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: add CPU name to standard benchmark labels #39214

Closed
martisch opened this issue May 22, 2020 · 17 comments
Closed

testing: add CPU name to standard benchmark labels #39214

martisch opened this issue May 22, 2020 · 17 comments

Comments

@martisch
Copy link
Contributor

@martisch martisch commented May 22, 2020

Currently only goarch and goos are printed as bechmark labels.

This proposal suggests to add the CPU name after a cpu label to the labels printed once at the top of benchmark output.

This will only happen when internal/cpu is able to detect the cpu type and otherwise the cpu label will not be printed.

Example with prototype http://golang.org/cl/234977:

goos: darwin
goarch: amd64
pkg: strconv
cpu: Intel(R) Core(TM) i7-3520M CPU @ 2.90GHz
BenchmarkAtof64Decimal-4        	24431032	        46.8 ns/op

Optionally disabled/enabled CPU features could be printed in addition:
features: aesni, avx, sse41 ...

This will help to understand, reproduce and document benchmark results better in context of different system configurations.

Some commit messages with benchmarks made by Go contributors already contain this information, but added manually:
https://go-review.googlesource.com/c/go/+/230737
https://go-review.googlesource.com/c/crypto/+/169037
https://go-review.googlesource.com/c/go/+/40693
https://go-review.googlesource.com/c/go/+/10367
https://go-review.googlesource.com/c/go/+/76470
https://go-review.googlesource.com/c/go/+/171736
https://go-review.googlesource.com/c/go/+/125316
https://go-review.googlesource.com/c/go/+/171731
https://go-review.googlesource.com/c/crypto/+/39693
....

Related Proposal: #28398

@gopherbot gopherbot added this to the Proposal milestone May 22, 2020
@mvdan
Copy link
Member

@mvdan mvdan commented May 22, 2020

Should we also print information about the memory and disk models/speeds? I imagine this will greatly depend on what the bottleneck for a benchmark is.

@martisch
Copy link
Contributor Author

@martisch martisch commented May 22, 2020

I would leave it at CPU information for now as this is a common factor for many microbenchmarks. For me this also identifies the system I was benchmarking on, which is helpful when examining logs of benchmarks later.

@mvdan
Copy link
Member

@mvdan mvdan commented May 22, 2020

I agree; I was thinking outloud more than anything. There could be many other factors at play too, such as what kernel version and if any special boot options were used (such as intel mitigations which could slow down syscalls).

@gopherbot
Copy link

@gopherbot gopherbot commented May 22, 2020

Change https://golang.org/cl/234977 mentions this issue: testing: print CPU name for benchmarks

@martisch
Copy link
Contributor Author

@martisch martisch commented May 22, 2020

@mvdan agreed. This could potentially be expanded in the future with a flag for printing even more information (ram, caches, kernel, ...) but I would leave that to a future extra and separate proposal.

The same information could then be easily added to e.g. go env or other commands with information for bug reporting if considered helpful.

@rsc rsc changed the title proposal: add CPU name to standard benchmark labels proposal: testing: add CPU name to standard benchmark labels Jun 10, 2020
@rsc rsc added this to Incoming in Proposals Jun 10, 2020
@martisch
Copy link
Contributor Author

@martisch martisch commented Jul 22, 2020

Having gathered benchmarks from multiple machines with different CPUs again lately for comparison and the reactions posted/emojis are all seem to be positive. Does anyone object to adding this for go1.16?

@rsc rsc moved this from Incoming to Active in Proposals Sep 16, 2020
@rsc
Copy link
Contributor

@rsc rsc commented Sep 16, 2020

Seems OK. Do we know how to procure this information for a wide enough variety of architectures and operating systems?

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Sep 16, 2020

Seems OK. Do we know how to procure this information for a wide enough variety of architectures and operating systems?

Linux: cat /proc/cpuinfo
mac: sysctl -n machdep.cpu.brand_string
Windows: wmic cpu get

@martisch
Copy link
Contributor Author

@martisch martisch commented Sep 16, 2020

On all operating systems for 386 and amd64 we can use the CPUID instruction (see http://golang.org/cl/234977) to get the CPU name.

Generally for linux on other architectures /proc/cpuinfo can be read.

For ARM on all operating systems another (more indirect) but not as universal approach can be to read the model and vendor IDs with CPU instructions and then map those to common architecture names for the most common CPUs (e.g. Cortex-A55, Cortex-A77 ...).

@ceseo
Copy link
Contributor

@ceseo ceseo commented Sep 16, 2020

For ppc64x, you have to rely on the OS. Currently, there is no exposed hardware CPU ID to userspace. On Linux, you can get that from /proc/cpuinfo or directly from the auxiliary vector (via the AT_PLATFORM variable — see LD_SHOW_AUXV=1)

@martisch
Copy link
Contributor Author

@martisch martisch commented Sep 16, 2020

@ceseo thanks for pointing this out.

I will work on to ammend my protoype patch for Linux on all CPU architectures to use AT_PLATFORM from the aux vector. We already use the same mechanism to get HWCAP bits from AT_HWCAP in the auxillary vector. It seems this could also work on BSDs.

@ceseo
Copy link
Contributor

@ceseo ceseo commented Sep 16, 2020

@martisch please bear in mind that, if you want to add the processor capabilities in the future (VSX, etc), you will have to read both AT_HWCAP and AT_HWCAP2 on ppc64x.

@martisch
Copy link
Contributor Author

@martisch martisch commented Sep 17, 2020

@ceseo both are already used on ppc64x to identify processor capabilities:

case _AT_HWCAP:

// HWCAP2 feature bits

@martisch
Copy link
Contributor Author

@martisch martisch commented Sep 21, 2020

I tested a bit around and AT_PLATFORM is mostly not useful as it will only print along the lines of x86_64, aarch64 and similar.

Parsing /proc/cpuinfo will cover all Linux based combinations but unfortunately is more complex than a first glance might suggest. Every architecture on Linux has its own format to store the specify CPU name in differently named fields.

On *BSDs it seems we can get information using sysctl calls that are already implemented in the runtime. I have however not tested how differ across architectures and some *BSDs we may need to implement a sysctlbyname call first if the MIBs are not static.

So in a first iteration it seems we could cover all of x86, amd64 and Linux leaving these not supported initially:

aix/ppc64
darwin/arm64
freebsd/arm
freebsd/arm64
js/wasm
netbsd/arm
netbsd/arm64
openbsd/arm
openbsd/arm64
plan9/arm
windows/arm

@rsc
Copy link
Contributor

@rsc rsc commented Sep 23, 2020

It sounds like between the commands and auxv we have getting the info covered.
No one is arguing against this, so it seems like a likely accept.

@rsc rsc moved this from Active to Likely Accept in Proposals Sep 23, 2020
@rsc
Copy link
Contributor

@rsc rsc commented Sep 30, 2020

No change in consensus, so accepted.

@rsc rsc moved this from Likely Accept to Accepted in Proposals Sep 30, 2020
@rsc rsc removed this from the Proposal milestone Sep 30, 2020
@rsc rsc added this to the Backlog milestone Sep 30, 2020
@rsc rsc changed the title proposal: testing: add CPU name to standard benchmark labels testing: add CPU name to standard benchmark labels Sep 30, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Oct 20, 2020

Change https://golang.org/cl/263804 mentions this issue: testing: print cpu type as label for benchmarks

@gopherbot gopherbot closed this in b7a2d41 Oct 20, 2020
@golang golang locked and limited conversation to collaborators Oct 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Proposals
Accepted
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants