build: bench-ratchet infrastructure — deterministic rebaseline, Go 1.26.5 baseline, pre-push gate - #780
Conversation
6dd6a2e to
ea34ee4
Compare
|
Restacked onto The original split accidentally retained The baseline was recaptured under the exclusive benchmark baton from exact reachable
Final validation:
No #781 implementation code is included in this PR. |
mparrett
left a comment
mparrett
left a comment
There was a problem hiding this comment.
Reviewed the forced-rebaseline semantics, regenerated baseline, and pre-push integration.
Validated locally:
go test -count=1 ./cmd/bench-ratchetprek validate-config .pre-commit-config.yaml- semantic baseline comparison: 3 measured benchmarks changed across all 6 profiles
- prospective merge with current
main: deterministic benchmark ratchet passes
No blocking findings. One non-blocking process caveat: the comments reference a repository jj push alias, but that alias is not tracked here, so JJ enforcement depends on external configuration.
|
@nnunley What do you think about sharing/documenting any jj-specific workflow pieces, like the |
|
Reviewed cmd/bench-ratchet/main.go, main_test.go, and the ratchet.md delta (skipped the baseline.json regen itself beyond confirming it's a wholesale regen, not hand-edited). forceRebaseline and filterUnstableBenchmarks are both well tested — the new tests actually exercise the cross-profile propagation and the six-variant filter, not just happy-path. Two things worth your own judgment call rather than a rubber stamp from me, since they're repo-policy rather than correctness:
Nothing blocking from a correctness read. |
|
@nnunley — nooga's read covers correctness, and the two items he left open are policy calls on your own infra, so they belong with you rather than with me. On the pre-push hook, one measurement that isn't on the thread yet: On the M3 baseline, nooga's own reading limits the blast radius: wall-clock is gated only within the same machine profile, and only allocations and bytes compare globally, which are machine-independent. That reads fine to me. The one addition I'd want is a line in Two mechanical things:
|
|
Correcting myself on the pre-push hook, because I had the central fact backwards. I said That inverts my argument rather than weakening it. With no server-side enforcement, this hook would be the only thing standing between a perf regression and What survives is the cost, not the duplication: about a minute on top of The rebase and the |
nooga
left a comment
nooga
left a comment
There was a problem hiding this comment.
LGTM — mparrett's two approvals plus a local build/vet/test pass. The 3.6k-line delta is the regenerated docs/perf/baseline.json, not code.
Needs a rebase (6 behind main). Note this has a real conflict with #684 in cmd/bench-ratchet/main.go; since #684 isn't finished yet, this lands first and #684 rebases over it.
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.
PreparedCall and an argument slice on every invocation, and resolved the target through resolveBytecodeCall, which packs a rest list for variadic fns before the preparation could reject them. The IR compiler performs thousands of short reductions per compile, so BenchmarkIRCompile gained ~1,900 allocs/op (+2.4% bytes/op, over the ratchet's 2% deterministic bar) and InitFromLGB slowed ~10%. Bisect and numbers in nooga#791. PrepareCallInto(p, fn, arity) fills a caller-owned PreparedCall, so the hot loops keep it on their stack; the argument slots live in a new Frame.prepArgs that survives pool reuse and does not alias argbuf, which installBytecodeCall overwrites on a same-frame tail call. The target is inspected with unwrapBytecodeFn, split out of resolveBytecodeCall, so variadic and wrong-arity fns are rejected before any packing. PrepareCall remains as the allocating form. On the M3, BenchmarkIRCompile goes from 116,149 to 113,722 allocs/op and 5,080,290 to 4,935,179 bytes/op, below the nooga#780 baseline (114,255 / 4,963,229) because variadic reducers no longer pay for a preparation they cannot use. TestPrepareCallIntoDoesNotAllocate pins the zero-allocation contract. Refs nooga#791.
PreparedCall and an argument slice on every invocation, and resolved the target through resolveBytecodeCall, which packs a rest list for variadic fns before the preparation could reject them. The IR compiler performs thousands of short reductions per compile, so BenchmarkIRCompile gained ~1,900 allocs/op (+2.4% bytes/op, over the ratchet's 2% deterministic bar) and InitFromLGB slowed ~10%. Bisect and numbers in #791. PrepareCallInto(p, fn, arity) fills a caller-owned PreparedCall, so the hot loops keep it on their stack; the argument slots live in a new Frame.prepArgs that survives pool reuse and does not alias argbuf, which installBytecodeCall overwrites on a same-frame tail call. The target is inspected with unwrapBytecodeFn, split out of resolveBytecodeCall, so variadic and wrong-arity fns are rejected before any packing. PrepareCall remains as the allocating form. On the M3, BenchmarkIRCompile goes from 116,149 to 113,722 allocs/op and 5,080,290 to 4,935,179 bytes/op, below the #780 baseline (114,255 / 4,963,229) because variadic reducers no longer pay for a preparation they cannot use. TestPrepareCallIntoDoesNotAllocate pins the zero-allocation contract. Refs #791.
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.
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.
…o space (#684) * perf(bench-ratchet): seed the baseline from a window, reduced in ratio 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. * fix(bench-ratchet): gate M3 preservation on Arch, not the model string 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> * fix(bench-ratchet): group seed candidates by machine key, not filename 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. * perf(bench-ratchet): re-seed baseline.json from the perf-data window 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. * fix(bench-ratchet): skip a machine key with fewer than -seed-min-window 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> * perf(bench-ratchet): re-seed baseline.json from the perf-data window 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> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
3 participants
Three bench-ratchet infrastructure changes, split out of a local stack (the companion perf change follows separately with A/B evidence).
cmd/bench-ratchet: a forced rebaseline now also resets the deterministic (alloc/bytes) bars instead of carrying stale ones forward; unit tests cover the reset path.docs/perf/ratchet.mdupdated to match.docs/perf/baseline.jsonafter the toolchain moved from Go 1.26.3. Numbers are from the same Apple M3 machine as the existing baseline; single-machine baselines are what the ratchet fingerprint expects, flagging for reviewer awareness.Each change is independent of the others in content but ordered (baseline regen assumes the deterministic-bar fix; the push gate assumes a current baseline).
Validation:
make generatedeterministic across two runs,make check-generatedclean,go test ./...including e2e passes on this head.