From 6eda1173e99ee9f7954621abce89d1a855d21b08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 11 Aug 2026 19:35:18 +0200 Subject: [PATCH 1/4] perf(runtime): gate dynamic-index collection probes by header (#7865) --- crates/perry-runtime/src/value/dyn_index.rs | 79 ++++++++++++++++--- .../value/dyn_index_collection_tag_tests.rs | 59 ++++++++++++++ 2 files changed, 125 insertions(+), 13 deletions(-) create mode 100644 crates/perry-runtime/src/value/dyn_index_collection_tag_tests.rs diff --git a/crates/perry-runtime/src/value/dyn_index.rs b/crates/perry-runtime/src/value/dyn_index.rs index 524855baaa..0f78787b74 100644 --- a/crates/perry-runtime/src/value/dyn_index.rs +++ b/crates/perry-runtime/src/value/dyn_index.rs @@ -2,6 +2,57 @@ use super::*; +#[cfg(test)] +thread_local! { + static TEST_COLLECTION_REGISTRY_PROBES: std::cell::Cell<(u64, u64)> = + const { std::cell::Cell::new((0, 0)) }; +} + +#[cfg(test)] +fn test_collection_registry_probe_count() -> (u64, u64) { + TEST_COLLECTION_REGISTRY_PROBES.with(std::cell::Cell::get) +} + +#[inline(always)] +fn probe_set_registry(addr: usize) -> bool { + #[cfg(test)] + TEST_COLLECTION_REGISTRY_PROBES.with(|counts| { + let (maps, sets) = counts.get(); + counts.set((maps, sets.wrapping_add(1))); + }); + crate::set::is_registered_set(addr) +} + +#[inline(always)] +fn probe_map_registry(addr: usize) -> bool { + #[cfg(test)] + TEST_COLLECTION_REGISTRY_PROBES.with(|counts| { + let (maps, sets) = counts.get(); + counts.set((maps.wrapping_add(1), sets)); + }); + crate::map::is_registered_map(addr) +} + +/// Read the GC type/flags that both dynamic-index dispatchers already need, +/// after header-less TypedArray and Buffer receivers have been routed. +/// A collection tag only selects a registry; registration still proves ownership. +#[inline(always)] +fn receiver_gc_tag(addr: usize) -> Option<(u8, u8)> { + unsafe { + crate::value::addr_class::try_read_gc_header(addr) + .map(|header| (header.obj_type, header.gc_flags)) + } +} + +#[inline(always)] +fn is_registered_collection(addr: usize, obj_type: u8) -> bool { + match obj_type { + crate::gc::GC_TYPE_SET => probe_set_registry(addr), + crate::gc::GC_TYPE_MAP => probe_map_registry(addr), + _ => false, + } +} + fn finite_nonnegative_i32_index(index: f64) -> Option { let bits = index.to_bits(); if (bits & TAG_MASK) == INT32_TAG { @@ -229,7 +280,11 @@ pub extern "C" fn js_dyn_index_get(value: f64, index: f64) -> f64 { } return f64::from_bits(TAG_UNDEFINED); } - if crate::set::is_registered_set(raw_ptr) || crate::map::is_registered_map(raw_ptr) { + // #7865: the receiver's managed-header type can rule both collections out + // before either thread-local registry/hash probe. The registry remains the + // authority for a matching tag; the tag only selects which one to ask. + let receiver_tag = receiver_gc_tag(raw_ptr); + if receiver_tag.is_some_and(|(obj_type, _)| is_registered_collection(raw_ptr, obj_type)) { let Some(index) = finite_nonnegative_u32_index(index) else { return f64::from_bits(TAG_UNDEFINED); }; @@ -323,12 +378,7 @@ pub extern "C" fn js_dyn_index_get(value: f64, index: f64) -> f64 { return value; } } - if raw_ptr >= crate::gc::GC_HEADER_SIZE { - let gc_hdr = unsafe { - (raw_ptr as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader - }; - let obj_type = unsafe { (*gc_hdr).obj_type }; - let gc_flags = unsafe { (*gc_hdr).gc_flags }; + if let Some((obj_type, gc_flags)) = receiver_tag { if obj_type == crate::gc::GC_TYPE_LAZY_ARRAY || (gc_flags & crate::gc::GC_FLAG_FORWARDED) != 0 { @@ -541,7 +591,10 @@ pub extern "C" fn js_dyn_index_set(obj: f64, index: f64, value: f64) -> f64 { } return value; } - if crate::set::is_registered_set(raw_ptr) || crate::map::is_registered_map(raw_ptr) { + // #7865: reuse the header byte the array/object split below needs. Plain + // receivers skip both registries; Map/Set tags still require confirmation. + let receiver_tag = receiver_gc_tag(raw_ptr); + if receiver_tag.is_some_and(|(obj_type, _)| is_registered_collection(raw_ptr, obj_type)) { return value; } // Mirror the #63/#321 guard on the get side: heuristic-derived @@ -589,11 +642,7 @@ pub extern "C" fn js_dyn_index_set(obj: f64, index: f64, value: f64) -> f64 { return value; } } - let is_array = unsafe { - let gc_header = - (raw_ptr as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader; - (*gc_header).obj_type == crate::gc::GC_TYPE_ARRAY - }; + let is_array = receiver_tag.is_some_and(|(obj_type, _)| obj_type == crate::gc::GC_TYPE_ARRAY); if is_array { crate::array::js_array_set_index_or_string( raw_ptr as *mut crate::array::ArrayHeader, @@ -713,3 +762,7 @@ static KEEP_JS_DYN_INDEX_SET: extern "C" fn(f64, f64, f64) -> f64 = js_dyn_index #[cfg(feature = "keepalive-anchors")] #[used] static KEEP_JS_IS_UNDEFINED_OR_BARE_NAN: extern "C" fn(f64) -> i32 = js_is_undefined_or_bare_nan; + +#[cfg(test)] +#[path = "dyn_index_collection_tag_tests.rs"] +mod collection_tag_tests; diff --git a/crates/perry-runtime/src/value/dyn_index_collection_tag_tests.rs b/crates/perry-runtime/src/value/dyn_index_collection_tag_tests.rs new file mode 100644 index 0000000000..e61131294b --- /dev/null +++ b/crates/perry-runtime/src/value/dyn_index_collection_tag_tests.rs @@ -0,0 +1,59 @@ +//! Receiver-tag gating for dynamic-index Map/Set registry probes (#7865). + +use super::{js_dyn_index_get, js_dyn_index_set}; + +fn probes() -> (u64, u64) { + super::test_collection_registry_probe_count() +} + +fn arm_both_registries() -> (*mut crate::map::MapHeader, *mut crate::set::SetHeader) { + let map = crate::map::js_map_alloc(4); + crate::map::js_map_set(map, 1.0, 10.0); + let set = crate::set::js_set_alloc(4); + crate::set::js_set_add(set, 20.0); + assert!(crate::map::is_registered_map(map as usize)); + assert!(crate::set::is_registered_set(set as usize)); + (map, set) +} + +#[test] +fn plain_array_dynamic_indexing_never_probes_collection_registries() { + let (_map, _set) = arm_both_registries(); + let mut array = crate::array::js_array_alloc(2); + array = crate::array::js_array_push_f64(array, 10.0); + let receiver = crate::value::js_nanbox_pointer(array as i64); + let before = probes(); + + assert_eq!(js_dyn_index_get(receiver, 0.0), 10.0); + assert_eq!(js_dyn_index_set(receiver, 0.0, 30.0), 30.0); + assert_eq!(js_dyn_index_get(receiver, 0.0), 30.0); + + assert_eq!( + probes(), + before, + "GC_TYPE_ARRAY must bypass both Map/Set registry probes" + ); +} + +#[test] +fn collection_receivers_still_use_their_authoritative_registries() { + let (map, set) = arm_both_registries(); + let map_receiver = crate::value::js_nanbox_pointer(map as i64); + let set_receiver = crate::value::js_nanbox_pointer(set as i64); + + let before = probes(); + assert_eq!(js_dyn_index_get(map_receiver, 0.0), 1.0); + assert_eq!(probes(), (before.0 + 1, before.1)); + + let before = probes(); + assert_eq!(js_dyn_index_get(set_receiver, 0.0), 20.0); + assert_eq!(probes(), (before.0, before.1 + 1)); + + let before = probes(); + assert_eq!(js_dyn_index_set(map_receiver, 0.0, 99.0), 99.0); + assert_eq!(probes(), (before.0 + 1, before.1)); + + let before = probes(); + assert_eq!(js_dyn_index_set(set_receiver, 0.0, 88.0), 88.0); + assert_eq!(probes(), (before.0, before.1 + 1)); +} From 467f7e44909190c5ce96e779c5031dcf44140df9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 11 Aug 2026 20:13:45 +0200 Subject: [PATCH 2/4] docs(changelog): record dynamic-index header gates --- changelog.d/7880-dyn-index-header-gates.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 changelog.d/7880-dyn-index-header-gates.md diff --git a/changelog.d/7880-dyn-index-header-gates.md b/changelog.d/7880-dyn-index-header-gates.md new file mode 100644 index 0000000000..fdd485857a --- /dev/null +++ b/changelog.d/7880-dyn-index-header-gates.md @@ -0,0 +1,14 @@ +### `perf(runtime)`: gate dynamic-index collection probes by the GC header (#7865) + +`js_dyn_index_get` and `js_dyn_index_set` used to consult both the `Map` and +`Set` side registries on every dynamic index operation once either registry had +been armed. They now read the receiver's already-required `GcHeader` first and +consult only the registry selected by its object type. Registry membership +remains the authoritative ownership check; the header is only a prefilter. + +Dedicated probe counters lock in the structural saving: ordinary array indexing +touches neither collection registry, while `Map` and `Set` receivers still reach +exactly their own registry. On the quiet M1 mini, an amplified `interp` run was +about 0.3% faster but within noise, and a targeted 20-million-iteration dynamic +index workload was exactly unchanged. This lands for the eliminated registry +lookups and the regression coverage, not for a claimed wall-clock speedup. From 1357b87fa5dbeb40213f04880ec87009ea0cf4ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 11 Aug 2026 20:35:20 +0200 Subject: [PATCH 3/4] fix(runtime): validate raw dynamic-index receivers before header read --- crates/perry-runtime/src/value/dyn_index.rs | 28 +++++++++++++++---- .../value/dyn_index_collection_tag_tests.rs | 22 +++++++++++++++ 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/crates/perry-runtime/src/value/dyn_index.rs b/crates/perry-runtime/src/value/dyn_index.rs index 0f78787b74..d774754082 100644 --- a/crates/perry-runtime/src/value/dyn_index.rs +++ b/crates/perry-runtime/src/value/dyn_index.rs @@ -53,6 +53,24 @@ fn is_registered_collection(addr: usize, obj_type: u8) -> bool { } } +/// A legacy raw-I64 receiver has no NaN-box tag proving it came from a managed +/// allocation. Validate membership without dereferencing it before asking for +/// a `GcHeader`; the old magnitude-only `is_valid_obj_ptr` check admitted any +/// aligned address in the platform heap range, including unmapped addresses. +#[inline(always)] +fn raw_i64_receiver_is_managed(addr: usize) -> bool { + if !matches!( + crate::arena::classify_heap_space(addr), + crate::arena::HeapSpace::Unknown + ) { + return true; + } + addr.checked_sub(crate::gc::GC_HEADER_SIZE) + .is_some_and(|header| { + crate::gc::gc_malloc_header_is_tracked(header as *const crate::gc::GcHeader) + }) +} + fn finite_nonnegative_i32_index(index: f64) -> Option { let bits = index.to_bits(); if (bits & TAG_MASK) == INT32_TAG { @@ -591,17 +609,17 @@ pub extern "C" fn js_dyn_index_set(obj: f64, index: f64, value: f64) -> f64 { } return value; } + // A raw-I64 fallback is only a heuristic until arena/malloc membership + // proves it. Do this before `receiver_gc_tag`, which reads addr - 8. + if !jsval.is_pointer() && !raw_i64_receiver_is_managed(raw_ptr) { + return value; + } // #7865: reuse the header byte the array/object split below needs. Plain // receivers skip both registries; Map/Set tags still require confirmation. let receiver_tag = receiver_gc_tag(raw_ptr); if receiver_tag.is_some_and(|(obj_type, _)| is_registered_collection(raw_ptr, obj_type)) { return value; } - // Mirror the #63/#321 guard on the get side: heuristic-derived - // pseudo-pointers from non-pointer dataflow must not be dereferenced. - if !jsval.is_pointer() && !crate::object::is_valid_obj_ptr(raw_ptr as *const u8) { - return value; - } // #5579 / Issue #957 (set side): a STRING index (`obj["foo"] = v`) must // route through the ordinary receiver-aware `[[Set]]`, NOT the numeric // element path below. A NaN-boxed string index otherwise reached the diff --git a/crates/perry-runtime/src/value/dyn_index_collection_tag_tests.rs b/crates/perry-runtime/src/value/dyn_index_collection_tag_tests.rs index e61131294b..fd35368a42 100644 --- a/crates/perry-runtime/src/value/dyn_index_collection_tag_tests.rs +++ b/crates/perry-runtime/src/value/dyn_index_collection_tag_tests.rs @@ -57,3 +57,25 @@ fn collection_receivers_still_use_their_authoritative_registries() { assert_eq!(js_dyn_index_set(set_receiver, 0.0, 88.0), 88.0); assert_eq!(probes(), (before.0, before.1 + 1)); } + +#[test] +fn unmapped_legacy_raw_i64_is_rejected_before_the_gc_header_read() { + // An aligned, finite raw-I64 candidate inside the platform's permissive + // heap magnitude range, but not inside any arena or tracked malloc object. + // The pre-review ordering passed it to `receiver_gc_tag` and attempted to + // read the unmapped header at 4 GiB - 8. + let raw_bits = 0x0000_0001_0000_0000_u64; + let raw_receiver = f64::from_bits(raw_bits); + assert!(!raw_receiver.is_nan()); + assert!(crate::value::addr_class::is_plausible_heap_addr( + raw_bits as usize + )); + assert!(matches!( + crate::arena::classify_heap_space(raw_bits as usize), + crate::arena::HeapSpace::Unknown + )); + + let before = probes(); + assert_eq!(js_dyn_index_set(raw_receiver, 0.0, 42.0), 42.0); + assert_eq!(probes(), before); +} From cc763d5ac4c234b0d8309b869ddf727e52a5b4d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 11 Aug 2026 20:53:35 +0200 Subject: [PATCH 4/4] test(runtime): prove raw dyn index skips header reads --- crates/perry-runtime/src/value/dyn_index.rs | 36 +++++++++++++------ .../value/dyn_index_collection_tag_tests.rs | 18 ++++++++-- 2 files changed, 41 insertions(+), 13 deletions(-) diff --git a/crates/perry-runtime/src/value/dyn_index.rs b/crates/perry-runtime/src/value/dyn_index.rs index d774754082..40e3530338 100644 --- a/crates/perry-runtime/src/value/dyn_index.rs +++ b/crates/perry-runtime/src/value/dyn_index.rs @@ -4,21 +4,29 @@ use super::*; #[cfg(test)] thread_local! { - static TEST_COLLECTION_REGISTRY_PROBES: std::cell::Cell<(u64, u64)> = - const { std::cell::Cell::new((0, 0)) }; + static TEST_DYN_INDEX_DISPATCH_COUNTS: std::cell::Cell<(u64, u64, u64)> = + const { std::cell::Cell::new((0, 0, 0)) }; } #[cfg(test)] fn test_collection_registry_probe_count() -> (u64, u64) { - TEST_COLLECTION_REGISTRY_PROBES.with(std::cell::Cell::get) + TEST_DYN_INDEX_DISPATCH_COUNTS.with(|counts| { + let (maps, sets, _) = counts.get(); + (maps, sets) + }) +} + +#[cfg(test)] +fn test_receiver_gc_header_read_count() -> u64 { + TEST_DYN_INDEX_DISPATCH_COUNTS.with(|counts| counts.get().2) } #[inline(always)] fn probe_set_registry(addr: usize) -> bool { #[cfg(test)] - TEST_COLLECTION_REGISTRY_PROBES.with(|counts| { - let (maps, sets) = counts.get(); - counts.set((maps, sets.wrapping_add(1))); + TEST_DYN_INDEX_DISPATCH_COUNTS.with(|counts| { + let (maps, sets, header_reads) = counts.get(); + counts.set((maps, sets.wrapping_add(1), header_reads)); }); crate::set::is_registered_set(addr) } @@ -26,9 +34,9 @@ fn probe_set_registry(addr: usize) -> bool { #[inline(always)] fn probe_map_registry(addr: usize) -> bool { #[cfg(test)] - TEST_COLLECTION_REGISTRY_PROBES.with(|counts| { - let (maps, sets) = counts.get(); - counts.set((maps.wrapping_add(1), sets)); + TEST_DYN_INDEX_DISPATCH_COUNTS.with(|counts| { + let (maps, sets, header_reads) = counts.get(); + counts.set((maps.wrapping_add(1), sets, header_reads)); }); crate::map::is_registered_map(addr) } @@ -39,8 +47,14 @@ fn probe_map_registry(addr: usize) -> bool { #[inline(always)] fn receiver_gc_tag(addr: usize) -> Option<(u8, u8)> { unsafe { - crate::value::addr_class::try_read_gc_header(addr) - .map(|header| (header.obj_type, header.gc_flags)) + crate::value::addr_class::try_read_gc_header(addr).map(|header| { + #[cfg(test)] + TEST_DYN_INDEX_DISPATCH_COUNTS.with(|counts| { + let (maps, sets, header_reads) = counts.get(); + counts.set((maps, sets, header_reads.wrapping_add(1))); + }); + (header.obj_type, header.gc_flags) + }) } } diff --git a/crates/perry-runtime/src/value/dyn_index_collection_tag_tests.rs b/crates/perry-runtime/src/value/dyn_index_collection_tag_tests.rs index fd35368a42..5a3a860fff 100644 --- a/crates/perry-runtime/src/value/dyn_index_collection_tag_tests.rs +++ b/crates/perry-runtime/src/value/dyn_index_collection_tag_tests.rs @@ -6,6 +6,10 @@ fn probes() -> (u64, u64) { super::test_collection_registry_probe_count() } +fn header_reads() -> u64 { + super::test_receiver_gc_header_read_count() +} + fn arm_both_registries() -> (*mut crate::map::MapHeader, *mut crate::set::SetHeader) { let map = crate::map::js_map_alloc(4); crate::map::js_map_set(map, 1.0, 10.0); @@ -75,7 +79,17 @@ fn unmapped_legacy_raw_i64_is_rejected_before_the_gc_header_read() { crate::arena::HeapSpace::Unknown )); - let before = probes(); + let probes_before = probes(); + assert_eq!(header_reads(), 0); + + assert_eq!( + js_dyn_index_get(raw_receiver, 0.0).to_bits(), + crate::value::TAG_UNDEFINED + ); + assert_eq!(probes(), probes_before); + assert_eq!(header_reads(), 0); + assert_eq!(js_dyn_index_set(raw_receiver, 0.0, 42.0), 42.0); - assert_eq!(probes(), before); + assert_eq!(probes(), probes_before); + assert_eq!(header_reads(), 0); }