ref: exclude benchmark harness from measurement via collection steady-state

Toggle callgrind collection off once at process start and make the
PauseTiming()/ResumeTiming() toggles unconditional. Collection is now
only enabled inside the benchmark loop, so State setup, timer reads and
instrument-hooks zero/dump requests no longer appear in the measurement,
and the codspeed_in_benchmark_loop_ gating flag is no longer needed.

The toggle is inlined via CALLGRIND_TOGGLE_COLLECT directly (instead of
calling the instrument-hooks wrapper) so no toggle frame shows up in
flamegraphs; the counted boundary shrinks to the ResumeTiming() epilogue
(~6 instructions). SkipWithMessage/SkipWithError restore the toggle
parity when a benchmark is skipped mid-loop without pausing.

Refs COD-2033

Author: not-matthias

google_benchmark/include/benchmark/benchmark.h

@@ -949,15 +949,6 @@ class BENCHMARK_EXPORT BENCHMARK_INTERNAL_CACHELINE_ALIGNED State {
 #if defined(CODSPEED_ANALYSIS) || defined(CODSPEED_WALLTIME)
   codspeed::CodSpeed* codspeed_;
 #endif
-#ifdef CODSPEED_ANALYSIS
-  // True between the implicit ResumeTiming() in StartKeepRunning() and the
-  // implicit PauseTiming() in FinishKeepRunning(). Only explicit user
-  // PauseTiming()/ResumeTiming() calls (inside the benchmark loop) may
-  // toggle callgrind collection: CALLGRIND_TOGGLE_COLLECT is parity-based,
-  // and collection starts enabled, so the implicit bracket calls must not
-  // toggle or they would invert the collection state for the whole run.
-  bool codspeed_in_benchmark_loop_;
-#endif
 #ifdef CODSPEED_WALLTIME
   uint64_t resume_timestamp_;
 #endif

google_benchmark/src/benchmark.cc

@@ -61,6 +61,24 @@
 #include "thread_timer.h"
 
 namespace benchmark {
+
+#ifdef CODSPEED_ANALYSIS
+// Callgrind cost collection is kept off outside the benchmark loop (one
+// initial toggle below, inverting the --collect-atstart=yes default) and
+// toggled on/off by ResumeTiming()/PauseTiming(). This keeps harness code and
+// paused sections out of the measurement without flushing the simulated cache
+// (unlike CALLGRIND_STOP_INSTRUMENTATION). The toggle must be the first
+// statement in PauseTiming() and the last in ResumeTiming().
+#define CODSPEED_TOGGLE_COLLECT() CALLGRIND_TOGGLE_COLLECT
+namespace {
+struct CodSpeedCollectInit {
+  CodSpeedCollectInit() { CODSPEED_TOGGLE_COLLECT(); }
+} const codspeed_collect_init;
+}  // namespace
+#else
+#define CODSPEED_TOGGLE_COLLECT()
+#endif
+
 // Print a list of benchmarks. This option overrides all other options.
 BM_DEFINE_bool(benchmark_list_tests, false);
 
@@ -199,9 +217,6 @@ State::State(std::string name, IterationCount max_iters,
 #if defined(CODSPEED_ANALYSIS) || defined(CODSPEED_WALLTIME)
       codspeed_(codspeed),
 #endif
-#ifdef CODSPEED_ANALYSIS
-      codspeed_in_benchmark_loop_(false),
-#endif
 #ifdef CODSPEED_WALLTIME
       resume_timestamp_(0),
 #endif
@@ -275,15 +290,7 @@ void State::PauseTiming() {
 #ifdef CODSPEED_WALLTIME
   uint64_t pause_timestamp = measurement_current_timestamp();
 #endif
-#ifdef CODSPEED_ANALYSIS
-  // Suspend callgrind cost collection for the paused section. Toggling
-  // collection (unlike stopping instrumentation) does not flush the
-  // simulated cache, so it adds no artificial cold-cache cost to the
-  // measured region.
-  if (codspeed_in_benchmark_loop_) {
-    callgrind_toggle_collect();
-  }
-#endif
+  CODSPEED_TOGGLE_COLLECT();
 
   // Add in time accumulated so far
   BM_CHECK(started_ && !finished_ && !skipped());
@@ -322,12 +329,7 @@ void State::ResumeTiming() {
   BM_CHECK(resume_timestamp_ == 0);
   resume_timestamp_ = measurement_current_timestamp();
 #endif
-#ifdef CODSPEED_ANALYSIS
-  // Re-enable callgrind cost collection after a paused section.
-  if (codspeed_in_benchmark_loop_) {
-    callgrind_toggle_collect();
-  }
-#endif
+  CODSPEED_TOGGLE_COLLECT();
 }
 
 void State::SkipWithMessage(const std::string& msg) {
@@ -342,6 +344,8 @@ void State::SkipWithMessage(const std::string& msg) {
   total_iterations_ = 0;
   if (timer_->running()) {
     timer_->StopTimer();
+    // Skipped mid-loop without pausing: restore the collection-off state.
+    CODSPEED_TOGGLE_COLLECT();
   }
 }
 
@@ -357,6 +361,8 @@ void State::SkipWithError(const std::string& msg) {
   total_iterations_ = 0;
   if (timer_->running()) {
     timer_->StopTimer();
+    // Skipped mid-loop without pausing: restore the collection-off state.
+    CODSPEED_TOGGLE_COLLECT();
   }
 }
 
@@ -379,22 +385,12 @@ void State::StartKeepRunning() {
   manager_->StartStopBarrier();
   if (!skipped()) {
     ResumeTiming();
-#ifdef CODSPEED_ANALYSIS
-    // Arm collection toggling only after the implicit ResumeTiming() above,
-    // so that only explicit user pauses inside the loop toggle collection.
-    codspeed_in_benchmark_loop_ = true;
-#endif
   }
 }
 
 void State::FinishKeepRunning() {
   BM_CHECK(started_ && (!finished_ || skipped()));
   if (!skipped()) {
-#ifdef CODSPEED_ANALYSIS
-    // Disarm before the implicit PauseTiming() below so it does not toggle
-    // collection off for the rest of the process.
-    codspeed_in_benchmark_loop_ = false;
-#endif
     PauseTiming();
   }
   // Total iterations has now wrapped around past 0. Fix this.