From 7368abfd5dea3d8219e5f6a2ebecf35e42f8867f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 16 Aug 2026 10:12:38 +0200 Subject: [PATCH 1/2] fix(gc): root the values the uint8 Buffer callback dispatcher held across user callbacks (#8179) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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= 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)". --- .../src/object/typed_array_proto_thunks.rs | 281 ++++++++++++++---- .../perry-runtime/src/typedarray/iterate.rs | 49 ++- .../perry-runtime/src/typedarray/transform.rs | 32 +- ...st_gap_gc_uint8_buffer_callback_rooting.ts | 170 +++++++++++ test-parity/gc_repsel_corpus.txt | 16 + 5 files changed, 486 insertions(+), 62 deletions(-) create mode 100644 test-files/test_gap_gc_uint8_buffer_callback_rooting.ts diff --git a/crates/perry-runtime/src/object/typed_array_proto_thunks.rs b/crates/perry-runtime/src/object/typed_array_proto_thunks.rs index a5e5144b0f..2731192459 100644 --- a/crates/perry-runtime/src/object/typed_array_proto_thunks.rs +++ b/crates/perry-runtime/src/object/typed_array_proto_thunks.rs @@ -539,28 +539,178 @@ fn validate_comparator(args: &[f64]) -> *const crate::closure::ClosureHeader { } } +/// A Buffer-backed `Uint8Array` receiver (or `map`/`toSorted` result) kept +/// LIVE for the duration of a user-callback loop. +/// +/// `dispatch_uint8_buffer_method` runs user code with the receiver reachable +/// only through a raw `usize` parameter and the native stack, which an +/// evacuating minor does not scan. `array::buffer_receiver_dispatch` roots it +/// at the boundary; the `%TypedArray%.prototype` thunk and +/// `dispatch_buffer_method`'s catch-all — the other two entries — do not, so +/// the root belongs here, where all three get it. +/// +/// # Why the ADDRESS is read once per dispatch, not once per element +/// +/// A raw address held across a call is normally a stale copy +/// (`docs/src/internals/gc-rooting-invariant.md`). A Buffer is the documented +/// exception: `buffer_alloc` allocates it with `arena_alloc_gc_old` and sets +/// `GC_FLAG_TENURED` (`buffer/header.rs`) — the same old-arena space +/// `typed_array_alloc` describes as "non-movable space: raw data pointers are +/// handed out". Every `%TypedArray%` sibling in `typedarray/iterate.rs` and +/// `typedarray/transform.rs` relies on exactly that invariant and holds its +/// `ta` / `recv` in a plain local across callbacks; doing something different +/// here would be inconsistent without being safer. +/// +/// The one collector arm that relocates an old-arena page is old-page defrag, +/// and it is opt-in (`PERRY_GC_OLD_DEFRAG=1`, `gc/oldgen_defrag.rs`, +/// deliberately default-off until a fragmentation workload exists that can +/// exercise it). Making that arm safe is a TREE-WIDE property of every holder +/// of an old-arena raw address, not something one dispatcher can establish — +/// so this type buys liveness plus a single post-validation read, and the +/// invariant is named rather than re-checked 2.6M times per `forEach`. +/// Re-reading it per element measured **+19 %** on a `Uint8Array` +/// forEach/map/reduce benchmark, for a knob that is off. +/// +/// The callback closure below is the opposite case and IS re-read per element: +/// it genuinely moves. +struct RootedUint8Buffer<'s> { + handle: crate::gc::RuntimeHandle<'s>, +} + +impl<'s> RootedUint8Buffer<'s> { + #[inline] + fn new(scope: &'s crate::gc::RuntimeHandleScope, addr: usize) -> Self { + Self { + handle: scope.root_nanbox_f64(pointer_value(addr)), + } + } + + /// The address and the receiver value from ONE handle read. + /// + /// Taken after `validate_callback` (which allocates on its throw path) and + /// before the loop, so it is a post-allocation read. On Darwin a + /// `perry_thread_local!` access is a `tlv_get_addr` CALL, which is why + /// this is one read and not two. + #[inline] + fn live(&self) -> (usize, f64) { + let value = self.handle.get_nanbox_f64(); + ( + (value.to_bits() & crate::value::POINTER_MASK) as usize, + value, + ) + } +} + +/// A callback closure rooted for the duration of a loop that invokes it once +/// per element, with its address RE-READ from the root before every call. +/// +/// **This is the live half of #8179.** A closure is an ordinary nursery +/// allocation (`GC_TYPE_CLOSURE`, `GcAllocationPolicy::ArenaOrMalloc`, with a +/// `GcMoveHookKind::ClosureDynamicProps` move hook — it both moves and dies), +/// and a callback handed in by a frameless caller — the arrow in +/// `Uint8Array.prototype.map.call(u, v => v + 1)` — is reachable ONLY through +/// a raw parameter plus the native stack, which an evacuating minor does not +/// scan. `js_array_map` has carried the same root since gh #6206 / #6081; this +/// dispatcher had it on one of its three entries. +/// +/// `test-files/test_gap_gc_uint8_buffer_callback_rooting.ts` reproduces it on +/// the SHIPPED DEFAULT (`TypeError: value is not a function`, exit 1) and, +/// under `PERRY_GC_SCHEDULE_RATE=1 PERRY_GC_SCHEDULE_SEED=7 +/// PERRY_GC_PROTECT_FROMSPACE=1`, faults precisely — on the FIRST scheduled +/// collection: +/// +/// ```text +/// [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 seed runs 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`. + +macro_rules! rooted_callback { + ($name:ident, $slow:ident, $($arg:ident),+) => { + struct $name<'s> { + handle: crate::gc::RuntimeHandle<'s>, + } + + impl<'s> $name<'s> { + #[inline] + fn new( + scope: &'s crate::gc::RuntimeHandleScope, + callback: *const crate::closure::ClosureHeader, + ) -> Self { + Self { + handle: scope.root_raw_const_ptr(callback), + } + } + + /// `with_const_ptr`, not a bare `get_raw_const_ptr`: the pointer is + /// an ARGUMENT to a C-ABI call, a position `across_*` cannot + /// express (it hands the address back only AFTER the call returns). + /// + /// The pre-call address is never bound across a call either way — + /// the handle is re-read on EVERY iteration, immediately before the + /// pointer is consumed, and the root is a `RawTagged` slot that + /// `scan_runtime_handle_roots_mut` both marks and REWRITES. So a + /// collection during call `i` is reflected in the address call + /// `i + 1` uses, which is precisely the ordering `across_*` exists + /// to enforce. + #[inline] + fn call(&self, $($arg: f64),+) -> f64 { + self.handle + .with_const_ptr::(|cb| { + crate::closure::$slow(cb, $($arg),+) + }) + } + } + }; +} + +rooted_callback!(RootedCallback2, js_closure_call2, arg0, arg1); +rooted_callback!(RootedCallback3, js_closure_call3, arg0, arg1, arg2); +rooted_callback!(RootedCallback4, js_closure_call4, arg0, arg1, arg2, arg3); + pub(crate) unsafe fn dispatch_uint8_buffer_method( addr: usize, method: &str, args: &[f64], ) -> Option { let len = uint8_len(addr); - let receiver = pointer_value(addr); - let mut args_ptr = std::ptr::null(); - if !args.is_empty() { - args_ptr = args.as_ptr(); - } + // Root the receiver for the whole dispatch. Its ADDRESS is then taken from + // the root once per arm — after the callback validation, before the loop — + // rather than being carried in from the parameter; see `RootedUint8Buffer` + // for why once-per-arm and not once-per-element. + let scope = crate::gc::RuntimeHandleScope::new(); + let recv = RootedUint8Buffer::new(&scope, addr); + // Shadow the raw parameter: nothing below may reach the receiver except + // through the rooted handle. + #[allow(unused_variables, clippy::let_unit_value)] + let addr = (); let result = match method { - "set" => super::dispatch_buffer_method(addr, method, args_ptr, args.len()), + "set" => { + let mut args_ptr = std::ptr::null(); + if !args.is_empty() { + args_ptr = args.as_ptr(); + } + let (addr, _) = recv.live(); + super::dispatch_buffer_method(addr, method, args_ptr, args.len()) + } "at" => match uint8_at_index(args, len) { - Some(index) => uint8_get(addr, index) as f64, + Some(index) => uint8_get(recv.live().0, index) as f64, None => undefined(), }, - "entries" | "keys" | "values" => uint8_iterator(addr, method), + "entries" | "keys" | "values" => uint8_iterator(recv.live().0, method), "slice" | "subarray" => { let start = uint8_relative_index_arg(args, 0, len, 0); let end = uint8_relative_index_arg(args, 1, len, len); + let (addr, _) = recv.live(); let result = crate::buffer::js_buffer_slice( addr as *mut crate::buffer::BufferHeader, start as i32, @@ -576,6 +726,7 @@ pub(crate) unsafe fn dispatch_uint8_buffer_method( let from = uint8_relative_index_arg(args, 1, len, 0); let final_ = uint8_relative_index_arg(args, 2, len, len); let count = final_.saturating_sub(from).min(len.saturating_sub(to)); + let (addr, receiver) = recv.live(); if count > 0 { let block: Vec = (0..count).map(|i| uint8_get(addr, from + i)).collect(); for (i, value) in block.into_iter().enumerate() { @@ -585,45 +736,54 @@ pub(crate) unsafe fn dispatch_uint8_buffer_method( receiver } "fill" => { + // `to_uint8` runs ToNumber, which fires a user `valueOf` on an + // object argument — so it is a collection point, and it is + // sequenced BEFORE the `recv.live()` read on purpose. let value = to_uint8(arg_or_undefined(args, 0)); let start = uint8_relative_index_arg(args, 1, len, 0); let end = uint8_relative_index_arg(args, 2, len, len); + let (addr, receiver) = recv.live(); for i in start..end { uint8_set(addr, i, value); } receiver } "map" => { - let cb = validate_callback(args); - let out = uint8_alloc_like(addr, len); + let cb = RootedCallback3::new(&scope, validate_callback(args)); + // `uint8_alloc_like` allocates, so the receiver read is + // sequenced AFTER it. + let out = uint8_alloc_like(recv.live().0, len) as usize; + let (addr, receiver) = recv.live(); for i in 0..len { let value = uint8_get(addr, i) as f64; - let mapped = crate::closure::js_closure_call3(cb, value, i as f64, receiver); - uint8_set(out as usize, i, to_uint8(mapped)); + let mapped = cb.call(value, i as f64, receiver); + uint8_set(out, i, to_uint8(mapped)); } - pointer_value(out as usize) + pointer_value(out) } "filter" => { - let cb = validate_callback(args); + let cb = RootedCallback3::new(&scope, validate_callback(args)); + let (addr, receiver) = recv.live(); let mut kept = Vec::new(); for i in 0..len { let value = uint8_get(addr, i) as f64; - let keep = crate::closure::js_closure_call3(cb, value, i as f64, receiver); + let keep = cb.call(value, i as f64, receiver); if crate::value::js_is_truthy(keep) != 0 { kept.push(value as u8); } } - let out = uint8_alloc_like(addr, kept.len()); + let out = uint8_alloc_like(recv.live().0, kept.len()); for (i, value) in kept.into_iter().enumerate() { uint8_set(out as usize, i, value); } pointer_value(out as usize) } "every" => { - let cb = validate_callback(args); + let cb = RootedCallback3::new(&scope, validate_callback(args)); + let (addr, receiver) = recv.live(); for i in 0..len { let value = uint8_get(addr, i) as f64; - let keep = crate::closure::js_closure_call3(cb, value, i as f64, receiver); + let keep = cb.call(value, i as f64, receiver); if crate::value::js_is_truthy(keep) == 0 { return Some(bool_value(false)); } @@ -631,10 +791,11 @@ pub(crate) unsafe fn dispatch_uint8_buffer_method( bool_value(true) } "some" => { - let cb = validate_callback(args); + let cb = RootedCallback3::new(&scope, validate_callback(args)); + let (addr, receiver) = recv.live(); for i in 0..len { let value = uint8_get(addr, i) as f64; - let keep = crate::closure::js_closure_call3(cb, value, i as f64, receiver); + let keep = cb.call(value, i as f64, receiver); if crate::value::js_is_truthy(keep) != 0 { return Some(bool_value(true)); } @@ -647,7 +808,8 @@ pub(crate) unsafe fn dispatch_uint8_buffer_method( // node: `2`). Its sibling `findLastIndex` was already served below, // which is why the hole survived: the two are always cited together. "find" | "findLast" => { - let cb = validate_callback(args); + let cb = RootedCallback3::new(&scope, validate_callback(args)); + let (addr, receiver) = recv.live(); let indexes: Box> = if method == "find" { Box::new(0..len) } else { @@ -655,7 +817,7 @@ pub(crate) unsafe fn dispatch_uint8_buffer_method( }; for i in indexes { let value = uint8_get(addr, i) as f64; - let keep = crate::closure::js_closure_call3(cb, value, i as f64, receiver); + let keep = cb.call(value, i as f64, receiver); if crate::value::js_is_truthy(keep) != 0 { return Some(value); } @@ -663,7 +825,8 @@ pub(crate) unsafe fn dispatch_uint8_buffer_method( undefined() } "findIndex" | "findLastIndex" => { - let cb = validate_callback(args); + let cb = RootedCallback3::new(&scope, validate_callback(args)); + let (addr, receiver) = recv.live(); let indexes: Box> = if method == "findIndex" { Box::new(0..len) } else { @@ -671,7 +834,7 @@ pub(crate) unsafe fn dispatch_uint8_buffer_method( }; for i in indexes { let value = uint8_get(addr, i) as f64; - let keep = crate::closure::js_closure_call3(cb, value, i as f64, receiver); + let keep = cb.call(value, i as f64, receiver); if crate::value::js_is_truthy(keep) != 0 { return Some(i as f64); } @@ -679,48 +842,59 @@ pub(crate) unsafe fn dispatch_uint8_buffer_method( -1.0 } "forEach" => { - let cb = validate_callback(args); + let cb = RootedCallback3::new(&scope, validate_callback(args)); + let (addr, receiver) = recv.live(); for i in 0..len { let value = uint8_get(addr, i) as f64; - let _ = crate::closure::js_closure_call3(cb, value, i as f64, receiver); + let _ = cb.call(value, i as f64, receiver); } undefined() } - "includes" => uint8_includes(addr, args), - "indexOf" => uint8_index_of(addr, args), - "lastIndexOf" => uint8_last_index_of(addr, args), - "join" => uint8_join(addr, arg_or_undefined(args, 0)), - "toLocaleString" => uint8_join(addr, undefined()), + "includes" => uint8_includes(recv.live().0, args), + "indexOf" => uint8_index_of(recv.live().0, args), + "lastIndexOf" => uint8_last_index_of(recv.live().0, args), + "join" => uint8_join(recv.live().0, arg_or_undefined(args, 0)), + "toLocaleString" => uint8_join(recv.live().0, undefined()), "reduce" | "reduceRight" => { - let cb = validate_callback(args); + let cb = RootedCallback4::new(&scope, validate_callback(args)); if len == 0 && args.len() < 2 { crate::array::throw_reduce_of_empty(); } let reverse = method == "reduceRight"; - let (mut accumulator, indexes): (f64, Box>) = - if args.len() >= 2 { - let iter: Box> = if reverse { - Box::new((0..len).rev()) - } else { - Box::new(0..len) - }; - (args[1], iter) - } else if reverse { - ( - uint8_get(addr, len - 1) as f64, - Box::new((0..len - 1).rev()), - ) + let (addr, receiver) = recv.live(); + let (accumulator, indexes): (f64, Box>) = if args.len() >= 2 + { + let iter: Box> = if reverse { + Box::new((0..len).rev()) } else { - (uint8_get(addr, 0) as f64, Box::new(1..len)) + Box::new(0..len) }; + (args[1], iter) + } else if reverse { + ( + uint8_get(addr, len - 1) as f64, + Box::new((0..len - 1).rev()), + ) + } else { + (uint8_get(addr, 0) as f64, Box::new(1..len)) + }; + // The accumulator is the one value in this dispatcher that is + // routinely a NURSERY object: a string/object/array seed, or a + // fresh one returned by every callback. Held in a Rust local it is + // a pre-move NaN-boxed address that an evacuating minor rewrites + // nowhere. `js_array_reduce` (array/iter_methods.rs) has rooted its + // accumulator since the 2026-07-02 audit; the Buffer-receiver + // reduce and both TypedArray reduces never did. + let acc = scope.root_nanbox_f64(accumulator); for i in indexes { let value = uint8_get(addr, i) as f64; - accumulator = - crate::closure::js_closure_call4(cb, accumulator, value, i as f64, receiver); + let next = cb.call(acc.get_nanbox_f64(), value, i as f64, receiver); + acc.set_nanbox_f64(next); } - accumulator + acc.get_nanbox_f64() } "reverse" => { + let (addr, receiver) = recv.live(); if len > 1 { let mut i = 0usize; let mut j = len - 1; @@ -737,6 +911,7 @@ pub(crate) unsafe fn dispatch_uint8_buffer_method( } "sort" | "toSorted" => { let cmp = validate_comparator(args); + let (addr, receiver) = recv.live(); let out_addr = if method == "sort" { addr } else { @@ -746,8 +921,9 @@ pub(crate) unsafe fn dispatch_uint8_buffer_method( if cmp.is_null() { values.sort_unstable(); } else { + let cmp = RootedCallback2::new(&scope, cmp); values.sort_by(|a, b| { - let r = crate::closure::js_closure_call2(cmp, *a as f64, *b as f64); + let r = cmp.call(*a as f64, *b as f64); if r < 0.0 { std::cmp::Ordering::Less } else if r > 0.0 { @@ -767,13 +943,14 @@ pub(crate) unsafe fn dispatch_uint8_buffer_method( } } "toReversed" => { - let out = uint8_alloc_like(addr, len); + let out = uint8_alloc_like(recv.live().0, len); + let (addr, _) = recv.live(); for i in 0..len { uint8_set(out as usize, i, uint8_get(addr, len - 1 - i)); } pointer_value(out as usize) } - "with" => uint8_with(addr, args), + "with" => uint8_with(recv.live().0, args), _ => return None, }; Some(result) diff --git a/crates/perry-runtime/src/typedarray/iterate.rs b/crates/perry-runtime/src/typedarray/iterate.rs index 0e66da4bf1..7db4a3c9fb 100644 --- a/crates/perry-runtime/src/typedarray/iterate.rs +++ b/crates/perry-runtime/src/typedarray/iterate.rs @@ -228,17 +228,31 @@ pub extern "C" fn js_typed_array_reduce( crate::array::throw_reduce_of_empty(); } let recv = ta_receiver_value(ta); - let (mut accumulator, start_idx) = if has_initial != 0 { + let (accumulator, start_idx) = if has_initial != 0 { (initial, 0) } else { (load_at(ta, 0), 1) }; + // #8179: root the accumulator. It is the one value in these loops + // that is routinely a NURSERY object — a string/object/array seed, or + // a fresh one returned by every callback — and a bare Rust local + // holding a NaN-boxed pre-move address is a copy the collector cannot + // see. `js_array_reduce` (array/iter_methods.rs) has rooted its + // accumulator since the 2026-07-02 audit; the TypedArray siblings and + // the Buffer-receiver dispatcher never did. + // + // The receiver (`ta` / `recv`) needs no equivalent: `typed_array_alloc` + // places every TypedArray in the old arena as TENURED, documented + // there as non-movable because raw data pointers are handed out. + let scope = crate::gc::RuntimeHandleScope::new(); + let acc = scope.root_nanbox_f64(accumulator); for i in start_idx..len { let v = load_at(ta, i); - accumulator = - crate::closure::js_closure_call4(callback, accumulator, v, i as f64, recv); + let next = + crate::closure::js_closure_call4(callback, acc.get_nanbox_f64(), v, i as f64, recv); + acc.set_nanbox_f64(next); } - accumulator + acc.get_nanbox_f64() } } @@ -267,18 +281,37 @@ pub extern "C" fn js_typed_array_reduce_right( crate::array::throw_reduce_of_empty(); } let recv = ta_receiver_value(ta); - let (mut accumulator, start_idx) = if has_initial != 0 { + let (accumulator, start_idx) = if has_initial != 0 { (initial, len) } else { (load_at(ta, len - 1), len - 1) }; + // #8179: root the accumulator. It is the one value in these loops + // that is routinely a NURSERY object — a string/object/array seed, or + // a fresh one returned by every callback — and a bare Rust local + // holding a NaN-boxed pre-move address is a copy the collector cannot + // see. `js_array_reduce` (array/iter_methods.rs) has rooted its + // accumulator since the 2026-07-02 audit; the TypedArray siblings and + // the Buffer-receiver dispatcher never did. + // + // The receiver (`ta` / `recv`) needs no equivalent: `typed_array_alloc` + // places every TypedArray in the old arena as TENURED, documented + // there as non-movable because raw data pointers are handed out. + let scope = crate::gc::RuntimeHandleScope::new(); + let acc = scope.root_nanbox_f64(accumulator); if start_idx > 0 { for i in (0..start_idx).rev() { let v = load_at(ta, i); - accumulator = - crate::closure::js_closure_call4(callback, accumulator, v, i as f64, recv); + let next = crate::closure::js_closure_call4( + callback, + acc.get_nanbox_f64(), + v, + i as f64, + recv, + ); + acc.set_nanbox_f64(next); } } - accumulator + acc.get_nanbox_f64() } } diff --git a/crates/perry-runtime/src/typedarray/transform.rs b/crates/perry-runtime/src/typedarray/transform.rs index a3751ca289..e162da1d9a 100644 --- a/crates/perry-runtime/src/typedarray/transform.rs +++ b/crates/perry-runtime/src/typedarray/transform.rs @@ -209,9 +209,23 @@ pub extern "C" fn js_typed_array_sort_with_comparator( } // Non-BigInt kinds: `load_at` yields plain numeric f64s (no heap // pointers), so the owned buffer is GC-inert by construction. + // #8179: root the comparator. A comparator that allocates can trigger a + // moving minor that relocates its own closure header, and this raw + // parameter plus the native stack is its only reference — an + // evacuating minor does not scan the native stack. `sorted_bigint_lanes` + // above has rooted it since it was written ("the comparator closure + // itself is re-derived from a rooted handle per call"); the two + // non-BigInt arms were missed. + let scope = crate::gc::RuntimeHandleScope::new(); + let cmp_handle = scope.root_raw_const_ptr(comparator); let mut buf: Vec = (0..len).map(|i| load_at(ta_clean, i)).collect(); buf.sort_by(|a, b| { - let r = crate::closure::js_closure_call2(comparator, *a, *b); + // `with_const_ptr`: an argument-position read, which `across_*` + // cannot express. The handle is re-read per comparison, so a + // relocation during comparison `i` is reflected in `i + 1`. + let r = cmp_handle.with_const_ptr::(|cmp| { + crate::closure::js_closure_call2(cmp, *a, *b) + }); if r < 0.0 { std::cmp::Ordering::Less } else if r > 0.0 { @@ -280,9 +294,23 @@ pub extern "C" fn js_typed_array_to_sorted_with_comparator( return out; } // Non-BigInt kinds: plain numeric f64s — the owned buffer is GC-inert. + // #8179: root the comparator. A comparator that allocates can trigger a + // moving minor that relocates its own closure header, and this raw + // parameter plus the native stack is its only reference — an + // evacuating minor does not scan the native stack. `sorted_bigint_lanes` + // above has rooted it since it was written ("the comparator closure + // itself is re-derived from a rooted handle per call"); the two + // non-BigInt arms were missed. + let scope = crate::gc::RuntimeHandleScope::new(); + let cmp_handle = scope.root_raw_const_ptr(comparator); let mut buf: Vec = (0..len).map(|i| load_at(ta, i)).collect(); buf.sort_by(|a, b| { - let r = crate::closure::js_closure_call2(comparator, *a, *b); + // `with_const_ptr`: an argument-position read, which `across_*` + // cannot express. The handle is re-read per comparison, so a + // relocation during comparison `i` is reflected in `i + 1`. + let r = cmp_handle.with_const_ptr::(|cmp| { + crate::closure::js_closure_call2(cmp, *a, *b) + }); if r < 0.0 { std::cmp::Ordering::Less } else if r > 0.0 { diff --git a/test-files/test_gap_gc_uint8_buffer_callback_rooting.ts b/test-files/test_gap_gc_uint8_buffer_callback_rooting.ts new file mode 100644 index 0000000000..fb242563da --- /dev/null +++ b/test-files/test_gap_gc_uint8_buffer_callback_rooting.ts @@ -0,0 +1,170 @@ +// #8179: every value `dispatch_uint8_buffer_method` carried across a user +// callback lived in a bare Rust local — +// +// * the receiver `addr` (re-read as `uint8_get(addr, i)` on the NEXT +// iteration) and the NaN-boxed `receiver` handed to the callback as its +// 3rd/4th argument, +// * `map`'s freshly allocated `out` buffer, written after every call, +// * `sort`/`toSorted`'s `out_addr`, permuted under a user comparator, +// * `reduce`/`reduceRight`'s `accumulator`, which is a NURSERY object +// whenever the seed or a callback result is a string/object/array, +// * and the callback closure itself, which on two of the three entries into +// this dispatcher had no root at all (gh #6206 / #6081: a closure is +// non-movable but IS swept, and an arrow at a frameless call site is +// reachable only through that raw parameter). +// +// The callbacks below allocate inside a LOOP on purpose. A back-edge poll is +// the only safepoint reachable from inside user JS (`js_gc_loop_safepoint`), +// so an allocation-free callback gives the seeded GC schedule nothing to fire +// on and the whole file passes vacuously. Run it as +// +// PERRY_GC_SCHEDULE_RATE=1 PERRY_GC_SCHEDULE_SEED=7 \ +// PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_VERIFY_EVACUATION=1 \ +// PERRY_GC_DIAG=1 ./out +// +// (`PERRY_GC_SCHEDULE_SEED` implies forced evacuation, so survivors actually +// MOVE.) Two things must be true before a green run means anything: a +// `[gc-fromspace-protect] … retired_set=#N` line must appear, and the +// instrument's own exit verdict must report `copying_minors` and +// `moved_objects` above zero — a run with zero copying minors protects +// nothing. Pre-fix this file dies on the FIRST scheduled collection +// (`[gc-schedule] FAILURE (signal 10) … safepoints=1`); post-fix the same seed +// reports `safepoints=5306 copying_minors=5306 moved_objects=25760` and +// exits 0. +// +// BOTH dispatch entries are exercised. A statically typed receiver reaches +// `dispatch_uint8_buffer_method` through `dispatch_buffer_method`'s catch-all, +// while a fused `js_array_*` helper reaches it through +// `array::buffer_receiver_dispatch` — and only the latter had rooted the +// receiver and the callback at the boundary. +// +// Perry's `new Uint8Array([…])` is a Buffer (`BufferHeader`), which is what +// makes this dispatcher run at all; see #8137/#8173. + +function churn(n: number): string { + let s = ""; + for (let k = 0; k < n; k++) { + s = "g" + (k + n) + ":" + s.length; + } + return s; +} + +const SRC = [3, 1, 4, 1, 5, 9, 2, 6, 5, 3, 5, 8, 9, 7, 9, 3]; +const ROUNDS = 300; + +let mapMismatch = 0; +let filterMismatch = 0; +let forEachMismatch = 0; +let reduceMismatch = 0; +let reduceRightMismatch = 0; +let sortMismatch = 0; +let findMismatch = 0; +let protoMapMismatch = 0; +let protoReduceMismatch = 0; + +const EXPECT_MAP = "6,2,8,2,10,18,4,12,10,6,10,16,18,14,18,6"; +const EXPECT_FILTER = "4,2,6,8"; +const EXPECT_REDUCE = "s-3-1-4-1-5-9-2-6-5-3-5-8-9-7-9-3"; +const EXPECT_REDUCE_RIGHT = "s-3-9-7-9-8-5-3-5-6-2-9-5-1-4-1-3"; +const EXPECT_SORT = "1,1,2,3,3,3,4,5,5,5,6,7,8,9,9,9"; + +for (let r = 0; r < ROUNDS; r++) { + const u = new Uint8Array(SRC); + + const doubled = u.map((v: number) => { + churn(12); + return (v * 2) % 256; + }); + if (doubled.join(",") !== EXPECT_MAP) { + mapMismatch++; + } + + const evens = u.filter((v: number) => { + churn(6); + return v % 2 === 0; + }); + if (evens.join(",") !== EXPECT_FILTER) { + filterMismatch++; + } + + let seen = ""; + u.forEach((v: number, i: number) => { + churn(4); + seen = seen + v; + if (i === 0 && v !== 3) { + forEachMismatch++; + } + }); + if (seen !== "3141592653589793") { + forEachMismatch++; + } + + // A STRING accumulator: the one value in this loop that is a nursery + // object on every single iteration. + const joined = u.reduce((acc: string, v: number) => { + churn(8); + return acc + "-" + v; + }, "s"); + if (joined !== EXPECT_REDUCE) { + reduceMismatch++; + } + + const joinedRight = u.reduceRight((acc: string, v: number) => { + churn(8); + return acc + "-" + v; + }, "s"); + if (joinedRight !== EXPECT_REDUCE_RIGHT) { + reduceRightMismatch++; + } + + // The comparator runs over a `Vec` while `out_addr` sits in a Rust + // local; `toSorted` makes that local a FRESH buffer rather than the + // receiver. + const sorted = u.toSorted((a: number, b: number) => { + churn(3); + return a - b; + }); + if (sorted.join(",") !== EXPECT_SORT) { + sortMismatch++; + } + + const found = u.find((v: number) => { + churn(5); + return v > 8; + }); + if (found !== 9) { + findMismatch++; + } + + // The reflective entry: `%TypedArray%.prototype` thunk → brand check → + // the same dispatcher, with NO boundary root on the receiver or callback. + const protoMapped = Uint8Array.prototype.map.call(u, (v: number) => { + churn(7); + return (v + 1) % 256; + }); + if (Array.prototype.join.call(protoMapped, ",") !== "4,2,5,2,6,10,3,7,6,4,6,9,10,8,10,4") { + protoMapMismatch++; + } + + const protoReduced = Uint8Array.prototype.reduce.call( + u, + (acc: string, v: number) => { + churn(6); + return acc + "." + v; + }, + "p", + ); + if (protoReduced !== "p.3.1.4.1.5.9.2.6.5.3.5.8.9.7.9.3") { + protoReduceMismatch++; + } +} + +console.log("map", mapMismatch); +console.log("filter", filterMismatch); +console.log("forEach", forEachMismatch); +console.log("reduce", reduceMismatch); +console.log("reduceRight", reduceRightMismatch); +console.log("sort", sortMismatch); +console.log("find", findMismatch); +console.log("protoMap", protoMapMismatch); +console.log("protoReduce", protoReduceMismatch); diff --git a/test-parity/gc_repsel_corpus.txt b/test-parity/gc_repsel_corpus.txt index 042a40c954..400ac0054a 100644 --- a/test-parity/gc_repsel_corpus.txt +++ b/test-parity/gc_repsel_corpus.txt @@ -779,3 +779,19 @@ test_gap_repsel_element_group_numeric test_gap_gc_container_value_rooting test_gap_gc_define_properties_key_rooting test_gap_gc_define_property_descriptor_rooting + +# #8179 (this PR): `dispatch_uint8_buffer_method` — the shared uint8 +# `%TypedArray%.prototype` dispatcher every Buffer-backed `Uint8Array` callback +# method funnels through — held the receiver address, the NaN-boxed receiver +# handed to callbacks, `map`'s freshly allocated result buffer, +# `sort`/`toSorted`'s permuted output and `reduce`'s accumulator in bare Rust +# locals across `js_closure_call{2,3,4}`, and rooted the callback closure on +# only ONE of its three entries. The CLOSURE is the live half: it is an +# ordinary nursery allocation that MOVES, and on two of those +# three entries the raw parameter was its only reference. Pre-fix this file +# throws `TypeError: value is not a function` on the shipped default and dies on +# the FIRST collection under `PERRY_GC_SCHEDULE_RATE=1 PERRY_GC_SCHEDULE_SEED=7 +# PERRY_GC_PROTECT_FROMSPACE=1`. Callbacks here allocate inside a LOOP on +# purpose — a back-edge poll is the only safepoint reachable from user JS, so an +# allocation-free callback makes the whole file pass vacuously. +test_gap_gc_uint8_buffer_callback_rooting From 7c6212c8afb6b32e2da170c9d2dbcf6faa4aa494 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 16 Aug 2026 10:13:02 +0200 Subject: [PATCH 2/2] perf(closure): hoist per-element closure dispatch out of the array-callback loops (#8180) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. --- ...8-callback-rooting-and-hoisted-dispatch.md | 96 +++++++ .../perry-runtime/src/array/iter_methods.rs | 78 ++++-- .../perry-runtime/src/array/reduce_right.rs | 8 +- crates/perry-runtime/src/array/sort.rs | 11 +- .../src/array/typed_array_receiver_tests.rs | 2 +- crates/perry-runtime/src/closure/dispatch.rs | 7 +- .../src/closure/dispatch/calln.rs | 41 +-- .../src/closure/dispatch/direct.rs | 247 ++++++++++++++++++ crates/perry-runtime/src/closure/mod.rs | 5 +- .../src/object/typed_array_proto_thunks.rs | 18 +- .../perry-runtime/src/typedarray/iterate.rs | 61 +++-- .../perry-runtime/src/typedarray/transform.rs | 30 ++- 12 files changed, 510 insertions(+), 94 deletions(-) create mode 100644 changelog.d/8188-uint8-callback-rooting-and-hoisted-dispatch.md create mode 100644 crates/perry-runtime/src/closure/dispatch/direct.rs diff --git a/changelog.d/8188-uint8-callback-rooting-and-hoisted-dispatch.md b/changelog.d/8188-uint8-callback-rooting-and-hoisted-dispatch.md new file mode 100644 index 0000000000..f7c6375e7a --- /dev/null +++ b/changelog.d/8188-uint8-callback-rooting-and-hoisted-dispatch.md @@ -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= +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. diff --git a/crates/perry-runtime/src/array/iter_methods.rs b/crates/perry-runtime/src/array/iter_methods.rs index c4dba84b4c..db3e02f247 100644 --- a/crates/perry-runtime/src/array/iter_methods.rs +++ b/crates/perry-runtime/src/array/iter_methods.rs @@ -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 @@ -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)); @@ -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; } @@ -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)); } } } @@ -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 @@ -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::(); - 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 = @@ -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 @@ -396,7 +408,7 @@ 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; } @@ -404,7 +416,7 @@ pub extern "C" fn js_array_map_discard(arr: *const ArrayHeader, callback: *const 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()); } } } @@ -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(); @@ -473,7 +489,7 @@ pub extern "C" fn js_array_filter( } }; let callback = cb_handle.get_raw_const_ptr::(); - 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 { @@ -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); @@ -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; @@ -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); @@ -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; @@ -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() { @@ -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; } @@ -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() { @@ -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; } @@ -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); @@ -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); } @@ -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); @@ -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); } @@ -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( @@ -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); @@ -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 { @@ -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, diff --git a/crates/perry-runtime/src/array/reduce_right.rs b/crates/perry-runtime/src/array/reduce_right.rs index 2b333860a1..7c9b9cf2ba 100644 --- a/crates/perry-runtime/src/array/reduce_right.rs +++ b/crates/perry-runtime/src/array/reduce_right.rs @@ -1,7 +1,7 @@ //! Array.prototype.reduceRight. use super::*; use crate::array::throw_reduce_of_empty; -use crate::closure::{js_closure_call4, ClosureHeader}; +use crate::closure::ClosureHeader; #[inline(always)] unsafe fn array_elements_ptr(arr: *const ArrayHeader) -> *const f64 { @@ -101,6 +101,10 @@ pub extern "C" fn js_array_reduce_right( // Root the accumulator too: it can hold a heap value while a GC runs // between iterations. + // #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); let acc_rooted = scope.root_nanbox_f64(accumulator); if start_idx > 0 { for i in (0..start_idx).rev() { @@ -108,7 +112,7 @@ pub extern "C" fn js_array_reduce_right( continue; }; // Spec callback `(accumulator, currentValue, currentIndex, array)`. - let next = js_closure_call4( + let next = cb_site.call( callback, acc_rooted.get_nanbox_f64(), element, diff --git a/crates/perry-runtime/src/array/sort.rs b/crates/perry-runtime/src/array/sort.rs index 2c05a65367..5f047036fc 100644 --- a/crates/perry-runtime/src/array/sort.rs +++ b/crates/perry-runtime/src/array/sort.rs @@ -1,7 +1,7 @@ //! Mutating sort — default + comparator, plus the spec-ops path for exotic //! receivers (index accessors, sparse storage, inherited prototype elements). use super::*; -use crate::closure::{js_closure_call2, ClosureHeader}; +use crate::closure::ClosureHeader; // --------------------------------------------------------------------------- // SortCompare helpers shared by the dense fast paths, the exotic spec path, @@ -13,14 +13,14 @@ use crate::closure::{js_closure_call2, ClosureHeader}; #[derive(Clone, Copy)] pub(crate) struct ComparatorCall { comparator: *const ClosureHeader, - direct: Option f64>, + direct: crate::closure::DirectCall2, } impl ComparatorCall { pub(crate) fn new(comparator: *const ClosureHeader) -> Self { ComparatorCall { comparator, - direct: crate::closure::resolve_call2_direct(comparator), + direct: crate::closure::DirectCall2::resolve(comparator), } } @@ -40,10 +40,7 @@ impl ComparatorCall { /// which relocation does not change. #[inline(always)] pub(crate) fn compare_at(&self, comparator: *const ClosureHeader, a: f64, b: f64) -> f64 { - let r = match self.direct { - Some(f) => f(comparator, a, b), - None => js_closure_call2(comparator, a, b), - }; + let r = self.direct.call(comparator, a, b); if !r.is_nan() { return r; } diff --git a/crates/perry-runtime/src/array/typed_array_receiver_tests.rs b/crates/perry-runtime/src/array/typed_array_receiver_tests.rs index 8370e0a516..637639e572 100644 --- a/crates/perry-runtime/src/array/typed_array_receiver_tests.rs +++ b/crates/perry-runtime/src/array/typed_array_receiver_tests.rs @@ -239,7 +239,7 @@ fn typed_read_back(ta: *mut ArrayHeader, len: usize) -> Vec { /// A two-argument comparator closure, built the way `array/tests.rs` builds /// its `map` callback: a bare `extern "C"` function behind a capture-less -/// `ClosureHeader`, which is what `resolve_call2_direct` expects. +/// `ClosureHeader`, which is what `DirectCall2::resolve` expects. extern "C" fn descending_cmp( _closure: *const crate::closure::ClosureHeader, a: f64, diff --git a/crates/perry-runtime/src/closure/dispatch.rs b/crates/perry-runtime/src/closure/dispatch.rs index 503881cce8..0e82eb3d6d 100644 --- a/crates/perry-runtime/src/closure/dispatch.rs +++ b/crates/perry-runtime/src/closure/dispatch.rs @@ -8,12 +8,14 @@ //! - `errors`: the not-callable throw path + #922 circuit breaker //! - `validate`: closure-pointer validation (`get_valid_func_ptr`, GC stubs) //! - `calln`: per-arity `js_closure_callN` FFI entry points +//! - `direct`: hoisted per-arity dispatch for callback loops (#8180) //! - `value_call`: the dynamic value-call / V8-trampoline / spread bridges use super::*; mod bound; mod calln; +mod direct; mod errors; mod validate; mod value_call; @@ -26,15 +28,14 @@ pub use errors::throw_not_callable; pub use validate::{clean_closure_ptr, dispatch_proxy_callee_or_throw, get_valid_func_ptr}; -pub(crate) use calln::{ - dispatch_registered_call, dispatch_rest_or_declared_arity, resolve_call2_direct, -}; +pub(crate) use calln::{dispatch_registered_call, dispatch_rest_or_declared_arity}; pub use calln::{ js_closure_call0, js_closure_call1, js_closure_call10, js_closure_call11, js_closure_call12, js_closure_call13, js_closure_call14, js_closure_call15, js_closure_call16, js_closure_call2, js_closure_call3, js_closure_call4, js_closure_call5, js_closure_call6, js_closure_call7, js_closure_call8, js_closure_call9, }; +pub use direct::{DirectCall1, DirectCall2, DirectCall3, DirectCall4}; pub use value_call::{ js_closure_call_apply_with_spread, js_closure_call_array, js_native_call_value, diff --git a/crates/perry-runtime/src/closure/dispatch/calln.rs b/crates/perry-runtime/src/closure/dispatch/calln.rs index 26904bfb39..e626c9d5de 100644 --- a/crates/perry-runtime/src/closure/dispatch/calln.rs +++ b/crates/perry-runtime/src/closure/dispatch/calln.rs @@ -1,6 +1,11 @@ -//! Per-arity `js_closure_callN` FFI entry points (0..=16), the `resolve_call2_direct` -//! hot-loop helper, and the shared `dispatch_registered_call` / -//! `dispatch_rest_or_declared_arity` routing helpers. +//! Per-arity `js_closure_callN` FFI entry points (0..=16) and the shared +//! `dispatch_registered_call` / `dispatch_rest_or_declared_arity` routing +//! helpers. +//! +//! The hot-loop counterpart -- resolve a closure ONCE and call it directly for +//! the rest of the loop -- lives in the sibling `direct` module (#8180). It +//! subsumes the `resolve_call2_direct` helper that used to sit here and had +//! exactly one consumer. use super::*; @@ -52,36 +57,6 @@ pub extern "C" fn js_closure_call1(closure: *const ClosureHeader, arg0: f64) -> } } -/// Resolve a 2-arg closure call once: returns Some(typed_fn_ptr) when -/// the closure can be invoked via a direct call without per-call -/// dispatch adjustments (no rest-bundling, no arity-padding, no -/// bound-method routing). Returns None when the call must go through -/// the slow `js_closure_call2` path. Hot loops that call the same -/// closure many times (e.g. `array.sort((a,b) => a-b)`) can hoist -/// this resolution out of the loop and skip ~50M HashMap lookups -/// over a 1.25M-element sort. -#[inline] -pub(crate) fn resolve_call2_direct( - closure: *const ClosureHeader, -) -> Option f64> { - let func_ptr = get_valid_func_ptr(closure); - if func_ptr.is_null() - || func_ptr == BOUND_METHOD_FUNC_PTR - || func_ptr == BOUND_FUNCTION_FUNC_PTR - { - return None; - } - if lookup_closure_rest(func_ptr).is_some() { - return None; - } - if let Some(declared) = lookup_closure_arity(func_ptr) { - if declared > 2 { - return None; - } - } - Some(unsafe { std::mem::transmute(func_ptr) }) -} - /// Call a closure with 2 arguments, returning f64 #[no_mangle] pub extern "C" fn js_closure_call2(closure: *const ClosureHeader, arg0: f64, arg1: f64) -> f64 { diff --git a/crates/perry-runtime/src/closure/dispatch/direct.rs b/crates/perry-runtime/src/closure/dispatch/direct.rs new file mode 100644 index 0000000000..f482ed5404 --- /dev/null +++ b/crates/perry-runtime/src/closure/dispatch/direct.rs @@ -0,0 +1,247 @@ +//! Hoisted per-arity closure dispatch for callback loops (#8180). +//! +//! `js_closure_callN` re-derives, on EVERY call, three things that cannot +//! change while one closure is being called in a loop: +//! +//! 1. `get_valid_func_ptr` — two address-band checks plus a volatile +//! `CLOSURE_MAGIC` probe through `*(closure + 12)` and a volatile load of +//! `closure->func_ptr` (`dispatch/validate.rs`); +//! 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 when +//! it hits (`closure/registry.rs`); +//! 3. the `match` over `DispatchStrategy` before the indirect jump. +//! +//! Over `array.forEach(cb)` on a million elements that is a million repeats of +//! one answer. `array/sort.rs`'s `ComparatorCall` already hoists it for the +//! 2-argument comparator — introduced to "skip ~50M HashMap lookups over a +//! 1.25M-element sort" — but that was the only consumer. This module +//! generalises the same shape to the arities the array-callback helpers use +//! and gives it one place to live. +//! +//! # Why hoisting is sound +//! +//! Each input is invariant for a FIXED closure: +//! +//! * `closure->func_ptr` is written once by `js_closure_alloc` and never +//! mutated, and a `ClosureHeader` is non-movable, so `get_valid_func_ptr` +//! answers the same address every time. (A moving collection cannot change +//! it either — `direct` is a static CODE address; that is the same argument +//! `ComparatorCall::compare_at` documents.) +//! * `lookup_closure_rest` / `lookup_closure_arity` are keyed by that +//! func_ptr, and both registries are insert-only per key — the registration +//! happens at closure creation, before the closure can be passed anywhere. +//! * `BOUND_METHOD_FUNC_PTR` / `BOUND_FUNCTION_FUNC_PTR` are 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 +//! exactly one. A site that can retarget its callee (a dynamic property read +//! per element, say) must not use these types. +//! +//! # Interaction with rooting +//! +//! `call` takes the closure pointer as a parameter rather than caching it, so +//! a caller that roots its callback in a `RuntimeHandleScope` (#8179, gh +//! #6206) passes the CURRENT address after every user-code window. The +//! resolved target stays valid regardless: relocation does not move code. +//! +//! # Fallback +//! +//! `resolve` answers `None` for a bound method/function, a rest parameter, a +//! declared arity above the call arity, and an invalid closure pointer. Those +//! calls go through `js_closure_callN` unchanged, which keeps the +//! proxy-callee/throw path, the rest bundling and the undefined-padding in +//! exactly one place. + +use super::*; + +/// Resolve a closure ONCE for a fixed call arity: `Some(func_ptr)` when every +/// call at `arity` can jump straight to the compiled body (no bound-method +/// routing, no rest bundling, no undefined-padding). See the module docs for +/// why the answer is invariant. +#[inline] +pub(crate) fn resolve_direct_func_ptr( + closure: *const ClosureHeader, + arity: u32, +) -> Option<*const u8> { + let func_ptr = get_valid_func_ptr(closure); + if func_ptr.is_null() + || func_ptr == BOUND_METHOD_FUNC_PTR + || func_ptr == BOUND_FUNCTION_FUNC_PTR + { + return None; + } + if lookup_closure_rest(func_ptr).is_some() { + return None; + } + if let Some(declared) = lookup_closure_arity(func_ptr) { + if declared > arity { + return None; + } + } + Some(func_ptr) +} + +macro_rules! define_direct_call_site { + ( + $(#[$meta:meta])* + $site:ident, $arity:literal, $slow:ident, $($arg:ident),+ + ) => { + $(#[$meta])* + #[derive(Clone, Copy)] + pub struct $site(Option f64>); + + impl $site { + /// Resolve `closure` once, before the loop. + #[inline] + pub fn resolve(closure: *const ClosureHeader) -> Self { + $site(resolve_direct_func_ptr(closure, $arity).map(|func_ptr| unsafe { + std::mem::transmute::< + *const u8, + extern "C" fn(*const ClosureHeader, $(define_direct_call_site!(@f64 $arg)),+) -> f64, + >(func_ptr) + })) + } + + /// Invoke with the CURRENT closure address (see the module docs on + /// rooting). Falls back to the full dispatcher when the closure + /// did not resolve. + #[inline] + pub fn call(&self, closure: *const ClosureHeader, $($arg: f64),+) -> f64 { + match self.0 { + Some(func) => func(closure, $($arg),+), + None => $slow(closure, $($arg),+), + } + } + + /// Whether the direct target was resolved. Test-only: a "fast + /// path" nobody can prove ran is not a fast path. + #[cfg(test)] + #[allow(dead_code)] + pub(crate) fn is_direct(&self) -> bool { + self.0.is_some() + } + } + }; + (@f64 $arg:ident) => { f64 }; +} + +define_direct_call_site!( + /// A 1-argument callback resolved once for a whole loop. + DirectCall1, + 1, + js_closure_call1, + arg0 +); + +define_direct_call_site!( + /// A 2-argument callback (comparators, `Map`/`Set` visitors) resolved once + /// for a whole loop. + DirectCall2, + 2, + js_closure_call2, + arg0, + arg1 +); + +define_direct_call_site!( + /// A 3-argument callback — `(element, index, array)`, the shape every + /// `Array.prototype` iteration method uses — resolved once for a whole + /// loop. + DirectCall3, + 3, + js_closure_call3, + arg0, + arg1, + arg2 +); + +define_direct_call_site!( + /// A 4-argument callback — `(accumulator, element, index, array)`, the + /// `reduce`/`reduceRight` shape — resolved once for a whole loop. + DirectCall4, + 4, + js_closure_call4, + arg0, + arg1, + arg2, + arg3 +); + +#[cfg(test)] +mod tests { + use super::*; + + // A capture-less body behind a real `ClosureHeader`, the same way + // `array/tests.rs` and `array/typed_array_receiver_tests.rs` build theirs. + extern "C" fn add3(_c: *const ClosureHeader, a: f64, b: f64, c: f64) -> f64 { + a * 100.0 + b * 10.0 + c + } + + extern "C" fn sum2(_c: *const ClosureHeader, a: f64, b: f64) -> f64 { + a + b + } + + fn closure_for(body: *const u8) -> *const ClosureHeader { + crate::closure::js_closure_alloc(body, 0) + } + + #[test] + fn a_plain_callback_resolves_and_answers_identically_to_the_slow_path() { + let c = closure_for(add3 as *const u8); + let site = DirectCall3::resolve(c); + // ASSERT THE SUBJECT IS LIVE. Without this the test passes just as + // happily when `resolve` always answers `None` and every call falls + // back — a "fast path" nobody can prove ran (CLAUDE.md, the fourth way + // a gate cannot fail). + assert!( + site.is_direct(), + "an unregistered, capture-less, non-bound callback must resolve to \ + a direct target -- otherwise this whole module is inert" + ); + assert_eq!(site.call(c, 1.0, 2.0, 3.0), 123.0); + assert_eq!( + site.call(c, 1.0, 2.0, 3.0), + js_closure_call3(c, 1.0, 2.0, 3.0) + ); + } + + #[test] + fn a_declared_arity_above_the_call_arity_is_declined() { + let c = closure_for(add3 as *const u8); + // Same body, asked for at a LOWER arity than it declares: the call + // must keep going through `js_closure_call2`, which pads with + // undefined via `dispatch_with_arity`. + crate::closure::js_register_closure_arity(add3 as *const u8, 3); + assert!( + !DirectCall2::resolve(c).is_direct(), + "declared arity 3 > call arity 2 must decline: a direct 2-arg call \ + would leave the third parameter as whatever was in the register" + ); + // ...and the same body at its own arity still resolves. + assert!(DirectCall3::resolve(c).is_direct()); + } + + #[test] + fn a_rest_closure_is_declined() { + let c = closure_for(sum2 as *const u8); + assert!( + DirectCall2::resolve(c).is_direct(), + "precondition: it resolves before being registered as rest" + ); + crate::closure::js_register_closure_rest(sum2 as *const u8, 1); + assert!( + !DirectCall2::resolve(c).is_direct(), + "a rest parameter needs `dispatch_rest_bundled` to build the rest \ + array; a direct call would hand the body a bare f64" + ); + } + + #[test] + fn an_invalid_closure_pointer_is_declined_and_falls_back() { + // `get_valid_func_ptr` rejects the small-handle band, so this is the + // shape a NaN-boxed stdlib handle takes if one reaches a callback slot. + let site = DirectCall1::resolve(0x40 as *const ClosureHeader); + assert!(!site.is_direct()); + } +} diff --git a/crates/perry-runtime/src/closure/mod.rs b/crates/perry-runtime/src/closure/mod.rs index d3542d319c..05e2464433 100644 --- a/crates/perry-runtime/src/closure/mod.rs +++ b/crates/perry-runtime/src/closure/mod.rs @@ -46,11 +46,12 @@ pub use dispatch::{ js_closure_call13, js_closure_call14, js_closure_call15, js_closure_call16, js_closure_call2, js_closure_call3, js_closure_call4, js_closure_call5, js_closure_call6, js_closure_call7, js_closure_call8, js_closure_call9, js_closure_call_apply_with_spread, js_closure_call_array, - js_function_bind, js_native_call_value, throw_not_callable, + js_function_bind, js_native_call_value, throw_not_callable, DirectCall1, DirectCall2, + DirectCall3, DirectCall4, }; pub(crate) use dispatch::{ coerce_call_this, rebind_explicit_this, reify_function_method_value, - reset_throw_not_callable_counter, resolve_call2_direct, + reset_throw_not_callable_counter, }; pub use unbox::js_closure_unbox_callee_checked; diff --git a/crates/perry-runtime/src/object/typed_array_proto_thunks.rs b/crates/perry-runtime/src/object/typed_array_proto_thunks.rs index 2731192459..480efc99a5 100644 --- a/crates/perry-runtime/src/object/typed_array_proto_thunks.rs +++ b/crates/perry-runtime/src/object/typed_array_proto_thunks.rs @@ -633,10 +633,14 @@ impl<'s> RootedUint8Buffer<'s> { /// verdict showing the subject was live: `safepoints=5306 /// scheduled_collections=5306 copying_minors=5306 moved_objects=25760`. +/// #8180: each type also carries the closure's dispatch strategy, resolved +/// ONCE for the whole loop (`closure/dispatch/direct.rs`) instead of +/// re-derived per element by `js_closure_callN`. macro_rules! rooted_callback { - ($name:ident, $slow:ident, $($arg:ident),+) => { + ($name:ident, $site:ident, $($arg:ident),+) => { struct $name<'s> { handle: crate::gc::RuntimeHandle<'s>, + site: crate::closure::$site, } impl<'s> $name<'s> { @@ -647,6 +651,7 @@ macro_rules! rooted_callback { ) -> Self { Self { handle: scope.root_raw_const_ptr(callback), + site: crate::closure::$site::resolve(callback), } } @@ -660,21 +665,22 @@ macro_rules! rooted_callback { /// `scan_runtime_handle_roots_mut` both marks and REWRITES. So a /// collection during call `i` is reflected in the address call /// `i + 1` uses, which is precisely the ordering `across_*` exists - /// to enforce. + /// to enforce. The resolved `site` needs no such treatment: it is a + /// static CODE address, which relocation does not change. #[inline] fn call(&self, $($arg: f64),+) -> f64 { self.handle .with_const_ptr::(|cb| { - crate::closure::$slow(cb, $($arg),+) + self.site.call(cb, $($arg),+) }) } } }; } -rooted_callback!(RootedCallback2, js_closure_call2, arg0, arg1); -rooted_callback!(RootedCallback3, js_closure_call3, arg0, arg1, arg2); -rooted_callback!(RootedCallback4, js_closure_call4, arg0, arg1, arg2, arg3); +rooted_callback!(RootedCallback2, DirectCall2, arg0, arg1); +rooted_callback!(RootedCallback3, DirectCall3, arg0, arg1, arg2); +rooted_callback!(RootedCallback4, DirectCall4, arg0, arg1, arg2, arg3); pub(crate) unsafe fn dispatch_uint8_buffer_method( addr: usize, diff --git a/crates/perry-runtime/src/typedarray/iterate.rs b/crates/perry-runtime/src/typedarray/iterate.rs index 7db4a3c9fb..d782f59a1c 100644 --- a/crates/perry-runtime/src/typedarray/iterate.rs +++ b/crates/perry-runtime/src/typedarray/iterate.rs @@ -29,6 +29,10 @@ pub extern "C" fn js_typed_array_map( let kind = (*ta).kind; let len = (*ta).length as usize; let recv = ta_receiver_value(ta); + // #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); // 23.2.3.20 step 5: A is TypedArraySpeciesCreate(O, « len ») — BEFORE // the callback loop (so a throwing constructor/@@species getter aborts // before any callback runs). @@ -39,7 +43,7 @@ pub extern "C" fn js_typed_array_map( }; for i in 0..len { let v = load_at(ta, i); - let r = crate::closure::js_closure_call3(callback, v, i as f64, recv); + let r = cb_site.call(callback, v, i as f64, recv); crate::typedarray_props::species_result_store(result_addr, i, r); } species::result_as_ptr(result) @@ -61,13 +65,17 @@ pub extern "C" fn js_typed_array_filter( let kind = (*ta).kind; let len = (*ta).length as usize; let recv = ta_receiver_value(ta); + // #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); // 23.2.3.10: the callback runs for every element FIRST (collecting the // kept values), THEN A = TypedArraySpeciesCreate(O, « captured »). The // @@species getter is therefore observed after all callbacks. let mut kept: Vec = Vec::new(); for i in 0..len { let v = load_at(ta, i); - let r = crate::closure::js_closure_call3(callback, v, i as f64, recv); + let r = cb_site.call(callback, v, i as f64, recv); if crate::value::js_is_truthy(r) != 0 { kept.push(v); } @@ -97,9 +105,13 @@ pub extern "C" fn js_typed_array_every( unsafe { let len = (*ta).length as usize; let recv = ta_receiver_value(ta); + // #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); for i in 0..len { let v = load_at(ta, i); - let r = crate::closure::js_closure_call3(callback, v, i as f64, recv); + let r = cb_site.call(callback, v, i as f64, recv); if crate::value::js_is_truthy(r) == 0 { return f64::from_bits(crate::value::TAG_FALSE); } @@ -121,9 +133,13 @@ pub extern "C" fn js_typed_array_some( unsafe { let len = (*ta).length as usize; let recv = ta_receiver_value(ta); + // #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); for i in 0..len { let v = load_at(ta, i); - let r = crate::closure::js_closure_call3(callback, v, i as f64, recv); + let r = cb_site.call(callback, v, i as f64, recv); if crate::value::js_is_truthy(r) != 0 { return f64::from_bits(crate::value::TAG_TRUE); } @@ -143,9 +159,13 @@ pub extern "C" fn js_typed_array_for_each( unsafe { let len = (*ta).length as usize; let recv = ta_receiver_value(ta); + // #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); for i in 0..len { let v = load_at(ta, i); - let _ = crate::closure::js_closure_call3(callback, v, i as f64, recv); + let _ = cb_site.call(callback, v, i as f64, recv); } } } @@ -165,9 +185,13 @@ pub extern "C" fn js_typed_array_find( unsafe { let len = (*ta).length as usize; let recv = ta_receiver_value(ta); + // #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); for i in 0..len { let v = load_at(ta, i); - let r = crate::closure::js_closure_call3(callback, v, i as f64, recv); + let r = cb_site.call(callback, v, i as f64, recv); if crate::value::js_is_truthy(r) != 0 { return v; } @@ -189,9 +213,13 @@ pub extern "C" fn js_typed_array_find_index( unsafe { let len = (*ta).length as usize; let recv = ta_receiver_value(ta); + // #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); for i in 0..len { let v = load_at(ta, i); - let r = crate::closure::js_closure_call3(callback, v, i as f64, recv); + let r = cb_site.call(callback, v, i as f64, recv); if crate::value::js_is_truthy(r) != 0 { return i as f64; } @@ -228,6 +256,10 @@ pub extern "C" fn js_typed_array_reduce( crate::array::throw_reduce_of_empty(); } let recv = ta_receiver_value(ta); + // #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); let (accumulator, start_idx) = if has_initial != 0 { (initial, 0) } else { @@ -248,8 +280,7 @@ pub extern "C" fn js_typed_array_reduce( let acc = scope.root_nanbox_f64(accumulator); for i in start_idx..len { let v = load_at(ta, i); - let next = - crate::closure::js_closure_call4(callback, acc.get_nanbox_f64(), v, i as f64, recv); + let next = cb_site.call(callback, acc.get_nanbox_f64(), v, i as f64, recv); acc.set_nanbox_f64(next); } acc.get_nanbox_f64() @@ -281,6 +312,10 @@ pub extern "C" fn js_typed_array_reduce_right( crate::array::throw_reduce_of_empty(); } let recv = ta_receiver_value(ta); + // #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); let (accumulator, start_idx) = if has_initial != 0 { (initial, len) } else { @@ -302,13 +337,7 @@ pub extern "C" fn js_typed_array_reduce_right( if start_idx > 0 { for i in (0..start_idx).rev() { let v = load_at(ta, i); - let next = crate::closure::js_closure_call4( - callback, - acc.get_nanbox_f64(), - v, - i as f64, - recv, - ); + let next = cb_site.call(callback, acc.get_nanbox_f64(), v, i as f64, recv); acc.set_nanbox_f64(next); } } diff --git a/crates/perry-runtime/src/typedarray/transform.rs b/crates/perry-runtime/src/typedarray/transform.rs index e162da1d9a..4340503e6a 100644 --- a/crates/perry-runtime/src/typedarray/transform.rs +++ b/crates/perry-runtime/src/typedarray/transform.rs @@ -116,6 +116,7 @@ pub extern "C" fn js_typed_array_sort_default(ta: *mut TypedArrayHeader) -> *mut /// the comparator body itself) can trigger a GC that would otherwise sweep — /// or move — the first while its pointer sits in a bare Rust local. unsafe fn bigint_lane_compare( + site: crate::closure::DirectCall2, comparator: *const ClosureHeader, a_bits: u64, b_bits: u64, @@ -131,7 +132,7 @@ unsafe fn bigint_lane_compare( }; let a_handle = scope.root_nanbox_f64(box_lane(a_bits)); let b_handle = scope.root_nanbox_f64(box_lane(b_bits)); - let r = crate::closure::js_closure_call2( + let r = site.call( comparator, a_handle.get_nanbox_f64(), b_handle.get_nanbox_f64(), @@ -161,9 +162,12 @@ unsafe fn sorted_bigint_lanes( ) -> Vec { let scope = crate::gc::RuntimeHandleScope::new(); let cmp_handle = scope.root_raw_const_ptr(comparator); + // #8180: one resolution for the whole sort, not one per comparison. + let cmp_site = crate::closure::DirectCall2::resolve(comparator); let mut lanes: Vec = std::slice::from_raw_parts(data_ptr(ta) as *const u64, len).to_vec(); lanes.sort_by(|&a, &b| { bigint_lane_compare( + cmp_site, cmp_handle.get_raw_const_ptr::(), a, b, @@ -218,14 +222,14 @@ pub extern "C" fn js_typed_array_sort_with_comparator( // non-BigInt arms were missed. let scope = crate::gc::RuntimeHandleScope::new(); let cmp_handle = scope.root_raw_const_ptr(comparator); + // #8180: one resolution for the whole sort, not one per comparison. + let cmp_site = crate::closure::DirectCall2::resolve(comparator); let mut buf: Vec = (0..len).map(|i| load_at(ta_clean, i)).collect(); buf.sort_by(|a, b| { // `with_const_ptr`: an argument-position read, which `across_*` // cannot express. The handle is re-read per comparison, so a // relocation during comparison `i` is reflected in `i + 1`. - let r = cmp_handle.with_const_ptr::(|cmp| { - crate::closure::js_closure_call2(cmp, *a, *b) - }); + let r = cmp_handle.with_const_ptr::(|cmp| cmp_site.call(cmp, *a, *b)); if r < 0.0 { std::cmp::Ordering::Less } else if r > 0.0 { @@ -303,14 +307,14 @@ pub extern "C" fn js_typed_array_to_sorted_with_comparator( // non-BigInt arms were missed. let scope = crate::gc::RuntimeHandleScope::new(); let cmp_handle = scope.root_raw_const_ptr(comparator); + // #8180: one resolution for the whole sort, not one per comparison. + let cmp_site = crate::closure::DirectCall2::resolve(comparator); let mut buf: Vec = (0..len).map(|i| load_at(ta, i)).collect(); buf.sort_by(|a, b| { // `with_const_ptr`: an argument-position read, which `across_*` // cannot express. The handle is re-read per comparison, so a // relocation during comparison `i` is reflected in `i + 1`. - let r = cmp_handle.with_const_ptr::(|cmp| { - crate::closure::js_closure_call2(cmp, *a, *b) - }); + let r = cmp_handle.with_const_ptr::(|cmp| cmp_site.call(cmp, *a, *b)); if r < 0.0 { std::cmp::Ordering::Less } else if r > 0.0 { @@ -379,9 +383,13 @@ pub extern "C" fn js_typed_array_find_last( unsafe { let len = (*ta).length as usize; let recv = ta_receiver_value(ta); + // #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); for i in (0..len).rev() { let v = load_at(ta, i); - let r = crate::closure::js_closure_call3(callback, v, i as f64, recv); + let r = cb_site.call(callback, v, i as f64, recv); if crate::value::js_is_truthy(r) != 0 { return v; } @@ -403,9 +411,13 @@ pub extern "C" fn js_typed_array_find_last_index( unsafe { let len = (*ta).length as usize; let recv = ta_receiver_value(ta); + // #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); for i in (0..len).rev() { let v = load_at(ta, i); - let r = crate::closure::js_closure_call3(callback, v, i as f64, recv); + let r = cb_site.call(callback, v, i as f64, recv); if crate::value::js_is_truthy(r) != 0 { return i as f64; }