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 output should include settings like GOAMD64 #53782

Open
mvdan opened this issue Jul 11, 2022 · 2 comments
Open

testing: benchmark output should include settings like GOAMD64 #53782

mvdan opened this issue Jul 11, 2022 · 2 comments
Labels
FeatureRequest NeedsInvestigation
Milestone

Comments

@mvdan
Copy link
Member

@mvdan mvdan commented Jul 11, 2022

go test -bench has printed system information like goos, goarch, and cpu as part of benchmark results for some time now:

go/src/testing/benchmark.go

Lines 265 to 272 in 59ab6f3

fmt.Fprintf(b.w, "goos: %s\n", runtime.GOOS)
fmt.Fprintf(b.w, "goarch: %s\n", runtime.GOARCH)
if b.importPath != "" {
fmt.Fprintf(b.w, "pkg: %s\n", b.importPath)
}
if cpu := sysinfo.CPU.Name(); cpu != "" {
fmt.Fprintf(b.w, "cpu: %s\n", cpu)
}

As a companion to goarch and cpu, I think we should also include the value of Go environment variables like GOAMD64 - similar env vars include GOARM, GOMIPS64, etc. They are listed together in go help environment.

This matters just like cpu does: benchmark numbers will vary between CPUs, and some benchmarks might benefit significantly from GOAMD64=v3 compared to the default of GOAMD64=v1.

For the sake of reducing verbosity, we could only print these if they are set to a non-default value. GOAMD64=v1 wouldn't be printed, but GOAMD64=v3 would be.

Arguably there are lots of other factors that could contribute to higher or lower benchmark results, such as what kernel the sytem is running, which C toolchain was used to build cgo, what Go version is being used, etc. However, env vars like GOAMD64 feel much more in line with the goarch and cpu lines we already print, and they are a set of Go build options which directly affect performance.

@mknyszek mknyszek added FeatureRequest NeedsInvestigation labels Jul 11, 2022
@mknyszek mknyszek added this to the Backlog milestone Jul 11, 2022
@mknyszek
Copy link
Contributor

@mknyszek mknyszek commented Jul 11, 2022

CC @bcmills @aclements

I think this makes sense, and should be straightforward to do. I also don't think it would be too verbose to always print them (for the relevant platforms; i.e. don't print "GOARM" for "GOARCH=amd64"). We just need to pick names for the keys. Maybe just "goamd64," "gomips," etc.?

@mvdan
Copy link
Member Author

@mvdan mvdan commented Jul 11, 2022

Yeah, just lowercasing the names seems good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest NeedsInvestigation
Projects
None yet
Development

No branches or pull requests

2 participants