Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 96 additions & 0 deletions changelog.d/8188-uint8-callback-rooting-and-hoisted-dispatch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
### Fixed — GC rooting in the uint8 Buffer callback dispatcher (#8179)

`dispatch_uint8_buffer_method` — the shared uint8 `%TypedArray%.prototype`
dispatcher that 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, so both `Uint8Array.prototype.map.call(u, fn)` and a
statically typed `u.map(fn)` were exposed.

`test-files/test_gap_gc_uint8_buffer_callback_rooting.ts` fails on the **shipped
default** before the fix (`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` it 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` and 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 showing the subject was
live: `safepoints=5306 scheduled_collections=5306 copying_minors=5306
moved_objects=25760`, zero faults, exit 0. The witness is registered in
`test-parity/gc_repsel_corpus.txt`.

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 already holds its receiver in
a plain local across callbacks on that invariant. It is now rooted for
*liveness* (the raw parameter is otherwise its only reference on two of three
entries) with its address read from the root once per arm rather than once per
element. The one collector 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, and
re-reading per element cost +28 % on the `Uint8Array` benchmark for a knob that
is off.

Two sibling families get the same treatment: `js_typed_array_reduce` /
`js_typed_array_reduce_right` now root their accumulator (as `js_array_reduce`
has since the 2026-07-02 audit), and 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` beside them already did.

### Performance — hoisted per-element closure dispatch (#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:
`get_valid_func_ptr` (two address-band checks, a volatile `CLOSURE_MAGIC` probe
through `*(closure + 12)`, a volatile `func_ptr` load), `resolve_strategy` (a
`perry_thread_local!` single-slot cache — on Darwin a `tlv_get_addr` CALL plus a
load and a compare even on a hit), and the `DispatchStrategy` match before the
indirect jump.

New `closure/dispatch/direct.rs` generalises `array/sort.rs`'s `ComparatorCall`
trick — 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, 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.
`resolve_call2_direct` is deleted rather than left standing beside it.

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.

Instructions retired, quiet M1 mini, best of 5, arms interleaved:

| benchmark | main | +#8179 | +#8179+#8180 | vs main |
|---|---|---|---|---|
| 21M plain-`Array` callback invocations | 5,027,207,909 | 5,027,015,176 | 3,970,592,563 | **−21.0 %** |
| 7.9M Buffer-`Uint8Array` callback invocations | 1,979,043,224 | 2,535,814,615 | 1,978,697,176 | **−0.02 %** |

Peak RSS is flat: 46,628,864 B on both arms of the first benchmark;
14,794,752 → 14,876,672 B (+0.55 %, 20 pages) on the second.

`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`), not 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.
78 changes: 63 additions & 15 deletions crates/perry-runtime/src/array/iter_methods.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Higher-order array methods.
use super::*;
use crate::closure::{js_closure_call3, js_closure_call4, ClosureHeader};
use crate::closure::ClosureHeader;
use std::ptr;

/// NaN-box an array header pointer as the JS `array` receiver value passed as
Expand Down Expand Up @@ -195,6 +195,10 @@ pub extern "C" fn js_array_forEach(arr: *const ArrayHeader, callback: *const Clo
let length = (*arr).length;
let scope = crate::gc::RuntimeHandleScope::new();
let rooted = RootedIterArray::new(&scope, arr);
// #8180: resolve the callback's dispatch ONCE. It is invariant for a
// fixed closure (see closure/dispatch/direct.rs), and this loop calls
// exactly one.
let cb_site = crate::closure::DirectCall3::resolve(callback);
// The override is a movable `ObjectHeader` held across user callbacks
// that allocate — root it for the duration of the loop.
let self_handle = self_override.map(|recv| scope.root_nanbox_f64(recv));
Expand All @@ -210,7 +214,7 @@ pub extern "C" fn js_array_forEach(arr: *const ArrayHeader, callback: *const Clo
continue;
}
let element = crate::array::array_spec_get(arr, i as u32);
js_closure_call3(callback, element, i as f64, self_value(&rooted));
cb_site.call(callback, element, i as f64, self_value(&rooted));
}
return;
}
Expand All @@ -222,7 +226,7 @@ pub extern "C" fn js_array_forEach(arr: *const ArrayHeader, callback: *const Clo
// dispatch path supports call3 safely, so bound native
// methods like `array.forEach(console.log)` can observe the
// source array just like Node.
js_closure_call3(callback, element, i as f64, self_value(&rooted));
cb_site.call(callback, element, i as f64, self_value(&rooted));
}
}
}
Expand Down Expand Up @@ -261,6 +265,10 @@ pub extern "C" fn js_array_map(
let length = (*arr).length;
let scope = crate::gc::RuntimeHandleScope::new();
let rooted = RootedIterArray::new(&scope, arr);
// #8180: resolve the callback's dispatch ONCE. It is invariant for a
// fixed closure (see closure/dispatch/direct.rs), and this loop calls
// exactly one.
let cb_site = crate::closure::DirectCall3::resolve(callback);
// Root the callback closure across the iteration. A callback allocated
// by a frameless caller (arrow/method — #6081) is reachable ONLY via
// this raw param + the native stack, which an evacuating minor does NOT
Expand Down Expand Up @@ -307,7 +315,7 @@ pub extern "C" fn js_array_map(
};
// JS .map() callback receives (element, index, array).
let callback = cb_handle.get_raw_const_ptr::<ClosureHeader>();
let mapped = js_closure_call3(callback, element, i as f64, rooted.receiver());
let mapped = cb_site.call(callback, element, i as f64, rooted.receiver());
if is_plain {
let result = result_arr(&result_rooted);
let result_elements =
Expand Down Expand Up @@ -363,6 +371,10 @@ pub extern "C" fn js_array_map_discard(arr: *const ArrayHeader, callback: *const
let length = (*arr).length;
let scope = crate::gc::RuntimeHandleScope::new();
let rooted = RootedIterArray::new(&scope, arr);
// #8180: resolve the callback's dispatch ONCE. It is invariant for a
// fixed closure (see closure/dispatch/direct.rs), and this loop calls
// exactly one.
let cb_site = crate::closure::DirectCall3::resolve(callback);
// The callback needs the same root its sibling `js_array_map` gives it
// (#6081), and for the same reason: a callback allocated by a frameless
// caller — the arrow in `xs.map(x => …)` — is reachable ONLY through this
Expand Down Expand Up @@ -396,15 +408,15 @@ pub extern "C" fn js_array_map_discard(arr: *const ArrayHeader, callback: *const
continue;
}
let element = crate::array::array_spec_get(arr, i as u32);
let _ = js_closure_call3(current_callback(), element, i as f64, rooted.receiver());
let _ = cb_site.call(current_callback(), element, i as f64, rooted.receiver());
}
return;
}
for i in 0..length as usize {
let Some(element) = rooted.present(i) else {
continue;
};
let _ = js_closure_call3(current_callback(), element, i as f64, rooted.receiver());
let _ = cb_site.call(current_callback(), element, i as f64, rooted.receiver());
}
}
}
Expand Down Expand Up @@ -443,6 +455,10 @@ pub extern "C" fn js_array_filter(
let length = (*arr).length;
let scope = crate::gc::RuntimeHandleScope::new();
let rooted = RootedIterArray::new(&scope, arr);
// #8180: resolve the callback's dispatch ONCE. It is invariant for a
// fixed closure (see closure/dispatch/direct.rs), and this loop calls
// exactly one.
let cb_site = crate::closure::DirectCall3::resolve(callback);
// Root the callback across the loop — see js_array_map / gh #6206.
let cb_handle = scope.root_raw_const_ptr(callback);
let _tg = DenseThisGuard::bind_undefined();
Expand Down Expand Up @@ -473,7 +489,7 @@ pub extern "C" fn js_array_filter(
}
};
let callback = cb_handle.get_raw_const_ptr::<ClosureHeader>();
let keep = js_closure_call3(callback, element, i as f64, rooted.receiver());
let keep = cb_site.call(callback, element, i as f64, rooted.receiver());
// Proper truthy check: handles NaN-boxed booleans (TAG_FALSE != 0.0 but is falsy)
if crate::value::js_is_truthy(keep) != 0 {
if is_plain {
Expand Down Expand Up @@ -527,6 +543,10 @@ pub extern "C" fn js_array_find(arr: *const ArrayHeader, callback: *const Closur
let length = (*arr).length;
let scope = crate::gc::RuntimeHandleScope::new();
let rooted = RootedIterArray::new(&scope, arr);
// #8180: resolve the callback's dispatch ONCE. It is invariant for a
// fixed closure (see closure/dispatch/direct.rs), and this loop calls
// exactly one.
let cb_site = crate::closure::DirectCall3::resolve(callback);
let _tg = DenseThisGuard::bind_undefined();
let exotic = crate::array::array_iteration_is_exotic(arr);

Expand All @@ -536,7 +556,7 @@ pub extern "C" fn js_array_find(arr: *const ArrayHeader, callback: *const Closur
} else {
rooted.get_or_undefined(i)
};
let result = js_closure_call3(callback, element, i as f64, rooted.receiver());
let result = cb_site.call(callback, element, i as f64, rooted.receiver());
// Proper truthy check: handles NaN-boxed booleans
if crate::value::js_is_truthy(result) != 0 {
return element;
Expand Down Expand Up @@ -583,6 +603,10 @@ pub extern "C" fn js_array_findIndex(
let length = (*arr).length;
let scope = crate::gc::RuntimeHandleScope::new();
let rooted = RootedIterArray::new(&scope, arr);
// #8180: resolve the callback's dispatch ONCE. It is invariant for a
// fixed closure (see closure/dispatch/direct.rs), and this loop calls
// exactly one.
let cb_site = crate::closure::DirectCall3::resolve(callback);
let _tg = DenseThisGuard::bind_undefined();
let exotic = crate::array::array_iteration_is_exotic(arr);

Expand All @@ -592,7 +616,7 @@ pub extern "C" fn js_array_findIndex(
} else {
rooted.get_or_undefined(i)
};
let result = js_closure_call3(callback, element, i as f64, rooted.receiver());
let result = cb_site.call(callback, element, i as f64, rooted.receiver());
// Proper truthy check: handles NaN-boxed booleans
if crate::value::js_is_truthy(result) != 0 {
return i as i32;
Expand Down Expand Up @@ -624,6 +648,10 @@ pub extern "C" fn js_array_find_last(
let length = (*arr).length as usize;
let scope = crate::gc::RuntimeHandleScope::new();
let rooted = RootedIterArray::new(&scope, arr);
// #8180: resolve the callback's dispatch ONCE. It is invariant for a
// fixed closure (see closure/dispatch/direct.rs), and this loop calls
// exactly one.
let cb_site = crate::closure::DirectCall3::resolve(callback);
let _tg = DenseThisGuard::bind_undefined();
let exotic = crate::array::array_iteration_is_exotic(arr);
for i in (0..length).rev() {
Expand All @@ -632,7 +660,7 @@ pub extern "C" fn js_array_find_last(
} else {
rooted.get_or_undefined(i)
};
let result = js_closure_call3(callback, element, i as f64, rooted.receiver());
let result = cb_site.call(callback, element, i as f64, rooted.receiver());
if crate::value::js_is_truthy(result) != 0 {
return element;
}
Expand Down Expand Up @@ -662,6 +690,10 @@ pub extern "C" fn js_array_find_last_index(
let length = (*arr).length as usize;
let scope = crate::gc::RuntimeHandleScope::new();
let rooted = RootedIterArray::new(&scope, arr);
// #8180: resolve the callback's dispatch ONCE. It is invariant for a
// fixed closure (see closure/dispatch/direct.rs), and this loop calls
// exactly one.
let cb_site = crate::closure::DirectCall3::resolve(callback);
let _tg = DenseThisGuard::bind_undefined();
let exotic = crate::array::array_iteration_is_exotic(arr);
for i in (0..length).rev() {
Expand All @@ -670,7 +702,7 @@ pub extern "C" fn js_array_find_last_index(
} else {
rooted.get_or_undefined(i)
};
let result = js_closure_call3(callback, element, i as f64, rooted.receiver());
let result = cb_site.call(callback, element, i as f64, rooted.receiver());
if crate::value::js_is_truthy(result) != 0 {
return i as i32;
}
Expand Down Expand Up @@ -757,6 +789,10 @@ pub extern "C" fn js_array_some(arr: *const ArrayHeader, callback: *const Closur
let length = (*arr).length;
let scope = crate::gc::RuntimeHandleScope::new();
let rooted = RootedIterArray::new(&scope, arr);
// #8180: resolve the callback's dispatch ONCE. It is invariant for a
// fixed closure (see closure/dispatch/direct.rs), and this loop calls
// exactly one.
let cb_site = crate::closure::DirectCall3::resolve(callback);
let _tg = DenseThisGuard::bind_undefined();
let exotic = crate::array::array_iteration_is_exotic(arr);

Expand All @@ -773,7 +809,7 @@ pub extern "C" fn js_array_some(arr: *const ArrayHeader, callback: *const Closur
None => continue,
}
};
let result = js_closure_call3(callback, element, i as f64, rooted.receiver());
let result = cb_site.call(callback, element, i as f64, rooted.receiver());
if crate::value::js_is_truthy(result) != 0 {
return f64::from_bits(TAG_TRUE);
}
Expand Down Expand Up @@ -816,6 +852,10 @@ pub extern "C" fn js_array_every(arr: *const ArrayHeader, callback: *const Closu
let length = (*arr).length;
let scope = crate::gc::RuntimeHandleScope::new();
let rooted = RootedIterArray::new(&scope, arr);
// #8180: resolve the callback's dispatch ONCE. It is invariant for a
// fixed closure (see closure/dispatch/direct.rs), and this loop calls
// exactly one.
let cb_site = crate::closure::DirectCall3::resolve(callback);
let _tg = DenseThisGuard::bind_undefined();
let exotic = crate::array::array_iteration_is_exotic(arr);

Expand All @@ -832,7 +872,7 @@ pub extern "C" fn js_array_every(arr: *const ArrayHeader, callback: *const Closu
None => continue,
}
};
let result = js_closure_call3(callback, element, i as f64, rooted.receiver());
let result = cb_site.call(callback, element, i as f64, rooted.receiver());
if crate::value::js_is_truthy(result) == 0 {
return f64::from_bits(TAG_FALSE);
}
Expand All @@ -857,6 +897,10 @@ pub extern "C" fn js_array_flatMap(
let length = (*arr).length;
let scope = crate::gc::RuntimeHandleScope::new();
let rooted = RootedIterArray::new(&scope, arr);
// #8180: resolve the callback's dispatch ONCE. It is invariant for a
// fixed closure (see closure/dispatch/direct.rs), and this loop calls
// exactly one.
let cb_site = crate::closure::DirectCall3::resolve(callback);
// Root the result across callbacks and pushes (a push both allocates
// — possibly triggering a moving GC — and may reallocate the array).
let result_rooted = scope.root_nanbox_f64(f64::from_bits(
Expand All @@ -879,7 +923,7 @@ pub extern "C" fn js_array_flatMap(
let Some(element) = rooted.present(i) else {
continue;
};
let mapped = js_closure_call3(callback, element, i as f64, rooted.receiver());
let mapped = cb_site.call(callback, element, i as f64, rooted.receiver());
// Root first: detecting a lazy array may materialize it, and a
// push in the inner loop can move the callback result's target.
sub_rooted.set_nanbox_f64(mapped);
Expand Down Expand Up @@ -958,6 +1002,10 @@ pub extern "C" fn js_array_reduce(
let length = (*arr).length as usize;
let scope = crate::gc::RuntimeHandleScope::new();
let rooted = RootedIterArray::new(&scope, arr);
// #8180: resolve the callback's dispatch ONCE. It is invariant for a
// fixed closure (see closure/dispatch/direct.rs), and this loop calls
// exactly one.
let cb_site = crate::closure::DirectCall4::resolve(callback);

if length == 0 {
if has_initial != 0 {
Expand Down Expand Up @@ -1005,7 +1053,7 @@ pub extern "C" fn js_array_reduce(
continue;
};
// Spec callback is `(accumulator, currentValue, currentIndex, array)`.
let next = js_closure_call4(
let next = cb_site.call(
callback,
acc_rooted.get_nanbox_f64(),
element,
Expand Down
Loading
Loading