fix: exclude paused sections from instrumentation measurement

[not-matthias]

PauseTiming/ResumeTiming now stop/start Callgrind instrumentation in
CODSPEED_ANALYSIS mode, ensuring setup/teardown code is not included
in benchmark measurements.

Depends on CodSpeedHQ/instrument-hooks#29

[codspeed-hq]

Merging this PR will improve performance by ×72

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 9 improved benchmarks
✅ 453 untouched benchmarks

Performance Changes

	Mode	Benchmark	BASE	HEAD	Efficiency
⚡	Simulation	BM_large_setup	13,179,310.3 ns	883.1 ns	×15,000
⚡	Simulation	BM_large_setup_teardown	13,179,436.1 ns	989.7 ns	×13,000
⚡	Simulation	BM_large_setup	10,561,210.6 ns	913.9 ns	×12,000
⚡	Simulation	BM_large_setup_teardown	10,561,302.5 ns	933.6 ns	×11,000
⚡	WallTime	BM_Vector_PushBack[10000]	39.7 µs	34 µs	+16.63%
⚡	WallTime	BM_Vector_Reserve[10000]	36.1 µs	31.2 µs	+15.46%
⚡	WallTime	BM_Vector_PushBack[1000]	4.3 µs	3.7 µs	+14.83%
⚡	WallTime	BM_Vector_Reserve[1000]	3.6 µs	3.2 µs	+14.59%
⚡	WallTime	BM_Vector_Reserve[100]	390.6 ns	350.7 ns	+11.36%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing cod-2033-codspeed-cpp-includes-setupteardown-in-instrumentation-mode (e47ecff) with main (f5a917f)}

[avalanche-staging]

Congrats! CodSpeed is installed 🎉

🆕 468 new benchmarks were detected.

You will start to see performance impacts in the reports once the benchmarks are run from your default branch.

Detected benchmarks

• FooTest[MyFixture] (WallTime): < 1 ns
• BM_sleep_50ns (WallTime): 125.9 ns
• BM_FibonacciRecursive[35] (WallTime): 67.6 ms
• BarTest[MyFixture] (WallTime): < 1 ns
• BM_sleep_10us (WallTime): 10.1 µs
• BM_sleep_10ns (WallTime): 83.5 ns
• BM_FibonacciIterative[50] (WallTime): 62.9 ns
• BM_sleep_100ns (WallTime): 173.5 ns
• BM_sleep_100us (WallTime): 100.1 µs
• IntTest[MyTemplatedFixture, int] (WallTime): < 1 ns
• BM_sleep_1us (WallTime): 1.1 µs
• BM_sleep_50ms (WallTime): 50 ms
• BM_sleep_1ms (WallTime): 1 ms
• BM_sleep_50us (WallTime): 50.1 µs
• DoubleTest[MyTemplatedFixture, double] (WallTime): < 1 ns
• TestA[MyTemplate1, int] (WallTime): < 1 ns
• TestB[MyTemplate2, int, double] (WallTime): < 1 ns
• BM_RLE_Encode_SmallRuns[100000] (WallTime): 1.6 ms
• BM_RLE_Encode_SmallRuns[100] (WallTime): 1.8 µs
• BM_RLE_Encode_SmallRuns[10000] (WallTime): 154.5 µs
• ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

---

[greptile-apps]

Greptile Summary

This PR wires PauseTiming / ResumeTiming into Callgrind's toggle-collect mechanism so that setup/teardown code bracketed by those calls is excluded from instruction counts in CODSPEED_ANALYSIS mode. It also bumps the instrument-hooks submodule to a commit that exposes callgrind_toggle_collect().

• measurement.hpp gains an inline bool measurement_collecting guard (correctly using C++17 inline to share one definition across TUs) and two ALWAYS_INLINE helpers that pair the flag with callgrind_toggle_collect(), with no-op stubs for non-analysis builds.
• benchmark.cc drops the #ifdef CODSPEED_WALLTIME guard around the measurement.hpp include (now always needed) and inserts measurement_pause_timing() / measurement_resume_timing() calls in PauseTiming, ResumeTiming, SkipWithMessage, and SkipWithError.

Confidence Score: 4/5

The core toggle-collect logic is correct for the common case, but a benchmark that ends its loop body with PauseTiming() will leave measurement_collecting=false; the next benchmark then starts with collection already logically off, corrupting its instruction count. One targeted fix to measurement_start() resolves it.

The missing reset of measurement_collecting between benchmarks means any test suite where a benchmark uses PauseTiming-for-teardown will silently record wrong instruction counts for the benchmark that follows it. This is a real, reproducible defect on the changed path — not a theoretical concern — but it only manifests in multi-benchmark runs with that teardown pattern.

core/include/measurement.hpp — measurement_start() needs to reset measurement_collecting to true so subsequent benchmarks always begin with collection in the expected state.

Important Files Changed

Filename	Overview
core/include/measurement.hpp	Adds inline measurement_collecting flag and pause/resume helpers; correctly uses C++17 inline for ODR safety, but the flag is never reset to true between benchmarks, so if a benchmark ends mid-pause the next one inherits wrong toggle state
google_benchmark/src/benchmark.cc	Calls measurement_pause/resume_timing in PauseTiming, ResumeTiming, SkipWithMessage, SkipWithError; the call sites are structurally sound but rely on the missing between-benchmark reset being fixed
core/CMakeLists.txt	Bumps instrument-hooks GIT_TAG to a specific commit hash that adds callgrind_toggle_collect(); pinned by hash consistent with the repository's convention
core/instrument-hooks	Submodule pointer updated to match new CMakeLists.txt GIT_TAG; no code changes in this repo

Sequence Diagram

sequenceDiagram
    participant User as User Benchmark
    participant State as State (benchmark.cc)
    participant Meas as measurement.hpp
    participant CG as Callgrind

    Note over State,CG: State::end() — range-for setup
    State->>State: StartKeepRunning() → ResumeTiming()
    State->>Meas: measurement_resume_timing()
    Note right of Meas: measurement_collecting=true → no-op
    State->>Meas: measurement_start()
    Meas->>CG: CALLGRIND_ZERO_STATS
    Meas->>CG: CALLGRIND_START_INSTRUMENTATION

    Note over User,CG: Benchmark loop body
    User->>State: PauseTiming()
    State->>Meas: measurement_pause_timing()
    Note right of Meas: collecting=true → toggle OFF
    Meas->>CG: callgrind_toggle_collect()
    Note right of CG: collection OFF

    User->>State: ResumeTiming()
    State->>Meas: measurement_resume_timing()
    Note right of Meas: collecting=false → toggle ON
    Meas->>CG: callgrind_toggle_collect()
    Note right of CG: collection ON

    Note over User,CG: Loop ends
    State->>Meas: measurement_stop()
    Meas->>CG: CALLGRIND_STOP_INSTRUMENTATION
    State->>State: FinishKeepRunning() → PauseTiming()
    State->>Meas: measurement_pause_timing()
    Note right of Meas: collecting=true → toggle OFF, collecting=false
    Meas->>CG: callgrind_toggle_collect()

Loading

_{Reviews (6): Last reviewed commit: "fix: exclude paused sections from instru..." | Re-trigger Greptile}

[GuillaumeLagrange]

olgtm

[GuillaumeLagrange]

I had not noticed that at the valgrind level there was only toggle. I dont think we should expose explicit start stop then, lgtm