Skip to content

Commit f282919

Browse files
author
Ralph Küpper
committed
perf(array): serve the object keys walk from the array's own words (#7768)
The object field-get funnel already proves `keys` is a live `GC_TYPE_ARRAY` and caps the index below its capacity, then called `js_array_get` per key — which re-establishes both facts from scratch: a `clean_arr_ptr` forwarding walk, a lazy-header probe, the exotic-receiver classifications and a descriptor-flag read. That funnel was 78% of all `js_array_get_f64` samples on `gc-handoff/apps/asyncpipe_big.ts`. `keys_array_len_capped_to_capacity` paid the same toll through `js_array_length` once per property read. `keys_array_slot` serves the dense, descriptor-free, non-forwarded case from the array's own words and delegates everything else — a hole, an out-of-range index, a forwarded or descriptor-carrying array, a null pointer — so no general semantics move. A test-only per-thread fallback counter pins both directions, so a fast path that stopped applying and one that started swallowing a shape it should have delegated are equally red. Also switches the #7768 receiver-tag read to `addr_class::try_read_gc_header`: this file's usual `>= GC_HEADER_SIZE + 0x1000` floor sits BELOW the handle band, and `js_array_length` reaches it before proxy/handle receivers are routed. Keeps the addr-class ratchet green too.
1 parent b921c9c commit f282919

9 files changed

Lines changed: 164 additions & 23 deletions

File tree

crates/perry-runtime/src/array/collection_tag_tests.rs

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,65 @@ fn a_stale_registry_entry_over_recycled_bytes_does_not_read_as_a_map() {
216216
assert_eq!(js_map_size(map), 3);
217217
}
218218

219+
/// `keys_array_slot` must be the general getter, minus the work the field-get
220+
/// funnel already did — and must refuse every shape it cannot serve on those
221+
/// terms rather than guess. Both directions are asserted against the fallback
222+
/// counter, so "stopped applying" and "started swallowing" are equally red.
223+
#[test]
224+
fn keys_array_slot_matches_the_general_getter_and_delegates_what_it_cannot_serve() {
225+
let dense_keys = dense(&[10.0, 20.0, 30.0]);
226+
227+
let before = crate::array::test_keys_array_slot_fallbacks();
228+
for i in 0..3u32 {
229+
let fast = unsafe { crate::array::keys_array_slot(dense_keys, i) };
230+
let general = crate::array::js_array_get(dense_keys, i);
231+
assert_eq!(
232+
fast.bits(),
233+
general.bits(),
234+
"slot {i} must read identically through both paths"
235+
);
236+
}
237+
assert_eq!(
238+
crate::array::test_keys_array_slot_fallbacks(),
239+
before,
240+
"a dense, descriptor-free keys array is exactly what the fast path \
241+
exists for — it must not delegate"
242+
);
243+
244+
// Out of range, and a hole, both delegate: the general getter walks the
245+
// prototype chain for those and the dense words cannot answer.
246+
let before = crate::array::test_keys_array_slot_fallbacks();
247+
let oob = unsafe { crate::array::keys_array_slot(dense_keys, 7) };
248+
assert_eq!(oob.bits(), crate::array::js_array_get(dense_keys, 7).bits());
249+
assert_eq!(
250+
crate::array::test_keys_array_slot_fallbacks(),
251+
before + 1,
252+
"an out-of-range index must reach the general getter"
253+
);
254+
255+
let holey = js_array_alloc_with_length(3);
256+
js_array_set_f64(holey, 1, 42.0);
257+
let before = crate::array::test_keys_array_slot_fallbacks();
258+
let hole = unsafe { crate::array::keys_array_slot(holey, 0) };
259+
assert_eq!(hole.bits(), crate::array::js_array_get(holey, 0).bits());
260+
assert_eq!(
261+
crate::array::test_keys_array_slot_fallbacks(),
262+
before + 1,
263+
"a HOLE reads through the prototype chain, so it must delegate"
264+
);
265+
let filled = unsafe { crate::array::keys_array_slot(holey, 1) };
266+
assert_eq!(filled.bits(), crate::array::js_array_get(holey, 1).bits());
267+
268+
// A null / low pointer must delegate rather than dereference.
269+
let before = crate::array::test_keys_array_slot_fallbacks();
270+
let _ = unsafe { crate::array::keys_array_slot(std::ptr::null(), 0) };
271+
assert_eq!(
272+
crate::array::test_keys_array_slot_fallbacks(),
273+
before + 1,
274+
"a null keys pointer must delegate, never be dereferenced"
275+
);
276+
}
277+
219278
/// The invariant the whole gate rests on: a registered collection's address IS
220279
/// its `arena_alloc_gc` header, so its `obj_type` is a complete answer.
221280
///

crates/perry-runtime/src/array/header.rs

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -94,16 +94,15 @@ pub(crate) fn array_object_flags(arr: *const ArrayHeader) -> u16 {
9494
/// stopped being true when they moved into the managed arena.
9595
#[inline]
9696
pub(crate) fn array_receiver_gc_tag(arr: *const ArrayHeader) -> (u8, u16) {
97-
if (arr as usize) < crate::gc::GC_HEADER_SIZE + 0x1000 {
98-
return (0, 0);
99-
}
100-
// SAFETY: the same `arr - GC_HEADER_SIZE` read `clean_arr_ptr` performs on
101-
// this pointer (forwarding chain, lazy/object rejection), under the same
102-
// magnitude guard.
103-
unsafe {
104-
let gc_header =
105-
(arr as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader;
106-
((*gc_header).obj_type, (*gc_header)._reserved)
97+
// `try_read_gc_header` rather than this file's usual
98+
// `>= GC_HEADER_SIZE + 0x1000` floor: that floor sits BELOW the handle
99+
// band, and `js_array_length` reaches here before its proxy/handle
100+
// receivers have been routed. The canonical predicate rejects the bands
101+
// without touching memory, and rejecting an address the old floor would
102+
// have read costs nothing — a handle is not a Map either way.
103+
match unsafe { crate::value::addr_class::try_read_gc_header(arr as usize) } {
104+
Some(header) => (header.obj_type, header._reserved),
105+
None => (0, 0),
107106
}
108107
}
109108

crates/perry-runtime/src/array/indexing.rs

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -545,6 +545,20 @@ fn array_get_property_by_key(arr: *const ArrayHeader, key: *const crate::StringH
545545
/// FOR DENSE KEYS/PROPERTY ARRAYS ONLY — general JS arrays may have
546546
/// `length > capacity` (sparse), where this cap would be incorrect.
547547
pub(crate) unsafe fn keys_array_len_capped_to_capacity(arr: *const ArrayHeader) -> usize {
548+
// #7768: a well-formed dense keys array answers from its own two words.
549+
// `js_array_length` re-derives the same number through a proxy probe, a
550+
// second header read for its lazy/object arms, and a `clean_arr_ptr`
551+
// forwarding walk — once per property read on the field-get funnel.
552+
// `length <= capacity` is exactly the well-formed case; the sparse and
553+
// corrupted shapes this cap exists for fall through unchanged.
554+
if let Some(header) = crate::value::addr_class::try_read_gc_header(arr as usize) {
555+
if header.obj_type == crate::gc::GC_TYPE_ARRAY
556+
&& header.gc_flags & crate::gc::GC_FLAG_FORWARDED == 0
557+
&& (*arr).length <= (*arr).capacity
558+
{
559+
return (*arr).length as usize;
560+
}
561+
}
548562
let raw = js_array_length(arr) as usize;
549563
if arr.is_null() {
550564
raw
@@ -553,6 +567,66 @@ pub(crate) unsafe fn keys_array_len_capped_to_capacity(arr: *const ArrayHeader)
553567
}
554568
}
555569

570+
/// Read slot `index` of a dense internal keys/property array.
571+
///
572+
/// The object field-get funnel has already proved `keys` is a live
573+
/// `GC_TYPE_ARRAY` — it reads the `GcHeader` and returns `undefined` otherwise
574+
/// — and has capped `index` below the array's own capacity (see
575+
/// [`keys_array_len_capped_to_capacity`]). Those are precisely the two facts
576+
/// [`js_array_get_f64`] re-establishes from scratch on every call: a
577+
/// `clean_arr_ptr` forwarding walk, a lazy-header probe, the exotic-receiver
578+
/// classifications and a descriptor-flag read — per key examined, per property
579+
/// read. On `gc-handoff/apps/asyncpipe_big.ts` that one funnel was 78% of all
580+
/// `js_array_get_f64` samples.
581+
///
582+
/// Falls back to the general getter for anything it cannot serve on those
583+
/// terms — a forwarded array (which `clean_arr_ptr` would relocate), one
584+
/// carrying index descriptors, an out-of-range index, or a hole (which reads
585+
/// through the prototype chain) — so no general semantics move. Keys arrays
586+
/// are dense and descriptor-free, so the fallback is the cold arm.
587+
#[inline]
588+
pub(crate) unsafe fn keys_array_slot(
589+
keys: *const ArrayHeader,
590+
index: u32,
591+
) -> crate::value::JSValue {
592+
if let Some(header) = crate::value::addr_class::try_read_gc_header(keys as usize) {
593+
if header.obj_type == crate::gc::GC_TYPE_ARRAY
594+
&& header.gc_flags & crate::gc::GC_FLAG_FORWARDED == 0
595+
&& header._reserved & crate::gc::OBJ_FLAG_ARRAY_DESCRIPTORS == 0
596+
&& index < (*keys).length
597+
&& index < (*keys).capacity
598+
{
599+
let elements =
600+
(keys as *const u8).add(std::mem::size_of::<ArrayHeader>()) as *const f64;
601+
let raw = std::ptr::read(elements.add(index as usize));
602+
if raw.to_bits() != crate::value::TAG_HOLE {
603+
return crate::value::JSValue::from_bits(raw.to_bits());
604+
}
605+
}
606+
}
607+
#[cfg(test)]
608+
KEYS_ARRAY_SLOT_FALLBACKS.with(|c| c.set(c.get().wrapping_add(1)));
609+
crate::array::js_array_get(keys, index)
610+
}
611+
612+
/// Times [`keys_array_slot`] could NOT serve a slot from the dense words and
613+
/// had to delegate. Asserted in both directions by
614+
/// `array::collection_tag_tests` — zero for the dense keys arrays the fast path
615+
/// exists for, non-zero for every shape it must refuse — so a fast path that
616+
/// silently stopped applying, or one that started swallowing a shape it should
617+
/// have delegated, both go red.
618+
/// Per THREAD — `cargo test` runs every case on its own thread in one process,
619+
/// so a process-global counter would be moved by whatever else is running.
620+
#[cfg(test)]
621+
thread_local! {
622+
static KEYS_ARRAY_SLOT_FALLBACKS: std::cell::Cell<u64> = const { std::cell::Cell::new(0) };
623+
}
624+
625+
#[cfg(test)]
626+
pub(crate) fn test_keys_array_slot_fallbacks() -> u64 {
627+
KEYS_ARRAY_SLOT_FALLBACKS.with(|c| c.get())
628+
}
629+
556630
/// Auto-opt dead-strip anchor: codegen emits a bare `js_array_length` symbol in
557631
/// native-region wrappers (`__perry_wrap_*`) and elsewhere, so it must be a
558632
/// `#[no_mangle]` C export AND survive dead-stripping even when no Rust caller

crates/perry-runtime/src/array/mod.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ pub use self::immutable::{
105105
pub(crate) use self::indexing::{
106106
array_has_own_index, array_iteration_is_exotic, array_proto_iterator_modified,
107107
array_prototype_addr, array_prototype_has_index_flag, array_spec_get, array_spec_has_index,
108-
invalidate_array_index_fast_path, keys_array_len_capped_to_capacity,
108+
invalidate_array_index_fast_path, keys_array_len_capped_to_capacity, keys_array_slot,
109109
note_array_proto_iterator_write, note_object_prototype_index_write, object_prototype_addr,
110110
object_prototype_addr_matches, object_prototype_has_index_flag,
111111
PERRY_ARRAY_INDEX_FAST_PATH_INVALIDATED,
@@ -120,7 +120,9 @@ pub use self::indexing::{
120120
scan_prototype_addr_cache_roots_mut,
121121
};
122122
#[cfg(test)]
123-
pub(crate) use self::indexing::{test_array_proto_addr_cache, test_object_proto_addr_cache};
123+
pub(crate) use self::indexing::{
124+
test_array_proto_addr_cache, test_keys_array_slot_fallbacks, test_object_proto_addr_cache,
125+
};
124126
pub use self::is_array::js_array_is_array;
125127
pub(crate) use self::iter_methods::throw_reduce_of_empty;
126128
pub use self::iter_methods::{

crates/perry-runtime/src/map.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -196,18 +196,23 @@ fn register_map(ptr: *mut MapHeader, entries: *mut f64, capacity: usize) {
196196
/// receiver-tag gates (#7768) are asserted against this: a plain-array element
197197
/// read must not move it. Remove those gates and the assertion fails, which is
198198
/// the point — a fast path nobody can prove ran is not a fast path.
199+
///
200+
/// Per THREAD, not per process: the registries themselves are thread-local, and
201+
/// `cargo test` runs every case on its own thread in one process, so a global
202+
/// counter would be moved by whatever else happens to be running.
199203
#[cfg(test)]
200-
pub(crate) static TEST_MAP_REGISTRY_PROBES: std::sync::atomic::AtomicU64 =
201-
std::sync::atomic::AtomicU64::new(0);
204+
thread_local! {
205+
static TEST_MAP_REGISTRY_PROBES: std::cell::Cell<u64> = const { std::cell::Cell::new(0) };
206+
}
202207

203208
#[cfg(test)]
204209
pub(crate) fn test_map_registry_probe_count() -> u64 {
205-
TEST_MAP_REGISTRY_PROBES.load(std::sync::atomic::Ordering::Relaxed)
210+
TEST_MAP_REGISTRY_PROBES.with(|c| c.get())
206211
}
207212

208213
pub fn is_registered_map(addr: usize) -> bool {
209214
#[cfg(test)]
210-
TEST_MAP_REGISTRY_PROBES.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
215+
TEST_MAP_REGISTRY_PROBES.with(|c| c.set(c.get().wrapping_add(1)));
211216
// #7469: nothing has ever been registered ⟹ nothing can be found. Checked
212217
// first because it is the only arm that costs neither a thread-local
213218
// resolution nor a hash.

crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ pub extern "C" fn js_object_get_field_by_name(
175175
crate::array::keys_array_len_capped_to_capacity(keys);
176176
if key_count <= 4096 {
177177
for i in 0..key_count {
178-
let kv = crate::array::js_array_get(keys, i as u32);
178+
let kv = crate::array::keys_array_slot(keys, i as u32);
179179
if crate::string::js_string_key_matches(kv, key) {
180180
super::super::prop_plan::read_plan_record(
181181
keys as usize,

crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1601,7 +1601,7 @@ pub(crate) fn get_field_by_name_object_tail(
16011601
if let Some(field_idx) = cached {
16021602
let idx = field_idx as usize;
16031603
let cache_hit_valid = if idx < key_count {
1604-
let key_val = crate::array::js_array_get(keys, field_idx);
1604+
let key_val = crate::array::keys_array_slot(keys, field_idx);
16051605
// #1781: SSO-aware match — pre-fix the `is_string()` here
16061606
// false-invalidated cache hits for ≤5-byte keys stored
16071607
// as SHORT_STRING_TAG values.
@@ -1676,7 +1676,7 @@ pub(crate) fn get_field_by_name_object_tail(
16761676
}
16771677

16781678
for i in 0..key_count {
1679-
let key_val = crate::array::js_array_get(keys, i as u32);
1679+
let key_val = crate::array::keys_array_slot(keys, i as u32);
16801680
// #1781: accept inline SSO short keys here too — the
16811681
// slow-path lookup is what backs `obj[k]` for ≤5-byte
16821682
// keys after a field-cache miss.

crates/perry-runtime/src/set.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -224,17 +224,18 @@ fn register_set(ptr: *mut SetHeader, elements: *mut f64, capacity: usize) {
224224
/// Every entry into [`is_registered_set`]. Twin of
225225
/// `map::TEST_MAP_REGISTRY_PROBES` — see that counter for what it pins down.
226226
#[cfg(test)]
227-
pub(crate) static TEST_SET_REGISTRY_PROBES: std::sync::atomic::AtomicU64 =
228-
std::sync::atomic::AtomicU64::new(0);
227+
thread_local! {
228+
static TEST_SET_REGISTRY_PROBES: std::cell::Cell<u64> = const { std::cell::Cell::new(0) };
229+
}
229230

230231
#[cfg(test)]
231232
pub(crate) fn test_set_registry_probe_count() -> u64 {
232-
TEST_SET_REGISTRY_PROBES.load(std::sync::atomic::Ordering::Relaxed)
233+
TEST_SET_REGISTRY_PROBES.with(|c| c.get())
233234
}
234235

235236
pub fn is_registered_set(addr: usize) -> bool {
236237
#[cfg(test)]
237-
TEST_SET_REGISTRY_PROBES.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
238+
TEST_SET_REGISTRY_PROBES.with(|c| c.set(c.get().wrapping_add(1)));
238239
// #7469: nothing registered ⟹ nothing to find, without a thread-local
239240
// resolution or a hash. See `map::is_registered_map` for the pairing.
240241
if set_registry_never_used() {

scripts/addr_class_allowlist.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,3 +151,4 @@ crates/perry-runtime/src/child_process/value_util.rs | * | pre-existing GcHeader
151151
crates/perry-runtime/src/closure/dispatch/ | * | pre-existing GcHeader probe predating addr_class; address validated by call-site guards (magnitude/registry/is_valid_obj_ptr) -- migrate to addr_class::try_read_gc_header in a follow-up (split of closure/dispatch.rs)
152152
crates/perry-runtime/src/bun_compat/string_width.rs | 0xE0000..=0xE007F | Unicode "Tags" codepoint block (U+E0000..U+E007F) tested against a char, not a handle-band address
153153
crates/perry-runtime/src/bun_compat/width_tables.rs | * | pure Unicode East-Asian-width codepoint-range table; every hex literal is a Unicode code point (e.g. U+F0000 / U+100000 SPUA-A/B planes), never a handle-band address
154+
crates/perry-runtime/src/array/collection_tag_tests.rs | * | unit tests for the #7768 receiver-tag gate: they read AND re-stamp `GcHeader.obj_type` on an address they allocated themselves, which is the whole subject under test. `try_read_gc_header` cannot serve them (it hands out a shared reference, and the recycling test must WRITE the tag to model `arena_alloc_gc` handing the bytes to the next owner).

0 commit comments

Comments
 (0)