testing: add -benchtime=100x (x suffix for exact count)

[wmorrow]

Currently benchmarks built using testing's Benchmark interface run a variable number of iterations, auto-adjusting to run for at least -benchiters seconds. This complicates HW PMU counter collection and A/B comparisons because the amount of work is (potentially) variable and the time adjustment code can muddle the benchmark results. It can also easily overshoot the amount of time you expect it to take:

$ go test archive/zip -benchtime 30s -run=Benchmark -bench=BenchmarkZip64Test$
goos: linux
goarch: amd64
pkg: archive/zip
BenchmarkZip64Test-8        1000          59382178 ns/op
PASS
ok      archive/zip     65.380s

The proposal

is to add a new flag to go test that circumvents the adjustment process and runs a benchmark for some exact user-defined number of iterations:

$ go test archive/zip -benchiterations 500 -run=Benchmark -bench=BenchmarkZip64Test$
goos: linux
goarch: amd64
pkg: archive/zip
BenchmarkZip64Test-8         500          59419401 ns/op
PASS
ok      archive/zip     29.775s

References

A draft of this change is already on Gerrit (+92617)

[bcmills]

(CC @rsc @josharian; see also #19128 and #10930.)

[rsc]

I'm reluctant to add more complexity to the benchmark flags. We just added benchsplit last round.
/cc @aclements

[josharian]

The main time I've wanted this recently is when working around benchmarks that are non-linear (i.e. broken) and hard to fix. But I share the reluctance to add more complexity here.

[aclements]

@dr2chase had some recent situation where there would have been useful, though I'm blanking on the details.

I'm also reluctant to add more flags. Sometimes, however, the non-linearity isn't the benchmark's fault, but GC's. For example, 1,000 iterations vs 2,000 iterations could be the difference between 1 GC cycle and 3 GC cycles.

[rsc]

It bothers me that this is only meaningful for a single benchmark at a time. Is there something different we can do that addresses the underlying problem?

[aclements]

We could be less aggressive about rounding to "nice" iteration counts, the effect of this would be damped.

For smoothing out GCs specifically, we could track the allocation rate of the benchmark to predict the number of GC cycles. If the number of cycles would be small, bias the next selected iteration count toward a nice value that would stay away from the edge between n and n+1 GC cycles.

[cespare]

Possibly related to #23423.

We could be less aggressive about rounding to "nice" iteration counts, the effect of this would be damped.

This behavior has annoyed me in the past. On #23423 I wrote

[...] it would've been nice if go test had picked a smaller value than 1000 here. It took 10s when I asked for 5s.

[wmorrow]

From my perspective the goal is to have some way to perform a set amount of work in a benchmark, rather than a (potentially) variable amount based on execution time or some other heuristic. Anything that can fix that amount of work to a reproducible value would be fine.

Specifically I'm thinking of cases where I want to compare hardware counters on two different hosts or between two versions of Go (with and without some patch). Assuming the two instances (hosts/toolchains/etc) have some different performance profile, the faster instance will likely perform a different amount of work than the slower instance if only because it uses different iteration counts when scaling. If the two instances also settle to different iteration counts the problem is exacerbated.

Another strategy I can think of off the top of my head is to read some datafile (perhaps a previous log?) and use the number of iterations set there. It's certainly not elegant but I'm also dubious that a heuristic based on measured performance of the instance can accurately reproduce a set amount of work. I think at some point you'll have to have an iteration count recorded or passed somewhere.

[meirf]

I'm reluctant to add more complexity to the benchmark flags. We just added benchsplit last round.

As a quick note, the benchsplit proposal was accepted but the feature was not added. The review hasn't been merged. I'm not mentioning this as a nit; rather there is some debate on that review which relates to the complexity/reluctance (and needs a decision to move forward.)

https://go-review.googlesource.com/c/go/+/47411/2/src/testing/benchmark.go#317
https://go-review.googlesource.com/c/go/+/47411/5/src/testing/benchmark.go#296

[rsc]

It seems like if you want to do right by performance counters generally, you need to have the testing package collect them around the runs and report them, including dividing by the iteration count. For example the 1000 iterations are preceded by 1, 10, and 100 iterations, trying to guess how many will be needed for benchtime. Really you don't want to see the performance counters for those trial runs at all. I don't know of any proposals for adding performance counters, though, and that would be quite a complex API.

Alternatively, the big jumps you see, where sometimes a benchmark runs for 1000 iterations and sometimes 2000 to reach benchtime, would go away if we stopped rounding so aggressively, as @aclements suggested in #24735 (comment)? Would that be enough to address the issue here, @wmorrow?

Originally I put the "nice" numbers in for eye-balling times more easily, but it really makes little sense given that you need to run 10 or so and run them through benchstat to have reliable statistics anyway.

[josharian]

a12cc71 took a small step towards removing aggressive rounding. I’m all for ditching it entirely. We should also consider eliminating or reducing the multiplier as well. (See that commit message for details.)

[aclements]

I don't know of any proposals for adding performance counters, though, and that would be quite a complex API.

#21295

@wmorrow, it seems like any performance counter comparison needs to account for warm-up and divide by the number of iterations anyway. If you're using perf for this, it seems like perf record --switch-output (maybe with -s if you just want counts) would make it easy to capture just the last run of the benchmark, and then the counts should be divided by the iteration count reported in the benchmark output.

[gopherbot]

Change https://golang.org/cl/112155 mentions this issue: testing: stop rounding b.N

[josharian]

I wanted to see the impact, so I hacked together CL 112155.

Thinking about that CL raises another concern.

Suppose we have a benchmark with high variance. We use our estimates to try to get near the benchtime. We are more likely to exceed the benchtime if we get a particularly slow run. A particularly fast run is more likely to trigger another benchmark run.

The current approach thus introduces bias.

One simple way to fix this would be to decide when our estimate is going to be "close enough", that is, when we are one iteration way from being done, and then stick with that final iteration even if it falls short of the benchtime.

There are probably other fixes, too...including setting a fixed number of iterations up front. :)

[josharian]

Oh, and note that making our iteration estimates ever more accurate (e.g. by eliminating rounding) actually exacerbates the bias-for-slow-runs problem, rather than ameliorating it.

[wmorrow]

I don't know of any proposals for adding performance counters, though, and that would be quite a complex API.

I agree, I think you'd end up having to rewrite perf/etc or call out to it and probably lose some expressiveness in the process.

If you're using perf for this, it seems like perf record --switch-output (maybe with -s if you just want counts) would make it easy to capture just the last run of the benchmark, and then the counts should be divided by the iteration count reported in the benchmark output.

If I'm reading this correctly you're suggesting adding a SIGUSR2 (or some other signal) when restarting the benchmark with some new N so that the outer perf can segment the perf.data files accordingly?

That could work to some extent (at least to ensure there's some known amount of work represented in the perf record), but it seems like a complicated solution to the problem. I suppose you save on having to define iteration counts for each benchmark but you have to do more work after the run to ensure you're matching the right perf data to the right benchmark. It's also not portable for collecting data in situations where we're not using perf (as an example we use MAMBO to track visited basic blocks).

[warpfork]

I have a very similar desire to what @wmorrow describes: I want to compare profiling info across code changes, or across packages which implement similar interfaces, or of the same code on different hardware, and so forth -- all situations where the iteration count detected by the bench tool is (quite sensibly) going to vary -- and have all the numbers be reasonably comparable with a bare minimum of further normalization.

In order to get stable human-handy info like this, it seems I want to either be able to declare a fixed number for b.N; or, have some way to get that number out afterwards so I can divide e.g. prof stats into average-per-op numbers.

A potential hack to work around this would be to also write Test1000Times_Foo functions to match every Benchmark_Foo function I have in the codebase; but this seems like a lot of boilerplate. It's a practical truism that the subjects of my benchmark functions are also what I want to evaluate any pprof'ing, etc, against, and so it would be nice if the test and benchmark run tooling would DWIM for this.

[rsc]

We're discussing a lot of complexity but even if we add it, it's hard to see whether it solves the real problems with warm-ups and the like. The SIGUSR2 is kind of weird but interesting. I bet if we sent SIGUSR2 to test binaries in general some packages would get mad, like when we tried to have package testing install a SIGINT handler.

[theckman]

To add my color from the other issue I opened (since GitHub search didn't lead me here):

I'm in the process of helping do a technical review of a Go book for newbies, and the author wants to show the performance benefits of using concurrency for web requests over serializing them. Originally, the author wanted to teach readers how to write benchmarks first, and to then use these benchmarking skills to compare a concurrent vs serial implementation. The issue here is that because b.N can't be set to a constant value of 1, the author doesn't want to cause a DDoS against golang.org (and another site) from people running this benchmark. Because we're looking to measure the performance difference on the magnitude of milliseconds, running the benchmark once should give us an accurate-enough result to show the performance improvement.

I have been in a similar spot too, where I wanted to benchmark something but only wanted the high-level/one run result because of the overall complexity of the thing I'm benchmarking. It would have at least reduced the time I was waiting for benchmarks to finish. As I recall, I ended up having to implement a package main that invoked my function with a time.Since. I would have much rather written a benchmark and invoked go test with the right inputs to do what I wanted.

Edit: For posterity, here's the other reason I'd seen this asked for (from my comment in the other issue):

While I'm less convinced of this use case, another ask I've seen in the Go community for this functionality is around using a constant value for your benchmarks so that you can compare the total wallclock time of the benchmarks. The reason I'm not convinced in this situation is that because most people do (and should) care about the ns/op result on a per-benchmark basis, not the total wallclock time it took the benchmark suite to run. I think they wanted to use this as a way to automatically detect large changes in performance, without needing to parse the results and keep track of each benchmark between runs.

[josharian]

FWIW, my reaction to the "I really just want it to run once" style of benchmarking is that package main and time.Since is the right answer. Otherwise you'll always have to remember to set b.N=1 on every benchmark invocation; the opportunity for accidental DDOS is high.

I'd argue that doing so might also make for good pedagogy: Start with time.Since, discuss the shortcomings of that approach, graduate to benchmarks, and discuss their benefits.

[bcmills]

because b.N can't be set to a constant value of 1, the author doesn't want to cause a DDoS against golang.org (and another site) from people running this benchmark.

If the author is concerned about the amount of traffic, then:
a. Congratulations are in order for a successful book! 🙂
b. They should provide a server that readers can start locally to produce (or simulate) the desired latency characteristics.
c. Or, they should target the benchmark against the book's own website. Since traffic is proportional to readers, and readers are proportional to sales, the more traffic the book produces, the more machines the author can afford in order to handle the traffic!

More seriously, though: benchmarks are experiments, and a low N is bad for benchmarking for exactly the same reason that it's bad for a scientific study. Ideally we should compute it based on the measured variability rather than a fixed duration, but deliberately setting it much too low is not a good practice to teach either way.

Because we're looking to measure the performance difference on the magnitude of milliseconds, running the benchmark once should give us an accurate-enough result to show the performance improvement.

-benchtime already fills a similar niche: if you know that the operation only takes a few hundred milliseconds, just run it for a few hundred milliseconds.

[theckman]

The point was to keep the example simple and digestable for readers who are newer to programming. Adding more complexity to the example makes it easier for people to become overwhelmed.

Thank you for the feedback.

[magical]

Like @warpfork i'm interested in an option like this to help with profiling.

go test -cpuprofile makes it very easy to identify hotspots in the code, but once identified i want to be able to reliably compare profiles both before and after a change. go tool pprof reports time in two units: percentage of runtime, and absolute runtime. It can't report time per iteration like go test -bench does because it has no concept of iterations. Percentage is out because, due to the potato paradox, comparing percentages can be extremely counterintuitive (if a function accounts for 99% of runtime and you make it twice as fast, it now takes 98%). That leaves absolute runtime, but if benchmarks always run for a constant amount of time, then this becomes as useless as percentages.

Things mostly work out today because, due to rounding, it is likely that go test -bench will select the same number of iterations each time--as long as your optimizations aren't extreme enough to kick you into a higher bucket. If CL 112155 goes in rounding goes away it seems like this problem will get worse.

My desired workflow goes something like this

1. pick a benchmark to work on
2. run go test -bench=foo to find a suitable number of iterations N to run it for
3. run go test -bench=foo -benchiter=N -run=NONE -cpuprofile=cpu.prof; go tool pprof -top cpu.prof to identify hot functions
4. make changes to the code
5. run go test -bench=foo -benchiter=N -run=NONE -cpuprofile=cpu.prof; go tool pprof -top cpu.prof again to see if changes had the intended effect by comparing absolute run times from step 3
6. repeat steps 4-5

I worked around this issue at first by fudging with -benchtime to get the number of iterations i wanted, but later took the more direct route of patching a -benchiter option directly into go test.

I could do what @josharian suggested and write a main program which starts a CPU profile and runs a copy of the benchmark code for a fixed number of iterations, but it seems a shame to give up all the tooling around go test.

[rsc]

@bradfitz suggested changing -benchtime to accept a little bit more than a time.Duration, so that you can say -benchtime=100x. Maybe that's good enough?

[josharian]

SGTM. I'd still like to finish up CL 112155 and think more about the bias concerns raised in #24735 (comment).

[magical]

SGTM too. I'd be happy to take a stab at implementing this.

Another thought: for the examples @theckman brought up about wanting to run a benchmark only once, it seems like a workaround would be to use -benchtime=0s, since benchmarks are always run at least once.

[gopherbot]

Change https://golang.org/cl/130675 mentions this issue: testing: allow passing an iteration count to -benchtime

[magical]

Oh, I didn't notice @mrosier-qdt had already uploaded a CL. Oh well, here's my take on it.

[josharian]

SGTM. I'd still like to finish up CL 112155 and think more about the bias concerns raised in #24735 (comment).

I've broken out the bias concern as #27168. (And I'll eventually get back to the CL.)

[rsc]

Looks like people generally agree with -benchtime=100x and there is a pending CL. Proposal accepted.

[gopherbot]

Change https://golang.org/cl/139258 mentions this issue: testing: implement -benchtime=100x

[magical]

@rsc I see you've uploaded a CL of your own. Should i abandon https://golang.org/cl/130675?

[bradfitz]

@magical, sorry, I don't think we realized a CL was already open for it. I'll let @rsc decide how he wants to go forward.