perf(codegen): put the numeric array push's GC bookkeeping behind one live test (push_num 0.149 -> 0.069) - #7839
Merged
Merged
Conversation
📝 WalkthroughWalkthroughNumeric array pushes now store values unconditionally and guard GC bookkeeping with live-bit heap-pointer checks. Array admission masks cover additional numeric layouts. New LLVM IR tests verify bookkeeping placement, runtime routing, pointer-push behavior, and non-constant guard conditions. ChangesNumeric push GC bookkeeping
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ArrayPushCodegen
participant HeapPointerCheck
participant NumericSlotStore
participant GCBookkeepingOps
ArrayPushCodegen->>NumericSlotStore: store numeric value unconditionally
ArrayPushCodegen->>HeapPointerCheck: test live bits for heap pointers
HeapPointerCheck-->>ArrayPushCodegen: computed pointer-carry result
ArrayPushCodegen->>GCBookkeepingOps: emit addref, layout note, and write barrier when needed
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
… live test
The inline array-append tier emitted `js_string_addref_if_heap_string`,
`js_gc_note_slot_layout` and a seq_cst load of
`PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT` on EVERY element. On
`bench/push_num.ts` — 20,000,000 pushes of a double into a `number[]` — all
three are dead on all 20M of them.
The static proof that retires them cannot be made for the shape that matters:
`keep.push(base + j)` is an `Expr::Binary { Add }`, and
`expr_produces_non_pointer_bits_by_construction` answers `false` there
unconditionally, because `+` is string concatenation for non-numeric operands.
This is #7511's answer to the identical problem on class-field stores, applied
to the array append: ask the question ONCE inline, on the live bits, and branch
over all three calls. The array's half of the proof rides the header test the
`nofwd` block already performs — the integrity mask widens from 0x0407 to
0x3C07, so reaching the inline store additionally proves ELEMENT_SHAPE,
TYPED_LAYOUT_INTACT and ALL_POINTERS clear, the three states in which
`js_gc_note_slot_layout` does real work for a non-pointer value.
A guard, not an elision: Perry does not validate declared types, so a
`number`-annotated value that is a heap string at runtime takes the guarded arm
and records the slot exactly as it always did.
proggeramlug
force-pushed
the
perf/7830-push-numeric-guard
branch
from
August 11, 2026 10:54
98b9956 to
8a828ec
Compare
…push guard Two sabotage-verified IR gates, prompted by review of the #7831/#7837 family against #7839's guard. `a_declared_type_lie_is_routed_to_the_runtime_tier_not_the_guard` — a `number[]` really can hold heap strings at runtime, and `is_numeric_expr` admits an element read off one (#7810). What keeps that value off the inline guard is `expr_produces_canonical_raw_f64` excluding every READ, which routes it to the pre-existing runtime numeric tier instead. Widening that predicate to admit a read fails this test. `the_guard_branches_on_the_live_bits_not_on_a_constant` — pins the guard's condition to a computed register and its predicate to the full heap-tag set. Hard-wiring the branch to `false` fails this test; it is invisible to every output-equality probe, because the elided bookkeeping is a GC-liveness fact rather than an arithmetic one.
proggeramlug
marked this pull request as ready for review
August 11, 2026 11:35
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/perry-codegen/src/expr/array_push_guard_tests.rs (1)
243-289: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that the write barrier remains in the guarded control-flow path.
These tests verify only
js_gc_note_slot_layoutandjs_string_addref_if_heap_string. They do not verifyjs_write_barrier_slot.If
emit_numeric_push_store_pointer_testedreturns barrier bits,crates/perry-codegen/src/expr/array_push.rsLines 1025-1036 emit the parent-generation gate afterapush.gc_bookkeeping.done. The current tests still pass, but every numeric push pays the barrier gate again.Assert that the barrier call remains emitted and that its parent-generation gate is reachable only from
apush.gc_bookkeeping.🤖 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 `@crates/perry-codegen/src/expr/array_push_guard_tests.rs` around lines 243 - 289, Extend the numeric push guard tests around emit_numeric_push_store_pointer_tested to verify js_write_barrier_slot is emitted and remains in the guarded control-flow path. Assert the barrier call is absent from the inline inbounds block and that its parent-generation gate is reachable only after apush.gc_bookkeeping.done, preserving the existing assertions for the note and addref calls.
🤖 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.
Nitpick comments:
In `@crates/perry-codegen/src/expr/array_push_guard_tests.rs`:
- Around line 243-289: Extend the numeric push guard tests around
emit_numeric_push_store_pointer_tested to verify js_write_barrier_slot is
emitted and remains in the guarded control-flow path. Assert the barrier call is
absent from the inline inbounds block and that its parent-generation gate is
reachable only after apush.gc_bookkeeping.done, preserving the existing
assertions for the note and addref calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9d611dda-7e33-4f74-b934-23611e7f41c0
📒 Files selected for processing (4)
changelog.d/7839-numeric-push-pointer-tested.mdcrates/perry-codegen/src/expr/array_push.rscrates/perry-codegen/src/expr/array_push_guard_tests.rscrates/perry-codegen/src/expr/mod.rs
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.
What
The inline array-append tier emitted three GC-bookkeeping obligations on every
element:
js_string_addref_if_heap_string,js_gc_note_slot_layout, and aseq_cst load of
PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNTto gatejs_write_barrier_slot. Ongc-handoff/bench/push_num.ts— 20,000,000 pushes ofa double into a
number[]— all three are dead on all 20M of them.This is #7511's answer to the identical problem on class-field stores, applied to
the array append: ask the question once, inline, on the live bits, and branch
over all three calls.
Why the static proof cannot do it
array_store_needs_layout_noterests onexpr_produces_non_pointer_bits_by_construction, and the shape that matters iskeep.push(base + j)— anExpr::Binary { Add }, where that predicate answersfalseunconditionally because+is string concatenation for non-numericoperands. It fires only for a bare canonical-i32 local, which is why
keep.push(j)andkeep.push(base + j)compile to materially different loopstoday.
The two halves of the guard
emit_may_carry_heap_pointer_check— already the codegen mirrorof
layout_pointer_bearing_bits/decode_heap_addr, already contract-testedover the whole 16-bit tag space. The store itself stays unconditional and
outside the branch; only the bookkeeping moves.
nofwdblock already performs: theintegrity mask widens
0x0407→0x3C07, so reaching the inline storeadditionally proves
GC_ARRAY_ELEMENT_SHAPE,GC_OBJ_TYPED_LAYOUT_INTACTandGC_LAYOUT_ALL_POINTERSclear — the three states in whichjs_gc_note_slot_layoutdoes real work for a non-pointer value. An array inany of them takes
js_array_push_f64, which notes the slot exactly as before.Same
and, sameicmp, wider constant.GC_LAYOUT_SIDE_MASKis deliberately not in the mask: skipping the note thereleaves a stale set bit over a non-pointer, and
mark_field_into_worklistre-validates every slot word, so the cost is one rejected visit and never a
stranded child — the identical argument
class_field_store_needs_layout_notealready ships.
A guard, not an elision.
the_guarded_arm_still_reaches_every_call_it_movedasserts the calls are still emitted, so a future "simplification" to an outright
elision fails there rather than as heap corruption.
Gated on
is_numeric_expr, so a pointer-pushing loop emits byte-identical IR.Measured — absolute seconds, quiet M1 mini
best-of-25, interleaved with rotated start position, exit codes checked, output
byte-identical to
node --experimental-strip-types. Load 1.17 before / 1.99after, zero foreign benchmark or compiler processes at both ends.
main@1ee158d27push_num0.1494 → 0.0692 s, −53.7%. It was the only benchmark in the corpusPerry lost to both node and scriptc; it is now 0.60× node and 0.74×
scriptc. The distributions are disjoint — this PR's worst of 25 (0.0896) is
below scriptc's best (0.0931).
No regression anywhere else — by construction, not by timing
24 of the 25 corpus programs compile BYTE-IDENTICALLY between
mainand thisbranch (
cmpovergc-handoff/m0810's full list plus the_realarms). Onlypush_numdiffers. Nothing else can have moved.The two byte-identical pairs were timed anyway as an in-run noise floor: the same
binary under two names read +0.19% (
push_cls) and −0.05% (churn)apart, so the host's floor on this run is ~0.2% and the −53.7% is ~250× it.
_realarmschurn_real,churn_alloc_real,push_cls_real,cycles_real,tree_real,retain_realare all in the byte-identical set, so their bootstrap penalty isunchanged by construction.
The declared-type lie (#7831/#7837) — reviewed, and it cannot reach this guard
A peer raised the case my own probes could not reach: a
number[]fed a heapstring built by
+. I checked it rather than assumed, and the answer is worthmore than the test would have been.
gc-handoff/m0810/numarr_lie.tsdoes not reach the guard. IR census: 0apush.gc_bookkeepingblocks. Its pushed value is(s1 + s2 + String(i))withs1/s2typedany, sois_numeric_expris false,guarded_numeric_bookkeepingis false, and the push keeps the historical unguarded tier untouched by this PR.
Empirically it prints
2000 string hello wor0 hello wor1999 6000on pre-#7839main, on this branch, and on a deliberately sabotaged compiler with the guard
hard-wired to
false— so it cannot fail on a broken guard. It is a fineregression test for the pre-existing path; it is not a detector for #7839.
Four further lie shapes — a
numberparameter, anumberobject field, amodule-level
numberglobal, and an element read off anumber[]— all emitzero guard blocks. The reason is structural, and is the clearest statement of
why this guard is safe:
is_numeric_exprdoes admit a read off anumber[](fix(codegen): stop typing a symbol-keyed element read as a number (#7796) #7810), so an erasedannotation alone would put a heap string on a numeric push path.
expr_produces_canonical_raw_f64excludes every read ("cold fallbacksreturn boxed bits"), so
keep_guarded_numeric_pushstays true and those pushesroute to the pre-existing runtime numeric tier
(
js_array_numeric_push_f64_unboxedbehind its feedback guard), whichvalidates the value at runtime.
canonical raw f64 by construction — a machine FP op. Such a value cannot be a
pointer except by ARM NaN-payload propagation from a NaN-boxed operand, which
is exactly what the live-bits test catches.
Two sabotage-verified detectors
Both are IR gates that run in
cargo test, and both were confirmed to fail ona deliberately broken compiler — the bar the runtime probes could not clear:
the_guard_branches_on_the_live_bits_not_on_a_constantbr i1 false(bookkeeping arm unreachable)a_declared_type_lie_is_routed_to_the_runtime_tier_not_the_guardexpr_produces_canonical_raw_f64widened to admit a readThe first pins the guard's condition to a computed register and its predicate to
the full heap-tag comparand set. The second pins the routing above, so a future
widening of the numeric proof surfaces here rather than as a guard resting on an
annotation.
Correctness
cargo test --release -p perry-codegen --lib— 861 passed, 0 failed,including five IR-census tests for this change.
the recorded node expectations.
gc-handoff/apps/iso_miss.tsprintschecksum 437840 misses 0, plainand under
PERRY_GC_SCHEDULE_RATE=1.numarr_lie.tsmatches node underPERRY_GC_SCHEDULE_RATE=1,…_ALLOC_KB=0and
PERRY_GC_VERIFY_EVACUATION=1 PERRY_GC_FORCE_EVACUATE=1.cargo fmt --all -- --checkclean.Honest limitations
The runtime probes are not detectors, which is why the two IR gates exist. A
sabotaged compiler with the guard hard-wired off passed every runtime probe I
wrote, and passed
numarr_lie.tstoo. Soundness rests on the routing argumentabove and on the two sabotage-verified gates, not on a runtime detector.
I am not citing the from-space protector as evidence. Under
PERRY_GC_DIAG=1it prints zero[gc-fromspace-protect] retired_set=lineson these programs, so the instrument never ran and that arm proves nothing.
Side observation for anyone extending this: for these shapes the layout note
appears less load-bearing than the surrounding comments imply, most likely
because the arrays end up
GC_LAYOUT_UNKNOWNand are tag-scanned anyway.Not run locally
The gap suite — it needs a quiet host, and the shared mini had several sessions
contending for it all day.
Pre-existing red gates, not from this branch
scripts/check_file_size.shfails onmain@1ee158d27 forcrates/perry-hir/src/lower/pre_scan.rs(2011 lines) andcrates/perry-runtime/src/gc/layout.rs(2023) — both were under the cap at0a2bf15bd. Neither is touched here.