fix(bench-ratchet): report out-of-scope baseline entries as one line, not MISSING rows (stacks on #561) - #570
Merged
Conversation
…hor-drift warning
Single cold samples made the ratchet lie. The committed baseline was
captured with -count=1, no warmup, and mean aggregation; its anchor
(1.847 ns/op) landed in the slow mode of a bimodal distribution the
historical .runs/ captures show clearly on an M3 (~1.06 ns on a P-core
vs ~1.85 ns E-core/throttled — 55 vs 35 of 92 recorded anchor samples).
Since every benchmark is reported as a multiple of the anchor, a
subsequent clean run reads as +38%/+31%/+17% phantom regressions while
raw wall times actually improved across the board (2026-07-18 incident).
Three changes, hyperfine-style:
- reduceSamples: same-named -count reps reduce by discarding the FIRST
rep as warmup (cold caches, first-touch var resolution, CPU clock
ramp — the suite bench visibly climbs 264->319ms across in-process
reps) and taking the MEDIAN of the rest, not the mean — one GC pause
or contention spike no longer skews the stored baseline. Single-rep
captures (legacy .jsonl, explicit -count 1) pass through unchanged.
- defaultCount 1 -> 4, and the suite jobs' count:1 pins -> 4: warmup +
3 kept samples, the minimum for a meaningful median (a median of 2
does not exist). The old pin rationale ("variance is negligible")
is what this incident falsified; a suite pass is ~2.4s today, so the
default gate grows by roughly a minute, not the "minutes per pass"
the comment assumed.
- check mode now prints a loud WARNING when the anchor drifts >15%
from the baseline on the same machine class: the anchor divides
every ratio, so a drifted anchor scales every verdict by the same
bogus factor. This would have flagged the incident on first run
instead of after three chased regressions.
Raw per-rep samples are still retained in the baseline for forensics;
per-sample ratios compute against the reduced anchor.
The committed docs/perf/baseline.json still carries the polluted
anchor and needs a recapture (bench-ratchet update -force) from a
clean checkout on a quiet machine — left out of this change so the
recapture can be reviewed on its own.
… not MISSING rows A partial profile (the fast gate, pr-fast) never runs the full-profile pkg/vm fleet, so check reports drowned real signal in 130+ MISSING rows. jobsSelect matches baseline entries against the run's capture jobs (pkg + family segment + variant label); out-of-scope entries collapse to a single summary count in both text and markdown reporters. nil jobs (aggregate-from-file, scope unknown) keeps the classic behavior. In-scope-but-absent entries still report MISSING — which immediately exposed that the suite [bytecode] rows had been silently absent (see the companion vm/rt fixes).
mparrett
reviewed
Jul 18, 2026
| } | ||
| // go test -bench matches per slash-segment and the job filters are | ||
| // family-anchored, so match the family segment only. | ||
| family := rest |
Collaborator
There was a problem hiding this comment.
[P2] Preserve slash-specific -filter semantics
This truncates the benchmark name to its family before applying the original regexp. For a supported manual filter such as -filter "^BenchmarkMapAssoc/HAMT-Assoc/100$", go test does run that exact sub-benchmark (it applies slash-separated regexp segments), but jobsSelect tests the whole regexp only against BenchmarkMapAssoc and returns false. If that targeted sub-benchmark disappears, the report will classify its baseline row as out of scope and suppress the MISSING signal this change is meant to preserve. Suggest reproducing Go test’s slash-segment matching here, or derive scope from an equivalent matcher.
mparrett
self-requested a review
July 20, 2026 23:57
mparrett
approved these changes
Jul 20, 2026
mparrett
left a comment
mparrett
left a comment
Collaborator
There was a problem hiding this comment.
One comment to follow up on. Let me know if it makes sense. Thanks!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extracted from the
catalog-dispatch-facetsstack — it's a standalone reporting fix that was riding along with unrelated IR work, and it's newly relevant: every ratchet report during the 2026-07-18 baseline investigation was flooded with 133 MISSING rows (baseline entries outside the current run's scope), burying the handful of real verdict lines. This collapses them to a single summary line.Stacked on #561 (rebased onto it cleanly, same two files, disjoint regions — aggregation vs reporting;
go test ./cmd/bench-ratchetgreen on the combined tree). This branch contains #561's commit; only the top commit is under review. Land #561 first and this becomes a one-commit diff.Together with #561 (sampling) and #564 (baseline recapture), this completes the ratchet-trustworthiness set: clean samples, clean priors, readable reports.