Repository navigation
testing: don't truncate allocs/op #24631
Description
Activity
Just in case, here's the complete result string, which shows that the program is doing as much work in both cases:
$ go run main.go 100000000 12.7 ns/op 4 B/op 1 allocs/op $ go run main.go 100000000 12.4 ns/op 4 B/op 0 allocs/op $ go run main.go 100000000 12.3 ns/op 4 B/op 1 allocs/op $ go run main.go 100000000 12.2 ns/op 4 B/op 0 allocs/op
The number of reported allocs varies between 999999 and 1000003; when it' the former 0 allocs are reported, when it's the latter 1 alloc is reported.
Either we fix the flakiness in the
memstatcalls (I don't know if that can be done reliably) or we stop using integer truncated division to compute the result (where 999999 / 1000000 = 0) and instead we round to the nearest integer. This will give us more accurate results in the cases that are more likely to be flaky (very fast benchmarks that only allocate once or twice).Change https://golang.org/cl/104055 mentions this issue:
testing: round (instead of truncating) in AllocsPerOp@teh-cmc, your benchmark loop is slightly non-linear, which I believe is what's causing this. The loop needs to repeat the benchmark b.N times, but your loop repeats it b.N-1 times. Either:
for i := 1; i < b.N+1; i++ { Eface = uint32(i) }
Or, better, since it a keeps the standard benchmark loop form:
for i := 0; i < b.N; i++ { Eface = uint32(i+1) }
Regardless, perhaps we shouldn't be truncating the reported allocs/op to an integer at all. The benchmark format is allowed to contain floating point numbers, so we could print out a few decimal places, like we already do for ns/op and MB/s.
Silly me. That makes sense @aclements, thanks.
I do agree with you and @ALTree that this shouldn't get truncated down to zero in any case though, that seems quite deceptive.
In #19128 (comment), @rsc suggested:
I'd also like to have a check somewhere (in package testing or benchstat, probably the former) that b.N really is scaling linearly.
I wonder whether that check would be sensitive enough to catch the nonlinearity here.
At any rate, it sounds like just fixing the rounding would help. Shall we retitle the bug for that?
- changed the title
[-]testing: Benchmark reports flaky number of allocations[/-][+]testing: don't truncate allocs/op[/+]on Apr 2, 2018 OTOH, unlike ns/op and MB/s, allocs/op really is a whole number for a given iteration. I worry slightly that the random background allocations that aren't coupled to iterations (e.g., when a GC cycle runs) would almost always cause this number to be slightly above a whole number and that that could confuse people.
Is allocs/op really a whole number? I would think that any API that involves a cache could legitimately end up with a fractional mean.
For a given iteration, yes, it's a whole number. But caches are another good example of iteration-to-iteration variance and yet another example where summarizing a benchmark result distribution as just a mean can lose a great deal of information.
Reacted by Josh Bleecher Snyder and Emmanuel T OdekeThe code under test was buggy which caused the flake. When we added allocs/op we explicitly decided to truncate it, to report the number of allocs that happen every single time. It's too late to change that definition.
- locked and limited conversation to collaborators
on Apr 9, 2019
testing.Benchmarkgives me flaky allocation reports in the following case:The generated code for
Eface = uint32(i)is what you'd expect:Results:
$ go run main.go 4 B/op 1 allocs/op $ go run main.go 4 B/op 1 allocs/op $ go run main.go 4 B/op 1 allocs/op $ go run main.go 4 B/op 0 allocs/op $ go run main.go 4 B/op 0 allocs/op $ go run main.go 4 B/op 1 allocs/opI.e. while the number of allocated bytes per op is always correct, the number of allocations per op is sometimes wrong (expecting
1 allocs/op).I guess I'm hitting some kind of rounding error here, but then again why would this behavior not be deterministic? Maybe due to the heuristics around the value of
b.N?I'm not sure what I'm missing here?
Thanks.