perf(bench-ratchet): seed the baseline from a window, reduced in ratio space - #684
Conversation
ae6ccee to
5601f15
Compare
|
#564 landed on This PR still lists the pre-squash #564 commits ( No rush and no action needed from my side — flagging it since the blocker you were waiting on is cleared. |
5601f15 to
335b170
Compare
|
Thanks for the rebase. The argument for ratio-space reduction convinced me, and the The open question: this lands the method but not the result. One nit: M3 preservation now keys only on I ran the unit tests, not a seed against the real perf-data timeline, so the corpus figures |
Rides along. I will re-seed and commit The technical reason not to split it: MechanismRebase (23 commits behind), fix the M3 guard, then re-derive as the last step before merge so the window is current: git worktree add ../lg-perf-data origin/perf-data # 364 timeline snapshots
go run ./cmd/bench-ratchet \
-perf-data-dir ../lg-perf-data/timeline \
-baseline docs/perf/baseline.json \
seed-baseline
go run ./cmd/bench-ratchet check # exit 0 on current mainNo measurement runs: it derives over snapshots already on M3 nitTaking it. if mb.Machine.Arch != opt.archPrefix && strings.Contains(mb.Machine.CPUModel, "M3") {
Ordering against #740#740 writes
Staying outThe 5% budget still has no measured floor under it, which is a measurement question rather than a seeding one. Out of scope here. The corpus figures in the body are still only mine. The seed output will put the tier counts and window sizes where you can check them against |
|
The method discussion looks settled, but the diff doesn't yet do what the PR is for: |
fd8f67d to
139b05e
Compare
|
Both done and pushed; the branch is rebased onto current The guard. The re-seed. Five amd64 tiers rebuilt from full 5/5 windows, newest contributing SHAs Two things to know before you read the diff:
What it does not fix. The arm64/Apple M3 tier is still the 2026-07-18 capture, carried over rather than seeded, because there is no CI counterpart to derive it from. Your point on #780 about that profile having a single owner is unchanged by this PR. I will rebase again over #780 once it lands, per your ordering. One thing still open from my 08-21 note: if the v1.12.2 backfill for #740 has already been dispatched, it wants a re-dispatch after this merges, since |
nnunley
left a comment
There was a problem hiding this comment.
Requesting changes on exact head 139b05e5. The ratio-space reduction and committed medians check out independently, but content/filename mismatch handling can silently overwrite a correct machine window.
seedBaseline groups by filename slug (seed.go:127–137), seedOneMachine warns and returns the content key (:201–216, :264), then the outer loop assigns merged.Machines[key] = mb (:159). An Intel-named file carrying EPYC content is therefore reduced separately and can overwrite the proper five-run EPYC reduction with a one-file result. The current mismatch test has no proper EPYC window, so it misses this collision. Please group candidates by the machine key read from content (while retaining the filename warning), and add a test with both the normal EPYC window and an Intel-named/EPYC-content file.
Also rebase over merged #780 and regenerate baseline.json last. GitHub reports a real main.go conflict; both this PR’s seed flags and #780’s rebaselineForce must survive. The carried M3 is pre-#780 (f154c7… versus current-main dadb8e…), so taking this branch’s JSON would restore stale bars.
Independent validation: 25 current five-window snapshots reproduce all five amd64 profiles with zero discrepancies in anchors, ratios, derived ns/op, bytes, allocs, or quorum membership; all 906 committed ratio identities hold.
139b05e to
7a0059f
Compare
|
@nnunley — fixed, rebased, and re-seeded, in that order. Head is The grouping collision. You were right about both the mechanism and the test gap. The filename warning stays. It no longer decides anything, but the filename is what the seed log prints and what someone greps for, so a divergence still has to be visible. The architecture filter runs on content as before; a file named for the seeded architecture whose profile is a different one is now reported, while a file named for another architecture is skipped quietly.
The perf-data claim was wrong. The code carried a claim that perf-data holds Intel-slugged files with EPYC content. It does not. All 382 timeline files agree with their names as of 2026-09-02, and the branch is append-only with no deletions or renames in its history, so there was never a file of that shape to observe. I removed the claim from the comment and said so in the commit message: this is a guard on a silent, order-dependent failure, not a repair of live data. The bug is real; my evidence for it having already bitten was not. The rebase. Onto current
The re-seed. Five amd64 tiers, all from full 5/5 windows, newest contributing SHAs Running it after the rebase is what settles the stale-bars problem you flagged: the carried arm64/Apple M3 profile is now byte-identical to the one on
One thing still open from my 08-21 note, unchanged by any of this: if the v1.12.2 backfill for #740 has already been dispatched, it wants a re-dispatch after this merges. |
7a0059f to
c0fbea9
Compare
|
@nnunley rebased onto current The #740 ordering question is closed. One new fix. A dry re-seed against the current corpus wrote a tier I had not seen before: A new runner CPU landed its first timeline snapshot on 2026-09-06. The coherence check cannot vote on fewer than three snapshots and the benchmark quorum passes everything at (1+1)/2 = 1, so every floor for that tier would have been a single observation, which is the case the window exists to prevent. The re-seed. Everything from your 09-02 review is otherwise unchanged at this head: content-keyed grouping, the EPYC-window-plus-misnamed-file test, and Ready for another look. The min-window gate is the only logic new since your review. If the corpus moves enough between review and merge to matter, I will re-seed once more at the end; the command and the numbers are in the commit message either way. |
…o space seed-baseline took the newest snapshot per machine key. One snapshot is one CI run, and one CI run is one sample: seeded from the newest versus a median of five on the same corpus, 22.6% of the 758 (tier, benchmark) floors differ by more than the 5% regression budget and 11.3% by more than 10%. Some of that is real code movement across the window and some is sampling, but either way a fifth of the gate's thresholds were set by a single observation. Take a window (-seed-window, default 5) and median it. Median rather than min because `update` already takes a min over history when it ratchets, and a second minimum would stack into a floor no clean run can reach. Reduce in ratio space, deriving ns_per_op back from the window's anchor: raw ns_per_op carries host speed and ratio_to_anchor does not. Over the 24 most recent amd64 snapshots, anchor deviation from the tier median spans -22.4%..+3.1% while ratio_to_anchor holds to a median 0.03%, worst 1.75%. Gate on that coherence rather than on anchor drift. Gating on the anchor was the obvious design and the corpus refutes it: a588a69 sits 22.4% off its window's anchor, is uniformly 22.4% fast in raw ns/op across all 162 of its benchmarks, and agrees with its window on every ratio to within 0.1% — the host was fast and the anchor divided it back out. That check would have discarded three good captures, two of them snapshots this baseline is seeded from. What needs rejecting is the mixed capture, where the ratios move together while the raw numbers look ordinary. Also, from reading the seam rather than the samples: - Enforce the arch filter on file CONTENT, not just the filename, and warn when the two disagree. That divergence put an EPYC profile under an Intel key once already, and the log printed the filename. - Parse snapshot names with an anchored regexp. A positional split on "-" yields a plausible-looking wrong SHA, and the machine slug is full of dashes. - Skip a benchmark present in under half the window instead of seeding it from one observation; check reports it as NEW, which is honest. - Report b.N movement across the window. b.N is an output of the timing loop, so it moves when per-op cost moves, and ns/op is N-dependent wherever iterations share state. - Mark every matching exclusion prefix, not the first. BenchmarkClojureTestSuite is a prefix of BenchmarkClojureTestSuiteCompileAndRun, so the longer entry was reported as stale on every run while it was in fact filtering. Provenance: every number above is a re-derivation over snapshots already committed on perf-data — reproduce with -seed-window 1 against -seed-window 5 and diff ratio_to_anchor. The tight-core-plus-slow-tail characterisation that motivated windowing in the first place (roughly 2 launches in 20 landing 15-29% high, byte-identical binaries landing 3.7% apart) is from a dedicated fixed-performance EC2 box in a sibling project, on different benchmark shapes; whether that tail generalises here is what a null control would settle.
…g alone The carry-over that preserves the local arm64 Apple M3 profile matched on CPUModel alone, so an amd64 model string containing "M3" matched as well and the stale entry overwrote the fresh seed the same run had just computed for that tier. Require the profile's Arch to differ from the tier being seeded, mirroring the existing skip at seed.go:200. The regression test fails against the bare guard with the carried-over 999 rather than the seeded 100. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e slug seed-baseline grouped snapshots by the machine slug in the filename, then stored each group's result under the key read from the file's content. A snapshot named for one machine but carrying another's profile therefore formed a group of its own, was reduced on its own, and was written under the key its content named — overwriting the window the correctly-named files had just produced for that same key. Which result survived depended on map-key order, so a single mis-named file could silently replace a five-run reduction with a one-file one. Grouping on the content key removes the collision: a mis-named file is one more candidate in the window it belongs to, subject to the same coherence check and the same median as its neighbours. The filename warning stays, because the filename is what the log prints and what a human greps for, even though it no longer decides anything. No snapshot in perf-data has this shape as of 2026-09-02 — all 382 files agree with their names, and the branch is append-only with no deletions or renames in its history — so this is a guard on a silent failure rather than a repair of live data. The regression test seeds a full five-snapshot EPYC window plus one Intel-named/EPYC-content file that is uniformly slow rather than incoherent, so the coherence check keeps it and grouping is the only thing under test. Against the old code it reports 200 ns/op, the stray file's own number, instead of the window's 100.
The committed baseline predates go1.26.5, so `check` on current main reads benchmarks against bars captured by a different toolchain — #781 reports InitFromLGB at +274% allocs against it. This re-seeds the five amd64 tiers from the perf-data timeline with the default window of 5: go run ./cmd/bench-ratchet -perf-data-dir <perf-data>/timeline seed-baseline All five tiers reduce from full 5/5 windows. Newest contributing SHAs are dadb8e0, 33fb9e3, e344ff2, 98ad2bf and f3ca5f9. Three tiers reported benchmarks below quorum and skipped them rather than seeding from one observation; the seed log lists the b.N movement per tier. It deletes far more than it adds. A fresh seed writes only the gated fields, so the per-sample arrays and the `best_since` provenance go with them. Both are `omitempty` reporting data that `update` and `aggregate` repopulate as runs accumulate; `check` gates on ns_per_op, ratio_to_anchor, allocs_per_op and bytes_per_op, all present. All 766 committed ratio identities hold. The arm64/Apple M3 tier is carried over byte-for-byte from main rather than seeded — it gates developer machines and has no CI counterpart to derive from (#651). Because this seeds after the rebase onto merged #780, the carried profile is main's current one at dadb8e0, not the pre-#780 capture.
…ow snapshots A machine that has only just started reporting has no window to disagree with. rejectIncoherent cannot vote on fewer than three snapshots and the benchmark quorum passes everything at (1+1)/2 = 1, so a one-snapshot tier would be seeded with every floor a single observation, which is the case the window exists to prevent. Seen live on 2026-09-06: a new runner CPU (AMD EPYC 9V45) landed its first timeline snapshot, and a re-seed against that corpus wrote the tier as "157 benchmarks from 1/1 snapshots". Skipping it leaves check ungated on that machine until the runs accrue, the same call the benchmark-level quorum already makes. Default is 3, the smallest window the coherence check can vote on. -seed-min-window must not exceed -seed-window, so the -seed-window 1 comparison mode now needs -seed-min-window 1 alongside it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Regenerated as the last step after rebasing onto main, so the carried
arm64/Apple M3 profile is the one main holds and the amd64 windows are
current:
go run ./cmd/bench-ratchet -perf-data-dir <perf-data>/timeline seed-baseline
Corpus: perf-data at f8ed429, 426 timeline files, window 5. Five amd64
tiers seeded from full 5/5 windows; newest contributing SHAs 477a5d3,
b0397f6, 928c217, 8757950, f3ca5f9. The 8370C tier reports 16
benchmarks below quorum. The new AMD EPYC 9V45 key has one snapshot and is
skipped by -seed-min-window. All 768 seeded ratio identities hold.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
c0fbea9 to
257a58c
Compare
|
Rebased onto All 15 required checks green. |
Follow-up to #564.
seed-baselinecurrently takes the newest snapshot per machine key. This takes a window of five and medians it, reduces in ratio space rather than rawns/op, gates on whether a snapshot agrees with its window instead of on how far its anchor drifted, and lands the re-seededdocs/perf/baseline.jsonalongside the method, sincecheckgates on timing only wherebaseline.Machines[key]exists and a baseline whose provenance no longer matches the tool degrades to no gate rather than to a failure.The re-seed
Regenerated as the last step after rebasing, so the carried arm64/Apple M3 profile is byte-identical to the one on
mainand the amd64 windows are current:Corpus:
perf-dataatf8ed4298, 426 timeline files, window 5, as of 2026-09-07. Five amd64 tiers seeded from full 5/5 windows, newest contributing SHAs477a5d36,b0397f63,928c2170,87579506, andf3ca5f9b. The 8370C tier reports 16 benchmarks below quorum and skips them. The newAMD EPYC 9V45key has one snapshot and is skipped by-seed-min-window(see below). All 768 seeded ratio identities hold.A fresh seed writes only the gated fields, so the per-sample arrays and
best_sinceprovenance go with it; both areomitemptyreporting data thatupdateandaggregaterepopulate as runs accumulate.checkgates onns_per_op,ratio_to_anchor,allocs_per_op, andbytes_per_op, all present.Correction to my #564 review
I said there to skip a candidate whose anchor is an outlier against its own tier. I built that, ran it against perf-data, and the corpus refutes it.
Snapshot
a588a69d2759(EPYC 9V74) sits 22.4% off its window's anchor. It is also uniformly 22.4% fast in rawns/opacross all 162 of its benchmarks, and agrees with its window on everyratio_to_anchorto within 0.1%. The host was fast that day and the anchor divided it back out, which is what the anchor is for. Anchor-deviation gating would have discarded it and two more like it.Over the 24 most recent amd64 snapshots as of 2026-08-05:
ratio_to_anchoroffset from tier medianSo the gate is on ratio coherence: a snapshot is rejected when its ratios sit more than
-seed-coherence-tolerance(default 5%) off the rest of the window. That catches the case that damages a baseline, the mixed capture, where the anchor caught the slow tail and the benchmarks did not, so every ratio is uniformly wrong while the raw numbers look ordinary. Nothing in the current corpus trips it, which is the expected result and not evidence the check is idle.Why a window
One snapshot is one CI run, and one CI run is one sample. Seeding from the newest versus a median of five, same corpus, same day (2026-08-05):
Part of that spread is real code movement across the window and part is sampling; I can't separate them without a null control. Either way, seeding from one snapshot sets a fifth of the gate's thresholds from a single observation of it.
Median rather than min:
updatealready takes a min over history when it ratchets, and seeding with a second minimum stacks two of them into a floor no clean run reaches.Why ratio space
Raw
ns_per_opcarries host speed;ratio_to_anchordoes not (see the table above). Reducing the quantity that carries host speed and then dividing imports that speed into the stored floor. The two approaches agree whenever a window comes from one host and diverge exactly when it does not, which a shared runner pool guarantees.ns_per_opis derived back from the reduced ratio and the window's anchor, soratio_to_anchor == ns_per_op / anchor.ns_per_opholds by construction.Additional fixes
-seed-min-windowsnapshots (default 3) is skipped and reported. The coherence check cannot vote on fewer than three, and the benchmark quorum passes everything at (1+1)/2 = 1, so a one-snapshot tier would be seeded with every floor a single observation. A new runner CPU landed one on 2026-09-06. Skipping leavescheckungated on that machine until the runs accrue, the same call the benchmark-level quorum makes.Machine.Arch != -seed-archas well as the model string, mirroring the skip in the candidate scan. Without it an amd64 model string containing "M3" would overwrite its own fresh seed.-yields a plausible-looking wrong SHA, and the machine slug is full of dashes. Unparseable names are skipped and reported: 120 in the current corpus, all predating the machine-slug naming.checkreports it as NEW, which is honest.b.Nmovement across the window is reported, not acted on.b.Nis an output of the timing loop, so it moves when per-op cost moves, andns/opis N-dependent wherever iterations share state. Excluding on it would shrink the gate silently.BenchmarkClojureTestSuiteis a prefix ofBenchmarkClojureTestSuiteCompileAndRun, so the longer entry was reported as stale on every run while it was in fact filtering.filterUnstableBenchmarks(from build: bench-ratchet infrastructure — deterministic rebaseline, Go 1.26.5 baseline, pre-push gate #780) sources its prefixes fromunstableBenchmarksinseed.gorather than a second copy of the list.Verification
go test ./cmd/bench-ratchet: 14 seed tests as of 2026-09-07, includingTestSeedBaselineKeepsUniformlyFastSnapshot(the negative result above),TestSeedBaselineRejectsMixedCapture,TestSeedBaselineGroupsMisnamedFileIntoContentWindow(a full EPYC window plus one Intel-named EPYC file),TestSeedBaselinePreservesOnlyOffArchM3, andTestSeedBaselineSkipsMachineBelowMinWindow. Each guard test fails with its guard removed.perf-datatimeline, output in the commit message.-seed-window 1 -seed-min-window 1reproduces the current behaviour, which is how the comparison table above was produced.Not addressed here
The 5% budget still has no measured floor under it. Nothing in this PR tells you whether 5% is above or below the gap between two builds that cannot differ on these tiers; a comment-only-edit control run through the existing dispatch would. Worth doing before the budget is tightened.
The
perf-release-baselineworkflow from #740 has not run yet, so there is no tighten-merge for this seed to clobber. Dispatch the v1.12.2 backfill after this merges and it tightens on top of the fresh seed.