fix: exclude warmup measurements

Author: not-matthias

google_benchmark/include/benchmark/benchmark.h

@@ -956,6 +956,7 @@ class BENCHMARK_EXPORT BENCHMARK_INTERNAL_CACHELINE_ALIGNED State {
  private:
   bool started_;
   bool finished_;
+  bool is_warmup_;
   internal::Skipped skipped_;
 
   // items we don't need on the first cache line
@@ -977,6 +978,8 @@ class BENCHMARK_EXPORT BENCHMARK_INTERNAL_CACHELINE_ALIGNED State {
         ,
         codspeed::CodSpeed* codspeed = NULL
 #endif
+        ,
+        bool is_warmup = false
   );
 
   void StartKeepRunning();

google_benchmark/src/benchmark.cc

@@ -190,6 +190,8 @@ State::State(std::string name, IterationCount max_iters,
              ,
              codspeed::CodSpeed* codspeed
 #endif
+             ,
+             bool is_warmup
              )
     : total_iterations_(0),
       batch_leftover_(0),
@@ -202,6 +204,7 @@ State::State(std::string name, IterationCount max_iters,
 #endif
       started_(false),
       finished_(false),
+      is_warmup_(is_warmup),
       skipped_(internal::NotSkipped),
       range_(ranges),
       complexity_n_(0),
@@ -275,7 +278,7 @@ void State::PauseTiming() {
   timer_->StopTimer();
 
 #ifdef CODSPEED_WALLTIME
-  if (resume_timestamp_ != 0) {
+  if (!is_warmup_ && resume_timestamp_ != 0) {
     measurement_add_benchmark_timestamps(resume_timestamp_, pause_timestamp);
     resume_timestamp_ = 0;
   }
@@ -566,8 +569,9 @@ void RunBenchmarks(const std::vector<BenchmarkInstance>& benchmarks,
 
 #ifdef CODSPEED_WALLTIME
       auto* codspeed = codspeed::CodSpeed::getInstance();
-      if (codspeed != nullptr) {
+      if (codspeed != nullptr && runner.IsFirstRepetition()) {
         codspeed->start_benchmark(runner.GetBenchmarkName());
+        measurement_start();
       }
 #endif
 
@@ -601,6 +605,7 @@ void RunBenchmarks(const std::vector<BenchmarkInstance>& benchmarks,
 
 #ifdef CODSPEED_WALLTIME
       if (codspeed != nullptr) {
+        measurement_stop();
         codspeed->end_benchmark();
       }
 

google_benchmark/src/benchmark_api_internal.cc

@@ -118,13 +118,13 @@ State BenchmarkInstance::__codspeed_root_frame__Run(
     IterationCount iters, int thread_id, internal::ThreadTimer* timer,
     internal::ThreadManager* manager,
     internal::PerfCountersMeasurement* perf_counters_measurement,
-    ProfilerManager* profiler_manager) const {
-#ifdef CODSPEED_WALLTIME
+    ProfilerManager* profiler_manager, bool is_warmup) const {
+#if defined(CODSPEED_SIMULATION) || defined(CODSPEED_WALLTIME)
   State st(name_.function_name, iters, args_, thread_id, threads_, timer,
-           manager, perf_counters_measurement, profiler_manager, codspeed::CodSpeed::getInstance());
+           manager, perf_counters_measurement, profiler_manager, codspeed::CodSpeed::getInstance(), is_warmup);
 #else
   State st(name_.function_name, iters, args_, thread_id, threads_, timer,
-           manager, perf_counters_measurement, profiler_manager);
+           manager, perf_counters_measurement, profiler_manager, is_warmup);
 #endif
   benchmark_.Run(st);
   return st;

google_benchmark/src/benchmark_api_internal.h

@@ -53,9 +53,9 @@ class BenchmarkInstance {
   void Teardown() const;
 
   State __codspeed_root_frame__Run(IterationCount iters, int thread_id, internal::ThreadTimer* timer,
-            internal::ThreadManager* manager,
-            internal::PerfCountersMeasurement* perf_counters_measurement,
-            ProfilerManager* profiler_manager) const;
+                internal::ThreadManager* manager,
+                internal::PerfCountersMeasurement* perf_counters_measurement,
+                ProfilerManager* profiler_manager, bool is_warmup = false) const;
 
 #ifdef CODSPEED_SIMULATION
   State RunSimulation(

google_benchmark/src/benchmark_runner.cc

@@ -135,14 +135,14 @@ BenchmarkReporter::Run CreateRunReport(
 void RunInThread(const BenchmarkInstance* b, IterationCount iters,
                  int thread_id, ThreadManager* manager,
                  PerfCountersMeasurement* perf_counters_measurement,
-                 ProfilerManager* profiler_manager_) {
+                 ProfilerManager* profiler_manager_, bool is_warmup = false) {
   internal::ThreadTimer timer(
       b->measure_process_cpu_time()
           ? internal::ThreadTimer::CreateProcessCpuTime()
           : internal::ThreadTimer::Create());
 
   State st = b->__codspeed_root_frame__Run(iters, thread_id, &timer, manager,
-                    perf_counters_measurement, profiler_manager_);
+                    perf_counters_measurement, profiler_manager_, is_warmup);
   BM_CHECK(st.skipped() || st.iterations() >= st.max_iterations)
       << "Benchmark returned before State::KeepRunning() returned false!";
   {
@@ -285,7 +285,7 @@ BenchmarkRunner::BenchmarkRunner(
   }
 }
 
-BenchmarkRunner::IterationResults BenchmarkRunner::DoNIterations() {
+BenchmarkRunner::IterationResults BenchmarkRunner::DoNIterations(bool is_warmup) {
   BM_VLOG(2) << "Running " << b.name().str() << " for " << iters << "\n";
 
   std::unique_ptr<internal::ThreadManager> manager;
@@ -295,13 +295,13 @@ BenchmarkRunner::IterationResults BenchmarkRunner::DoNIterations() {
   for (std::size_t ti = 0; ti < pool.size(); ++ti) {
     pool[ti] = std::thread(&RunInThread, &b, iters, static_cast<int>(ti + 1),
                            manager.get(), perf_counters_measurement_ptr,
-                           /*profiler_manager=*/nullptr);
+                           /*profiler_manager=*/nullptr, is_warmup);
   }
   // And run one thread here directly.
   // (If we were asked to run just one thread, we don't create new threads.)
   // Yes, we need to do this here *after* we start the separate threads.
   RunInThread(&b, iters, 0, manager.get(), perf_counters_measurement_ptr,
-              /*profiler_manager=*/nullptr);
+              /*profiler_manager=*/nullptr, is_warmup);
 
   // The main thread has finished. Now let's wait for the other threads.
   manager->WaitForAllThreads();
@@ -405,7 +405,7 @@ void BenchmarkRunner::RunWarmUp() {
 
   for (;;) {
     b.Setup();
-    i_warmup = DoNIterations();
+    i_warmup = DoNIterations(/*is_warmup=*/true);
     b.Teardown();
 
     const bool finish = ShouldReportIterationResults(i_warmup);
@@ -486,12 +486,6 @@ void BenchmarkRunner::DoOneRepetition() {
     RunWarmUp();
   }
 
-  // IMPORTANT: We must not sample the warmup otherwise the flamegraph timings will be incorrect since we
-  // divide by the iteration count.
-#ifdef CODSPEED_WALLTIME
-  measurement_start();
-#endif
-
   IterationResults i;
   // We *may* be gradually increasing the length (iteration count)
   // of the benchmark until we decide the results are significant.
@@ -525,9 +519,6 @@ void BenchmarkRunner::DoOneRepetition() {
            "if we did more iterations than we want to do the next time, "
            "then we should have accepted the current iteration run.");
   }
-#ifdef CODSPEED_WALLTIME
-  measurement_stop();
-#endif
 
   // Produce memory measurements if requested.
   MemoryManager::Result* memory_result = nullptr;

google_benchmark/src/benchmark_runner.h

@@ -60,6 +60,10 @@ class BenchmarkRunner {
     return GetNumRepeats() != num_repetitions_done;
   }
 
+  bool IsFirstRepetition() const {
+    return num_repetitions_done == 0;
+  }
+
   void DoOneRepetition();
 
   RunResults&& GetResults();
@@ -104,7 +108,7 @@ class BenchmarkRunner {
     IterationCount iters;
     double seconds;
   };
-  IterationResults DoNIterations();
+  IterationResults DoNIterations(bool is_warmup = false);
 
   MemoryManager::Result* RunMemoryManager(IterationCount memory_iterations);