From ce747f29f5d284857af3d7e3f1e0b8204f58c638 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 30 Aug 2026 21:22:34 +0200 Subject: [PATCH 1/2] perf(runtime): index declared prototype objects by address Replace the linear reverse scan of declared-class prototype objects with a DeclPrototypeTable that owns both the authoritative forward map and its pointer-keyed inverse. All stores, removals, full scans, and incremental GC relocations now update both directions through the same type. If the forward table ever stops being injective, the reverse index is abandoned and lookups fall back to the authoritative scan. Debug builds continuously compare indexed answers with that scan. Add focused table invariants and compiled-program coverage for descriptors, identity, deletion, late materialization, and allocation churn. The integration test builds and selects its matching runtime archive so stale host caches cannot make the result ambiguous. Refs #9180 --- .../9180-decl-prototype-reverse-index.md | 33 ++ .../src/object/class_gc_roots.rs | 15 +- .../src/object/class_registry.rs | 1 + .../class_registry/decl_prototype_table.rs | 358 ++++++++++++++++++ .../src/object/class_registry/gc_roots.rs | 14 +- .../src/object/class_registry/state.rs | 37 +- crates/perry-runtime/src/proxy/metadata.rs | 7 +- ...ssue_9180_decl_prototype_reverse_lookup.rs | 242 ++++++++++++ 8 files changed, 673 insertions(+), 34 deletions(-) create mode 100644 changelog.d/9180-decl-prototype-reverse-index.md create mode 100644 crates/perry-runtime/src/object/class_registry/decl_prototype_table.rs create mode 100644 crates/perry/tests/issue_9180_decl_prototype_reverse_lookup.rs diff --git a/changelog.d/9180-decl-prototype-reverse-index.md b/changelog.d/9180-decl-prototype-reverse-index.md new file mode 100644 index 0000000000..b2cb20391a --- /dev/null +++ b/changelog.d/9180-decl-prototype-reverse-index.md @@ -0,0 +1,33 @@ +**"Which class's `.prototype` is this object?" is now O(1)** (#9180). Every +`Object.defineProperty`, `Object.getOwnPropertyDescriptor` and `delete` asked +`class_id_for_decl_prototype_object` about its receiver, and the answer was a +linear scan of every materialized declared-class prototype. The comment above +it explained that the table was small and the path was a cold reflection path; +a bundled application falsifies both — esbuild's `__export(exports, { … })` +runs `defineProperty` thousands of times during module init, the receivers are +never prototypes, and a miss walked the whole table. It was 3.10% of +`cc --help`. + +The registry now carries its own inverse. `CLASS_DECL_PROTOTYPE_OBJECTS` holds +a `DeclPrototypeTable` whose two maps are private to one file, so the six +existing mutation sites — the store, both GC root scanners, the per-slot GC +step, the test reset and the test seeds — go through methods that update both +directions together, and a seventh cannot be written without editing that file. +That matters more than it sounds: an earlier pointer-keyed cache with +hand-placed invalidation went stale at the sites it missed, and the symptom was +not a crash but `getOwnPropertyDescriptor(C.prototype, "g")` quietly returning +`undefined` where node returns an accessor descriptor. + +Two things keep it honest beyond privacy. The reverse map is an exact inverse +only while one address belongs to one class id and is never re-pointed; +`insert` is where either could break, checks both while inserting with one +extra reverse lookup, and on anything unusual abandons the index for good and +answers from the same authoritative forward-table scan as before. In a +`debug_assertions` build every reverse lookup is compared against that scan, so +the whole runtime test suite is checking the index rather than trusting it. + +Measured on Linux with 400 declared-class prototypes materialized, per +operation on non-prototype receivers: `getOwnPropertyDescriptor` 420 → 200 ns +(2.10×), `defineProperty` 1585 → 1055 ns (1.50×), `delete` 2685 → 2445 ns. The +scan's signature was the slope — `getOwnPropertyDescriptor` cost 180/210/305/420 +ns at 0/50/200/400 prototypes before, and is flat at ~200 ns after. diff --git a/crates/perry-runtime/src/object/class_gc_roots.rs b/crates/perry-runtime/src/object/class_gc_roots.rs index d968c67225..b0a221ac95 100644 --- a/crates/perry-runtime/src/object/class_gc_roots.rs +++ b/crates/perry-runtime/src/object/class_gc_roots.rs @@ -54,9 +54,9 @@ pub fn scan_class_inheritance_roots_mut(visitor: &mut crate::gc::RuntimeRootVisi CLASS_DECL_PROTOTYPE_OBJECTS.with(|table| { if let Ok(mut guard) = table.write() { if let Some(map) = guard.as_mut() { - for ptr in map.values_mut() { + map.visit_root_slots(|ptr| { visitor.visit_usize_slot(ptr); - } + }); } } }); @@ -85,9 +85,10 @@ pub(crate) fn test_seed_class_inheritance_roots(proto_cid: u32, proto_ptr: usize #[cfg(test)] pub(crate) fn test_seed_decl_class_prototype_root(class_id: u32, proto_ptr: usize) { CLASS_DECL_PROTOTYPE_OBJECTS.with(|table| { - let mut guard = table.write().unwrap(); - guard - .get_or_insert_with(std::collections::HashMap::new) + table + .write() + .unwrap() + .get_or_insert_with(Default::default) .insert(class_id, proto_ptr); }); } @@ -122,7 +123,7 @@ pub(crate) fn test_decl_class_prototype_root(class_id: u32) -> usize { .read() .unwrap() .as_ref() - .and_then(|m| m.get(&class_id).copied()) + .and_then(|m| m.get(class_id)) .unwrap_or(0) }) } @@ -148,7 +149,7 @@ pub(crate) fn test_clear_class_inheritance_roots(proto_cid: u32, closure_cid: u3 }); CLASS_DECL_PROTOTYPE_OBJECTS.with(|table| { if let Some(m) = table.write().unwrap().as_mut() { - m.remove(&proto_cid); + m.remove(proto_cid); } }); CLASS_PARENT_CLOSURES.with(|table| { diff --git a/crates/perry-runtime/src/object/class_registry.rs b/crates/perry-runtime/src/object/class_registry.rs index 27e330209c..a50575f88e 100644 --- a/crates/perry-runtime/src/object/class_registry.rs +++ b/crates/perry-runtime/src/object/class_registry.rs @@ -46,6 +46,7 @@ mod builtin_alias_construct; mod class_meta; mod construct; pub(crate) use construct::scan_current_new_target_root_mut; +pub mod decl_prototype_table; mod dispatch; mod function_prototype; mod gc_roots; diff --git a/crates/perry-runtime/src/object/class_registry/decl_prototype_table.rs b/crates/perry-runtime/src/object/class_registry/decl_prototype_table.rs new file mode 100644 index 0000000000..0844667271 --- /dev/null +++ b/crates/perry-runtime/src/object/class_registry/decl_prototype_table.rs @@ -0,0 +1,358 @@ +//! The declared-class `.prototype` registry, with its reverse index built in. +//! +//! # Why this is a type and not a `HashMap` +//! +//! `class_id_for_decl_prototype_object` answers "which declared class's +//! `.prototype` is this heap object?". Its callers +//! (`descriptor_state::disable_inline_guards_for_descriptor_target`, +//! `Object.getOwnPropertyDescriptor`, `delete`, the proxy and +//! `Reflect.metadata` paths) ask it about ARBITRARY objects, so the common +//! answer is "none". Until #9180 that answer cost a linear scan of every +//! materialized declared-class prototype, and on a large application — where +//! esbuild-style `__export(exports, { … })` runs `Object.defineProperty` +//! thousands of times during module init — the scan was 3.10% of `cc --help`. +//! +//! The obvious repair is a pointer-keyed reverse cache invalidated at the +//! writers. That was tried and it was wrong: the table has six mutation sites +//! (the store, the two GC root scanners, the per-slot GC step, the test reset +//! and the test seeds), a missed one leaves the cache stale, and a stale +//! reverse lookup does not crash — it silently reports "not a prototype", so +//! `getOwnPropertyDescriptor(C.prototype, "g")` starts returning `undefined` +//! where node returns an accessor descriptor. +//! +//! So the invalidation here is not maintained by diligence. Three properties +//! carry it, in decreasing order of how much they are asked to do. +//! +//! ## 1. Privacy — a seventh writer cannot forget +//! +//! `forward` and `reverse` are private to THIS FILE. No other module can name +//! them, so no other module can insert, remove, or rewrite an address; the +//! only ways in are the handful of methods below, and each updates both +//! directions in one statement sequence. Adding a mutation site now means +//! editing this file. +//! +//! ## 2. The fallback is the code that was already correct +//! +//! A targeted `reverse` update is an exact inverse only while `forward` is +//! *injective* — one prototype address per class id, never re-pointed. That +//! is how the registry is really built (every entry is a fresh +//! `js_object_alloc`, stored once, and afterwards only ever relocated by the +//! collector), but it is a precondition, not a theorem, and a precondition +//! that quietly stopped holding is exactly the failure mode above: with two +//! class ids on one address, retiring one entry drops the other's reverse +//! key and its lookups start missing. +//! +//! `insert` is the only place either half of that precondition can break, so it +//! checks both while inserting (with one extra reverse lookup), and when a +//! check trips it gives the index up for good. In that mode `class_id_for` +//! falls back to `scan_class_id_for`, the same authoritative forward-table +//! scan used before #9180. +//! +//! ## 3. The index is checked against its own ground truth +//! +//! In a `debug_assertions` build — i.e. throughout the runtime test suite — +//! every `class_id_for` compares the index against that same linear scan, and +//! every mutation re-checks the whole invariant. A mutation added here that +//! forgets `reverse` fails a test rather than shipping a wrong descriptor. +//! +//! # Why the GC cannot invalidate it either +//! +//! Evacuation rewrites the stored addresses through `visit_usize_slot`. Both +//! visit helpers below capture the slot's value, hand the slot to the +//! visitor, and re-key `reverse` from what the visitor left behind — so a +//! move updates both maps or neither. + +use crate::fast_hash::PtrHashMap; + +/// `class_id → *mut ObjectHeader` for materialized declared-class +/// prototypes, plus the pointer-keyed inverse. +#[derive(Default)] +pub struct DeclPrototypeTable { + /// The authoritative direction. Values are raw addresses (`usize` so the + /// table stays `Send + Sync`); they are GC roots, visited and rewritten + /// by the scanners below. + forward: PtrHashMap, + /// Exact inverse of `forward` while `reverse_index_abandoned` is false. + /// Never consulted for anything `forward` does not already say. + reverse: PtrHashMap, + /// Sticky: set when [`Self::insert`] sees something that would make + /// `reverse` less than an exact inverse (a second class id claiming an + /// address, or a class id re-pointed at a different object). From then on + /// the reverse lookup falls back to the linear scan, which is always + /// right. Never observed in practice — see the module docs. + reverse_index_abandoned: bool, +} + +impl DeclPrototypeTable { + /// Register `class_id`'s prototype object. + pub(crate) fn insert(&mut self, class_id: u32, ptr: usize) { + // Both halves of the injectivity precondition, checked in O(1) + // BEFORE the forward map changes. + let address_already_owned = + matches!(self.reverse.get(&ptr), Some(&owner) if owner != class_id); + let previous = self.forward.insert(class_id, ptr); + let class_was_re_pointed = previous.is_some_and(|previous| previous != ptr); + + if address_already_owned || class_was_re_pointed { + self.abandon_reverse_index(); + return; + } + if !self.reverse_index_abandoned { + self.reverse.insert(ptr, class_id); + } + self.debug_assert_consistent(); + } + + /// Forward lookup: the prototype object registered for `class_id`. + #[inline] + pub(crate) fn get(&self, class_id: u32) -> Option { + self.forward.get(&class_id).copied() + } + + /// Reverse lookup: the declared class whose `.prototype` is `ptr`. + /// O(1) — this is the whole point of the type. + #[inline] + pub(crate) fn class_id_for(&self, ptr: usize) -> Option { + if self.reverse_index_abandoned { + return self.scan_class_id_for(ptr); + } + let answer = self.reverse.get(&ptr).copied(); + debug_assert_eq!( + answer, + self.scan_class_id_for(ptr), + "decl-prototype reverse index disagrees with the forward table" + ); + answer + } + + /// Every registered class id. Read-only by construction (`u32` copies), + /// so a caller cannot reach the addresses through it. + pub(crate) fn class_ids(&self) -> impl Iterator + '_ { + self.forward.keys().copied() + } + + /// Drop `class_id`'s registration. Test-support only today; kept next to + /// `insert` so the pair is read together. + #[cfg(test)] + pub(crate) fn remove(&mut self, class_id: u32) { + if let Some(previous) = self.forward.remove(&class_id) { + if self.reverse.get(&previous) == Some(&class_id) { + self.reverse.remove(&previous); + } + } + self.debug_assert_consistent(); + } + + /// Hand every root slot to a GC visitor, then re-key `reverse` for any + /// address the visitor rewrote (evacuation). + pub(crate) fn visit_root_slots(&mut self, mut visit: impl FnMut(&mut usize)) { + let mut moved = false; + for slot in self.forward.values_mut() { + let before = *slot; + visit(slot); + moved |= *slot != before; + } + if moved && !self.reverse_index_abandoned { + self.rebuild_reverse(); + } + self.debug_assert_consistent(); + } + + /// Single-slot twin of [`Self::visit_root_slots`], for the step-wise + /// (cycle-based) root machine that visits one recorded class id at a time. + pub(crate) fn visit_root_slot_for(&mut self, class_id: u32, mut visit: impl FnMut(&mut usize)) { + let Some(slot) = self.forward.get_mut(&class_id) else { + return; + }; + let before = *slot; + visit(slot); + let after = *slot; + if after != before && !self.reverse_index_abandoned { + // Injective `forward` (see the module docs) means `before` was + // this class id's address and nobody else's, so the retarget is + // exact; if it were not, `reverse_index_abandoned` would already + // be set and this arm unreachable. + if self.reverse.get(&before) == Some(&class_id) { + self.reverse.remove(&before); + } + self.reverse.insert(after, class_id); + } + self.debug_assert_consistent(); + } + + /// Give up the O(1) index for the life of the table and answer every + /// future reverse lookup with the linear scan instead. + fn abandon_reverse_index(&mut self) { + self.reverse_index_abandoned = true; + self.reverse.clear(); + self.reverse.shrink_to_fit(); + } + + fn rebuild_reverse(&mut self) { + self.reverse.clear(); + for (&class_id, &ptr) in self.forward.iter() { + self.reverse.insert(ptr, class_id); + } + } + + /// Ground truth: what the pre-#9180 linear scan answered, and what this + /// table still answers once the index is abandoned. Also the oracle the + /// `debug_assert_eq!` in [`Self::class_id_for`] checks the index against. + /// The assertion is compiled out of release builds, so it costs nothing + /// there. + fn scan_class_id_for(&self, ptr: usize) -> Option { + self.forward + .iter() + .find(|(_, &candidate)| candidate == ptr) + .map(|(&class_id, _)| class_id) + } + + #[cfg(debug_assertions)] + fn debug_assert_consistent(&self) { + if self.reverse_index_abandoned { + debug_assert!( + self.reverse.is_empty(), + "abandoned decl-prototype index must not be consulted" + ); + return; + } + debug_assert_eq!( + self.forward.len(), + self.reverse.len(), + "decl-prototype reverse index lost or gained an entry" + ); + for (&class_id, &ptr) in self.forward.iter() { + debug_assert_eq!( + self.reverse.get(&ptr).copied(), + Some(class_id), + "decl-prototype reverse index is missing {ptr:#x} → {class_id}" + ); + } + } + + #[cfg(not(debug_assertions))] + #[inline(always)] + fn debug_assert_consistent(&self) {} + + #[cfg(test)] + fn is_abandoned(&self) -> bool { + self.reverse_index_abandoned + } +} + +#[cfg(test)] +mod tests { + use super::DeclPrototypeTable; + + #[test] + fn reverse_answers_what_the_linear_scan_answered() { + let mut table = DeclPrototypeTable::default(); + table.insert(7, 0x1000); + table.insert(9, 0x2000); + assert!(!table.is_abandoned()); + assert_eq!(table.class_id_for(0x1000), Some(7)); + assert_eq!(table.class_id_for(0x2000), Some(9)); + assert_eq!(table.class_id_for(0x3000), None); + assert_eq!(table.class_id_for(0), None); + assert_eq!(table.get(7), Some(0x1000)); + assert_eq!(table.get(11), None); + } + + #[test] + fn re_registering_the_same_address_is_not_a_retarget() { + let mut table = DeclPrototypeTable::default(); + table.insert(7, 0x1000); + table.insert(7, 0x1000); + assert!(!table.is_abandoned()); + assert_eq!(table.class_id_for(0x1000), Some(7)); + } + + #[test] + fn removing_a_class_retires_both_directions() { + let mut table = DeclPrototypeTable::default(); + table.insert(7, 0x1000); + table.insert(9, 0x2000); + table.remove(7); + assert_eq!(table.class_id_for(0x1000), None); + assert_eq!(table.get(7), None); + assert_eq!(table.class_id_for(0x2000), Some(9)); + } + + /// The evacuation case the naive pointer-keyed cache got wrong. + #[test] + fn evacuation_rekeys_the_reverse_index() { + let mut table = DeclPrototypeTable::default(); + table.insert(7, 0x1000); + table.insert(9, 0x2000); + table.visit_root_slots(|slot| *slot += 0x10_0000); + assert_eq!(table.class_id_for(0x1000), None); + assert_eq!(table.class_id_for(0x2000), None); + assert_eq!(table.class_id_for(0x10_1000), Some(7)); + assert_eq!(table.class_id_for(0x10_2000), Some(9)); + assert_eq!(table.get(7), Some(0x10_1000)); + } + + #[test] + fn stepwise_evacuation_rekeys_only_the_visited_class() { + let mut table = DeclPrototypeTable::default(); + table.insert(7, 0x1000); + table.insert(9, 0x2000); + table.visit_root_slot_for(7, |slot| *slot = 0x9000); + assert_eq!(table.class_id_for(0x1000), None); + assert_eq!(table.class_id_for(0x9000), Some(7)); + assert_eq!(table.class_id_for(0x2000), Some(9)); + table.visit_root_slot_for(1234, |_| unreachable!("unregistered class visited")); + } + + #[test] + fn a_visit_that_moves_nothing_leaves_the_index_alone() { + let mut table = DeclPrototypeTable::default(); + table.insert(7, 0x1000); + table.visit_root_slots(|_| {}); + table.visit_root_slot_for(7, |_| {}); + assert_eq!(table.class_id_for(0x1000), Some(7)); + let mut ids: Vec<_> = table.class_ids().collect(); + ids.sort_unstable(); + assert_eq!(ids, vec![7]); + } + + /// Two class ids on one address would make a targeted reverse update + /// drop the other one's key. The table gives the index up instead and + /// answers exactly as the pre-#9180 linear scan did — including through + /// a later evacuation, where the abandoned index must stay out of the way. + #[test] + fn a_shared_prototype_address_falls_back_to_the_scan() { + let mut table = DeclPrototypeTable::default(); + table.insert(7, 0x1000); + table.insert(9, 0x1000); + assert!(table.is_abandoned()); + assert!(matches!(table.class_id_for(0x1000), Some(7) | Some(9))); + assert_eq!(table.class_id_for(0x2000), None); + assert_eq!(table.get(7), Some(0x1000)); + assert_eq!(table.get(9), Some(0x1000)); + + table.insert(11, 0x3000); + assert_eq!(table.class_id_for(0x3000), Some(11)); + table.visit_root_slots(|slot| *slot += 0x10_0000); + assert_eq!(table.class_id_for(0x3000), None); + assert_eq!(table.class_id_for(0x10_3000), Some(11)); + assert!(matches!(table.class_id_for(0x10_1000), Some(7) | Some(9))); + table.visit_root_slot_for(11, |slot| *slot = 0x4000); + assert_eq!(table.class_id_for(0x4000), Some(11)); + assert_eq!(table.class_id_for(0x10_3000), None); + } + + /// Re-pointing one class at a different object is the other half of the + /// precondition; it degrades the same way rather than silently stranding + /// the old address in `reverse`. + #[test] + fn re_pointing_a_class_falls_back_to_the_scan() { + let mut table = DeclPrototypeTable::default(); + table.insert(7, 0x1000); + table.insert(7, 0x4000); + assert!(table.is_abandoned()); + assert_eq!(table.class_id_for(0x1000), None); + assert_eq!(table.class_id_for(0x4000), Some(7)); + assert_eq!(table.get(7), Some(0x4000)); + } +} diff --git a/crates/perry-runtime/src/object/class_registry/gc_roots.rs b/crates/perry-runtime/src/object/class_registry/gc_roots.rs index 4a4bea8362..b60b7412dd 100644 --- a/crates/perry-runtime/src/object/class_registry/gc_roots.rs +++ b/crates/perry-runtime/src/object/class_registry/gc_roots.rs @@ -119,9 +119,9 @@ pub fn scan_class_side_table_roots_mut(visitor: &mut crate::gc::RuntimeRootVisit CLASS_DECL_PROTOTYPE_OBJECTS.with(|table| { if let Ok(mut guard) = table.write() { if let Some(map) = guard.as_mut() { - for proto_addr in map.values_mut() { + map.visit_root_slots(|proto_addr| { visitor.visit_usize_slot(proto_addr); - } + }); } } }); @@ -264,7 +264,7 @@ fn class_side_table_root_snapshot() -> Vec { CLASS_DECL_PROTOTYPE_OBJECTS.with(|table| { if let Ok(guard) = table.read() { if let Some(map) = guard.as_ref() { - for &class_id in map.keys() { + for class_id in map.class_ids() { slots.push(ClassSideTableRootSlot::DeclPrototypeObject { class_id }); } } @@ -397,8 +397,10 @@ fn scan_class_side_table_root_slot( ClassSideTableRootSlot::DeclPrototypeObject { class_id } => { CLASS_DECL_PROTOTYPE_OBJECTS.with(|table| { if let Ok(mut guard) = table.write() { - if let Some(proto_addr) = guard.as_mut().and_then(|map| map.get_mut(class_id)) { - visitor.visit_usize_slot(proto_addr); + if let Some(map) = guard.as_mut() { + map.visit_root_slot_for(*class_id, |proto_addr| { + visitor.visit_usize_slot(proto_addr); + }); } } }); @@ -749,7 +751,7 @@ pub(crate) fn test_class_decl_prototype_object_root_addr(class_id: u32) -> usize table .read() .ok() - .and_then(|guard| guard.as_ref().and_then(|map| map.get(&class_id).copied())) + .and_then(|guard| guard.as_ref().and_then(|map| map.get(class_id))) .unwrap_or(0) }) } diff --git a/crates/perry-runtime/src/object/class_registry/state.rs b/crates/perry-runtime/src/object/class_registry/state.rs index 7638aa0bcd..01cfd58616 100644 --- a/crates/perry-runtime/src/object/class_registry/state.rs +++ b/crates/perry-runtime/src/object/class_registry/state.rs @@ -1,3 +1,4 @@ +use super::decl_prototype_table::DeclPrototypeTable; use super::*; use crate::object::class_image::{ImageTable, StaticAccessorTable, StaticMethodTable}; use std::collections::HashMap; @@ -326,7 +327,7 @@ crate::perry_thread_local! { /// inheritance shortcuts. Declared class prototypes need stable heap identity /// for `typeof C.prototype`, `Object.getPrototypeOf(new C())`, and /// `C.prototype.isPrototypeOf(instance)` without perturbing those paths. - pub static CLASS_DECL_PROTOTYPE_OBJECTS: RwLock>> = RwLock::new(None); + pub static CLASS_DECL_PROTOTYPE_OBJECTS: RwLock> = RwLock::new(None); } crate::perry_thread_local! { @@ -475,10 +476,9 @@ pub(crate) fn class_decl_prototype_object_root_store(class_id: u32, proto_ptr: * } CLASS_DECL_PROTOTYPE_OBJECTS.with(|table| { let mut guard = table.write().unwrap(); - if guard.is_none() { - *guard = Some(HashMap::new()); - } - guard.as_mut().unwrap().insert(class_id, proto_ptr as usize); + guard + .get_or_insert_with(DeclPrototypeTable::default) + .insert(class_id, proto_ptr as usize); }); crate::gc::runtime_write_barrier_root_raw_ptr(proto_ptr); } @@ -531,22 +531,23 @@ pub(crate) fn parent_closure_in_chain(class_id: u32) -> Option { /// Reverse lookup: which declared class's `.prototype` is this heap object? /// Used by `Object.getOwnPropertyDescriptor(C.prototype, name)` to surface -/// vtable accessors as own properties of the prototype object. Linear scan — -/// the table is small (one entry per materialized declared-class prototype) -/// and this only runs on the reflection slow path. +/// vtable accessors as own properties of the prototype object, and by +/// `descriptor_state::disable_inline_guards_for_descriptor_target` on every +/// `Object.defineProperty`. +/// +/// #9180: this was a linear scan over every materialized declared-class +/// prototype, on the strength of a "the table is small and this is a cold +/// reflection path" comment that a bundled application falsifies twice over +/// — it was 3.10% of `cc --help`. Callers ask about arbitrary objects, so the +/// common case is a MISS, and a miss walked the whole table. +/// [`DeclPrototypeTable`] carries the inverse of the map next to it and keeps +/// the two in step by construction; see that module for why the invalidation +/// is structural rather than enumerated. pub(crate) fn class_id_for_decl_prototype_object(ptr: usize) -> Option { if ptr == 0 { return None; } - CLASS_DECL_PROTOTYPE_OBJECTS.with(|table| { - table - .read() - .ok()? - .as_ref()? - .iter() - .find(|(_, &p)| p == ptr) - .map(|(k, _)| *k) - }) + CLASS_DECL_PROTOTYPE_OBJECTS.with(|table| table.read().ok()?.as_ref()?.class_id_for(ptr)) } /// #7757: a monomorphized specialization (`Gen$num`) must present the GENERIC's @@ -572,7 +573,7 @@ pub(crate) fn class_decl_prototype_object(class_id: u32) -> *mut ObjectHeader { CLASS_DECL_PROTOTYPE_OBJECTS.with(|table| { if let Ok(read) = table.read() { if let Some(map) = read.as_ref() { - return map.get(&class_id).copied().unwrap_or(0) as *mut ObjectHeader; + return map.get(class_id).unwrap_or(0) as *mut ObjectHeader; } } std::ptr::null_mut() diff --git a/crates/perry-runtime/src/proxy/metadata.rs b/crates/perry-runtime/src/proxy/metadata.rs index 4e1d58b900..3f2182416d 100644 --- a/crates/perry-runtime/src/proxy/metadata.rs +++ b/crates/perry-runtime/src/proxy/metadata.rs @@ -319,9 +319,10 @@ mod tests { register_test_class(cid); let fake_proto_ptr: usize = 0x1_0000; // arbitrary; only used as a map key crate::object::CLASS_DECL_PROTOTYPE_OBJECTS.with(|table| { - let mut guard = table.write().unwrap(); - guard - .get_or_insert_with(std::collections::HashMap::new) + table + .write() + .unwrap() + .get_or_insert_with(Default::default) .insert(cid, fake_proto_ptr); }); let target = f64::from_bits(POINTER_TAG | (fake_proto_ptr as u64 & POINTER_MASK)); diff --git a/crates/perry/tests/issue_9180_decl_prototype_reverse_lookup.rs b/crates/perry/tests/issue_9180_decl_prototype_reverse_lookup.rs new file mode 100644 index 0000000000..cb35ad1cad --- /dev/null +++ b/crates/perry/tests/issue_9180_decl_prototype_reverse_lookup.rs @@ -0,0 +1,242 @@ +//! Regression test for #9180: the reverse "which declared class's `.prototype` +//! is this heap object?" lookup. +//! +//! The lookup used to be a linear scan of `CLASS_DECL_PROTOTYPE_OBJECTS`; it is +//! now answered from an index maintained alongside that table. The failure mode +//! of getting an index like this wrong is SILENT — it does not crash, it reports +//! "not a prototype", and the class's vtable accessors stop being surfaced as +//! own properties of `C.prototype`. A first attempt at a pointer-keyed cache +//! shipped exactly that: `Object.getOwnPropertyDescriptor(C.prototype, "g")` +//! returned `undefined` where node returns a getter/setter descriptor. +//! +//! So this pins the observable surface rather than the data structure: +//! descriptor shapes for accessor and data properties, `hasOwnProperty`, +//! prototype identity, and `delete` — each of which routes through the reverse +//! lookup — with enough sibling classes materialized that a size-one table +//! cannot make a broken index look right, and repeated across allocation churn +//! because the collector rewrites the very addresses the index is keyed by. + +use std::path::PathBuf; +use std::process::Command; +use std::sync::Once; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn workspace_root() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("../..") + .canonicalize() + .expect("canonicalize workspace root") +} + +fn runtime_dir() -> PathBuf { + static BUILD_RUNTIME: Once = Once::new(); + BUILD_RUNTIME.call_once(|| { + let cargo = std::env::var_os("CARGO").unwrap_or_else(|| "cargo".into()); + let mut command = Command::new(cargo); + command.current_dir(workspace_root()).arg("build"); + if !cfg!(debug_assertions) { + command.arg("--release"); + } + let build = command + .args(["-p", "perry-runtime-static"]) + .output() + .expect("build static runtime archive"); + assert!( + build.status.success(), + "static runtime build failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&build.stdout), + String::from_utf8_lossy(&build.stderr) + ); + }); + + perry_bin() + .parent() + .expect("Perry binary directory") + .to_path_buf() +} + +fn compile_and_run(dir: &std::path::Path, source: &str) -> String { + let entry = dir.join("main.ts"); + let output = dir.join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + + let compile = Command::new(perry_bin()) + .current_dir(dir) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .arg("--no-auto-optimize") + .arg("--no-cache") + .env("PERRY_RUNTIME_DIR", runtime_dir()) + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + + let run = Command::new(&output) + .current_dir(dir) + .output() + .expect("run compiled binary"); + assert!( + run.status.success(), + "compiled binary failed\nstatus: {:?}\nstdout:\n{}\nstderr:\n{}", + run.status, + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + String::from_utf8_lossy(&run.stdout).into_owned() +} + +/// Sibling classes whose prototypes are materialized before the assertions, so +/// the table under test holds more than the one entry being asked about. +const FILLERS: &str = r#" +class F0 { f0() { return 0; } get g0() { return 0; } } +class F1 { f1() { return 1; } get g1() { return 1; } } +class F2 { f2() { return 2; } get g2() { return 2; } } +class F3 { f3() { return 3; } get g3() { return 3; } } +class F4 { f4() { return 4; } get g4() { return 4; } } +class F5 { f5() { return 5; } get g5() { return 5; } } +const FILL: any[] = [ + F0.prototype, F1.prototype, F2.prototype, + F3.prototype, F4.prototype, F5.prototype, +]; +function churn(n: number): number { + let sink = 0; + for (let i = 0; i < n; i++) { + const a: any[] = []; + for (let j = 0; j < 40; j++) a.push({ i, j, s: "x" + j }); + sink += a.length; + } + return sink; +} +"#; + +#[test] +fn class_prototype_accessors_reflect_as_own_descriptors() { + let dir = tempfile::tempdir().expect("tempdir"); + let stdout = compile_and_run( + dir.path(), + &format!( + r#"{FILLERS} +class Base {{ bm() {{ return "bm"; }} }} +class C extends Base {{ + m() {{ return "m"; }} + get g() {{ return "getter-g"; }} + set g(v: string) {{ (this as any)._g = v; }} +}} + +function report(tag: string) {{ + const acc: any = Object.getOwnPropertyDescriptor(C.prototype, "g"); + console.log(tag + ".accessor.present=" + (acc !== undefined)); + console.log(tag + ".accessor.hasGet=" + (acc !== undefined && typeof acc.get === "function")); + console.log(tag + ".accessor.hasSet=" + (acc !== undefined && typeof acc.set === "function")); + console.log(tag + ".accessor.get()=" + (acc !== undefined && acc.get ? acc.get.call({{}}) : "MISSING")); + console.log(tag + ".accessor.hasValue=" + (acc !== undefined && "value" in acc)); + const data: any = Object.getOwnPropertyDescriptor(C.prototype, "m"); + console.log(tag + ".data.present=" + (data !== undefined)); + console.log(tag + ".data.value()=" + (data !== undefined && data.value ? data.value.call({{}}) : "MISSING")); + console.log(tag + ".hasOwn.g=" + Object.prototype.hasOwnProperty.call(C.prototype, "g")); + console.log(tag + ".identity=" + (Object.getPrototypeOf(new C()) === C.prototype)); + console.log(tag + ".isProtoOf=" + C.prototype.isPrototypeOf(new C())); + // A miss must stay a miss: a plain object is not any class's prototype. + console.log(tag + ".plainMiss=" + (Object.getOwnPropertyDescriptor({{ a: 1 }}, "g") === undefined)); +}} + +console.log("fillers=" + FILL.length); +report("fresh"); +churn(2000); +report("afterChurn"); + +// A class whose prototype is first demanded AFTER the churn — the table grows +// again once addresses have already moved. +class Late {{ lm() {{ return "lm"; }} get lg() {{ return "lg"; }} }} +const late: any = Object.getOwnPropertyDescriptor(Late.prototype, "lg"); +console.log("late.hasGet=" + (late !== undefined && typeof late.get === "function")); +churn(1000); +const late2: any = Object.getOwnPropertyDescriptor(Late.prototype, "lg"); +console.log("late.afterChurn.hasGet=" + (late2 !== undefined && typeof late2.get === "function")); +"# + ), + ); + + assert!(stdout.contains("fillers=6"), "stdout:\n{stdout}"); + for tag in ["fresh", "afterChurn"] { + for (line, why) in [ + ( + format!("{tag}.accessor.present=true"), + "accessor descriptor", + ), + (format!("{tag}.accessor.hasGet=true"), "getter"), + (format!("{tag}.accessor.hasSet=true"), "setter"), + (format!("{tag}.accessor.get()=getter-g"), "getter call"), + ( + format!("{tag}.accessor.hasValue=false"), + "accessor is not a data descriptor", + ), + (format!("{tag}.data.present=true"), "method descriptor"), + (format!("{tag}.data.value()=m"), "method call"), + (format!("{tag}.hasOwn.g=true"), "hasOwnProperty"), + (format!("{tag}.identity=true"), "prototype identity"), + (format!("{tag}.isProtoOf=true"), "isPrototypeOf"), + (format!("{tag}.plainMiss=true"), "non-prototype receiver"), + ] { + assert!( + stdout.contains(&line), + "missing {why} ({line})\nstdout:\n{stdout}" + ); + } + } + assert!(stdout.contains("late.hasGet=true"), "stdout:\n{stdout}"); + assert!( + stdout.contains("late.afterChurn.hasGet=true"), + "stdout:\n{stdout}" + ); +} + +#[test] +fn deleting_a_prototype_method_removes_it_from_instances() { + let dir = tempfile::tempdir().expect("tempdir"); + let stdout = compile_and_run( + dir.path(), + &format!( + r#"{FILLERS} +class E {{ em() {{ return "em"; }} get eg() {{ return "eg"; }} }} +const proto: any = E.prototype; +console.log("fillers=" + FILL.length); +console.log("before=" + (typeof new E().em)); +console.log("delete.em=" + (delete proto.em)); +console.log("descAfter=" + (Object.getOwnPropertyDescriptor(proto, "em") === undefined)); +try {{ + (new E() as any).em(); + console.log("call=NO_THROW"); +}} catch (e: any) {{ + console.log("call=throw:" + (e && e.constructor ? e.constructor.name : "?")); +}} +console.log("delete.eg=" + (delete proto.eg)); +console.log("descEgAfter=" + (Object.getOwnPropertyDescriptor(proto, "eg") === undefined)); +churn(1500); +console.log("descAfterChurn=" + (Object.getOwnPropertyDescriptor(proto, "em") === undefined)); +"# + ), + ); + + for line in [ + "before=function", + "delete.em=true", + "descAfter=true", + "call=throw:TypeError", + "delete.eg=true", + "descEgAfter=true", + "descAfterChurn=true", + ] { + assert!(stdout.contains(line), "missing {line}\nstdout:\n{stdout}"); + } +} From 6f5eec83091e187d0b39837e1f3dfbd0a2eaa03a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 30 Aug 2026 23:27:43 +0200 Subject: [PATCH 2/2] docs: key prototype-index changelog to PR #9214 --- ...type-reverse-index.md => 9214-decl-prototype-reverse-index.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{9180-decl-prototype-reverse-index.md => 9214-decl-prototype-reverse-index.md} (100%) diff --git a/changelog.d/9180-decl-prototype-reverse-index.md b/changelog.d/9214-decl-prototype-reverse-index.md similarity index 100% rename from changelog.d/9180-decl-prototype-reverse-index.md rename to changelog.d/9214-decl-prototype-reverse-index.md