perf(interp): defer ref operand ownership to backing slots in the ARM64 JIT#151
Merged
Merged
Conversation
…64 JIT A ref pushed by LOCAL_GET/GLOBAL_GET/UPVAL_GET took a retain that its container consumer immediately released through guardRC — two dependent read-modify-writes on the same rc cell per element, making Sieve(256) 3x slower per array access than the constant-array TypedArraySum(256). Unify the existing raw const-marker deferral into a single owner model: a ref value is ownerStack (owns its retain) or defers it to backing storage (ownerConst/Local/Global/Upval). Producers push deferred with no retain; container consumers (ARRAY/STRUCT GET/SET, ARRAY_LEN, REF_IS_NULL, DROP, coro/error/string reads) borrow and elide the matching release. The retain materializes only where the value reaches interpreter-visible state: own (slot stores, call args, entry returns), settle (backing slot overwritten or frame dying), guard exit stubs, redeem (trap fallback and module completion), and before real calls. A committing back-edge flush rejects live deferred refs, keeping loop-carried deferrals threaded. The consume() peephole is subsumed. Sieve(256) default 5,052 -> 4,722 ns/op (-7%, jit 4,719); refcount exactness verified against a threaded oracle across deopt, call, tail-call, and completion paths.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe ARM64 JIT now tracks deferred reference ownership explicitly, borrows retains during native execution, and materializes or redeems them at interpreter-visible boundaries. Lowering, calls, exits, flushes, documentation, benchmarks, and regression tests were updated. ChangesDeferred reference ownership
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CompiledJIT
participant Flush
participant Redeem
participant Interpreter
CompiledJIT->>Flush: flush deferred operand stack
Flush->>Redeem: redeem deferred retains
Redeem->>Interpreter: transfer retained references
Interpreter->>CompiledJIT: return or trap
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #151 +/- ##
=======================================
Coverage 29.57% 29.57%
=======================================
Files 86 86
Lines 60181 60185 +4
=======================================
+ Hits 17796 17798 +2
- Misses 41118 41121 +3
+ Partials 1267 1266 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This was referenced Jul 16, 2026
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
interp/jit_arm64_test.go (1)
1313-1340: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInline the lockstep harness into each subtest.
balancedhides setup, execution, and assertions for four cases. Inline this harness pert.Run, including the native-entry assertion.As per coding guidelines, “Inline test setup, execution, and assertions unless the testing documentation explicitly allows a helper.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@interp/jit_arm64_test.go` around lines 1313 - 1340, Remove the shared balanced helper and inline its JIT/reference setup, 48-iteration lockstep execution, result comparison, refcount comparison, and reset logic directly into each of the four t.Run subtests. Preserve the existing assertions and add each case’s native-entry assertion within its respective subtest.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@interp/jit_arm64_test.go`:
- Around line 1092-1098: Extend the refcount assertions in the upvalue and
overwrite sub-cases of DeferredRefElision to compare rc[1:] with the threaded
execution state before each Reset(). Apply the same full-state comparison to the
additional cases around the referenced ranges, while preserving their existing
result and replacement-array assertions.
- Around line 1141-1155: Update the JIT regression tests around each listed
instance creation, including the test containing New(prog, WithTick(1),
WithThreshold(0)), to attach a profiler to every JIT instance and assert that
the vm_jit_native_entries_total metric is greater than zero after execution.
Preserve the existing assertions while ensuring each test proves native code was
entered rather than only fallback execution.
In `@interp/jit_arm64.go`:
- Around line 2484-2495: Remove the unreachable ctx.values ownership scan at
interp/jit_arm64.go lines 2484-2495; locals already owns transferred arguments,
so retain the surrounding control flow without this loop. Remove the equivalent
scan at interp/jit_arm64.go lines 2527-2541; locals handles arguments before
frame replacement, and no replacement logic is needed there.
- Around line 3228-3232: Materialize deferred ref ownership before transferring
refs into interpreter-visible storage: in interp/jit_arm64.go lines 3228-3232,
update the array mutation path to call own on the ref value before capturing pre
and storing the array element; apply the same ordering in lines 3481-3485 for
the struct-field mutation path. Use the existing ownership helper and preserve
the current mutation behavior.
- Around line 4094-4127: Update flush to pre-scan ctx.values at function entry
when commit is true, rejecting any ownerConst or other deferred owner before
emitting ARM64 instructions or mutating symbolic state. Preserve ownerStack
validation and the existing non-commit emission behavior, then perform the
current write/clear logic only after the commit validation succeeds.
---
Nitpick comments:
In `@interp/jit_arm64_test.go`:
- Around line 1313-1340: Remove the shared balanced helper and inline its
JIT/reference setup, 48-iteration lockstep execution, result comparison,
refcount comparison, and reset logic directly into each of the four t.Run
subtests. Preserve the existing assertions and add each case’s native-entry
assertion within its respective subtest.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1dbca822-b529-4ed7-9410-dc3cea6d5285
📒 Files selected for processing (8)
AGENTS.mddocs/architecture.mddocs/benchmarks.mddocs/jit-internals.mddocs/memory-model.mdinterp/jit.gointerp/jit_arm64.gointerp/jit_arm64_test.go
This was referenced Jul 17, 2026
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
1 participant
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.
Problem
docs/benchmarks.md's worst competitive row wasSieve(256): 5,052 ns adaptive vs wazero 645 ns (7.8x) — the representative pattern of a runtime-allocated array held in a local. A control experiment isolated the cause: per array access,TypedArraySum(256)(constant array) costs 2.28 ns while Sieve costs 6.89 ns — 3x for the same element type and JIT. The delta is pure refcount churn: every refLOCAL_GETemitted a retain and the consumingARRAY_GET/ARRAY_SETemitted a matchingguardRC+ release — two serialized read-modify-writes on the samerccell, per element, net zero.Change
The JIT fusion layer already had a deferral concept for compile-time constant ref markers (
rawrefs: no retain at push,box()materializes it,sideExit.retainbalances exits). This PR unifies that into a single ownership model and extends it to every slot-backed producer:valuegainsowner(ownerStack/ownerConst/ownerLocal/ownerGlobal/ownerUpval) andhome. OnlyownerStackcarries its own retain; every other owner borrows from backing storage.LOCAL_GET/GLOBAL_GET/UPVAL_GETof a ref, const markers, andDUPof a deferred ref push deferred with no retain.ARRAY_GET/SET,STRUCT_GET/SET,ARRAY_LEN,REF_IS_NULL,DROP, and coroutine/error/string reads borrow the container and elide itsguardRC/release when deferred (element/payload retains are unchanged).own— slot stores, call-argument transfer, entry-frame returnssettle— backing slot overwritten (*_SET) or frame dying (stitch, tail dispatch)materializeExits— guard exit stubs reload each deferred operand from its flushed VM stack home and retain on the cold path onlyredeem— stub-less deopts (trap fallback, module completion)directCall/selfCallown every live deferred operand before theBL(a callee trap adopts the caller's flushed stack)consume()peephole andsideExit.retainscalar field are subsumed and removed.Results (Apple M4 Pro, darwin/arm64, Go 1.26.2, median of 3 × 300ms, serial)
Other rows moved within noise or from earlier branch commits;
docs/benchmarks.mdrefreshed as one consistent measurement set (July 16, 2026).Safety
The RC invariant (exact and symmetric on every path, including deopts) is preserved by construction: each deferred value is retained exactly once on any path that hands it to the interpreter.
TestARM64_DeferredRefElisionadds a JIT-vs-threaded oracle asserting popped results and every heap refcount agree across 48 lockstep runs, covering: local/global/upval backers,DUPfan-out, backer overwrite (settle), terminal-op trap (redeem), module completion, call-argument transfer, and self tail-call forwarding. Each regression test was verified to fail with its fix reverted.Test plan
go test -race ./...— all 16 packages greenbenchmarkscorrectness suite across default/threaded/jit modesgo vet,make lintclean-tags=compareexternal comparison re-measured serially;docs/benchmarks.md,docs/jit-internals.md(new Reference Ownership section),docs/memory-model.md,AGENTS.md,docs/architecture.mdupdatedKnown follow-ups (out of scope)
Summary by CodeRabbit
refoperands, ensuring retains are properly owned/redeemed across calls, branches, exits, trap fallback, and module completion, and preventing deferred refs from persisting across committing loop back-edges.refelision scenarios and verifying exact JIT vs threaded interpreter parity, including heap refcount stability.refownership/flush rules; refreshed benchmark results with new measurement data.