perf(bytecode): compact lazy var metadata - #781
Conversation
e62e5ec to
19b4634
Compare
19b4634 to
26c86bf
Compare
|
Reviewed the lazy var-metadata implementation and tested it on a locally regenerated integration with current Validation on the integrated tree:
The PR is currently marked conflicting with |
|
Heads up before a full review: this PR is CONFLICTING against main, and it's not incidental — the conflict is entirely in the generated triad (pkg/rt/core_compiled.lgb, generated.manifest, generated.sums). #767 (fix(lower-go): assign each branch its own closure at a disagreeing join), merged 2026-08-28, touched pkg/rt/core/ir/lower_go.lg, which regenerates the same embedded core bundle this PR's var-metadata encoding also touches. That matters for the PR body's numbers: the InitFromLGB A/B (26,294 → 13,834 allocs/op, -22% wall) was measured against merge-base a665761, and the bundle #767 produces is a different one than what's on main now — the exact 485-var-metadata byte counts this change's win is proportional to could shift slightly on a rebase (unlikely to change the qualitative story, given the size of the win, but the reported numbers should be re-verified against current main rather than assumed to still hold exactly). Once it's rebased and go run ./cmd/lgprimgen -primitives pkg/rt -go-pkg github.com/nooga/let-go/pkg/rt -primitives-out pkg/rt/zz_primitives_generated.go
exit status 1 generate.lg ERROR: dependency staleness query failed (exit 1) is clean against current main, happy to do a full pass on the decoder/encoder/DefMetaPairs changes — the diff shape (encoder.go/decoder.go/def_meta_pairs.go/var.go, all with matching tests) looks reasonable at a skim, I just don't want to review logic against a stale bundle when the rebase is mechanical. |
|
Same follow-up as on #734, since this head carries the same generated artifacts.
Worth the rebase to clear the box, but the red is the driver gap rather than a signal about this change. #635 and #683 are the structural fix. |
nooga
left a comment
nooga
left a comment
There was a problem hiding this comment.
LGTM. Confirmed the format change is version-gated (TagIDMap|TagVer1) and TestKnownTagRejectsUnsupportedVersion pins that old decoders reject new bundles loudly. pkg/bytecode/pkg/compiler/pkg/vm tests pass locally. Rebase for the manifest and I'll merge.
Seeded from the 381-snapshot perf-data timeline, window 5 per machine key: go run ./cmd/bench-ratchet -perf-data-dir <perf-data>/timeline seed-baseline Five amd64 tiers are rebuilt from window medians, newest contributing SHAs dadb8e0, 69d88da, e344ff2, 98ad2bf and f3ca5f9. The arm64/Apple M3 tier is carried over unchanged, as before: it has no CI counterpart, so seeding cannot derive it. The line count falls because a fresh seed writes only the gated fields. The per-sample arrays and best_since provenance are omitempty reporting data that update/aggregate repopulate as runs accumulate; check gates on ns_per_op, ratio_to_anchor, allocs_per_op and bytes_per_op, all of which are present. This is what closes the gap #781 reported, where the committed baseline predated go1.26.5 and InitFromLGB read +274% allocs against it on main: that tier's allocs move 7391 -> 26294, which is what main actually does today. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
26c86bf to
f320426
Compare
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
What
Var metadata in the embedded core bundle is currently decoded into 485 eagerly-built maps during
InitFromLGB, before any of it is looked at. This change stores the metadata as compact lazyDefMetaPairsand builds the map on first use. Bundle size is unchanged (303,648 bytes); programs that never touch a var's metadata never pay for its map.Evidence
Exact A/B on the merge base: candidate
e62e5ec0441evs its parentmaina6657612,BenchmarkInitFromLGB,-benchmem -count=10 -run=^$, quiesced Apple M3, go1.26.5, benchstat:The deterministic axes are exact: 26,294 → 13,834 allocs/op and 1,599,988 → 1,004,581 B/op. Wall time improves -22% with the usual startup-bench noise (cand ±21%).
make bench-ratchetnote: the committeddocs/perf/baseline.jsonpredates go1.26.5 and is stale against currentmain— the ratchet fails onmainitself (InitFromLGB allocs +274% vs baseline). This change cuts that to +97% and improves every InitFromLGB axis relative to base (anchor-relative time +37.1% → +7.2%). The baseline refresh is in #780.Validation
make generatetwice is deterministic,make check-generatedpasses, andgo test ./...including the e2e suite passes on this head.