Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fmt: TestCountMallocs fails when run alone #8617

Closed
randall77 opened this issue Aug 29, 2014 · 9 comments
Closed

fmt: TestCountMallocs fails when run alone #8617

randall77 opened this issue Aug 29, 2014 · 9 comments
Milestone

Comments

@randall77
Copy link
Contributor

At tip, do
  go test fmt

--- FAIL: TestCountMallocs (0.00s)
    fmt_test.go:888: Fprintf(buf, "%x %x %x"): got 1 allocs, want <=0
FAIL
FAIL    fmt 0.051s

This test passes when run as part of all.bash.

Raising the repeat count to 1000 (instead of 100) fixes the test.  But we should
probably understand what is going on here.
@dvyukov
Copy link
Member

dvyukov commented Aug 29, 2014

Comment 1:

What does GODEBUG=allocfreetrace=1 say?

@ianlancetaylor
Copy link
Contributor

Comment 2:

Labels changed: added repo-main, release-go1.4.

@randall77
Copy link
Contributor Author

Comment 3:

Looks like it is the allocations at fmt_test.go:872 for the Fprintf interface arguments
(7, 8, and 9).
fmt_test.go:872:    {0, `Fprintf(buf, "%x %x %x")`, func() { mallocBuf.Reset();
Fprintf(&mallocBuf, "%x %x %x", 7, 8, 9) }},
tracealloc(0xc20806d910, 0x10, int)
goroutine 5 [running]:
runtime.gomallocgc(0x10, 0x518000, 0x1, 0xc20806d908)
        /usr/local/google/home/khr/sandbox/go-ro/src/pkg/runtime/malloc.go:317 +0x229 fp=0xc208037ce0 sp=0xc208037c48
runtime.newobject(0x518000, 0xc208037d98)
        /usr/local/google/home/khr/sandbox/go-ro/src/pkg/runtime/malloc.go:348 +0x4a fp=0xc208037d08 sp=0xc208037ce0
runtime.convT2E(0x518000, 0xc208037d90, 0x0, 0x0)
        /usr/local/google/home/khr/sandbox/go-ro/src/pkg/runtime/iface.go:141 +0xac fp=0xc208037d40 sp=0xc208037d08
fmt_test.func·007()
        /usr/local/google/home/khr/sandbox/go-ro/src/pkg/fmt/fmt_test.go:872 +0x130 fp=0xc208037e18 sp=0xc208037d40
testing.AllocsPerRun(0x64, 0x5c2158, 0x0)
        /usr/local/google/home/khr/sandbox/go-ro/src/pkg/testing/allocs.go:33 +0xc0 fp=0xc208037e48 sp=0xc208037e18
fmt_test.TestCountMallocs(0xc20805c090)
        /usr/local/google/home/khr/sandbox/go-ro/src/pkg/fmt/fmt_test.go:886 +0x271 fp=0xc208037f68 sp=0xc208037e48
testing.tRunner(0xc20805c090, 0x649440)
        /usr/local/google/home/khr/sandbox/go-ro/src/pkg/testing/testing.go:427 +0x8b fp=0xc208037f98 sp=0xc208037f68
runtime.goexit()
        /usr/local/google/home/khr/sandbox/go-ro/src/pkg/runtime/proc.c:1625 fp=0xc208037fa0 sp=0xc208037f98
created by testing.RunTests
        /usr/local/google/home/khr/sandbox/go-ro/src/pkg/testing/testing.go:509 +0x914
So maybe this test is just wrong after our interfaces-are-always-pointers conversion. 
Probably other such tests are only succeeding by chance.
There are 150 allocations for 100 iterations.  That's what I expect when allocating 3
8-byte items per iteration (tinyalloc is in play here, with one 16-byte alloc every
other call).

@dvyukov
Copy link
Member

dvyukov commented Aug 29, 2014

Comment 4:

I am curious why changing 100 to 1000 iterations or running the full suite fixes the
test?...

@bradfitz
Copy link
Contributor

Comment 5:

This is the tinyalloc thing, right?
This is because tinyallocs don't increment runtime.Mallocs, and that's what
testing.AllocsPerRun uses.

@randall77
Copy link
Contributor Author

Comment 6:

That's part of it, but not all of it.  Only every other allocation increments
runtime.Mallocs, so it measures 1.5 allocations per iteration.

@rsc
Copy link
Contributor

rsc commented Sep 15, 2014

Comment 7:

Filed issue #8734 to fix AllocsPerRun.

@josharian
Copy link
Contributor

Comment 8:

all.bash runs tests with -short, and this test is skipped in -short, which is why the
build is ok.
Now that AllocsPerRun is fixed the test is accurate (exactly 200/300 allocs per 100
iterations), so the allowances just need to be increased. CL coming.

@josharian
Copy link
Contributor

Comment 9:

This issue was closed by revision 73a82db.

Status changed to Fixed.

@rsc rsc added this to the Go1.4 milestone Apr 14, 2015
@rsc rsc removed the release-go1.4 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
Converting an integer to an interface{} allocates as of CL 130240043.

Fixes golang#8617.

LGTM=r
R=r
CC=golang-codereviews, khr
https://golang.org/cl/141700043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
Converting an integer to an interface{} allocates as of CL 130240043.

Fixes golang#8617.

LGTM=r
R=r
CC=golang-codereviews, khr
https://golang.org/cl/141700043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
Converting an integer to an interface{} allocates as of CL 130240043.

Fixes golang#8617.

LGTM=r
R=r
CC=golang-codereviews, khr
https://golang.org/cl/141700043
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants