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

testing: decide whether benchmark probe should use b.N==0 or b.N==1 #14863

Closed
rsc opened this issue Mar 18, 2016 · 5 comments

Comments

Projects
None yet
5 participants
@rsc
Copy link
Contributor

commented Mar 18, 2016

We need some kind of probe phase to gather sub-benchmark information.
It would be nice if that phase were clearly identified, for example by b.N==0.
Some benchmarks have tests that assume there was at least one iteration,
and those would need updating. We've now updated the standard library.

This is something people would have to update tests for, but of course non-test
programs continue working fine. And the testing package has never guaranteed
that b.N > 0. We do break code that depends on things it shouldn't, for example
when we updated sort.Sort in Go 1.6. But we don't want to break too much.

If, based on beta testing, we decide that there are just too many benchmarks
in real-world code that will need updating, then we should back off to using N==1
as the subtest probe.

Decide based on beta testing though, not by guessing now.

@rsc rsc assigned mpvl Mar 18, 2016

@rsc rsc added this to the Go1.7 milestone Mar 18, 2016

@bradfitz

This comment has been minimized.

Copy link
Member

commented Mar 18, 2016

We need some kind of probe phase to gather sub-benchmark information.

Is that statement already jumping to conclusions? Why can't the Benchmark signature be different and take more than just a *testing.B for benchmarks which want subtests?

(To save other clueless readers time, this is about bug #12166 with design doc https://github.com/golang/proposal/blob/master/design/12166-subtests.md)

@minux

This comment has been minimized.

Copy link
Member

commented Mar 19, 2016

@mpvl

This comment has been minimized.

Copy link
Member

commented Mar 19, 2016

On Saturday, March 19, 2016, Minux Ma notifications@github.com wrote:

I want to ask the same question. When a new feature breaks old code, there
seems like enough reason to redesign the new feature to not break existing
code (esp. when the standard library code is also broken.) Why
subbenchmarks have to be registered with a regular benchmark function?

Please see
https://github.com/golang/proposal/blob/master/design/12166-subtests.md

The feature can easily be made to not break these tests. The breaking
version is a bit nicer so this is just to see if we can make things nicer
without much effort. If not, no problem.
Note that the breakage is very rare (but easy to identify) and occurs only
in quite unconventional uses of the benchmark functionality that are in
addition making use of an undocumented and not guaranteed property of B.


You are receiving this because you were assigned.
Reply to this email directly or view it on GitHub
#14863 (comment)

@gopherbot

This comment has been minimized.

Copy link

commented Mar 23, 2016

CL https://golang.org/cl/21043 mentions this issue.

@mpvl

This comment has been minimized.

Copy link
Member

commented Mar 25, 2016

Trade-offs:
Advantage of N=0: more principled, allows easy way for tests to do a setup once.

Disadvantage: less compatible: some code expects N>1 and code may be called a different number of times from before.

Overall, we don't allow benchmarks to do both measurements and run subtests. So we can piggyback on the old N==1 phase to discover if there are subbenchmarks without changing the processing flow.

So overall, using the current N==1 as the "probe" phase is probably better. Moreover, to do setup once, a user can use a flag, or even use subtests:

func BenchmarkFoo(b *B) {
    // setup
   b.Run(b *B) {
      // doBench
   }
}

gopherbot pushed a commit that referenced this issue Mar 25, 2016

testing: probe with N=1
Change control flow to probe with N=1. This calls benchFunc
the same number of times as the old implementation in the
absence of subbenchmarks.

To be compatible with existing tools, benchmarking only
prints a line for "leaf" benchmarks. This means, though, that
the name of a benchmark can only be printed after the first
iteration.

Issue #14863

Change-Id: Ic7b9b89b058f8ebb5287755f24f9e47df8c9537c
Reviewed-on: https://go-review.googlesource.com/21043
Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

@mpvl mpvl closed this Mar 26, 2016

@golang golang locked and limited conversation to collaborators Mar 27, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.