perf(runtime): stop re-deriving the receiver on every dynamic-key read (−19%)

changelog.d/10570-dynamic-key-receiver-requeries.md

@@ -0,0 +1,33 @@
+Stop re-deriving the receiver on every dynamic-key property read. `o[k]` cost
+674 instructions per access — in a loop, with a constant key, on a
+two-property object — against node's ~4.5, and it did not amortize. A
+symbol-resolved profile showed three per-access re-derivations, the same shape
+as the array-push work: 674.1 -> 546.0, **-19.0%**.
+
+`keys_find_slot_by_bytes` ran `clean_arr_ptr` on `descriptor.keys` (16.2% of
+the loop) although that field is maintained by the COLLECTOR —
+`shapes::scan_shape_table_rekey_mut` writes the forwarded address back into
+every descriptor record when the keys array moves, and prunes descriptors whose
+keys array died. `try_data_get_bytes` read a thread-local on every access to
+ask whether the receiver is `process.env`; a sticky latch means that is touched
+only once such an object exists. And `is_anon_shape_class_id` took an `RwLock`
+read guard per access (16.1%, the largest frame left) to consult a set written
+only from module init; it is now answered from a lock-free open-addressed
+mirror, sound because the set is insert-only, and living in the class IMAGE
+beside `parent_dense` because `ImageTable` resolves every access through
+`current()`.
+
+A sticky "is the anon-shape set empty?" latch was tried first and measured
+ZERO — object literals ARE anon shapes, so the flag is true in essentially
+every real program. Caching the resolved slot against the key was dropped: a
+cached key pointer can be recycled into a WRONG slot rather than a miss, and
+would need its own GC root scanner.
+
+New fixture `test_gap_dynamic_key_read_paths.ts` covers the receiver shapes and
+the mutations that must invalidate what the fast path reads — key added and
+deleted after a first read, 64 keys to cross the indexed-lookup threshold,
+declared-class instances, prototype-chain reads, accessors, and the
+`.constructor`/`getPrototypeOf` verdicts the mirror answers. Byte-identical to
+node; two GC-stress seeds with from-space protection and
+PERRY_GC_FROMSPACE_SCAN_ABORT=1 ran 2,117 and 2,450 copying minors with
+dangling=0 and missing_rewrites=0.

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

@@ -73,7 +73,7 @@
 use crate::fast_hash::{PtrHashMap, PtrHashSet};
 use std::cell::OnceCell;
 use std::collections::HashMap;
-use std::sync::atomic::{AtomicU32, Ordering};
+use std::sync::atomic::{AtomicBool, AtomicU32, Ordering};
 use std::sync::{Arc, LockResult, OnceLock, RwLock, RwLockReadGuard, RwLockWriteGuard};
 use std::thread::ThreadId;
 
@@ -150,6 +150,16 @@ pub struct ClassImageTables {
     pub(crate) names: RwLock<Option<PtrHashMap<u32, String>>>,
     pub(crate) lengths: RwLock<Option<PtrHashMap<u32, u32>>>,
     pub(crate) anon_shape_class_ids: RwLock<Option<PtrHashSet<u32>>>,
+    /// Lock-free read mirror of `anon_shape_class_ids`, open-addressed.
+    /// `0` is an empty slot; `js_register_anon_shape_class_id` rejects id 0,
+    /// so the sentinel is unambiguous. Per IMAGE, like `parent_dense` — a
+    /// process-global mirror would answer one image's question out of
+    /// another's registrations, since `ImageTable` resolves every access
+    /// through `current()`.
+    pub(crate) anon_shape_fast: OnceLock<Box<[AtomicU32]>>,
+    /// Set when an insert exhausted its probe run: from then on an empty slot
+    /// no longer proves absence, so a miss must consult the locked set.
+    pub(crate) anon_shape_fast_overflow: AtomicBool,
 }
 
 impl ClassImageTables {
@@ -176,6 +186,8 @@ impl ClassImageTables {
             names: RwLock::new(None),
             lengths: RwLock::new(None),
             anon_shape_class_ids: RwLock::new(None),
+            anon_shape_fast: OnceLock::new(),
+            anon_shape_fast_overflow: AtomicBool::new(false),
         }
     }
 }
@@ -335,6 +347,77 @@ pub(crate) fn parent_dense_store(idx: usize, biased_parent: u32) {
         .store(biased_parent, Ordering::Release);
 }
 
+/// Slot count of the per-image anon-shape mirror. Power of two.
+pub(crate) const ANON_FAST_SLOTS: usize = 4096;
+const ANON_FAST_MASK: usize = ANON_FAST_SLOTS - 1;
+const ANON_FAST_MAX_PROBE: usize = 8;
+
+#[inline]
+fn anon_fast_index(class_id: u32) -> usize {
+    // splitmix32 finaliser: anon ids come from a counter, so the low bits
+    // alone would cluster one module's ids into a single probe run.
+    let mut z = class_id ^ 0x9E37_79B9;
+    z = (z ^ (z >> 16)).wrapping_mul(0x85EB_CA6B);
+    z = (z ^ (z >> 13)).wrapping_mul(0xC2B2_AE35);
+    ((z ^ (z >> 16)) as usize) & ANON_FAST_MASK
+}
+
+#[inline]
+fn anon_fast_table() -> &'static [AtomicU32] {
+    current()
+        .anon_shape_fast
+        .get_or_init(|| (0..ANON_FAST_SLOTS).map(|_| AtomicU32::new(0)).collect())
+}
+
+/// `Some(verdict)` when the calling image's mirror can answer; `None` when the
+/// caller must fall back to the locked set.
+#[inline]
+pub(crate) fn anon_fast_lookup(class_id: u32) -> Option<bool> {
+    let table = anon_fast_table();
+    let mut i = anon_fast_index(class_id);
+    for _ in 0..ANON_FAST_MAX_PROBE {
+        let v = table[i].load(Ordering::Acquire);
+        if v == class_id {
+            return Some(true);
+        }
+        if v == 0 {
+            return if current().anon_shape_fast_overflow.load(Ordering::Acquire) {
+                None
+            } else {
+                Some(false)
+            };
+        }
+        i = (i + 1) & ANON_FAST_MASK;
+    }
+    None
+}
+
+/// Publish `class_id` into the calling image's mirror. Idempotent.
+pub(crate) fn anon_fast_insert(class_id: u32) {
+    let table = anon_fast_table();
+    let mut i = anon_fast_index(class_id);
+    for _ in 0..ANON_FAST_MAX_PROBE {
+        let v = table[i].load(Ordering::Acquire);
+        if v == class_id {
+            return;
+        }
+        if v == 0
+            && table[i]
+                .compare_exchange(0, class_id, Ordering::AcqRel, Ordering::Acquire)
+                .is_ok()
+        {
+            return;
+        }
+        if table[i].load(Ordering::Acquire) == class_id {
+            return;
+        }
+        i = (i + 1) & ANON_FAST_MASK;
+    }
+    current()
+        .anon_shape_fast_overflow
+        .store(true, Ordering::Release);
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

crates/perry-runtime/src/object/class_registry/class_meta.rs

@@ -590,11 +590,37 @@ pub static ANON_SHAPE_CLASS_IDS: ImageTable<RwLock<Option<PtrHashSet<u32>>>> =
 /// Mark `class_id` as a synthetic anon-shape class so `.constructor`
 /// reads on instances of that class return the global `Object`
 /// constructor rather than the synthetic class ref.
+/// The lock-free read mirror lives in the class IMAGE
+/// (`class_image::anon_fast_lookup` / `anon_fast_insert`), next to
+/// `parent_dense`, because `ANON_SHAPE_CLASS_IDS` is an `ImageTable` and every
+/// access resolves through `current()`. A process-global mirror would answer
+/// one image's question out of another image's registrations.
+///
+/// Why a mirror at all: `is_anon_shape_class_id` is asked once per dynamic
+/// property read from `native_get::try_data_get_bytes`, and taking an `RwLock`
+/// read guard to do it was 16.1% of an `o[k]` loop — the largest single frame
+/// left in that profile. A guard is an atomic read-modify-write on a lock word
+/// shared by every thread in the image, paid to consult a set written only
+/// from module init.
+///
+/// A sticky "is the set empty" flag does NOT work here, and that was measured
+/// before this was written: object literals ARE anon shapes, so such a flag is
+/// true in essentially every real program and the lock is taken anyway.
+///
+/// The set is INSERT-ONLY — the registrar below only ever calls `insert`, and
+/// nothing removes — which is what makes an open-addressed mirror sound with
+/// no reclamation scheme: an entry, once published, stays valid for the life
+/// of the image.
 #[no_mangle]
 pub unsafe extern "C" fn js_register_anon_shape_class_id(class_id: u32) {
     if class_id == 0 {
         return;
     }
+    // Mirror FIRST: a reader that sees the id here is right, and one that does
+    // not yet see it falls through to the locked set below, which the write
+    // guard is about to update. The reverse order would let a reader miss in
+    // both.
+    super::super::class_image::anon_fast_insert(class_id);
     let mut guard = ANON_SHAPE_CLASS_IDS.write().unwrap();
     if guard.is_none() {
         *guard = Some(new_ptr_hash_set());
@@ -629,6 +655,9 @@ pub fn is_anon_shape_class_id(class_id: u32) -> bool {
     if class_id == 0 {
         return false;
     }
+    if let Some(verdict) = super::super::class_image::anon_fast_lookup(class_id) {
+        return verdict;
+    }
     if let Ok(guard) = ANON_SHAPE_CLASS_IDS.read() {
         if let Some(set) = guard.as_ref() {
             return set.contains(&class_id);

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

@@ -111,6 +111,70 @@ pub(crate) unsafe fn keys_find_slot_by_bytes(
     None
 }
 
+/// [`keys_array_dense_slots`] for a keys array the caller read out of a LIVE
+/// `ShapeDescriptor`.
+///
+/// `descriptor.keys` is maintained by the COLLECTOR. When the keys array
+/// moves, `shapes::scan_shape_table_rekey_mut` writes the forwarded address
+/// back into every descriptor record in that family —
+/// `unsafe { (*record).keys = addr as u64 }` — and a descriptor whose keys
+/// array died is pruned in the same pass (`shape_keys_address_is_recycled`).
+/// So the pointer read out of a live descriptor already IS the resolved live
+/// head, and `clean_arr_ptr` on it re-derives a guarantee the collector has
+/// already made.
+///
+/// Measured: `keys_array_dense_slots` was 16.2% of an `o[k]` read loop, and
+/// `clean_arr_ptr` is what it spends that on.
+///
+/// # Safety
+///
+/// `keys` must be `ShapeDescriptor::keys` from a descriptor read on this same
+/// straight-line path, with no allocation or safepoint since that read.
+#[inline]
+pub(crate) unsafe fn keys_array_dense_slots_resolved(
+    keys: *const crate::array::ArrayHeader,
+) -> (*const f64, usize) {
+    if keys.is_null() {
+        return (std::ptr::null(), 0);
+    }
+    let len = (*keys).length.min((*keys).capacity) as usize;
+    (crate::array::array_elements_ptr(keys) as *const f64, len)
+}
+
+/// [`keys_find_slot_by_bytes`] for a keys array obtained from a live
+/// descriptor — see [`keys_array_dense_slots_resolved`] for why the receiver
+/// needs no second resolution.
+///
+/// # Safety
+///
+/// As [`keys_array_dense_slots_resolved`].
+pub(crate) unsafe fn keys_find_slot_by_bytes_resolved(
+    keys: *const crate::array::ArrayHeader,
+    key_count: u32,
+    key_bytes: &[u8],
+) -> Option<u32> {
+    if key_count >= KEYS_INDEX_THRESHOLD {
+        // The indexed path owns its own receiver handling; hand it the
+        // unresolved entry so its behaviour is bit-for-bit what it was.
+        return keys_find_slot_by_bytes(keys, key_count, key_bytes);
+    }
+    let (slots, slot_len) = keys_array_dense_slots_resolved(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,

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

@@ -108,7 +108,15 @@ pub(crate) unsafe fn try_data_get_bytes(receiver: JSValue, key: &[u8]) -> Option
             let keys = descriptor.keys as usize as *const crate::array::ArrayHeader;
             if !keys.is_null() {
                 if let Some(slot) =
-                    super::keys_find_slot_by_bytes(keys, descriptor.logical_key_count, key)
+                    // `keys` came straight out of `descriptor` above with no
+                    // allocation in between, and the collector maintains that
+                    // field — so the resolved entry skips a `clean_arr_ptr`
+                    // that re-derives it.
+                    super::keys_find_slot_by_bytes_resolved(
+                        keys,
+                        descriptor.logical_key_count,
+                        key,
+                    )
                 {
                     let value = super::field_get_set::object_field_at_with_live(
                         object,

crates/perry-runtime/src/process/env_misc.rs

@@ -1317,7 +1317,22 @@ pub fn is_process_env_object(value: f64) -> bool {
 ///
 /// The pointer form of [`is_process_env_object`], for call sites that have
 /// already unboxed the target (`Object.assign`'s write funnel).
+/// Sticky: has a `process.env` object ever been materialised in this process?
+/// See [`is_process_env_ptr`].
+static ANY_PROCESS_ENV_OBJECT: std::sync::atomic::AtomicBool =
+    std::sync::atomic::AtomicBool::new(false);
+
 pub fn is_process_env_ptr(addr: usize) -> bool {
+    // Asked once per dynamic property read, from `native_get::try_data_get_bytes`.
+    // Reading `CACHED_ENV` is a thread-local access — `_tlv_get_addr` was 14.1%
+    // of an `o[k]` loop with half of it from here — and in a program that never
+    // materialises `process.env` the answer is always `false`. The latch is set
+    // the moment such an object is created, so the TLS is touched only once one
+    // exists. Monotone and never cleared: a stale `true` costs the old TLS read,
+    // never a wrong answer. `AtomicBool` holds no heap pointer, so not a GC root.
+    if !ANY_PROCESS_ENV_OBJECT.load(std::sync::atomic::Ordering::Acquire) {
+        return false;
+    }
     let cached = CACHED_ENV.with(|c| c.get());
     if cached == 0.0 {
         return false;
@@ -1429,6 +1444,7 @@ fn js_process_env_impl() -> f64 {
     }
     let boxed = f64::from_bits(JSValue::pointer(obj as *const u8).bits());
     CACHED_ENV.with(|c| c.set(boxed));
+    ANY_PROCESS_ENV_OBJECT.store(true, std::sync::atomic::Ordering::Release);
     boxed
 }
 

test-files/test_gap_dynamic_key_read_paths.ts

@@ -0,0 +1,60 @@
+// Dynamic-key property reads, `o[k]`, across the receiver shapes the fast path
+// in `native_get::try_data_get_bytes` classifies — and the mutations that must
+// invalidate what it reads.
+//
+// Three per-access re-derivations were removed behind this: the keys array was
+// re-resolved through `clean_arr_ptr` although the collector maintains
+// `ShapeDescriptor::keys`; a thread-local was read to answer "is this
+// process.env?"; and an RwLock read guard was taken to ask whether the
+// receiver's class id is an anon shape. The last is answered from a per-IMAGE
+// lock-free mirror, so anything that depends on the anon-shape verdict
+// (`.constructor`, `Object.getPrototypeOf`) is exercised here too.
+const plain: any = { a: 1, b: "two", c: true };
+for (const k of ["a", "b", "c", "missing"]) console.log("plain", k, String(plain[k]));
+
+// Anon shape: an object literal's `.constructor` must still be Object, which
+// is the verdict the mirror now answers.
+console.log("ctor is Object:", plain.constructor === Object);
+console.log("proto is Object.prototype:", Object.getPrototypeOf(plain) === Object.prototype);
+
+// Key added after first read: the shape changes, so the cached keys array must
+// not be reused.
+const grown: any = { x: 1 };
+console.log("before add", String(grown.y));
+grown.y = 42;
+console.log("after add", String(grown.y));
+
+// Key deleted after first read.
+const shrunk: any = { p: 1, q: 2 };
+console.log("before delete", String(shrunk.q));
+delete shrunk.q;
+console.log("after delete", String(shrunk.q));
+
+// Many keys, to cross the indexed-lookup threshold the resolved entry
+// deliberately delegates back to the original path for.
+const wide: any = {};
+for (let i = 0; i < 64; i++) wide["k" + i] = i;
+let sum = 0;
+for (let i = 0; i < 64; i++) sum += wide["k" + i];
+console.log("wide sum", sum);
+
+// A declared class instance is NOT an anon shape.
+class Holder { v: number; constructor(v: number) { this.v = v; } }
+const inst: any = new Holder(7);
+console.log("class read", inst["v"], "ctor", inst.constructor === Holder);
+
+// A prototype-chain read through a dynamic key.
+const parent: any = { inherited: "yes" };
+const child: any = Object.create(parent);
+child.own = "mine";
+console.log("own", child["own"], "inherited", child["inherited"]);
+
+// Accessors must not be answered from the data fast path.
+const acc: any = { get computed() { return "from-getter"; } };
+console.log("accessor", acc["computed"]);
+
+// Hot loop over one key, which is the shape the removals were measured on.
+const hot: any = { k: 1.5, other: 2 };
+let t = 0;
+for (let i = 0; i < 500; i++) t += hot["k"];
+console.log("hot total", t);