perf(ir): move inferred types to a positional side table — ~100x faster type writes - #558
Conversation
…t tuple set-type! was (swap! f assoc-in [:insts nid 5] t) — an interpreted assoc-in plus a full :insts spine path-copy per write. The typeinfer epoch flushes every inferred type through it, twice per optimize-fn, so writing the results back cost more than the entire worklist solve (3,500-inst fn: flush 261ms vs drain 227ms; ~94us per write). Store inferred types in a positional transient vector on the Function, exactly like the :source-info side table (add-source-info! precedent — "no swap!; was the #1 IR mutator by call volume"). Tuple slot 5 keeps the construction-time :unknown and remains the read fallback; type-of and the typeinfra seed read the side table first. No pass reads or writes types except through ir/type-of + ir/set-type!, so the change is contained to ir.data and the lattice seed. Measured (M2, origin/main 68abd70 base): - set-type! x3500: 327ms -> 0.9ms - flush-state-types! (3.5k): 261ms -> 3.1ms - typeinfra epoch (3.5k fn): 481ms -> 213ms - BenchmarkIRCompile: 22.0ms -> 15.4ms/op (-30%) - whole-core lgbgen --target=go: 3m22s -> 54s (3.8x) Gates: pkg/ir + compiler/vm/rt suites pass; make check-selfhost holds byte-identical (output-neutral); TestGogenAOTDiff parity passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ot 5's new role Review findings on the side-table change: - clone-inst carried the :source-info side-table entry but not :types, so a clone of a typed inst would read :unknown through type-of (the copied tuple only holds the construction-time default). Dormant today — clone-inst has no callers — but a silent de-optimization trap for its first user. Mirror the source-info carry. - The inst-layout comment still presented tuple slot 5 as THE type; note that it now holds only the construction default, with the :types side table authoritative after the first typeinfer flush. Gates re-run: pkg/ir + compiler/vm/rt suites green; make check-selfhost byte-identical. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@nnunley this builds on some of the solid work you already did. Let me know if it looks ok, and if it's compatible with your roadmap. |
…nc contract Review follow-ups on #558: - Regression test for the side-table invariants that type-of, the lattice seed, and clone-inst each implement by hand: fallback to the tuple's construction-time :unknown, side-table precedence, overwrite, sparse growth (gap nids keep the fallback), clone carry (typed and untyped), and the seed's merged view. The clone carry was a review catch, so the contract now has a tripwire. - set-type! comment: state explicitly that it bypasses the Function atom (no CAS, watches don't fire) and why that's safe — compilation is single-threaded per Function, the same assumption :source-info already relies on. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Perf (pkg/vm micros) — base vs head, same runnerBase bench-ratchet — 0 regression(s) > 12.0% budget, 0 missing, 0 new
Showing 0 of 53 benchmarks (past budget). Full table in the run summary → |
|
Correction: I've revised the title and body. The original headline (3.8x whole-core, −30% |
nnunley
left a comment
There was a problem hiding this comment.
Reviewed in depth — this is a well-executed, high-impact change and I'm on board (leaving the formal approve for after one more look at the CI matrix, but consider this an LGTM in substance). The side table correctly mirrors the proven add-source-info! pattern, the contract test locks all six invariants, selfhost stays byte-identical, and the corrected measurement numbers are believable and honestly shaped (gain concentrated in 3k+-inst fns, IRCompile-within-noise on its tiny-fn corpus is exactly what the shape predicts).
Line notes below. Two bigger threads worth syncing on since they affect where this goes next:
Where this is heading
- This dovetails with the typeinfer roadmap rather than competing with it. The recorded direction is a bitset lattice over the closed vm type universe for the lattice ops constant factor; your side table fixes the storage constant factor. Complementary halves of the same bottleneck (typeinfer was measured at ~43% of lowering time).
- Heads-up on
catalog-dispatch-facets(unpushed stack on my side): it reifies the infer facets into the op catalog and slimstypeinfer.lgby ~130 lines. File-wise we don't collide (you:data.lg/lattice.lg; me:ir_ops.lg/ops.lg/typeinfer.lg), and thetype-of/set-type!accessor API you kept stable is exactly the seam that makes both rebases clean. I'll rebase onto this after it lands and re-run the catalog tests over the side-table storage. - Longer view (EPIC-017): the packed-inst/ops-as-types direction eventually turns insts into native structs with typed fields, which would subsume both the tuple slot and the side table. Your side table is the right interim form — it removes the spine-copy cost today without committing to a layout the packed-inst work would have to unwind.
- The
data/generated.lgdrift you flagged is real and now slightly wider; agreed on deferring to #566 rather than patching the generator here.
Benchmarking — you independently hit what we just spent a night diagnosing
Your mid-review correction (3.8x → 1.1–1.3x whole-core after moving to interleaved A/B medians) is the exact failure mode we just root-caused in the ratchet itself: the committed docs/perf/baseline.json anchor (1.847 ns/op) turned out to sit in the slow mode of a bimodal distribution — E-core/throttled captures vs the true ~1.0–1.1 ns P-core mode across 92 historical anchor samples. Single cold samples + mean aggregation + an only-tightens ratchet had baked phantom priors in permanently: clean runs read as +38% regressions while wall times had actually improved.
Two PRs in flight fix this infrastructure-side, and they're relevant to how this PR's numbers will read once landed:
- #561 — bench-ratchet sampling:
-count 4default, first rep discarded as warmup, median (not mean) of the rest, and a loud warning when the anchor drifts >15% from baseline (that warning would have flagged your original inflated numbers automatically). - #564 — full baseline recapture with the new sampling (anchor 1.005 ns/op), cross-validated against per-benchmark medians pooled from the 47 clean-anchor historical runs. Notably, that validation surfaced #462's VarDeref wins that the polluted baseline had been masking — the same masking would have eaten this PR's improvement too.
Practical upshot for this PR: your corrected methodology (interleaved, median-of-3, pristine clones) is exactly the house style now; no need to re-measure. Once #561/#564 land, the ratchet's own numbers become trustworthy for changes like this one, and allocs/op comparisons (anchor-independent) are the most robust single signal in the meantime.
… (review) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Added the TransientVector note in 55f4192. On the seam: The anchor saga is a striking parallel — same bimodal trap, found from opposite ends the same week. Agreed on allocs/op as the robust signal until #561/#564 land; that's also the one row of my original numbers that never moved through the correction (−3.4% both measurements). |
|
Reviewed for merge readiness: substantive feedback is addressed, stale review threads are resolved, CI is green, and no blockers remain. Good to go. |
nooga
left a comment
There was a problem hiding this comment.
Reviewed for correctness with a focus on bypass readers, transient aliasing, and the epoch interplay. Approving.
Slot-5 bypass audit (the main risk): swept the whole tree for direct inst-tuple slot-5 access. The only readers are type-of (pkg/rt/core/ir/data.lg:161) and the seed (pkg/rt/core/ir/lattice.lg:446), both implementing the same side-table-first/slot-5-fallback rule; the only writers are the three inst constructors (add-inst, add-block-arg-front!, add-inst-before!), which write the construction-time :unknown. The one other (nth row 5) in the tree (pkg/ir/ir_ops.lg:128) is an op-spec table row, not an inst. Every pass, lower_go.lg, and dump.lg read exclusively through ir/type-of; no Go code touches inst tuples. The alternate ir.data.generated layer keys [:insts nid :type] on keyword-map insts — already structurally incompatible and required by nothing in production, so it isn't a bypass; agree with punting it to #566.
Transient aliasing: no Function snapshot/copy/rollback pattern exists anywhere — every swap! on the Function rebinds the map but carries the same :types transient pointer, so there is exactly one canonical store per Function, identical to the :source-info precedent. All Function construction goes through ir.data/new-fn (build.lg is the only caller), so :types always exists and ensure-types! can't conj! onto nil. The discarded conj!/assoc! returns are sound: TransientVector.Conj/.Assoc mutate in place and return the receiver (pkg/vm/transient.go:220-247).
ensure-types! / sparse nids: growth loop fills to nid+1 with nil ((<= (count tys) nid) — no off-by-one; count re-reads the mutating transient each iteration). Nil gap entries read as absent and fall back to slot 5, so a sparse flush (only touched nids written, ascending order in flush-state-types!) leaves gap nids at :unknown — the contract test's case 4 pins this. Nil-as-absence can't collide with a legitimate write: fact-type never returns nil (nil fact reads :bottom), and pre-PR a nil in slot 5 also read back :unknown, so semantics are unchanged either way.
clone-inst: the carry copies the (immutable) type value into a distinct index via ensure-types! + assoc!, so a clone's later set-type! writes its own slot, never the source's. Noting clone-inst currently has no in-tree callers — the carry fix (9d8070e) plus the contract test still make it correct API.
Epoch interplay: epoch 1 flushes to the side table; epoch 2's seed reads side-table-first — exactly the view epoch 2 used to get from slot 5. Mid-drain type-of reads also match pre-PR behavior at every phase. Inline/LICM create fresh insts via add-inst (:unknown, re-inferred by the trailing epoch) — pre-existing, unchanged.
Ran: go test ./pkg/ir ./pkg/compiler ./pkg/vm ./pkg/rt ./pkg/genmanifest -count=1 (includes the new TestTypeSideTableContract and the genmanifest staleness check against the updated generated.sums), go test ./test/ -count=1, and go test ./test/e2e -run 'TestLoweringDeterminism|TestGogenAOTDiff' -count=1 — all green.
Non-blocking: the docstrings at pkg/rt/core/ir/passes/infer_arg_types.lg:10 ("stored back via ir/set-type! on the load-arg inst") and typeinfer.lg:143 ("types pulled from ir/type-of") describe the pre-state-threading flow — results now travel through the lattice state and reach set-type! only in the flush. Both predate this PR; worth a comment sweep whenever slot 5 is retired.
…tion (#578) * bench: refresh results on post-#552 main; update README benchmark section Recapture after the 2026-07-18/19 perf batch (#558-#571, #560): transducers -24% on the VM leg (and its variance collapses with #560's lock-free realized LazySeq access), map-filter AOT now beats the VM leg, -3-4% broadly elsewhere. README benchmark table switches from the dropped joker/go-joker/gloat lineup to the VM/AOT/babashka/JVM matrix results.md actually measures. * readme: fix benchmark prose per review map/filter is a 1.7x win over babashka, not neck-and-neck (that's persistent-map); AOT vs warm JVM compute is ~2x, not an order of magnitude (that figure is vs babashka); and the warm JVM only outruns the VM leg, not the AOT build. * readme: correct the AOT-vs-warm-JVM claim with in-process measurements The '2x ahead of warm JVM' figure came from subtracting measured startup from the one-shot JVM wall clock, but that remainder still contains JIT warmup and script compilation. Measured in-process on the same machine (12 iterations each): steady-state HotSpot fib(35) is ~76ms vs ~99ms for the AOT build (flat from iteration 0, as expected for native code). Warm JVM is ~1.3x ahead of AOT; what AOT wins is one-shot wall clock (~5x).
Dumps a .lgb as deterministic text (via disasm/decode-bundle +
disassemble-resolved) so bundle changes can be read and diffed:
diff -u <(./lg scripts/lgbdump.lg old.lgb) <(./lg scripts/lgbdump.lg new.lgb)
Three choices aim at diff quality: resolved LOAD_CONST/LOAD_VAR rows
print only the referenced identifier (a pool insertion shifts every
later index, turning a one-fn change into a whole-bundle diff — 16.7k
noise lines vs 763 real ones on the #558 bundle bump); anonymous fns get
a bare label for the same reason; and the LOAD_CONST-VOID/POP pairs that
top-level comments compile to (#600 removes them) collapse to one count
line per chunk.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Dumps a .lgb as deterministic text (via disasm/decode-bundle +
disassemble-resolved) so bundle changes can be read and diffed:
diff -u <(./lg scripts/lgbdump.lg old.lgb) <(./lg scripts/lgbdump.lg new.lgb)
Three choices aim at diff quality: resolved LOAD_CONST/LOAD_VAR rows
print only the referenced identifier (a pool insertion shifts every
later index, turning a one-fn change into a whole-bundle diff — 16.7k
noise lines vs 763 real ones on the nooga#558 bundle bump); anonymous fns get
a bare label for the same reason; and the LOAD_CONST-VOID/POP pairs that
top-level comments compile to (nooga#600 removes them) collapse to one count
line per chunk.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…601) Dumps a .lgb as deterministic text (via disasm/decode-bundle + disassemble-resolved) so bundle changes can be read and diffed: diff -u <(./lg scripts/lgbdump.lg old.lgb) <(./lg scripts/lgbdump.lg new.lgb) Three choices aim at diff quality: resolved LOAD_CONST/LOAD_VAR rows print only the referenced identifier (a pool insertion shifts every later index, turning a one-fn change into a whole-bundle diff — 16.7k noise lines vs 763 real ones on the #558 bundle bump); anonymous fns get a bare label for the same reason; and the LOAD_CONST-VOID/POP pairs that top-level comments compile to (#600 removes them) collapse to one count line per chunk. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Why
Type inference dominated IR compile time, but the cost was not in the worklist solver — it was in writing the results back.
set-type!was(swap! f assoc-in [:insts nid 5] t).assoc-inis interpreted core code (no native builtin), and each call path-copies the whole:instsspine, so a single type write cost ~94µs on a 3,500-inst function. The typeinfer epoch flushes every inferred type through it, and the epoch runs twice peroptimize-fn, so on that function the flush (261ms) cost more than the entire fixpoint drain (227ms). The solver itself is healthy: ~2.1 enqueues per inst to converge.What
Inferred types now live in a positional transient vector (
:types) on the Function, indexed by nid — the same shape:source-infoalready uses (add-source-info!made the same move earlier for the same reason: "no swap! (was the #1 IR mutator by call volume)").set-type!becomes an in-placeassoc!, plus anensure-types!grow helper mirroringensure-si!.type-ofreads the side table first; inst tuple slot 5 remains the construction-time:unknownand the fallback for nids the table doesn't cover.seed-state-from-inst-types!(the typeinfra epoch seed) reads the same merged view.clone-instcarries the:typesentry alongside its existing:source-infocarry, so a clone keeps its source's inferred type.No pass reads or writes types except through
ir/type-of/ir/set-type!, so the change is contained toir.dataand the lattice seed.Numbers
Measured on an M2, base
68abd70vs this branch, in pristine clones with alternating warm runs (cold first runs discarded).set-type!× 3,500 (3.5k-inst fn)lgbgen --target=goBenchmarkIRCompileCorrection (2026-07-18): this PR originally claimed 3.8x whole-core and −30%
BenchmarkIRCompile. Both base measurements were taken in a heavily loaded session and did not survive clean re-measurement; the figures above are from interleaved A/B runs in fresh clones and agree with an independent reproduction on another M2 (which measured the type-write speedup at 109x and whole-core at 1.10–1.14x).The shape of the win:
BenchmarkIRCompile's corpus is tiny functions (15–50 insts), where the flush is a small share of compile time, so it barely moves. The flush cost scales with instruction count, so the gain concentrates in large functions — whole-core lowering, which includes several 3k+-inst functions, nets the ~1.1–1.3x.To reproduce:
Validation
go test ./pkg/ir ./pkg/compiler ./pkg/vm ./pkg/rtgreen.TestTypeSideTableContractpins the side-table invariants thattype-of, the lattice seed, andclone-insteach implement: fallback, precedence, overwrite, sparse growth, clone carry, and the seed's merged view.make check-selfhostholds byte-identical: the change is output-neutral, and the natively lowered side-table code reproduces its own output.TestGogenAOTDiff(bytecode vs native execution over the gold fixtures) passes.Notes and caveats
nth … 5site, and the generated bridge — more churn than this change should carry.conj!/assoc!pattern is safe here becauseTransientVectormutates in place and returns the same pointer (unlike Clojure proper, where the return value must be threaded).ensure-si!/add-source-info!already rely on this.pkg/rt/core/ir/data/generated.lg(the spec-generated alternate data layer) is structurally incompatible with the active IR beyond this PR: it models keyword-map insts while the active layer has been positional since before the:source-infoside table (perf(ir): source-infos in a positional side table, off the inst tuple #306), and no production code requires it — yet it is still generated, compiled, and lowered. Adding:typesto the spec would not reconcile it. Worth a separate decision: rehabilitate the generator around the positional representation, or retire it from generation, lowering, and the README — now tracked in ir.data.generated has drifted structurally from the active IR data layer — retire or rehabilitate #566.🤖 Generated with Claude Code