perf(codegen): store reference values inline in the dynamic-key write IC (#8108) - #8183
Conversation
… IC (#8108) `lower_put_value_dyn_ic_inline`'s entry predicate ANDed in "the value tag is not pointer/string/bigint", so every reference-valued `o[k] = v` left the inline path before the receiver guards and took `js_put_value_set_dyn_ic` — one cross-crate call per write that re-validated, in Rust, exactly the guards the inline block had already proved. The tag now SELECTS a store arm. `put.dynic.store.scalar` keeps the pre-existing bare store and its `GC_STORE_AUDIT(POINTER_FREE)` claim unchanged; `put.dynic.store.ref` runs `emit_jsvalue_slot_store_scalar_aware_on_block` — byte-for-byte the static write PIC's pointer-capable store, reached under strictly stronger conditions, since the guards above it are that PIC's guards and this block additionally knows the value carries a reference tag. The one thing the outlined helper does that the inline arm does not, `canonicalize_typed_slot_store_bits`, is provably a no-op here: it returns early for every tag except `INT32_TAG`, which the reference arm excludes. No new rooting obligation. The target is materialised below every operand that can collect (the call site's existing evaluation-order argument), and all three bookkeeping helpers are `gc-leaf-function`, so nothing between the re-read and the store is a collection point. Measured best-of-5 on the quiet mini, release build, per-arm PERRY_RUNTIME_DIR and PERRY_CACHE_DIR, output verified against Node 26.5.1: o.x = { value: r + i } x4.8M 4.243G -> 2.860G instr (-32.6%) 690.8M -> 491.0M cycles (-28.9%) 212 -> 150 ms (-29.2%) 33248 -> 33200 KB peak RSS 7.85x -> 5.56x vs node o.x = produce(r, i) x12M 5.362G -> 5.337G instr (-0.45%) o.x = pointer x9.6M 2.2649G -> 2.2649G instr binary size 13,544,480 bytes both arms IPC moves 6.14 -> 5.83 on the improved cell, so the cycle win is smaller than the instruction win; both are reported. Arm B lands within 2.8% of the static-PIC ceiling for the same shape (2.860G vs 2.782G). This is #8108's measured prize reached by a different route, and the issue's own framing does not survive measurement on a3118cf: * `rhs_pointer` (9.21x) is ALREADY on the static write PIC. Its RHS is an `Expr::LocalGet`, which `put_value_rhs_is_safepoint_free` has always admitted. The safepoint gate never rejected it. * `rhs_call` (18.28x) would REGRESS. `const v = f(); o.x = v` is exactly the IR slice A would produce and it costs +21.4% instructions (5.362G -> 6.512G): the static PIC's hit block emits three unconditional `gc-leaf` bookkeeping calls whenever the value is not statically provable non-pointer, where the dyn IC proves it at runtime and stores bare. 95% of that cell is the closure call (425 of 447 instructions per iteration). * `rhs_allocating` is the real prize, and it needs no change to any safepoint rule. So `expr/proxy_reflect.rs`'s safepoint gate is left in place.
|
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)
📝 WalkthroughWalkthroughDynamic-key write ICs now inline stores for reference-tagged values with layout-aware write barriers. Scalar stores retain their direct path, and unsupported cases retain the semantic fallback. Native IR proofs and runtime coverage validate the new paths. ChangesDynamic-key reference store support
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
What
lower_put_value_dyn_ic_inline's entry predicate ANDed in "the value tag is notpointer/string/bigint", so every reference-valued
o[k] = vleft the inline pathbefore the receiver guards and took
js_put_value_set_dyn_ic— one cross-cratecall per write that re-validated, in Rust, exactly the guards the inline block had
already proved.
The tag now selects a store arm instead of gating entry:
put.dynic.store.scalar— the pre-existing bare store,GC_STORE_AUDIT(POINTER_FREE)claim unchanged, IR identical to before for non-reference values;
put.dynic.store.ref—emit_jsvalue_slot_store_scalar_aware_on_block, i.e.byte-for-byte the static write PIC's pointer-capable store, reached under
strictly stronger conditions: the guards above it are that PIC's guards, and this
block additionally knows the value carries a reference tag.
The one thing the outlined helper does that the inline arm does not —
canonicalize_typed_slot_store_bits— is provably a no-op here: it returns early forevery tag except
INT32_TAG, which the reference arm excludes.No new rooting obligation. The target is materialised below every operand that can
collect (the call site's existing evaluation-order argument), and all three bookkeeping
helpers are
gc-leaf-function, so nothing between the re-read and the store is acollection point.
scripts/gc_root_dominance_check.pyoverscripts/gc_root_dominance_corpus.sh: 0 violations, 0 unrooted allocas, emptyallowlist, 40/40 seeded violations caught, both modes.
Measurement
Best-of-5, release build, one
CARGO_TARGET_DIR/PERRY_RUNTIME_DIR/PERRY_CACHE_DIRper arm,PERRY_NO_AUTO_OPTIMIZE=1,(writes, sink)verifiedagainst Node 26.5.1. Wall/cycles from the quiet mini (
perry-macos, load ~1.5);instructions agreed with the dev box to within 0.4%. Base
a3118cfea.o.x = { value: r+i }×4.8Mo.x = produce(r, i)×12Mo.x = pointer×9.6M (static PIC)vs Node on the improved shape: 7.85x → 5.56x (node 27 ms). Arm B lands within
2.8% of the static-PIC ceiling for the same shape (2.860G vs 2.782G), so the write
path is close to spent — the residual 5.4x is allocation and object model, not the
store.
IPC drops 6.14 → 5.83 on the improved cell, so the cycle win is smaller than the
instruction win; both are reported (#8175). Binary size is unchanged:
13,544,480 bytes in both arms.
This is #8108's prize, reached by a different route — and its framing does not survive measurement
#8108 names three cells and proposes admitting a safepointing RHS into the static
write PIC. Measured on
a3118cfeabefore writing any code:rhs_pointer(9.21x) is already on the static PIC. Its RHS is anExpr::LocalGet, whichput_value_rhs_is_safepoint_freehas always admitted. Thesafepoint gate never rejected it; that cell's cost is the pointer store path.
rhs_call(18.28x) would REGRESS.const v = f(); o.x = vis exactly the IRslice A would produce, and it costs +21.4% instructions (5.362G → 6.512G, +18%
wall): the static PIC's hit block emits three unconditional
gc-leafbookkeepingcalls whenever the value is not statically provable non-pointer, where the dyn IC
proves it at runtime and stores bare. 95% of that cell is the closure call —
425 of 447 instructions per iteration — not the write.
rhs_allocatingis the real prize, and capturing it needs no change to anysafepoint rule, because the dyn IC already roots correctly.
So the gate at
expr/proxy_reflect.rsis left in place and #8108 is corrected onthe issue rather than implemented as specified.
Tests
Two IR tests in
native_proof_regressions.rs:dyn_ic_inline_store_barriers_a_reference_value— all three bookkeeping calls in thereference arm, their absence from the scalar arm, and (because an emitted block is
not a reached block) the
br i1into the reference arm;dyn_ic_inline_store_keeps_its_semantic_fallback_for_reference_values— the tag as anarm selector rather than an entry gate, plus the retained
js_put_value_set_dyn_icfallback.
Four sabotages, all four caught: drop the write barrier; drop the layout note +
string addref; route reference values back to
put.dynic.slow(leaving the arm as deadIR); leak a barrier into the scalar arm. With the change reverted the suite reads
1476 passed / 11 failed (the 9 pre-existing + these 2); with it, 1478 / 9, same
nine names.
test-files/test_gap_8108_dyn_ic_reference_store.tsis the behavioural half: everyvalue tag through one site, frozen / sealed / non-extensible / accessor / read-only
receivers, an inherited setter, a
Proxytrap, array and typed-array receivers, amid-loop shape transition, a throwing RHS that leaves no store, target→key→RHS
evaluation order, and a volume section whose producer is reached through an
any[]soit cannot be inlined into a rooted temp. Byte-identical to Node 26.5.1 under the default
GC and under
PERRY_GC_FORCE_EVACUATE=1 PERRY_GC_VERIFY_EVACUATION=1,PERRY_GEN_GC=0,PERRY_GC_ZEAL=1 PERRY_GC_PROTECT_FROMSPACE=1, andPERRY_WRITE_BARRIERS=0.Gates
cargo fmt --all -- --check,git diff --check, all 28lint-job steps generatedfrom
.github/workflows/test.yml(step 21 re-run with a real merge base),python3 scripts/check_gc_env_knobs.py(in no workflow, #8166),cargo test -p perry-runtime --lib(2445 / 0 / 4),cargo test -p perry-codegen --no-fail-fast(1478 / 9, zero new by name),cargo clippy -p perry-codegen --all-targets(no new warnings), and bothgc_root_dominance_check.pymodes.Refs #8108 (premise corrected there, with the measurement), #8184 (the follow-up this exposed), #6812.
Probe sources and the per-arm recipe (click to expand)
Each probe is a matrix cell body copied verbatim from
benchmarks/object-write-6812/matrix.tsand rescaled so a run is ~100-300 ms. Thereceiver is read out of an
any[], which is what keeps the store on the opaquesame-receiver
PutValuerather than the class-field route — a top-level (non-function)spelling of the same loop lowers differently and does not reach either write IC.
Which path a probe took (block labels carry numeric suffixes, so a bare
grep 'put.pic.guard:'reports zero for every one of them):A.
o.x = { value: r+i }— the improved shape (rhs_allocating)B.
o.x = produce(r, i)—rhs_callC.
const v = produce(r, i); o.x = v— the IR slice A would produceD.
o.x = pointer—rhs_pointer, already on the static PICSummary by CodeRabbit
Performance
Bug Fixes
Tests