Navigation Menu

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

Jump to bottom

x/perf: allow callers to specify precision of benchmark report #57711

Closed
nealeckard opened this issue Jan 9, 2023 · 4 comments
Closed

x/perf: allow callers to specify precision of benchmark report #57711

nealeckard opened this issue Jan 9, 2023 · 4 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@nealeckard
Copy link

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

go1.20-20221229-RC00

$ go version

Does this issue reproduce with the latest release?

I think so (I'm on an RC, so I'd assume so).

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

Debian Linux, x86_64

go env Output
$ go env

What did you do?

We're running three sets of benchmarks (using the Google benchmark library): "original", "user", and "solution" (see screenshot). We then pipe the benchmarks through benchstat for display to the user.

What did you expect to see?

The "original" is usually much slower than the solution, so when benchstat processes the results, it displays all three benchmarks with the precision of the slowest benchmark, resulting in displayed rounded-down zero values for the solution. It would be better if these zero values weren't rounded down, but instead displayed, for example, 0.234 microseconds (instead of just 0 microseconds).

What did you see instead?

5T9BeqzE9XF3gJv
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jan 9, 2023
@gopherbot gopherbot added this to the Unreleased milestone Jan 9, 2023
@thepudds
Copy link
Contributor

Hi @nealeckard, thanks for filing this.

FWIW, there is a complete rewrite of benchstat that is in progress, including this CL:
https://go-review.googlesource.com/c/perf/+/309969

It could be interesting to try that on your example to see if you get better or different behavior:

git clone https://go.googlesource.com/perf xperf-benchstat-new
cd xperf-benchstat-new
git fetch https://go.googlesource.com/perf refs/changes/69/309969/11 && git checkout -b change-309969 FETCH_HEAD
cd cmd/benchstat/
go build -o benchstat-new

@nealeckard
Copy link
Author

Thanks for the tip! The new version does indeed seem to solve the problem of precision for widely disparate benchmarks.

Screenshot 2023-01-09 6 17 06 PM

@thepudds
Copy link
Contributor

Hi @aclements, enthusiasm for your new version of benchstat continues to build... 😅

@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 10, 2023
@cagedmantis cagedmantis changed the title x/perf: Allow callers to specify precision of benchmark report x/perf: allow callers to specify precision of benchmark report Jan 10, 2023
@prattmic
Copy link
Member

Closing now that https://go-review.googlesource.com/c/perf/+/309969 is in

@golang golang locked and limited conversation to collaborators Jan 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@cagedmantis @prattmic @gopherbot @thepudds @nealeckard