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
internal/sysinfo: fallbacks for CPU brandname detection on darwin on non-Intel CPU #61658
Comments
Note that /proc/cpuinfo does not contain a "model name" entry when running a linux/arm64 container on Apple M1. |
This CL is designed to support a larger discussion which is taking place in golang#61658. Since golang#39214 (accepted proposal), testing attempts to output the CPU brand string to provide more context to shared benchmarks. sysinfo currently relies on internal/cpu for hardware based CPU brand string detection, which only works on Intel x86 (386 and amd64). This CL adds a trivial system based fallback implementation for darwin utilizing syscall, which returns proper brand string on now very common arm64 darwin devices. It also sets up the basic structure to support additional platform based system implementations (for example, adding /proc/cpuinfo system implementation for Linux), which could come in future CLs. Updates golang#61658
This CL is designed to support a larger discussion which is taking place in golang#61658. Since golang#39214 (accepted proposal), testing attempts to output the CPU brand string to provide more context to shared benchmarks. sysinfo currently relies on internal/cpu for hardware based CPU brand string detection, which only works on Intel x86 (386 and amd64). This CL adds a trivial system based fallback implementation for darwin utilizing syscall, which returns proper brand string on now very common arm64 darwin devices. It also sets up the basic structure to support additional platform based system implementations (for example, adding /proc/cpuinfo system implementation for Linux), which could come in future CLs. Updates golang#61658
Change https://go.dev/cl/515055 mentions this issue: |
I've added a first CL with the Darwin implementation only to aide in the discussion (linked by gopherbot above). With this change, benchmark output adds cpu name properly on arm64 Darwin machines:
(A sample Linux implementation still exists in the aforementioned https://github.com/mroth/xsysinfo repo, but I feel like there may be a bit more to discuss about the merits of the |
Support for This issue now only tracks adding Darwin support. |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
Run any benchmarks (e.g
go test -bench=.
) on adarwin/arm64
orlinux/arm64
machine.What did you expect to see?
A prelude to the benchmark output containing a
cpu
line, ala:Since b7a2d41 benchmark test output contains
cpu
field designed to aide in the recognizable and shareable usage of the output; however the current implementation only works on Intel x86.What did you see instead?
Prelude which contains goos goaarch and pkg, but no cpu field.
I see this behavior (as expected) on both
darwin/arm64
andlinux/arm64
hardware I have access to test on.Proposed solution discussion (and sample code)
(Opened as a "bug" issue as this change would not affect any exposed APIs, and thus is not a "proposal".)
Reasoning
The initial discussion around adding this information was in #39214. As
arm64
hardware becomes increasingly common (notably, all consumer machines sold by Apple today), we have more and more developers writing Go using this as their development environment. Having the CPU "brand name" would aide in the context understanding of benchmark output shared by these developers.Implementation
The existing
internal/sysinfo
implementation is async.Once
cached call tointernal/cpu.CPU.Name()
, which is able to pull the brand string directly from the hardware on OS on Intel x86 chipsets. There is unfortunately apparently no reliable CPU-based way to do the same on arm64.There exists an existing TODO comment line in this package suggesting fallback to platform specific implementations may be acceptable:
⭐ I have put together a functional sample module at https://github.com/mroth/xsysinfo that shows what these modifications could look like, with support for Linux and Darwin platforms. I would like to open a CL but would like to first align on what is the ideal path is prior to doing so.
Darwin implementation (Syscall)
The Darwin solution was quite simple to implement, as it is a simple syscall to get
machdep.cpu.brand_string
. As example, on my test hardware, this returnsApple M2 Pro
. This implementation should function on botharm64
and olderamd64
darwin devices (though on the amd64 variants the cpu-based check will succeed, so this fallback should never be executed).Linux implementation (ProcFS)
The Linux usage of
/proc/cpuinfo
does feel a bit brittle as it relies on string parsing, but is perhaps acceptable given this information is not exposed in the go API anywhere, and is only used for developer tooling UX convenience.In the future, the linux implementation may want to be extended to look for
/proc/device-tree/model
which seems to be in usage on embedded hardware (e.g. Raspberry Pis which may be being used for local development, etc.)Windows implementation (future)
I did not create one today, but I believe it should be possible in the future if ARM hardware becomes common on Windows.
Risks
The error case simply elides the
cpu
field output, which is already the status quo today.There is a risk the Linux implementation could become increasingly complex in the future if many edge cases are added, adding a maintenance burden. (If this is deemed too risky in discussion here, I would propose I open my CL for review with the darwin implementation only for now, which should remain relatively stable.)
cc @martisch who wrote #39214
The text was updated successfully, but these errors were encountered: