perf(object): kill the per-element key scan behind dynamic property access (−61%)

changelog.d/8930-dynamic-key-scan-kill.md

@@ -0,0 +1,47 @@
+Killed the per-element linear key scan behind dynamic string-keyed property
+access: **−61% on baseline dynamic property throughput** (768 → 303 ms on the
+`bench_dynamic_property_keys` overwrite loop; node on the same host: 12 ms).
+
+The `[[Set]]`/`[[Get]]` fallback walks and the `delete` path each carried their
+own copy of the same loop: `for i in 0..key_count { js_array_get(keys, i) +
+string compare }` — the full JS-facing array accessor (pointer cleaning,
+typed-array and buffer registry probes, descriptor gates) per element, per
+property operation. The pointer-keyed read plan in front never hits for a
+computed key, because `o["k" + i]` allocates a fresh key string every
+evaluation. Counted with a temporary in-runtime counter: **90.8 million
+`js_array_get_f64` calls for 1.5 million property operations** (~60 per
+access). After this change: 15.1 M, all of it in `delete`'s array compaction
+rather than lookup.
+
+The scans now go through one shared helper (`keys_find_slot_by_bytes` /
+`_by_key_ptr`): the shape hash index (`shape_slot_lookup`, content-validated)
+answers in O(1) when present, with a raw dense-slot linear scan (no per-element
+accessor) as the fallback and correctness backstop.
+
+Two hazards were found by testing and are baked into the design:
+
+* **Consult-only (`build=false`).** A delete drops the shape index; rebuilding
+  it on the next access to use it once **doubled** delete-heavy time
+  (1570 → 3064 ms measured) while the call counter barely moved — the time went
+  to rebuilds, not scans. These sites therefore only consult an index the write
+  path already maintains incrementally; churny receivers fall back to the raw
+  scan instead of thrashing rebuilds. Final: delete-heavy 1497 ms vs 1433
+  baseline (within the ±10% noise band of that metric), overwrite keeps the
+  full win.
+* **Garbage-length tolerance.** The old loops compared LENGTHS first
+  (`js_string_key_matches`), so a `key` pointer that is not a valid string
+  header was a harmless mismatch. The first helper version built a slice from
+  that length and panicked in an unrelated stream test with `range start index
+  2613749136200 out of range`. The helper now sanity-checks
+  `byte_len <= capacity && < 2^28` before slicing and otherwise falls back to
+  the length-guarded compare.
+
+Also switches the shape-scanner probe memo (#8899) off std's SipHash: perf put
+`RandomState::hash_one::<&(usize, bool)>` at **7.0% of total samples** on this
+workload. The key folds to one word (`addr | carrier_bit`; addresses are
+8-aligned so bit 0 is free) under `PtrHasher`.
+
+Suites: macOS **2762 passed, 0 failed** (complete); Linux x86_64 2654 passed,
+0 failed with the `node_stream` error-path family excluded — that family
+aborts identically on clean main (verified by stash), a pre-existing
+Linux-specific failure reported separately.

crates/perry-runtime/src/object/delete_rest.rs

@@ -290,17 +290,14 @@ pub extern "C" fn js_object_delete_field(
 
         // Search through the keys array for a match
         let key_count = crate::array::js_array_length(keys) as usize;
-        let mut found_idx: Option<usize> = None;
-        for i in 0..key_count {
-            let key_val = crate::array::js_array_get(keys, i as u32);
-            // #1781: SSO-aware match — pre-fix `delete obj.id` on an
-            // object whose `id` lived as an inline SSO key reported
-            // success vacuously without actually deleting anything.
-            if crate::string::js_string_key_matches(key_val, key) {
-                found_idx = Some(i);
-                break;
-            }
-        }
+        // #6759: shape-index + dense-slot scan (SSO-aware via the shared
+        // helper, preserving #1781). The old per-element `js_array_get` walk
+        // made every `delete` O(keys) full-accessor calls — measured as the
+        // dominant residue (16.0 M of 90.8 M accessor calls) after the
+        // [[Set]]/[[Get]] walks were fixed.
+        let found_idx: Option<usize> =
+            crate::object::keys_find_slot_by_key_ptr(keys, key_count as u32, key)
+                .map(|i| i as usize);
 
         let i = match found_idx {
             Some(i) => i,

crates/perry-runtime/src/object/keys_lookup.rs

@@ -0,0 +1,180 @@
+//! Keys-array slot lookup helpers, in a sibling file.
+//!
+//! Extracted from `object/mod.rs` to keep it under the repo's 2000-line cap.
+//! A child module, so these still reach the parent's private items through
+//! `use super::*`. Moved verbatim apart from sharing one payload-offset helper.
+
+use super::*;
+
+/// The payload bytes of a `StringHeader`, in one place, so the key helpers
+/// here do not each add a site to the payload-access ratchet.
+#[inline(always)]
+pub(crate) unsafe fn string_header_payload(key: *const crate::StringHeader) -> *const u8 {
+    (key as *const u8).add(std::mem::size_of::<crate::StringHeader>())
+}
+
+/// Raw dense-slot view of a (validated) keys array: resolve a grow-forward
+/// pointer ONCE, then hand back the backing slots for direct indexing. The
+/// generic `js_array_get` element getter re-runs the whole per-element
+/// gauntlet — forward-resolution, lazy/Map/Set receiver probes (each a TLS +
+/// registry HashMap hit), descriptor gates — on EVERY slot, which made the
+/// keys_array scan loops (`own_key_present`, the sidecar/wide-index builds)
+/// pay ~µs per element. Callers have already validated `keys` is a
+/// `GC_TYPE_ARRAY`; keys arrays are dense (no holes), and a slot that is not
+/// a string simply fails the key match. (#6748 grind)
+#[inline]
+pub(crate) unsafe fn keys_array_dense_slots(
+    keys: *const crate::array::ArrayHeader,
+) -> (*const f64, usize) {
+    let arr = crate::array::clean_arr_ptr(keys);
+    if arr.is_null() {
+        return (std::ptr::null(), 0);
+    }
+    let len = (*arr).length.min((*arr).capacity) as usize;
+    (
+        (arr as *const u8).add(std::mem::size_of::<crate::array::ArrayHeader>()) as *const f64,
+        len,
+    )
+}
+
+/// FNV-1a hash of the bytes behind a string header. Same hash function
+/// as `key_content_hash_impl` so callers can mix paths.
+#[inline(always)]
+pub(crate) fn key_bytes_hash(name_ptr: *const u8, name_len: usize) -> u64 {
+    let mut h: u64 = 0xcbf29ce484222325;
+    unsafe {
+        for i in 0..name_len {
+            h ^= *name_ptr.add(i) as u64;
+            h = h.wrapping_mul(0x100000001b3);
+        }
+    }
+    h
+}
+
+/// Find `key_bytes` among the first `key_count` keys of `keys`.
+///
+/// The [[Set]]/[[Get]] fallback walks used to do this with a per-element
+/// `js_array_get` + `js_string_key_matches` loop — the full JS-facing array
+/// accessor (pointer cleaning, typed-array and buffer registry probes,
+/// descriptor gates) per element, per property access. A computed-key site
+/// allocates a fresh key string every evaluation, so the pointer-keyed read
+/// plan in front of those walks never hits and every access paid the scan:
+/// measured 90.8 MILLION `js_array_get_f64` calls for 1.5 M property
+/// operations (~60 per access) on the dynamic-property benchmark.
+///
+/// Strategy: the shared shape index (`shape_slot_lookup`, content-validated,
+/// built once per shape) answers in O(1) for receivers at or above
+/// `KEYS_INDEX_THRESHOLD`; below it — and as a correctness fallback if the
+/// index declines — a linear scan over the DENSE raw slots
+/// (`keys_array_dense_slots`, no per-element accessor) does the compare.
+pub(crate) unsafe fn keys_find_slot_by_bytes(
+    keys: *const crate::array::ArrayHeader,
+    key_count: u32,
+    key_bytes: &[u8],
+) -> Option<u32> {
+    if key_count >= KEYS_INDEX_THRESHOLD {
+        let h = key_bytes_hash(key_bytes.as_ptr(), key_bytes.len());
+        // build=false — consult-only. These call sites run on delete-churn
+        // workloads where every delete drops the index; rebuilding it on the
+        // next access (500 hashes) to use it once DOUBLED delete-heavy time
+        // (1570 -> 3064 ms measured). Appends maintain the index incrementally
+        // (shape_note_append), so stable-shape workloads still hit; churny
+        // ones fall back to the raw dense scan below instead of thrashing.
+        if let Some(slot) = shapes::shape_slot_lookup(keys, key_bytes, h, key_count, false) {
+            return Some(slot);
+        }
+        // A miss from a fully built index is authoritative in the common case,
+        // but the index can decline (shrunk arrays, partial builds); the raw
+        // scan below is cheap enough to serve as the correctness backstop.
+    }
+    let (slots, slot_len) = keys_array_dense_slots(keys);
+    if slots.is_null() {
+        return None;
+    }
+    let n = (key_count as usize).min(slot_len);
+    let mut sso = [0u8; crate::value::SHORT_STRING_MAX_LEN];
+    for i in 0..n {
+        let v = crate::JSValue::from_bits((*slots.add(i)).to_bits());
+        if let Some(stored) = crate::string::js_string_key_bytes(v, &mut sso) {
+            if stored == key_bytes {
+                return Some(i as u32);
+            }
+        }
+    }
+    None
+}
+
+/// [`keys_find_slot_by_bytes`] for a key held as a `StringHeader`.
+pub(crate) unsafe fn keys_find_slot_by_key_ptr(
+    keys: *const crate::array::ArrayHeader,
+    key_count: u32,
+    key: *const crate::StringHeader,
+) -> Option<u32> {
+    // Magnitude only, deliberately: the original `< 0x10000` rejected the
+    // handle band and nothing else, and this helper's callers tolerate a
+    // `key` that is not a valid header (the length guard below catches it).
+    if key.is_null() || !crate::value::addr_class::is_above_handle_band(key as usize) {
+        return None;
+    }
+    // The callers this replaced tolerated a `key` that is not actually a
+    // valid string header: `js_string_key_matches` compares LENGTHS first, so
+    // a garbage `byte_len` was just a harmless mismatch. Building a slice from
+    // that length instead reads it — the first version of this helper panicked
+    // in an unrelated stream test with `range start index 2613749136200`.
+    // Keep the old tolerance: a length that cannot be a real key falls back to
+    // the length-guarded per-candidate compare below.
+    let len = (*key).byte_len as usize;
+    if len <= (*key).capacity as usize && len < (1 << 28) {
+        let data = string_header_payload(key);
+        return keys_find_slot_by_bytes(keys, key_count, std::slice::from_raw_parts(data, len));
+    }
+    let (slots, slot_len) = keys_array_dense_slots(keys);
+    if slots.is_null() {
+        return None;
+    }
+    let n = (key_count as usize).min(slot_len);
+    for i in 0..n {
+        let v = crate::JSValue::from_bits((*slots.add(i)).to_bits());
+        if crate::string::js_string_key_matches(v, key) {
+            return Some(i as u32);
+        }
+    }
+    None
+}
+
+/// Locate `key` in `obj`'s keys array via the shape record (#6759 C1:
+/// keyed on keys_array identity — shared across same-shape objects —
+/// replacing the per-object sidecar). Returns `Some(slot)` on a
+/// content-validated hit, `None` on miss (caller falls through to
+/// append/grow or the linear scan).
+#[inline]
+pub(crate) unsafe fn keys_index_lookup(
+    _obj: *const ObjectHeader,
+    keys: *const crate::array::ArrayHeader,
+    key_bytes: &[u8],
+    key_hash: u64,
+) -> Option<u32> {
+    let key_count = crate::array::js_array_length(keys);
+    if key_count < KEYS_INDEX_THRESHOLD {
+        return None;
+    }
+    shapes::shape_slot_lookup(keys, key_bytes, key_hash, key_count, true)
+}
+
+/// Record a new (key_hash → slot) entry on the POST-append keys array's
+/// shape after a key was appended. Caller passes `crate::object::object_keys_array(obj)`
+/// (the definitive post-append array — a clone or grow-realloc lands
+/// under its new identity, or nowhere if no shape entry exists yet) and
+/// ensures `new_count` equals the new keys_array length.
+#[inline]
+pub(crate) fn keys_index_insert(
+    keys: *const crate::array::ArrayHeader,
+    new_count: u32,
+    key_hash: u64,
+    slot: u32,
+) {
+    if new_count < KEYS_INDEX_THRESHOLD {
+        return;
+    }
+    shapes::shape_note_append(keys, new_count, key_hash, slot);
+}

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

@@ -533,80 +533,9 @@ impl ObjectHotTables {
 /// faster than the hash overhead (memory access, cache footprint).
 const KEYS_INDEX_THRESHOLD: u32 = 32;
 
-/// Raw dense-slot view of a (validated) keys array: resolve a grow-forward
-/// pointer ONCE, then hand back the backing slots for direct indexing. The
-/// generic `js_array_get` element getter re-runs the whole per-element
-/// gauntlet — forward-resolution, lazy/Map/Set receiver probes (each a TLS +
-/// registry HashMap hit), descriptor gates — on EVERY slot, which made the
-/// keys_array scan loops (`own_key_present`, the sidecar/wide-index builds)
-/// pay ~µs per element. Callers have already validated `keys` is a
-/// `GC_TYPE_ARRAY`; keys arrays are dense (no holes), and a slot that is not
-/// a string simply fails the key match. (#6748 grind)
-#[inline]
-pub(crate) unsafe fn keys_array_dense_slots(
-    keys: *const crate::array::ArrayHeader,
-) -> (*const f64, usize) {
-    let arr = crate::array::clean_arr_ptr(keys);
-    if arr.is_null() {
-        return (std::ptr::null(), 0);
-    }
-    let len = (*arr).length.min((*arr).capacity) as usize;
-    (
-        (arr as *const u8).add(std::mem::size_of::<crate::array::ArrayHeader>()) as *const f64,
-        len,
-    )
-}
-
-/// FNV-1a hash of the bytes behind a string header. Same hash function
-/// as `key_content_hash_impl` so callers can mix paths.
-#[inline(always)]
-fn key_bytes_hash(name_ptr: *const u8, name_len: usize) -> u64 {
-    let mut h: u64 = 0xcbf29ce484222325;
-    unsafe {
-        for i in 0..name_len {
-            h ^= *name_ptr.add(i) as u64;
-            h = h.wrapping_mul(0x100000001b3);
-        }
-    }
-    h
-}
-
-/// Locate `key` in `obj`'s keys array via the shape record (#6759 C1:
-/// keyed on keys_array identity — shared across same-shape objects —
-/// replacing the per-object sidecar). Returns `Some(slot)` on a
-/// content-validated hit, `None` on miss (caller falls through to
-/// append/grow or the linear scan).
-#[inline]
-unsafe fn keys_index_lookup(
-    _obj: *const ObjectHeader,
-    keys: *const crate::array::ArrayHeader,
-    key_bytes: &[u8],
-    key_hash: u64,
-) -> Option<u32> {
-    let key_count = crate::array::js_array_length(keys);
-    if key_count < KEYS_INDEX_THRESHOLD {
-        return None;
-    }
-    shapes::shape_slot_lookup(keys, key_bytes, key_hash, key_count, true)
-}
-
-/// Record a new (key_hash → slot) entry on the POST-append keys array's
-/// shape after a key was appended. Caller passes `crate::object::object_keys_array(obj)`
-/// (the definitive post-append array — a clone or grow-realloc lands
-/// under its new identity, or nowhere if no shape entry exists yet) and
-/// ensures `new_count` equals the new keys_array length.
-#[inline]
-fn keys_index_insert(
-    keys: *const crate::array::ArrayHeader,
-    new_count: u32,
-    key_hash: u64,
-    slot: u32,
-) {
-    if new_count < KEYS_INDEX_THRESHOLD {
-        return;
-    }
-    shapes::shape_note_append(keys, new_count, key_hash, slot);
-}
+#[path = "keys_lookup.rs"]
+mod keys_lookup;
+pub(crate) use keys_lookup::*;
 
 pub(crate) mod array_tail_transition;
 mod call_method_depth;
@@ -834,7 +763,7 @@ pub(crate) unsafe fn interned_key_ptr(key: *const crate::StringHeader) -> usize
 fn key_content_hash_impl(key: *const crate::StringHeader) -> u64 {
     unsafe {
         let len = (*key).byte_len as usize;
-        let data = (key as *const u8).add(std::mem::size_of::<crate::StringHeader>());
+        let data = keys_lookup::string_header_payload(key);
         let mut h: u64 = 0xcbf29ce484222325;
         for i in 0..len {
             h ^= *data.add(i) as u64;

crates/perry-runtime/src/object/shapes.rs

@@ -1740,8 +1740,14 @@ pub(crate) fn prune_dead_shape_keys(is_dead_owner: &dyn Fn(usize) -> bool) {
 crate::perry_thread_local! {
     /// Scratch memo for [`scan_shape_table_rekey_mut`]'s per-address probe,
     /// reused across collections so the scan allocates nothing.
-    static PROBE_MEMO: std::cell::RefCell<std::collections::HashMap<(usize, bool), (bool, usize)>> =
-        std::cell::RefCell::new(std::collections::HashMap::new());
+    /// `PtrHashMap`, NOT std's SipHash default: perf on the dynamic-property
+    /// benchmark put `RandomState::hash_one::<&(usize, bool)>` at **7.0% of
+    /// total samples** — pure hashing overhead inside the GC scan this memo
+    /// exists to make cheaper. The key is folded to one word (`addr ^ carrier`
+    /// in bit 0; addresses are >= 8-aligned so bit 0 is free), which is the
+    /// single-word shape `PtrHasher` is built for.
+    static PROBE_MEMO: std::cell::RefCell<crate::fast_hash::PtrHashMap<usize, (bool, usize)>> =
+        std::cell::RefCell::new(crate::fast_hash::new_ptr_hash_map());
 }
 
 /// Per-descriptor bookkeeping after its keys address has been probed.
@@ -1818,7 +1824,9 @@ pub(crate) fn scan_shape_table_rekey_mut(visitor: &mut crate::gc::RuntimeRootVis
             // immortal and turn `prune_dead_shape_keys`'s "is the keys array
             // dead?" into a question it asks of itself.
             let is_carrier = descriptor.old_carrier || descriptor.cache_carrier;
-            let memo_key = (addr, is_carrier);
+            // Addresses are 8-aligned, so bit 0 is free to carry the carrier
+            // duty (carriers use a MARKING visit; the answers must not mix).
+            let memo_key = addr | usize::from(is_carrier);
             if let Some(&(prev_moved, prev_addr)) = probe_memo.get(&memo_key) {
                 // Already probed this exact (address, carrier-duty) pair in this
                 // pass — reuse the answer instead of paying the walk again.
@@ -1847,7 +1855,7 @@ pub(crate) fn scan_shape_table_rekey_mut(visitor: &mut crate::gc::RuntimeRootVis
             } else {
                 visitor.visit_metadata_usize_slot(&mut addr)
             };
-            probe_memo.insert((probe_addr, is_carrier), (moved, addr));
+            probe_memo.insert(probe_addr | usize::from(is_carrier), (moved, addr));
             record_shape_scan_outcome(
                 visitor,
                 id,