[BUG] Flaky failure of PerfCountersTest.MultiThreaded

[musicinmybrain]

Describe the bug

While looking at enabling libpfm support in Fedora’s google-benchmark package, we’ve found that PerfCountersTest.MultiThreaded fails flakily on the ppc64le architecture relatively frequently (perhaps 1/4 to 1/2 of the time), and rarely on some other architectures, at least aarch64.

System
Which OS, compiler, and compiler version are you using:

• OS: Fedora (Rawhide)
• Compiler and version: gcc (GCC) 16.0.1 20260416 (Red Hat 16.0.1-0)

To reproduce
We see this while building the google-benchmark package, but there should not be anything unique here: I expect it will be possible to reproduce it in any ppc64le environment with -DBENCHMARK_ENABLE_LIBPFM:BOOL=ON.

I can do test-builds of Fedora packages on ppc64le in Fedora infrastructure, but I do not have interactive access to real ppc64le hardware.

Expected behavior
All tests pass.

Screenshots

74/84 Test #79: perf_counters_gtest ........................***Failed    0.20 sec
Running main() from gmock_main.cc
[==========] Running 9 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 9 tests from PerfCountersTest
[ RUN      ] PerfCountersTest.Init
[       OK ] PerfCountersTest.Init (0 ms)
[ RUN      ] PerfCountersTest.OneCounter
[       OK ] PerfCountersTest.OneCounter (0 ms)
[ RUN      ] PerfCountersTest.NegativeTest
A performance counter name was the empty string
Unknown performance counter name: not a counter name
A performance counter name was the empty string
Unknown performance counter name: not a counter name
Unknown performance counter name: bad event name
[       OK ] PerfCountersTest.NegativeTest (0 ms)
[ RUN      ] PerfCountersTest.Read1Counter
[       OK ] PerfCountersTest.Read1Counter (0 ms)
[ RUN      ] PerfCountersTest.Read2Counters
[       OK ] PerfCountersTest.Read2Counters (0 ms)
[ RUN      ] PerfCountersTest.ReopenExistingCounters
[       OK ] PerfCountersTest.ReopenExistingCounters (0 ms)
[ RUN      ] PerfCountersTest.CreateExistingMeasurements
Error reading lead 9 errno:2 No such file or directory
[       OK ] PerfCountersTest.CreateExistingMeasurements (0 ms)
[ RUN      ] PerfCountersTest.MultiThreaded
/builddir/build/BUILD/google-benchmark-1.9.5-build/benchmark-1.9.5/test/perf_counters_gtest.cc:269: Failure
Value of: Elapsed4Threads[0] / Elapsed2Threads[0]
Expected: (is > 0.1) and (is < 10)
  Actual: 20.085135325112532 (of type double), which doesn't match (is < 10)
[  FAILED  ] PerfCountersTest.MultiThreaded (191 ms)
[ RUN      ] PerfCountersTest.HardwareLimits
[       OK ] PerfCountersTest.HardwareLimits (0 ms)
[----------] 9 tests from PerfCountersTest (192 ms total)
[----------] Global test environment tear-down
[==========] 9 tests from 1 test suite ran. (192 ms total)
[  PASSED  ] 8 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] PerfCountersTest.MultiThreaded
 1 FAILED TEST

In ten scratch builds, this test failed three times. Failing values of Elapsed4Threads[0] / Elapsed2Threads[0] were (approximately) 20.1, 11.1, and 17.1.

Additional context
N/A

[musicinmybrain]

I just observed this on aarch64 (with value ~11.4), so this must not be specific to ppc64le after all.

[musicinmybrain]

In https://src.fedoraproject.org/rpms/google-benchmark/pull-request/5#comment-319974, Matthew Krupcale suggests that the bounds on these ratios might be loosened another order of magnitude, to 100.

Or, if the failures reported here do not reflect an actual bug, perhaps this test is not very useful at all?

[LebedevRI]

It is always a not very good idea to expect that measuring performance in tests would produce reasonable results.
Reading the source of truth (https://github.com/google/benchmark/pull/1153/changes),
what happens if you change measure() function to run each thread/job in sequence?
I don't really think the parallelizm matters for this test, and realistically,
we can only check that the results scale with the count of jobs...

[LebedevRI]

(fat-fingered it)

[musicinmybrain]

It is always a not very good idea to expect that measuring performance in tests would produce reasonable results. Reading the source of truth (https://github.com/google/benchmark/pull/1153/changes), what happens if you change measure() function to run each thread/job in sequence? I don't really think the parallelizm matters for this test, and realistically, we can only check that the results scale with the count of jobs...

Hmm, looking at

benchmark/test/perf_counters_gtest.cc

Lines 288 to 302 in 1eceef8

	// We need to first set up the counters, then start the threads, so the
	// threads would inherit the counters. But later, we need to first destroy
	// the thread pool (so all the work finishes), then measure the counters. So
	// the scopes overlap, and we need to explicitly control the scope of the
	// threadpool.
	auto counters =
	PerfCounters::Create({kGenericPerfEvent1, kGenericPerfEvent2});
	for (auto& t : threads) {
	t = std::thread(work);
	}
	*before = SnapshotAndCombine(counters);
	for (auto& t : threads) {
	t.join();
	}
	*after = SnapshotAndCombine(counters);

I am not sure quite how to actually make these run sequentially in a way that corresponds to what the original author was trying to test here, and maybe the following observation has something to do with why this test is so unreliable.

When the for (auto& t : threads) { t = std::thread(work); } sets up the thread pool, each thread begins execution, or at least is ready to be scheduled by the OS, immediately upon construction. If creating a thread is relatively slow compared to the do_work() function, or if the “main” thread’s execution is interrupted by the scheduler in the middle of the loop, then the first threads may be done or nearly done by the time the last threads begin. Only after all of the threads have been started do we have *before = SnapshotAndCombine(counters);.

We could write something like this:

  auto counters = PerfCounters::Create({kGenericPerfEvent1, kGenericPerfEvent2});
  *before = SnapshotAndCombine(counters);
  for (auto& t : threads) { t = std::thread(work); t.join(); } 
  *after = SnapshotAndCombine(counters);

…but if I understand this test correctly, that isn’t correct, because the first call to SnapshotAndCombine should be after all of the threads have inherited the counters.

I suppose if we wanted to (1) delay worker thread execution until after the first call to SnapshotAndCombine, (2) restrict ourselves to C++11 features, and (3) possibly run the worker threads serially rather than in parallel, we could use a std::mutex initially held by the parent thread, release it after creating the workers, and have each worker acquire it to do its work. That would ensure all the work was actually done between the SnapshotAndCombine calls.

[LebedevRI]

Thank you!