Respect min_time per thread in multi-threaded benchmarks (#2117)

src/benchmark_runner.cc

@@ -370,17 +370,36 @@ BenchmarkRunner::IterationResults BenchmarkRunner::DoNIterations() {
   return i;
 }
 
+double BenchmarkRunner::GetTimeForDecision(const IterationResults& i) const {
+  // real_time and manual_time are accumulated per thread in RunInThread (i.e.
+  // summed across all threads), and so is whole-process CPU time. To decide
+  // whether each thread has run for at least min_time we need the per-thread
+  // (wall-clock-equivalent) duration, so divide the summed value by the number
+  // of threads. Per-thread CPU time (the default) is left untouched. The summed
+  // values in i.results are kept intact for reporting, where they are divided
+  // by the total iteration count across all threads. This mirrors the scaling
+  // that was applied unconditionally before #1836 removed it from the reported
+  // path.
+  double seconds = i.seconds;
+  if (b.use_manual_time() || b.use_real_time() ||
+      b.measure_process_cpu_time()) {
+    seconds /= b.threads();
+  }
+  return seconds;
+}
+
 IterationCount BenchmarkRunner::PredictNumItersNeeded(
     const IterationResults& i) const {
+  const double seconds = GetTimeForDecision(i);
   // See how much iterations should be increased by.
   // Note: Avoid division by zero with max(seconds, 1ns).
-  double multiplier = GetMinTimeToApply() * 1.4 / std::max(i.seconds, 1e-9);
+  double multiplier = GetMinTimeToApply() * 1.4 / std::max(seconds, 1e-9);
   // If our last run was at least 10% of FLAGS_benchmark_min_time then we
   // use the multiplier directly.
   // Otherwise we use at most 10 times expansion.
   // NOTE: When the last run was at least 10% of the min time the max
   // expansion should be 14x.
-  const bool is_significant = (i.seconds / GetMinTimeToApply()) > 0.1;
+  const bool is_significant = (seconds / GetMinTimeToApply()) > 0.1;
   multiplier = is_significant ? multiplier : 10.0;
 
   // So what seems to be the sufficiently-large iteration count? Round up.
@@ -399,15 +418,17 @@ bool BenchmarkRunner::ShouldReportIterationResults(
   // Determine if this run should be reported;
   // Either it has run for a sufficient amount of time
   // or because an error was reported.
+  const double seconds = GetTimeForDecision(i);
+  // real_time_used is accumulated across all threads; use the per-thread value
+  // so the guard below scales the same way regardless of thread count.
+  const double real_time_used = i.results.real_time_used / b.threads();
   return (i.results.skipped_ != 0u) || FLAGS_benchmark_dry_run ||
-         i.iters >= kMaxIterations ||  // Too many iterations already.
-         i.seconds >=
-             GetMinTimeToApply() ||  // The elapsed time is large enough.
+         i.iters >= kMaxIterations ||       // Too many iterations already.
+         seconds >= GetMinTimeToApply() ||  // The elapsed time is large enough.
          // CPU time is specified but the elapsed real time greatly exceeds
          // the minimum time.
          // Note that user provided timers are except from this test.
-         ((i.results.real_time_used >= 5 * GetMinTimeToApply()) &&
-          !b.use_manual_time());
+         ((real_time_used >= 5 * GetMinTimeToApply()) && !b.use_manual_time());
 }
 
 double BenchmarkRunner::GetMinTimeToApply() const {

src/benchmark_runner.h

@@ -113,6 +113,11 @@ class BenchmarkRunner {
 
   bool ShouldReportIterationResults(const IterationResults& i) const;
 
+  // Returns the elapsed time, in seconds, that the min-time stopping decision
+  // should be based on. real/manual time (and whole-process CPU time) are
+  // accumulated across all threads, so this returns the per-thread value.
+  double GetTimeForDecision(const IterationResults& i) const;
+
   double GetMinTimeToApply() const;
 
   void FinishWarmUp(const IterationCount& i);

test/CMakeLists.txt

@@ -259,6 +259,7 @@ if (BENCHMARK_ENABLE_GTEST_TESTS)
   add_gtest(reporter_list_gtest)
   add_gtest(time_unit_gtest)
   add_gtest(min_time_parse_gtest)
+  add_gtest(min_time_threads_gtest)
   add_gtest(profiler_manager_gtest)
   add_gtest(benchmark_setup_teardown_cb_types_gtest)
   add_gtest(memory_results_gtest)

test/min_time_threads_gtest.cc

@@ -0,0 +1,83 @@
+#include <chrono>
+#include <thread>
+#include <vector>
+
+#include "benchmark/benchmark.h"
+#include "gtest/gtest.h"
+
+// Regression test for #2117: the --benchmark_min_time budget must be respected
+// *per thread*. Because real/manual time (and whole-process CPU time) are
+// accumulated across all threads, the min-time stopping decision has to divide
+// that accumulated time by the thread count. Otherwise a benchmark that uses
+// N threads stops after only min_time/N of wall-clock time per thread.
+
+namespace {
+
+using benchmark::Benchmark;
+using benchmark::ClearRegisteredBenchmarks;
+using benchmark::ConsoleReporter;
+using benchmark::RegisterBenchmark;
+using benchmark::RunSpecifiedBenchmarks;
+using benchmark::State;
+
+constexpr double kMinTime = 0.1;  // seconds, matches the flag set below.
+
+void BM_ThreadedSleep(State& state) {
+  for (auto _ : state) {
+    std::this_thread::sleep_for(std::chrono::microseconds(200));
+  }
+}
+
+class TestReporter : public ConsoleReporter {
+ public:
+  bool ReportContext(const Context& /*unused*/) override { return true; }
+  void PrintHeader(const Run&) override {}
+  void PrintRunData(const Run& run) override {
+    // Ignore aggregate rows (mean/median/stddev), keep the real runs.
+    if (run.repetition_index < 0) return;
+    runs.push_back(run);
+  }
+
+  // Wall-clock time a single thread spent in the run with `threads` threads.
+  double PerThreadWallTime(int threads) const {
+    for (const auto& run : runs) {
+      if (run.threads == threads) {
+        // real_accumulated_time is summed across all threads.
+        return run.real_accumulated_time / static_cast<double>(run.threads);
+      }
+    }
+    return -1.0;
+  }
+
+  std::vector<Run> runs;
+};
+
+}  // namespace
+
+TEST(MinTimeThreadsTest, MinTimeRespectedPerThread) {
+  RegisterBenchmark("BM_ThreadedSleep", BM_ThreadedSleep)
+      ->UseRealTime()
+      ->Threads(1)
+      ->Threads(4);
+
+  const char* argv[] = {"min_time_threads_gtest", "--benchmark_min_time=0.1s"};
+  int argc = 2;
+  benchmark::Initialize(&argc, const_cast<char**>(argv));
+
+  TestReporter reporter;
+  RunSpecifiedBenchmarks(&reporter, "BM_ThreadedSleep");
+  ClearRegisteredBenchmarks();
+
+  const double wall_1 = reporter.PerThreadWallTime(1);
+  const double wall_4 = reporter.PerThreadWallTime(4);
+  ASSERT_GT(wall_1, 0.0) << "single-threaded run missing";
+  ASSERT_GT(wall_4, 0.0) << "4-threaded run missing";
+
+  // Each thread should run for roughly min_time, independent of thread count.
+  EXPECT_GE(wall_1, 0.7 * kMinTime);
+  // The core assertion: with the bug the 4-thread run stops after ~min_time/4
+  // per thread, so this would be ~0.25 * wall_1 instead of ~wall_1.
+  EXPECT_GE(wall_4, 0.7 * wall_1)
+      << "per-thread wall time collapsed with more threads: 1-thread=" << wall_1
+      << "s 4-thread=" << wall_4 << "s";
+}