[jasoneplumb]

Summary

Closes the cyclescope build-out: docs/design.md with the trade-off rationale behind every load-bearing decision, plus bench/overhead_bench producing the measured numbers the doc cites.

Closes #4

Changes

• docs/design.md: per-thread buffer contracts, the suppression-flag design (the ODR self-deadlock and unoptimized-recursion hazards and how the guard closes them), teardown immortality, backend and emitter choices, measured overhead with repro commands, stated limits
• bench/overhead_bench.cpp + instrumented fib twin: scope-macro cost vs empty loop, hook cost vs plain fib, divergence-checked
• CMake: CYCLESCOPE_BUILD_BENCH (default ON, non-MSVC)

Measured (arm64 macOS, 8 logical cores, Apple clang, Release)

Scope macro 136 ns/event (1M scopes); compiler instrumentation 86 ns/call (fib(26), 635,621 calls). Framed in the doc as one machine, one data point, with the granularity guidance that follows.

Test plan

• Full suite 3/3 green locally; benchmark output reproduced in the doc; em-dash and reference sweeps clean

🤖 Generated with Claude Code

[claude]

Logic error: loop iterates one too many times, returning 2*fib(n+2)-1 instead of 2*fib(n+1)-1.

Starting from (a=0, b=1) = (fib(0), fib(1)), each iteration advances by one Fibonacci step. After n+1 iterations b = fib(n+2), so fib_call_count returns 2*fib(n+2)-1 — not the 2*fib(n+1)-1 stated in the comment (which is the correct total-call formula).

Concrete impact for fib_n=26:

• Actual calls T(26) = 2*fib(27)−1 = 392,835
• fib_call_count(26) returns 2*fib(28)−1 = 635,621 (~1.618× too high)
• measure_instrumentation divides by this inflated count, so the printed ns/call is ~39% lower than the true per-call overhead
• docs/design.md reports "635,621 calls" which propagates this error into the published numbers

Suggested change

	long b = 1;
	for (int i = 0; i < n; ++i) {

With i < n the loop runs n iterations, giving b = fib(n+1) and the correct return value 2*fib(n+1)-1.

[claude]

Review summary

One substantive issue found; everything else looks good.

Bug: fib_call_count is off-by-one (bench/overhead_bench.cpp line 31)

The loop runs n+1 iterations instead of n, so b ends up as fib(n+2) rather than fib(n+1). The function returns 2*fib(n+2)-1 instead of the stated 2*fib(n+1)-1.

Downstream effects:

• measure_instrumentation pre-allocates ~1.618x too many event slots (harmless but wasteful)
• More importantly, dividing (traced - plain) by the inflated call count makes the printed ns/call ~39% lower than reality (for fib(26): reported 635,621 calls vs actual 392,835)
• docs/design.md inherits the wrong "635,621 calls" figure and the understated per-call overhead

Fix: i < n + 1 → i < n.

---

Everything else reviewed and looks correct:

• CMake wiring (per-file COMPILE_OPTIONS, ENABLE_EXPORTS, sanitizer pass-through)
• overhead_fib.cpp instrumented-twin structure
• Suppression-flag + immortal-singleton design in trace.hpp / instrument.cpp
• Input validation in main() (strtoll, atoi, range checks)
• JSON footer guarantee and lock-free snapshot path in write_json

[jasoneplumb]

Finding applied and re-measured: the loop now computes fib(n+1) as stated, the call count for fib(26) is 392,835, and the honest per-call figure is 136.4 ns rather than the understated 86. The corrected doc notes what the corrected numbers show: both paths cost the same per event because the shared record path dominates. Single finding; merging on green.

🤖 Generated with Claude Code