ITADN

Flaky Callgrind bench: fast_time instant_elapsed has timing-dependent instruction count

#220Opensandersaares 创建于 2026-05-30
S
sandersaarescommented
## Summary The Callgrind benchmark `fast_time_clock_cg::clock_group::instant_elapsed` (`packages/fast_time/benches/fast_time_clock_cg.rs:78-84`) produces a **timing-dependent instruction count**, so its run-to-run comparison is not deterministic — defeating the purpose of an instruction-count benchmark. ## Details The scenario setup `make_clock_with_anchor()` calls `clock.now()` once (which records `cache_key = clock_gettime_nanos()` for the current `CLOCK_MONOTONIC_COARSE` tick). The measured region then calls `now()` again via `anchor.elapsed(&mut clock)`. Whether the second `now()` hits the cache fast path (`cache_key == platform_time` → return cached) or the arithmetic slow path (`saturating_sub` + `checked_add` + `Duration::from_nanos`) depends on whether `CLOCK_MONOTONIC_COARSE` advanced during the run. Under Valgrind that depends on how much wall-clock time elapses while the (heavily slowed) process runs, which varies with machine load. The two paths have materially different instruction counts, so the reported number flips between runs. Observed: the same code measured `instant_elapsed` at 154, 137, and 169 instructions across different runs/machines. ## Impact - Spurious "regression" / "improvement" deltas unrelated to code changes. - Reviewers cannot trust the `instant_elapsed` number in a Callgrind diff. The sibling `clock_now` scenario is unaffected: it uses a fresh `Clock` (`cache_key == 0`), so it always takes the slow path deterministically. ## Suggested fix Make the cache state at the start of the measured region deterministic, e.g.: - Drive `now()` through a mock/controllable binding in the Callgrind bench so the platform time is fixed, OR - Add two scenarios that each pin one path explicitly (a guaranteed cache-hit variant where setup primes the exact same tick, and a guaranteed cache-miss variant), instead of one scenario whose path is decided by wall-clock timing. ## Discovered While measuring the `u128`→`u64` clock change in PR #219.
0 条评论