Fix the two root causes of CodSpeed flakes: substituted baselines and harness-dominated benchmarks - #9011
Fix the two root causes of CodSpeed flakes: substituted baselines and harness-dominated benchmarks#9011connortsui20 wants to merge 2 commits into
Conversation
Merging this PR will degrade performance by 91.08%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | binary_search_vortex |
486.7 ns | 25,683.6 ns | -98.11% |
| ❌ | Simulation | binary_search_std |
486.1 ns | 15,222.2 ns | -96.81% |
| ⚡ | WallTime | cuda/bitpacked_u8/unpack/3bw[100M] |
351.5 µs | 299.3 µs | +17.42% |
| 🆕 | Simulation | slice_vortex_buffer |
N/A | 5.6 µs | N/A |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/codspeed-flake-rootcause (aa146bf) with develop (b15394a)
Footnotes
-
12 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
Two CI causes of reports flagging benchmarks that the PR cannot affect. Serialised develop baselines. All develop pushes shared one concurrency group and never cancel, so they ran strictly one at a time. A burst of merges leaves later commits without a finished baseline, and CodSpeed then falls back to an older comparison base with its own footnote: "No successful run was found on develop (X), so Y was used instead. There might be some changes unrelated to this pull request in this report." That footnote appears on four of twelve open-PR reports, and it lines up with the benchmarks that flip between two fixed values in opposite directions on unrelated PRs. Give each develop push its own group so baselines still run on every commit, as the comment intends, without queueing behind each other. Develop runs can now overlap, which raises peak demand on a preemptible runner pool. CUDA walltime suite triggered by any workflow edit. The paths filter matched .github/workflows/**, so every dependency bump ran the walltime CUDA benchmarks. Eleven CI-only PRs reported cuda/bitpacked_u8/unpack/3bw[100M] improving 18-19% from an identical base, each carrying CodSpeed's own warning that walltime on standard hosted runners produces inconsistent data. Only this workflow defines those jobs, so match just this file. Non-pull_request events still always run the CUDA suite, so develop keeps full coverage. Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015ypCZPxvLuJK9FgWKF8U4P
One divan bench_refs iteration carries a fixed ~146 instructions of
harness. Where the measured operation costs less than that, the reported
figure tracks the harness rather than the operation, and a small change in
harness cost swings it past CodSpeed's 10% flag threshold with the
measured code untouched. Per-iteration instruction counts (callgrind, the
same instrument class as CodSpeed Simulation) confirm the flagged
benchmarks are exactly the overhead-dominated ones:
true_count_vortex_buffer 1024: 187 Ir (78% harness) <- flagged
2048: 228 Ir (64% harness) <- flagged
16384: 802 Ir (18% harness)
65536: 2770 Ir (5% harness)
The fit is linear at ~0.04 Ir/bit with a ~146 Ir intercept, and no report
ever flagged 16384 or 65536. This also explains why #8861 did not settle
it: dropping 128 (97% harness) just promoted 1024 to worst offender.
- true_count: keep 16384 and 65536, where the popcount dominates.
- slice: BitBuffer::slice only adjusts an offset, a length and a refcount,
so its cost is independent of length -- every size measured exactly 125
Ir, which is why #8749 reported identical values for [1024] and [16384].
Measure one length with enough slices per iteration to dominate, varying
the offset so they cannot be folded together. No black_box is needed:
the slice clones an Arc, and that refcount traffic is a side effect the
optimiser cannot remove.
- binary_search: one search over 65536 elements is ~16 comparisons, and at
~71% harness the two implementations measured 203 vs 206 Ir --
indistinguishable, defeating the comparison the benchmark exists for.
Searching 64 targets per iteration separates them: 15133 Ir for vortex
against 10010 for std.
varbinview_compact was left alone: compact[(4096, 90)] is 1069 Ir, only
14% harness, so its reports belong to the comparison-base problem fixed in
the preceding commit rather than to harness overhead.
Signed-off-by: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ypCZPxvLuJK9FgWKF8U4P
e3b12c8 to
aa146bf
Compare
Rationale for this change
Follow-up to #8861 / #8807. Two causes behind the remaining flakes.
Benchmarks measuring divan's harness, not the operation. An iteration carries ~146 instructions of fixed harness, and only benchmarks below that were ever flagged.
true_countwas 78% / 64% harness at 1024 / 2048 bits, so those sizes are gone.sliceis O(1) — every size measured identically at 125 Ir, which is why #8749 reported the same value for[1024]and[16384]— so it's one length, batched.binary_searchmeasured 203 vs 206 Ir for the two impls, indistinguishable; batched they separate at 15,133 vs 10,010.CI attributing other commits' changes to a PR. Serialised develop baselines meant CodSpeed substituted an older base and said so — "No successful run was found on
develop…" — on 4 of 12 open-PR reports. Now one group per commit, which does let develop runs overlap on a spot pool. Separately the CUDA filter matched.github/workflows/**, so 11 dependency bumps ran the walltime suite; now justcodspeed.yml.The first report here was an instance: a YAML-only commit flagged three benchmarks that vanished once the base was correct.
What changes are included in this PR?
Three files. Checks: callgrind before and after each fix,
clippy --benches,+nightly fmt, smoke runs,yamllint --strict. Rejected after testing:codegen-units = 1, and tight-loopingtrue_count. Numbers are from a dev container, not a CodSpeed runner.