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

NominalCPUFrequency Test from SysInfoTest Suite Fails on M1 Mac #1053

Closed
anthonygraca opened this issue Nov 4, 2021 · 3 comments
Closed
Labels

Comments

@anthonygraca
Copy link

anthonygraca commented Nov 4, 2021

Describe the bug

When running abseil tests, the NominalCPUFrequency test from the SysInfoTest test suite fails from absl/base/internal/sysinfo_test.cc.

Steps to reproduce the bug

The command that I ran specifically on my M1 MacBook Pro is:

bazel test --test_tag_filters=-benchmark --test_output=errors @com_google_absl//...

The failing test should show as:

FAIL: //absl/base:sysinfo_test (see /private/var/tmp/_bazel_anthonygraca/e02ebe8c94fcab7aaef2c566ce188ffa/execroot/com_google_absl/bazel-out/darwin_arm64-fastbuild/testlogs/absl/base/sysinfo_test/test.log)
INFO: From Testing //absl/base:sysinfo_test:
==================== Test output for //absl/base:sysinfo_test:
Running main() from gmock_main.cc
[==========] Running 3 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 3 tests from SysinfoTest
[ RUN      ] SysinfoTest.NumCPUs
[       OK ] SysinfoTest.NumCPUs (0 ms)
[ RUN      ] SysinfoTest.NominalCPUFrequency
absl/base/internal/sysinfo_test.cc:57: Failure
Expected: (NominalCPUFrequency()) >= (1000.0), actual: 1 vs 1000
NominalCPUFrequency() did not return a reasonable value
[  FAILED  ] SysinfoTest.NominalCPUFrequency (0 ms)
[ RUN      ] SysinfoTest.GetTID
[       OK ] SysinfoTest.GetTID (23 ms)
[----------] 3 tests from SysinfoTest (23 ms total)

[----------] Global test environment tear-down
[==========] 3 tests from 1 test suite ran. (23 ms total)
[  PASSED  ] 2 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] SysinfoTest.NominalCPUFrequency

 1 FAILED TEST
================================================================================

What version of Abseil are you using?
Head: Commit d6c75d9

What operating system and version are you using

macOS Monterey 12.0.1

What compiler and version are you using?

Apple clang version 13.0.0 (clang-1300.0.29.3)
Target: arm64-apple-darwin21.1.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

What build system are you using?

bazel 5.0.0-pre.20211011.2

Additional context

It looks like the code that causes the issue is from absl/base/internal/sysinfo.cc between lines 177 - 186 in function GetNominalCPUFrequency().

The root cause stems from line 181 where if(sysctl(mib, 2, &freq, &size, nullptr, 0) == 0) evaluates to false, so that causes the function to never return freq. If we ignored the check and just returned the value in freq anyways, the test passes but I am not sure if that is an appropriate solution.

It looks like the sysctl() call returns an error status of ENOENT, meaning the input to the first parameter of sysctl() is invalid. However both values in the mib[2] array, CTL_HW and HW_CPU_FREQ are defined, so I am not sure why this is the problem. I think this may be a bug with macOS itself because this behavior can be replicated outside of abseil in an external program. Any ideas on how to approach this problem?

@derekmauro
Copy link
Member

You may want to see my comments in #728. If the only problem you are trying to solve is a failing test, this is safe to ignore since this code is never called. I should consider stripping this test out of the open source release. NominalCPUFrequency is only called in code private to Google and we do have tests on the platforms we use it on.

@anthonygraca
Copy link
Author

anthonygraca commented Nov 5, 2021

Thanks @derekmauro for looking into this and thank you for links to your prior comments. I will be more thorough on my search next time.

Do you have any recommendations for development workflow? I could temporarily DISABLE_* the failing tests (there are two other tests that fail but I just wanted to make an issue with this one) but I am not sure how to separate signal from noise if the failing tests turn out to be an actual issue.

Would it be helpful for you if I made a PR on removing the NominalCPUFrequency code?

@derekmauro
Copy link
Member

I will have to see what I can do, but please do not remove the NominalCPUFrequency code.

razmser pushed a commit to razmser/abseil-cpp that referenced this issue Sep 12, 2023
--
4324716dc5384f03dcd7e36e8cca0e944e4dac74 by Evan Brown <ezb@google.com>:

Clarify comments about API differences from std::{set,map} in btree headers.

We note that the most important API differences are mentioned in the next paragraph and that other API differences are minor. An example of another minor API difference is the note about std::launder in the comment for `extract`.

Motivation: readers shouldn't feel like they need to read the entire header file to understand why b-tree containers are not "drop-in replacements" for STL containers.
PiperOrigin-RevId: 408703780

--
7e8da4f14afd25d11713eee6b743ba31605332bf by Derek Mauro <dmauro@google.com>:

Remove the test for absl::base_internal::NominalCPUFrequency() from OSS code

This is an internal-only function that should never by called by OSS code.
By its nature fails on unsupported platforms.
Google code has tests for this function on supported internal platforms.

Fixes abseil#1053

PiperOrigin-RevId: 408692861

--
37bad2e003b17e3f82d6fd5d90ae183553c018b0 by Abseil Team <absl-team@google.com>:

To avoid triggering -Wmissing-variable-declarations warnings, ABSL_FLAG now
declares the Flag before defining it.

PiperOrigin-RevId: 408673990
GitOrigin-RevId: 4324716dc5384f03dcd7e36e8cca0e944e4dac74
Change-Id: I8c8ea73c4a4e38729c5bfdfa3fefb5d593e0536c
razmser pushed a commit to razmser/abseil-cpp that referenced this issue Sep 12, 2023
--
4324716dc5384f03dcd7e36e8cca0e944e4dac74 by Evan Brown <ezb@google.com>:

Clarify comments about API differences from std::{set,map} in btree headers.

We note that the most important API differences are mentioned in the next paragraph and that other API differences are minor. An example of another minor API difference is the note about std::launder in the comment for `extract`.

Motivation: readers shouldn't feel like they need to read the entire header file to understand why b-tree containers are not "drop-in replacements" for STL containers.
PiperOrigin-RevId: 408703780

--
7e8da4f14afd25d11713eee6b743ba31605332bf by Derek Mauro <dmauro@google.com>:

Remove the test for absl::base_internal::NominalCPUFrequency() from OSS code

This is an internal-only function that should never by called by OSS code.
By its nature fails on unsupported platforms.
Google code has tests for this function on supported internal platforms.

Fixes abseil#1053

PiperOrigin-RevId: 408692861

--
37bad2e003b17e3f82d6fd5d90ae183553c018b0 by Abseil Team <absl-team@google.com>:

To avoid triggering -Wmissing-variable-declarations warnings, ABSL_FLAG now
declares the Flag before defining it.

PiperOrigin-RevId: 408673990
GitOrigin-RevId: 4324716dc5384f03dcd7e36e8cca0e944e4dac74
Change-Id: I8c8ea73c4a4e38729c5bfdfa3fefb5d593e0536c
liuyangxy pushed a commit to fedora-riscv/abseil-cpp that referenced this issue Dec 7, 2023
This fails occasionally on aarch64, and upstream reports that it is not
meaningful except for Google internal users. See:

NominalCPUFrequency Test from SysInfoTest Suite Fails on M1 Mac
abseil/abseil-cpp#1053 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants