From f7bf40aed9f12af519a90b6a29a8fd09c782f413 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 18 Sep 2026 04:00:23 +0200 Subject: [PATCH 1/2] perf(runtime): stop re-deriving the receiver on every dynamic-key read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `o[k]` costs 674 instructions per access — in a loop, with a constant key, on a two-property object — against node's ~4.5. It does not amortize: every iteration pays the full lookup. `prop_read` is the second most common construct in real TypeScript at 258 per 1k lines across a 1.6M-line corpus, and dynamic-key access is how every map-like object and dispatch table reads. Profiled with symbols, three of the costs are the same shape as the array-push work: a helper re-deriving per access something already established. 1. `keys_find_slot_by_bytes` ran `clean_arr_ptr` — allocator-ownership plus forwarding classification — on `descriptor.keys`, 16.2% of the loop. That field 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 the family (`(*record).keys = addr as u64`), and a descriptor whose keys array died is pruned in the same pass. So the pointer read out of a live descriptor already IS the resolved live head. The resolved entry deliberately delegates back to the original for key_count >= KEYS_INDEX_THRESHOLD, where the indexed path owns its own receiver handling. 2. `try_data_get_bytes` asked `is_process_env_ptr` on every access, which reads a thread-local to answer "is this process.env?" — `_tlv_get_addr` was 14.1% of the loop with half of it from here. A sticky process-global latch means the thread-local is touched only once such an object exists. 3. `is_anon_shape_class_id` took an RwLock READ GUARD on every access — 16.1%, the largest single frame left after (1). 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. It is now answered from a lock-free open-addressed mirror. 674.1 -> 546.0 instructions per access, -19.0%, both arms built from this commit's parent. Measured by differencing two probes that differ only in access count, inside each binary, so driver dispatch and code layout cancel before the arms are compared; the bare-loop control reads 0.06 and -0.11, and callback / `new` / object-literal controls are flat to two decimals. The mirror is sound because the set is INSERT-ONLY — the registrar only ever calls `insert`, nothing removes — so an entry once published stays valid for the life of the image and an open-addressed table needs no reclamation scheme. Slot 0 is the empty sentinel and the registrar rejects class id 0. An insert that exhausts its probe run sets an overflow flag, after which a miss no longer proves absence and falls back to the locked set. It lives in the class IMAGE, beside `parent_dense`, NOT in a process-global: `ANON_SHAPE_CLASS_IDS` is an `ImageTable` and every access resolves through `current()`, so a process-global mirror would answer one image's question out of another image's registrations. The process-global version measured -24.3% and was wrong; this one is -19.0% and is not. Two things that did NOT work, recorded so they are not retried: * A sticky "is the anon-shape set empty?" latch measured ZERO. Object literals ARE anon shapes, so the flag is true in essentially every real program and the lock was taken anyway. That is why this is a mirror and not a latch. * Caching the resolved slot against the key was considered and dropped: a cached key pointer is a heap pointer whose address can be recycled, which would produce a WRONG slot rather than a miss, and it would need its own GC root scanner. Removing the re-derivations gets most of it with none of that. --- .../perry-runtime/src/object/class_image.rs | 85 ++++++++++++++++++- .../src/object/class_registry/class_meta.rs | 29 +++++++ .../perry-runtime/src/object/keys_lookup.rs | 64 ++++++++++++++ crates/perry-runtime/src/object/native_get.rs | 10 ++- crates/perry-runtime/src/process/env_misc.rs | 16 ++++ test-files/test_gap_dynamic_key_read_paths.ts | 60 +++++++++++++ 6 files changed, 262 insertions(+), 2 deletions(-) create mode 100644 test-files/test_gap_dynamic_key_read_paths.ts diff --git a/crates/perry-runtime/src/object/class_image.rs b/crates/perry-runtime/src/object/class_image.rs index 33e13bce26..50c3a61ae5 100644 --- a/crates/perry-runtime/src/object/class_image.rs +++ b/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>>, pub(crate) lengths: RwLock>>, pub(crate) anon_shape_class_ids: RwLock>>, + /// 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>, + /// 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 { + 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::*; diff --git a/crates/perry-runtime/src/object/class_registry/class_meta.rs b/crates/perry-runtime/src/object/class_registry/class_meta.rs index cc1692ccd3..62325eb83e 100644 --- a/crates/perry-runtime/src/object/class_registry/class_meta.rs +++ b/crates/perry-runtime/src/object/class_registry/class_meta.rs @@ -590,11 +590,37 @@ pub static ANON_SHAPE_CLASS_IDS: ImageTable>>> = /// 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); diff --git a/crates/perry-runtime/src/object/keys_lookup.rs b/crates/perry-runtime/src/object/keys_lookup.rs index 50e827606c..fcff25538c 100644 --- a/crates/perry-runtime/src/object/keys_lookup.rs +++ b/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 { + 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, diff --git a/crates/perry-runtime/src/object/native_get.rs b/crates/perry-runtime/src/object/native_get.rs index c674092e5b..072b8eb435 100644 --- a/crates/perry-runtime/src/object/native_get.rs +++ b/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, diff --git a/crates/perry-runtime/src/process/env_misc.rs b/crates/perry-runtime/src/process/env_misc.rs index e3023f35b9..681121e159 100644 --- a/crates/perry-runtime/src/process/env_misc.rs +++ b/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 } diff --git a/test-files/test_gap_dynamic_key_read_paths.ts b/test-files/test_gap_dynamic_key_read_paths.ts new file mode 100644 index 0000000000..a0eecfc0b7 --- /dev/null +++ b/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); From 9ff8322eba6a68039f24edb5f6670c346d44337c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 18 Sep 2026 04:01:12 +0200 Subject: [PATCH 2/2] docs: changelog fragment for #10570 --- .../10570-dynamic-key-receiver-requeries.md | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 changelog.d/10570-dynamic-key-receiver-requeries.md diff --git a/changelog.d/10570-dynamic-key-receiver-requeries.md b/changelog.d/10570-dynamic-key-receiver-requeries.md new file mode 100644 index 0000000000..1fd2514dbe --- /dev/null +++ b/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.