fix: exclude paused sections from instrumentation measurement by not-matthias · Pull Request #44 · CodSpeedHQ/codspeed-cpp

not-matthias · 2026-03-13T18:37:26Z

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 · 2026-03-13T18:47:41Z

Merging this PR will improve performance by ×32

⚠️

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
❌ 2 regressed benchmarks
✅ 451 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	`BASE`	`HEAD`	Efficiency
❌	WallTime	`BM_memcpy[8192]`	472.7 ns	540.1 ns	-12.49%
❌	WallTime	`BM_FibonacciRecursive_Darwin[35]`	35.3 ms	39.6 ms	-10.78%
⚡	Simulation	`BM_large_setup`	13,179,310.3 ns	882.8 ns	×15,000
⚡	Simulation	`BM_large_setup_teardown`	13,179,436.1 ns	989.2 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.1 µs	+16.49%
⚡	WallTime	`BM_Vector_Reserve[10000]`	36.1 µs	31.2 µs	+15.46%
⚡	WallTime	`BM_Vector_Reserve[1000]`	3.6 µs	3.2 µs	+14.54%
⚡	WallTime	`BM_Vector_PushBack[1000]`	4.3 µs	3.7 µs	+14.25%
⚡	WallTime	`BM_Vector_Reserve[100]`	390.6 ns	350.8 ns	+11.34%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

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

avalanche-staging · 2026-03-16T16:58:09Z

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 · 2026-06-05T16:30:52Z

Greptile Summary

This PR hooks PauseTiming / ResumeTiming (and the skip paths) into Callgrind's collection toggle so that setup/teardown code is excluded from instruction-count measurements in CODSPEED_ANALYSIS mode. A static bool measurement_collecting guard prevents double-toggling when FinishKeepRunning calls PauseTiming after the user has already paused at the end of a loop iteration.

Adds measurement_pause_timing / measurement_resume_timing in measurement.hpp (analysis-only) that call callgrind_toggle_collect() from the bumped instrument-hooks submodule, using a boolean guard to keep toggle parity correct across nested or repeated calls.
Wires the two new helpers into State::PauseTiming, State::ResumeTiming, State::SkipWithMessage, and State::SkipWithError in benchmark.cc, and removes the #ifdef CODSPEED_WALLTIME guard around the measurement.hpp include so analysis builds also see the header.

Confidence Score: 4/5

Safe to merge with minor follow-up; the core toggle logic is sound and the boolean guard correctly handles all PauseTiming/ResumeTiming parity scenarios.

The Callgrind toggle guard is designed correctly and handles double-pause, post-loop-pause, and skip paths. The main concern is measurement_collecting being a file-scope static in a header included transitively by the public benchmark/benchmark.h: every benchmark TU compiled with CODSPEED_ANALYSIS gets a silent independent copy, and the ALWAYS_INLINE functions that reference it are visible to user code. The current code only ever calls them from benchmark.cc, so runtime behavior is correct today, but the design is fragile under the public header exposure.

core/include/measurement.hpp — the static bool measurement_collecting definition should live in a .cpp file rather than a widely-included header.

Important Files Changed

Filename	Overview
core/include/measurement.hpp	Adds measurement_pause_timing / measurement_resume_timing helpers that use a boolean guard to prevent double-toggling of Callgrind collection; guard variable is defined as a file-scope static in a header, creating one copy per TU.
google_benchmark/src/benchmark.cc	Integrates measurement_pause_timing / measurement_resume_timing into PauseTiming, ResumeTiming, SkipWithMessage, and SkipWithError; measurement_resume_timing is called after timer_->StartTimer() (asymmetric vs. PauseTiming).
core/CMakeLists.txt	Bumps instrument-hooks GIT_TAG to b6c4a75, which adds the callgrind_toggle_collect C API used by the new helpers.
core/instrument-hooks	Submodule pointer updated to match the new instrument-hooks tag; no code changes in this repo.

Sequence Diagram

sequenceDiagram
    participant U as User Benchmark
    participant S as State (benchmark.cc)
    participant M as measurement.hpp
    participant CG as Callgrind

    Note over CG: collection ON (--collect-at-start=yes)
    U->>S: StartKeepRunning()
    S->>S: ResumeTiming()
    S->>M: measurement_resume_timing()
    Note over M: measurement_collecting==true → no-op (first benchmark)

    loop Benchmark iterations
        Note over CG: counting instructions
        U->>S: PauseTiming() [optional]
        S->>M: measurement_pause_timing()
        M->>CG: callgrind_toggle_collect() → OFF
        Note over M: measurement_collecting = false
        U->>S: ResumeTiming() [optional]
        S->>M: measurement_resume_timing()
        M->>CG: callgrind_toggle_collect() → ON
        Note over M: measurement_collecting = true
    end

    S->>S: FinishKeepRunning() → PauseTiming()
    S->>M: measurement_pause_timing()
    M->>CG: callgrind_toggle_collect() → OFF
    Note over M: measurement_collecting = false

    Note over S: Next benchmark
    S->>S: StartKeepRunning() → ResumeTiming()
    S->>M: measurement_resume_timing()
    M->>CG: callgrind_toggle_collect() → ON
    Note over M: measurement_collecting = true

_{Reviews (3): Last reviewed commit: "ref: exclude benchmark harness from meas..." | Re-trigger Greptile}

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

Replace the per-pause CALLGRIND_STOP/START_INSTRUMENTATION calls with CALLGRIND_TOGGLE_COLLECT. Toggling instrumentation flushes callgrind's simulated cache, so every benchmark measured an artificial cold-cache warmup phase and the cache-warming repetition in RunAnalysis was wasted, regressing 76 benchmarks. Toggling collection excludes paused sections without touching simulator state. Gate the toggles to explicit user PauseTiming()/ResumeTiming() calls: TOGGLE_COLLECT is parity-based and collection starts enabled, so the implicit StartKeepRunning()/FinishKeepRunning() bracket must not toggle or it would invert the collection state for the whole run. Bump instrument-hooks for the new callgrind_toggle_collect helper. Fixes COD-2033

…-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

GuillaumeLagrange

olgtm

GuillaumeLagrange · 2026-06-10T09:19:30Z

+  // simulated cache, so it adds no artificial cold-cache cost to the
+  // measured region.
+  if (codspeed_in_benchmark_loop_) {
+    callgrind_toggle_collect();


We are in an explicit "PauseTiming" function, using a toggle function rather than an explicit "stop_callgrind" function is most likely a source of bugs

GuillaumeLagrange · 2026-06-10T09:21:09Z

-  callgrind_start_instrumentation();
+  // Re-enable callgrind cost collection after a paused section.
+  if (codspeed_in_benchmark_loop_) {
+    callgrind_toggle_collect();


same remark here

GuillaumeLagrange · 2026-06-10T09:21:34Z

+  // 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.


Comments are a bit superfluous here

GuillaumeLagrange · 2026-06-10T09:23:57Z

+  // 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.


Can we trim down this comment and just focus on the sequence of events with the implicit pause/resume that explain why we need this?

Also, if we switch to explicit start and stop callgrind functions, is this still needed?

GuillaumeLagrange · 2026-06-10T09:33:48Z

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

not-matthias force-pushed the cod-2033-codspeed-cpp-includes-setupteardown-in-instrumentation-mode branch 2 times, most recently from 646ed4c to 696eaa1 Compare March 18, 2026 09:52

not-matthias marked this pull request as ready for review March 18, 2026 09:52

GuillaumeLagrange approved these changes Mar 18, 2026

View reviewed changes

greptile-apps Bot reviewed Jun 5, 2026

View reviewed changes

Comment thread google_benchmark/src/benchmark.cc Outdated

Comment thread google_benchmark/include/benchmark/benchmark.h Outdated

not-matthias force-pushed the cod-2033-codspeed-cpp-includes-setupteardown-in-instrumentation-mode branch from 4dd226e to 8f254f0 Compare June 8, 2026 09:31

not-matthias added 3 commits June 8, 2026 11:33

fix: exclude paused sections from instrumentation measurement

1eae3cc

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

not-matthias force-pushed the cod-2033-codspeed-cpp-includes-setupteardown-in-instrumentation-mode branch from 8f254f0 to 5f8b3af Compare June 8, 2026 09:33

not-matthias requested a review from GuillaumeLagrange June 8, 2026 09:53

GuillaumeLagrange requested changes Jun 10, 2026

View reviewed changes

GuillaumeLagrange approved these changes Jun 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: exclude paused sections from instrumentation measurement#44

fix: exclude paused sections from instrumentation measurement#44
not-matthias wants to merge 3 commits into
mainfrom
cod-2033-codspeed-cpp-includes-setupteardown-in-instrumentation-mode

not-matthias commented Mar 13, 2026 •

edited

Loading

Uh oh!

codspeed-hq Bot commented Mar 13, 2026 •

edited

Loading

Uh oh!

avalanche-staging Bot commented Mar 16, 2026 •

edited

Loading

Detected benchmarks

Uh oh!

greptile-apps Bot commented Jun 5, 2026 •

edited

Loading

Sequence Diagram

Uh oh!

Uh oh!

Uh oh!

GuillaumeLagrange left a comment

Uh oh!

GuillaumeLagrange Jun 10, 2026

Uh oh!

GuillaumeLagrange Jun 10, 2026

Uh oh!

GuillaumeLagrange Jun 10, 2026

Uh oh!

GuillaumeLagrange Jun 10, 2026

Uh oh!

GuillaumeLagrange commented Jun 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

not-matthias commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codspeed-hq Bot commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will improve performance by ×32

Performance Changes

Uh oh!

avalanche-staging Bot commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Congrats! CodSpeed is installed 🎉

Detected benchmarks

Uh oh!

greptile-apps Bot commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Sequence Diagram

Uh oh!

Uh oh!

Uh oh!

GuillaumeLagrange left a comment

Choose a reason for hiding this comment

Uh oh!

GuillaumeLagrange Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

GuillaumeLagrange Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

GuillaumeLagrange Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

GuillaumeLagrange Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

GuillaumeLagrange commented Jun 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

not-matthias commented Mar 13, 2026 •

edited

Loading

codspeed-hq Bot commented Mar 13, 2026 •

edited

Loading

avalanche-staging Bot commented Mar 16, 2026 •

edited

Loading

greptile-apps Bot commented Jun 5, 2026 •

edited

Loading