fix(gc)+perf(closure): root the uint8 Buffer callback dispatcher (#8179) and hoist per-element closure dispatch (#8180) - #8188
Merged
Conversation
|
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 (14)
📝 WalkthroughWalkthroughThe PR adds hoisted direct callback dispatch for arities one through four. It roots Uint8Array receivers, callbacks, comparators, and reduction accumulators across allocating calls. It adds GC stress coverage for Buffer-backed typed-array callbacks. ChangesCallback dispatch and GC rooting
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Uint8ArrayMethod
participant RuntimeHandleScope
participant DirectCall3
participant Callback
Uint8ArrayMethod->>RuntimeHandleScope: root receiver and callback
Uint8ArrayMethod->>DirectCall3: resolve callback once
loop Each element
Uint8ArrayMethod->>RuntimeHandleScope: read current rooted pointers
Uint8ArrayMethod->>DirectCall3: call callback
DirectCall3->>Callback: invoke direct target or fallback
end
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 |
added 2 commits
August 16, 2026 10:12
…ross user callbacks (#8179) `dispatch_uint8_buffer_method` — the shared uint8 `%TypedArray%.prototype` dispatcher every Buffer-backed `Uint8Array` callback method funnels through, on all three of its entries — kept the callback closure, the receiver, `map`'s freshly allocated result buffer, `sort`/`toSorted`'s permuted output and `reduce`/`reduceRight`'s accumulator in bare Rust locals across `js_closure_call{2,3,4}`. THE CLOSURE IS THE LIVE HALF. It is an ordinary nursery allocation (`GC_TYPE_CLOSURE`, with a `GcMoveHookKind::ClosureDynamicProps` move hook — it both moves and dies), and a callback handed in by a frameless caller is reachable only through that raw parameter plus the native stack, which an evacuating minor does not scan. `array::buffer_receiver_dispatch` rooted it at the boundary; the `%TypedArray%.prototype` thunk and `dispatch_buffer_method`'s catch-all did not. It is now rooted here, where all three entries get it, and RE-READ from the root before every call. `test-files/test_gap_gc_uint8_buffer_callback_rooting.ts` (registered in `test-parity/gc_repsel_corpus.txt`) fails on the SHIPPED DEFAULT before this change — `TypeError: value is not a function`, exit 1 — and under `PERRY_GC_SCHEDULE_RATE=1 PERRY_GC_SCHEDULE_SEED=<n> PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_VERIFY_EVACUATION=1` dies on the FIRST scheduled collection, for every seed tried (1, 7, 42): [gc-fromspace-protect] FAULT: signal 10 at 0x2454161058c last-known object: user_ptr=0x24541610580 obj_type=4 size=24 [gc-schedule] FAILURE (signal 10) under seed=7 [gc-schedule] safepoints=1 scheduled_collections=1 `obj_type=4` is `GC_TYPE_CLOSURE`; the faulting address is `user_ptr + 12` — `CLOSURE_TYPE_TAG_OFFSET`, i.e. `get_valid_func_ptr`'s `CLOSURE_MAGIC` probe reading a retired from-space closure header. After the fix the same seeds run to completion with the instrument's own liveness verdict proving the subject ran: `safepoints=5306 scheduled_collections=5306 copying_minors=5306 moved_objects=25760`, 0 faults, exit 0. THE RECEIVER IS THE OTHER HALF, and is treated differently on purpose. A Buffer is `arena_alloc_gc_old` + `GC_FLAG_TENURED` (`buffer/header.rs`) — the same old-arena space `typed_array_alloc` calls "non-movable space: raw data pointers are handed out" — and every `%TypedArray%` sibling in `typedarray/iterate.rs` / `typedarray/transform.rs` already holds its receiver in a plain local across callbacks on that invariant. The receiver is therefore rooted for LIVENESS (the raw parameter is otherwise its only reference on two of the three entries) and its address is read from the root ONCE per arm — after the callback validation, before the loop — instead of being carried in from the parameter. The one arm that relocates an old-arena page is old-page defrag, which is opt-in and default-off (`PERRY_GC_OLD_DEFRAG=1`); making that safe is a tree-wide property of every holder of an old-arena raw address, not something one dispatcher can establish, and re-reading per element measured +28 % on a `Uint8Array` forEach/map/reduce benchmark for a knob that is off. Two sibling families in the same shape get the same treatment: * `js_typed_array_reduce` / `js_typed_array_reduce_right` now root their accumulator, as their plain-array sibling `js_array_reduce` has since the 2026-07-02 audit. It is a nursery object whenever the seed or a callback result is a string/object/array. * the two non-BigInt arms of `js_typed_array_sort_with_comparator` / `js_typed_array_to_sorted_with_comparator` now root the comparator closure. `sorted_bigint_lanes`, directly above them, has done so since it was written — "the comparator closure itself is re-derived from a rooted handle per call (a comparator-triggered GC can relocate its own closure header)".
…llback loops (#8180) `js_closure_callN` re-derived, on EVERY element of every fused array-callback loop, three answers that cannot change while one closure is being called: 1. `get_valid_func_ptr` — two address-band checks, a volatile `CLOSURE_MAGIC` probe through `*(closure + 12)` and a volatile `func_ptr` load; 2. `resolve_strategy` — a `perry_thread_local!` single-slot cache, which on Darwin is a `tlv_get_addr` CALL plus a load and a compare even on a hit; 3. the `DispatchStrategy` match before the indirect jump. The tree already contained the answer, applied to exactly one call site: `array/sort.rs`'s `ComparatorCall`, introduced to "skip ~50M HashMap lookups over a 1.25M-element sort". New `closure/dispatch/direct.rs` generalises it to arities 1–4 as `DirectCall{1,2,3,4}` — resolve once, call directly, fall back to `js_closure_callN` for a bound method/function, a rest parameter, a declared arity above the call arity, or an invalid closure pointer, so the proxy-callee/throw path, the rest bundling and the undefined-padding stay in one place. `resolve_call2_direct` is DELETED rather than left standing beside it; `ComparatorCall` now holds a `DirectCall2`. Hoisted at 31 call sites: `array/iter_methods.rs` (14), `array/reduce_right.rs` (1), `typedarray/iterate.rs` (9), `typedarray/transform.rs` (5 — including the BigInt lane comparator, which resolved once per COMPARISON) and the uint8 `%TypedArray%.prototype` dispatcher's `RootedCallback{2,3,4}`. Measured on a quiet M1 mini, instructions retired, best of 5, arms interleaved, per-arm `PERRY_RUNTIME_DIR` + `PERRY_CACHE_DIR`, `PERRY_NO_AUTO_OPTIMIZE=1`: bench main(A) +8179(B) +8179+8180(C) C vs A arr 5,027,207,909 5,027,015,176 3,970,592,563 -21.0 % u8 1,979,043,224 2,535,814,615 1,978,697,176 -0.02 % `arr` is 21M plain-`Array` callback invocations (forEach/map/filter/reduce/ findIndex/some/every); `u8` is 7.9M Buffer-backed `Uint8Array` ones. Peak RSS is flat: `arr` 46,628,864 B on both A and C, `u8` 14,794,752 -> 14,876,672 B (+0.55 %, 20 pages, the handle stack and the resolved sites). #8179's rooting costs +28 % on the `u8` path on its own; this change pays all of it back and the plain-`Array` path is 21 % cheaper than main. SOUNDNESS. `closure->func_ptr` is written once at `js_closure_alloc` and never mutated; `lookup_closure_rest` / `lookup_closure_arity` are keyed by it and are insert-only per key, registered at closure creation; the two sentinels are process constants. The only way to observe a different strategy mid-loop is to call a DIFFERENT closure, and an array method calls one. Callers that root their callback pass the CURRENT address to `call`; the resolved target is a static CODE address, which relocation does not change — the argument `ComparatorCall::compare_at` already documents. The unit tests in `direct.rs` assert the fast path is LIVE (`is_direct()`), not merely that nothing threw, and assert each decline (higher declared arity, rest parameter, invalid pointer). `array/generic.rs`'s `js_arraylike_*` engine is deliberately NOT converted. Its per-element cost is dominated by generic array-like property access (`al_has` + `al_get`, full prototype-chain lookups) rather than by dispatch, and the spec order it implements reads `LengthOfArrayLike` BEFORE `IsCallable` — so a hoisted resolve would have to be sequenced after the existing `callable()` call rather than inserted at the top of the function, which is not the same mechanical edit and risks moving an observable throw.
proggeramlug
force-pushed
the
fix/8179-8180-array-callback
branch
from
August 16, 2026 08:19
dce5052 to
7c6212c
Compare
proggeramlug
marked this pull request as ready for review
August 16, 2026 08:21
7 tasks
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.
Two changes that had to land together: #8179 adds rooting traffic to exactly
the loops #8180 makes cheaper, so measuring them apart would misattribute
both. #8179 is first in the branch history, #8180 second, and the three-arm
measurement below is taken at those two commits.
Closes #8179. Closes #8180.
#8179 — unrooted values held across
js_closure_call*dispatch_uint8_buffer_methodis the shared uint8%TypedArray%.prototypedispatcher that every Buffer-backed
Uint8Arraycallback method funnelsthrough, from three entries. It kept the callback closure, the receiver,
map'sfreshly allocated result buffer,
sort/toSorted's permuted output andreduce/reduceRight's accumulator in bare Rust locals acrossjs_closure_call{2,3,4}.The closure is the live half. It is an ordinary nursery allocation
(
GC_TYPE_CLOSURE,GcAllocationPolicy::ArenaOrMalloc, with aGcMoveHookKind::ClosureDynamicPropsmove hook — it both moves and dies), and acallback handed in by a frameless caller is reachable only through that raw
parameter plus the native stack, which an evacuating minor does not scan.
array::buffer_receiver_dispatchrooted it at the boundary; the%TypedArray%.prototypethunk anddispatch_buffer_method's catch-all did not.The fixture fails before and passes after
test-files/test_gap_gc_uint8_buffer_callback_rooting.ts, registered intest-parity/gc_repsel_corpus.txt. Buffer-backedUint8Array, stringaccumulators, allocating callbacks — and the callbacks allocate inside a
loop, because a back-edge poll is the only safepoint reachable from user JS
and an allocation-free callback would make the file pass vacuously.
It reproduces on the shipped default, no instrument required:
SCHEDULE_RATE=1 SCHEDULE_SEED={1,7,42} PROTECT_FROMSPACE=1 VERIFY_EVACUATION=1mainTypeError: value is not a function, exit 1main+#81790, exit 0main+#8179+#81800, exit 0The pre-fix fault names the object:
obj_type=4isGC_TYPE_CLOSURE, and the faulting address isuser_ptr + 12—CLOSURE_TYPE_TAG_OFFSET, i.e.get_valid_func_ptr'sCLOSURE_MAGICprobereading a retired from-space closure header. It dies on the first scheduled
collection.
The instrument is armed, and the green run is not vacuous. Post-fix, the
same seeds print the schedule instrument's own exit verdict:
5306 copying minors and 25,760 objects actually moved, so "no fault" is a
verdict rather than "nothing ran".
The receiver is the latent half, and is deliberately not re-read per element
A Buffer is
arena_alloc_gc_old+GC_FLAG_TENURED(buffer/header.rs) — thesame old-arena space
typed_array_allocdocuments as "non-movable space: rawdata pointers are handed out" — and every
%TypedArray%sibling intypedarray/iterate.rs/typedarray/transform.rsalready holds its receiver ina plain local across callbacks on that invariant.
So the receiver is rooted for liveness (the raw parameter is otherwise its
only reference on two of the three entries) and its address is read from the
root once per arm — after the callback validation, before the loop — rather
than carried in from the parameter. The only collector arm that relocates an
old-arena page is old-page defrag, which is opt-in and default-off
(
PERRY_GC_OLD_DEFRAG=1, andgc/oldgen_defrag.rssays so explicitly). Makingthat arm safe is a tree-wide property of every holder of an old-arena raw
address, not something one dispatcher can establish — and I measured re-reading
per element at +28 % on the Uint8Array benchmark, for a knob that is off.
That trade is written into the type's doc comment so the next reader does not
have to re-derive it.
Adjacent things fixed rather than filed
js_typed_array_reduce/js_typed_array_reduce_rightnow root theiraccumulator, as their plain-array sibling
js_array_reducehas since the2026-07-02 audit. It is a nursery object whenever the seed or a callback
result is a string/object/array.
js_typed_array_sort_with_comparator/js_typed_array_to_sorted_with_comparatornow root the comparator closure.sorted_bigint_lanes, directly above them, has done so since it was written("the comparator closure itself is re-derived from a rooted handle per call") —
the two arms beside it were missed.
Raw-handle conversion form used at each site
The ratchet is unchanged at 990 (
scripts/raw_handle_debt.py, plus--self-test). No site in this PR is a bareget_raw_*_ptr.RootedCallback{2,3,4}::call(uint8 dispatcher)with_const_ptracross_*cannot express, since it hands the address back only after the call. The handle is re-read on every iteration immediately before the pointer is consumed, and theRawTaggedslot is onescan_runtime_handle_roots_mutboth marks and rewrites, so a collection during call i is reflected in the address call i+1 uses.js_typed_array_sort_with_comparator/..._to_sorted_...comparatorwith_const_ptrsort_by.reduceaccumulatorroot_nanbox_f64+get/set_nanbox_f64js_typed_array_reduce{,_right}accumulatorroot_nanbox_f64+get/set_nanbox_f64root_nanbox_f64+ onelive()per armscripts/gc_root_dominance_check.pyis structurally blind here — these are Rustlocals, not emitted IR — so the runtime instruments above are the only detector,
which is why the witness carries its verdict lines.
#8180 — ~368 instructions per array-callback invocation
js_closure_callNre-derived, on every element, three answers that cannotchange while one closure is being called:
get_valid_func_ptr(two address-bandchecks, a volatile
CLOSURE_MAGICprobe through*(closure + 12), a volatilefunc_ptrload),resolve_strategy(aperry_thread_local!single-slot cache —on Darwin a
tlv_get_addrcall plus a load and a compare even on a hit), andthe
DispatchStrategymatch before the indirect jump.New
closure/dispatch/direct.rsgeneralisesarray/sort.rs'sComparatorCalltrick — introduced to "skip ~50M HashMap lookups over a 1.25M-element sort",
and until now its only consumer — into
DirectCall{1,2,3,4}. Resolve once, calldirectly, fall back to
js_closure_callNfor a bound method/function, a restparameter, a declared arity above the call arity, or an invalid closure pointer,
so the proxy-callee/throw path, the rest bundling and the undefined-padding stay
in exactly one place.
resolve_call2_directis deleted rather than leftstanding beside its generalisation;
ComparatorCallnow holds aDirectCall2.Hoisted at 31 call sites:
array/iter_methods.rs(14),array/reduce_right.rs(1),typedarray/iterate.rs(9),typedarray/transform.rs(5 — including the BigInt lane comparator, whichresolved once per comparison) and the uint8 dispatcher's
RootedCallback{2,3,4}.Why hoisting is sound
Each input is invariant for a fixed closure:
closure->func_ptris written once byjs_closure_allocand never mutated,so
get_valid_func_ptranswers the same code address every time. A movingcollection cannot change it either — that is the same argument
ComparatorCall::compare_atalready documents, and it is why callers thatroot their callback pass the current header address to
callwhile theresolved target stays cached.
lookup_closure_rest/lookup_closure_arityare keyed by that func_ptr andare insert-only per key; registration happens at closure creation, before the
closure can be passed anywhere.
BOUND_METHOD_FUNC_PTR/BOUND_FUNCTION_FUNC_PTRare process constants.So the only way a loop could observe a different dispatch strategy mid-iteration
is by calling a different closure, and an array method calls one. A site that
can retarget its callee per element must not use these types, and the module doc
says so.
The unit tests in
direct.rsassert the fast path is live (is_direct()) —not merely that nothing threw — and assert each decline: declared arity above the
call arity, a rest parameter, an invalid closure pointer.
Not converted, on purpose
array/generic.rs'sjs_arraylike_*engine. Its per-element cost is dominatedby generic array-like property access (
al_has+al_get, fullprototype-chain lookups) rather than by dispatch, so the win would be small; and
the spec order it implements reads
LengthOfArrayLikebeforeIsCallable,so a hoisted resolve would have to be sequenced after the existing
callable()call rather than inserted at the top of the function. That is not the same
mechanical edit and it risks moving an observable throw.
#8103is a different defect and is not touched here.Measurement
Quiet M1 mini (load ~1.5). Instructions retired is primary — load-independent.
Best of 5, arms interleaved so host drift cannot land on one arm. Per-arm
PERRY_RUNTIME_DIRandPERRY_CACHE_DIR,PERRY_NO_AUTO_OPTIMIZE=1, builtwith an identical
-p perry -p perry-runtime-static -p perry-stdlib-staticseteach hop, and all three arms'
perry,libperry_runtime.a,libperry_stdlib.aand both benchmark binaries
cmp-verified pairwise different. Every run'sexit code checked; every arm's stdout matches node's.
arr— 21M plain-Arraycallback invocationsu8— 7.9M Buffer-Uint8Arraycallback invocationsReading it:
Arraypath (−0.004 %, inside the ±0.2 % first-repspread) — it does not touch those loops.
traffic, and it is why these had to be measured together.
(−0.02 %) while the plain-
Arraypath is 21 % cheaper than main.arris identical on main and the final arm;u8grows81,920 B of max RSS (20 pages, +0.55 %) and 16,448 B of peak footprint — the
transient-handle stack plus the resolved sites. No compute/RSS trade in either
direction.
The final commits differ from the exact sources measured only in doc comments
(the
PERRY_GC_ZEAL→PERRY_GC_SCHEDULE_*correction); verified by diffing themeasured blobs against the committed ones with doc lines filtered out — no
non-doc line differs.
Local validation
CI is deliberately not consulted (owner instruction); this is the gate.
All green:
cargo fmt --all -- --check,scripts/check_file_size.sh,scripts/raw_handle_debt.py(990, unchanged) and its--self-test,scripts/gc_runtime_root_holders.py,scripts/shape_descriptor_census.py,scripts/addr_class_inventory.py,scripts/check_gc_env_knobs.py,scripts/gc_store_site_inventory.py,scripts/gc_pin_sites.py,scripts/gc_gate_wiring_check.py,scripts/check_test_registration.py,scripts/gc_root_dominance_check.py --audit-poll-reach,scripts/workspace_architecture.py --check.cargo test -p perry-runtime --lib— 2481 passed / 0 failed / 4 ignored.main's reference is 2477 / 0 / 4; the delta is exactly the 4 newdirect.rstests.
cargo test -p perry-codegen --no-fail-fast— 1480 passed / 9 failed,identical to the recorded baseline. The 9 are the pre-existing
native_proof_buffer_views(6),loop_safepoint_purity(1),shadow_slot_hygiene(1) andtyped_feedback(1) failures. Zero new, andby construction: this PR touches no file in
perry-codegen— the whole diff isconfined to
crates/perry-runtime/,test-files/,test-parity/andchangelog.d/.Summary by CodeRabbit
Bug Fixes
Performance
Tests