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: Benchmark doesn't collect result of sub-benchmarks #18815

Closed
uvelichitel opened this issue Jan 27, 2017 · 8 comments
Closed

testing: Benchmark doesn't collect result of sub-benchmarks #18815

uvelichitel opened this issue Jan 27, 2017 · 8 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@uvelichitel
Copy link

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

1.7.4
go version devel +78860b2ad

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build805050938=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

I run program normal way, not "go test". I call testing.Benchmark() in code. Benchmark function call Run() method to execute sub benchmark.

func main() {
    result := testing.Benchmark(func(parentB *testing.B) {
        parentB.Run("example", func(b *testing.B) {
            for n := 0; n < b.N; n++ {
                println("ok")
            }
        })
    })
    println(result.String())
}

Behaviour already reproduced prooflink http://stackoverflow.com/questions/41861918/using-testing-benchmark-does-not-produce-any-output

What did you expect to see?

According to the documentation I expect

Benchmark benchmarks a single function. Useful for creating custom benchmarks that do not use the "go test" command.

If f calls Run, the result will be an estimate of running all its subbenchmarks that don't call Run in sequence in a single benchmark.

What did you see instead?

Sub benchmark run N times(no once) but result is
0 0 ns/op

@bradfitz bradfitz changed the title testing.Benchmark() function doesn't collect result of sub-benchmarks. testing: Benchmark doesn't collect result of sub-benchmarks Jan 27, 2017
@bradfitz bradfitz added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jan 27, 2017
@bradfitz bradfitz added this to the Go1.9Maybe milestone Jan 27, 2017
@ianlancetaylor
Copy link
Contributor

I think the bug is that Benchmark returns an empty BenchmarkResult if b.run1() returns false. It should return b.result.

@robpike
Copy link
Contributor

robpike commented Jan 27, 2017

There may be a but as @ianlancetaylor says, but I would still not expect the testing package to work properly unless the program is run with go test. The go test command sets up the environment for the test and benchmarks. The go run command does not.

@uvelichitel
Copy link
Author

b.run1() seems to return true. Sub - benchmark is evaluated N times, no once, checked with debug prints. RunParallel() works as expected in same circumstances.

@cespare
Copy link
Contributor

cespare commented Jan 27, 2017

@robpike I always thought this was an intended and documented use case for the testing package. Otherwise, what is the purpose of the Benchmark function?

func Benchmark(f func(b *B)) BenchmarkResult
    Benchmark benchmarks a single function. Useful for creating custom
    benchmarks that do not use the "go test" command.

    If f calls Run, the result will be an estimate of running all its
    subbenchmarks that don't call Run in sequence in a single benchmark.

@robpike
Copy link
Contributor

robpike commented Jan 27, 2017

The purpose of the Benchmark function is to run a benchmark under the auspices of "go test".
I'm not saying it mustn't work outside of "go test", just that it wouldn't surprise me if it didn't.

Here is the start of the package docs:

Package testing provides support for automated testing of Go packages. It is
intended to be used in concert with the ``go test'' command, which automates
execution of any function of the form ...

@mpvl
Copy link
Contributor

mpvl commented Jan 31, 2017

I agree this is a bug. Like Ian said, it should still return b.result.

@mpvl mpvl added Testing An issue that has been verified to require only test changes, not just a test failure. NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Feb 14, 2017
@gopherbot
Copy link

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

@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Mar 1, 2017
The run1 call removed in golang.org/cl/36990 was necessary to
initialize the duration of the benchmark. With it gone, the math in
launch() starts from 100. This doesn't work out well for second-long
benchmark methods. Put it back.

Updates #18815

Change-Id: I461f3466c805d0c61124a2974662f7ad45335794
Reviewed-on: https://go-review.googlesource.com/37530
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
@golang golang locked and limited conversation to collaborators Feb 28, 2018
@rsc rsc unassigned mpvl Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Development

No branches or pull requests

7 participants