perf(runtime): #6759 C3c-r — header-stamped stable shape ids, id-keyed FIELD_CACHE, PIC ABA fix

changelog.d/6803-header-shape-id-c3c-r.md

@@ -0,0 +1 @@
+perf(runtime): #6759 C3c-r — stable shape ids become header-resident for plain objects (stamped into the otherwise-dead `parent_class_id` word, allocated from a process-global range disjoint from class ids and never reused). FIELD_CACHE keys on the stamp so entries survive keys-array grow-reallocs and GC moves, and the property inline cache's miss handler now primes only shape-shared (address-immortal) keys arrays — closing a latent ABA where an owned array's recycled address could satisfy the unvalidated inline compare and read the wrong slot.

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

@@ -381,6 +381,14 @@ pub extern "C" fn js_object_delete_field(
         //    in `shape_slot_lookup` would also catch this lazily; dropping
         //    eagerly keeps the record from serving hash misses meanwhile.
         crate::object::shapes::shape_drop((*obj).keys_array);
+        // #6759 C3c: the compaction changed the layout under the SAME keys
+        //    address, so the stamped shape id no longer describes this
+        //    object. Ids are never reused, so clearing here (plus the
+        //    record drop above) makes every stale id-keyed cache entry a
+        //    permanent miss; the next resolve stamps a fresh id.
+        if (*obj).class_id == 0 && crate::object::shapes::is_shape_id((*obj).parent_class_id) {
+            (*obj).parent_class_id = 0;
+        }
 
         1
     }

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

@@ -1525,6 +1525,18 @@ pub(crate) fn get_field_by_name_object_tail(
             h
         };
         let keys_id = keys as usize;
+        // #6759 C3c: prefer the header-stamped stable shape id as the cache
+        // key — it survives owned grow-reallocs AND GC moves of the keys
+        // array, both of which change `keys_id` and orphaned the entry.
+        // Unstamped objects (stamp 0 / class instances) keep the address
+        // key; either way every hit below re-validates the stored key, so a
+        // colliding or foreign key can only miss, never mis-resolve.
+        let stamp = (*obj).parent_class_id;
+        let shape_key = if (*obj).class_id == 0 && super::super::shapes::is_shape_id(stamp) {
+            stamp as usize
+        } else {
+            keys_id
+        };
 
         // Clamp the keys length to capacity so a bogus/oversized length can't
         // drive the wide-key map build or the linear scan below into unbounded
@@ -1534,15 +1546,15 @@ pub(crate) fn get_field_by_name_object_tail(
 
         // Per-thread inline cache (`state().field_lookup.field_cache`):
         // fixed-size direct-mapped cache (no allocation, no HashMap).
-        // Each entry stores (keys_ptr, key_hash, field_index). Copied-minor
+        // Each entry stores (shape_key, key_hash, field_index). Copied-minor
         // nursery reset can reuse a keys-array address, so cache hits still
         // validate the key slot before returning a field.
         let st = crate::state::state();
-        let cache_idx = (keys_id.wrapping_add(key_hash as usize)) % super::FIELD_CACHE_SIZE;
+        let cache_idx = (shape_key.wrapping_add(key_hash as usize)) % super::FIELD_CACHE_SIZE;
         let cached = {
             let cache = &*st.field_lookup.field_cache.get();
             let entry = cache[cache_idx];
-            if entry.0 == keys_id && entry.1 == key_hash {
+            if entry.0 == shape_key && entry.1 == key_hash {
                 Some(entry.2)
             } else {
                 None
@@ -1631,10 +1643,27 @@ pub(crate) fn get_field_by_name_object_tail(
             // slow-path lookup is what backs `obj[k]` for ≤5-byte
             // keys after a field-cache miss.
             if crate::string::js_string_key_matches(key_val, key) {
-                // Cache this lookup for next time
+                // Cache this lookup for next time. #6759 C3c: stamp the
+                // object's stable shape id (allocating the record on first
+                // touch) and key the entry on it, so the entry survives the
+                // grow-reallocs and GC moves that retire `keys_id`.
                 {
+                    let store_key = if (*obj).class_id == 0 {
+                        let id =
+                            super::super::shapes::shape_id_for_keys_ensure(keys, key_count as u32);
+                        if id != 0 {
+                            (*(obj as *mut ObjectHeader)).parent_class_id = id;
+                            id as usize
+                        } else {
+                            keys_id
+                        }
+                    } else {
+                        keys_id
+                    };
+                    let store_idx =
+                        (store_key.wrapping_add(key_hash as usize)) % super::FIELD_CACHE_SIZE;
                     let cache = &mut *st.field_lookup.field_cache.get();
-                    cache[cache_idx] = (keys_id, key_hash, i as u32);
+                    cache[store_idx] = (store_key, key_hash, i as u32);
                 }
                 if key_count >= WIDE_KEY_INDEX_MIN_KEYS {
                     wide_key_index_note_hit(keys_id, key_bytes, i as u32);

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

@@ -197,6 +197,20 @@ pub(crate) fn is_timer_handle_method_key(key: &[u8]) -> bool {
     )
 }
 
+/// #6759 C3c: is `keys` safe to prime into a per-site PIC cache whose hit
+/// path does an UNVALIDATED compare-and-load? True only for
+/// `GC_FLAG_SHAPE_SHARED` arrays — those are shape-cache-resident
+/// (process-rooted, so the address can never be freed and recycled under a
+/// different shape). Conservative `false` for anything else.
+pub(crate) unsafe fn keys_cacheable_for_pic(keys: *const crate::array::ArrayHeader) -> bool {
+    if (keys as usize) < crate::gc::GC_HEADER_SIZE + 0x1000 {
+        return false;
+    }
+    let gc = (keys as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader;
+    (*gc).obj_type == crate::gc::GC_TYPE_ARRAY
+        && (*gc).gc_flags & crate::gc::GC_FLAG_SHAPE_SHARED != 0
+}
+
 /// Monomorphic inline cache miss handler (issue #51).
 ///
 /// Called when the codegen-emitted shape check (`obj->keys_array == cache[0]`)
@@ -432,8 +446,20 @@ pub extern "C" fn js_object_get_field_ic_miss(
                     // perf-comprehensive's hot loops that path was hit
                     // ~900k times per run (40% inclusive samples per
                     // perfcomp.profile).
-                    (*cache)[0] = keys as i64;
-                    (*cache)[1] = i as i64;
+                    //
+                    // #6759 C3c: only prime the cache for SHAPE-SHARED keys
+                    // arrays (literal shapes, class-keys arrays — both
+                    // shape-cache-resident, process-rooted, address-stable).
+                    // An OWNED keys array can die and have its address
+                    // recycled under a DIFFERENT shape, and this compare is
+                    // the one unvalidated fast path in the system — a stale
+                    // hit reads the wrong slot. Owned/wide receivers are
+                    // served by the validated, shape-id-keyed FIELD_CACHE
+                    // instead.
+                    if keys_cacheable_for_pic(keys) {
+                        (*cache)[0] = keys as i64;
+                        (*cache)[1] = i as i64;
+                    }
                     let field_ptr = (obj as *const u8)
                         .add(std::mem::size_of::<ObjectHeader>() + i * 8)
                         as *const f64;
@@ -739,3 +765,28 @@ pub extern "C" fn js_private_guard(
     }
     obj
 }
+
+#[cfg(test)]
+mod c3c_pic_tests {
+    /// #6759 C3c: the PIC only caches SHAPE-SHARED (process-rooted,
+    /// address-stable) keys arrays; an owned array's address can be
+    /// recycled under a different shape, which the unvalidated PIC hit
+    /// path cannot detect.
+    #[test]
+    fn pic_caches_only_shape_shared_keys() {
+        let _lock = crate::gc::global_side_table_test_lock();
+        unsafe {
+            let keys = crate::array::js_array_alloc(4);
+            assert!(
+                !super::keys_cacheable_for_pic(keys),
+                "a fresh owned keys array must not be PIC-cacheable"
+            );
+            let gc = (keys as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *mut crate::gc::GcHeader;
+            (*gc).gc_flags |= crate::gc::GC_FLAG_SHAPE_SHARED;
+            assert!(
+                super::keys_cacheable_for_pic(keys),
+                "a shape-shared keys array must stay PIC-cacheable"
+            );
+        }
+    }
+}

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

@@ -1656,6 +1656,20 @@ pub(crate) unsafe fn gc_object_meta_slot(user_ptr: usize) -> Option<*mut u64> {
 
 #[inline]
 unsafe fn set_object_keys_array(obj: *mut ObjectHeader, keys_array: *mut ArrayHeader) {
+    // #6759 C3c: a stamped shape id (plain objects reuse the otherwise-dead
+    // `parent_class_id` word) described the OLD keys array — clear it on a
+    // pointer CHANGE so the stamp invariant (`stamp != 0 ⟹ stamp == id of
+    // current keys`) holds; the resolve paths re-stamp on their next
+    // successful lookup. A same-pointer update (in-place append) keeps the
+    // stamp: slots are append-only, existing mappings stay valid — and the
+    // C3a grow-migration keeps the id itself alive across reallocs, so the
+    // fresh stamp after a grow resolves to the SAME id.
+    if (*obj).class_id == 0
+        && (*obj).keys_array != keys_array
+        && shapes::is_shape_id((*obj).parent_class_id)
+    {
+        (*obj).parent_class_id = 0;
+    }
     // GC_STORE_AUDIT(BARRIERED): keys_array pointer field is followed by an object-slot barrier.
     (*obj).keys_array = keys_array;
     crate::gc::runtime_write_barrier_slot(

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

@@ -26,12 +26,12 @@ pub(crate) struct Shape {
     /// incrementally (append-only while shared); shorter ⟹ a delete
     /// compacted it — drop and rebuild on next lookup.
     indexed_len: u32,
-    /// #6759 Phase C3a: stable shape identity, allocated once at record
-    /// birth and preserved by [`shape_keys_grown`] when an owned keys
-    /// array reallocates — the identity consumers re-key on in C3b
-    /// (FIELD_CACHE, typed_feedback exactness) so they stop churning on
+    /// #6759 Phase C3a/C3c: stable shape identity, allocated once at
+    /// record birth and preserved by [`shape_keys_grown`] when an owned
+    /// keys array reallocates. Stamped into a plain object's
+    /// `parent_class_id` header word (dead weight for `class_id == 0`)
+    /// and used as the FIELD_CACHE key, so lookups stop churning on
     /// capacity doublings and GC moves. 0 is never allocated ("no id").
-    #[allow(dead_code)]
     shape_id: u32,
     /// FNV-1a content hash of key bytes → candidate slots (collisions
     /// resolved by the per-hit content validation).
@@ -40,26 +40,71 @@ pub(crate) struct Shape {
 
 pub(crate) struct ShapeTable {
     entries: RefCell<crate::fast_hash::PtrHashMap<usize, Shape>>,
-    /// #6759 Phase C3a: monotonic ShapeId allocator (1-based; 0 = none).
-    /// u32 wrap is theoretical (one id per shape BIRTH, not per object);
-    /// on wrap the allocator skips 0 and collision risk is bounded by the
-    /// validation-on-hit trust model like every other accelerator here.
-    next_id: std::cell::Cell<u32>,
 }
 
 impl ShapeTable {
     pub(crate) fn new() -> Self {
         ShapeTable {
             entries: RefCell::new(crate::fast_hash::new_ptr_hash_map()),
-            next_id: std::cell::Cell::new(1),
         }
     }
+}
+
+/// #6759 C3c: ShapeIds live in their own u32 range, disjoint from every
+/// real class id (user counter tops out far below; builtin reserved
+/// ranges sit at `0x7FFF_FF00..=0x7FFF_FFFF` and `0xFFFF_0000..`), so a
+/// stamp in a plain object's `parent_class_id` can never be mistaken for
+/// inheritance data — and vice versa.
+pub(crate) const SHAPE_ID_BASE: u32 = 0x8000_0000;
+/// Exclusive end of the ShapeId range (2^30 ids ≈ one per shape BIRTH,
+/// unreachable in practice).
+pub(crate) const SHAPE_ID_END: u32 = 0xC000_0000;
+
+/// #6759 C3c: PROCESS-GLOBAL allocator (supersedes the per-thread counter
+/// C3a landed with). Global uniqueness matters because the worker
+/// serializer replays `parent_class_id` verbatim: a deep-copied object's
+/// stamp arriving on another thread must never alias an id that thread
+/// allocated for a different shape. Monotonic — ids are NEVER reused, so
+/// a stale stamp or cache entry can only miss, not falsely hit.
+static SHAPE_ID_NEXT: std::sync::atomic::AtomicU32 =
+    std::sync::atomic::AtomicU32::new(SHAPE_ID_BASE);
+
+#[inline]
+pub(crate) fn is_shape_id(v: u32) -> bool {
+    (SHAPE_ID_BASE..SHAPE_ID_END).contains(&v)
+}
+
+fn alloc_shape_id() -> u32 {
+    use std::sync::atomic::Ordering;
+    let id = SHAPE_ID_NEXT.fetch_add(1, Ordering::Relaxed);
+    if id >= SHAPE_ID_END {
+        // Range exhausted: park the counter (every subsequent call lands
+        // here again, so it can never wrap back into the valid range) and
+        // stop handing out ids — 0 disables the acceleration, never
+        // correctness.
+        SHAPE_ID_NEXT.store(SHAPE_ID_END, Ordering::Relaxed);
+        return 0;
+    }
+    id
+}
 
-    fn alloc_shape_id(&self) -> u32 {
-        let id = self.next_id.get();
-        self.next_id.set(id.wrapping_add(1).max(1));
-        id
+/// #6759 C3c: get-or-create the shape record for `keys` and return its
+/// stable id (0 only if the id range is exhausted). Used by the resolve
+/// paths to stamp a plain object's header after a successful lookup.
+pub(crate) fn shape_id_for_keys_ensure(keys: *const ArrayHeader, key_count: u32) -> u32 {
+    let keys_id = keys as usize;
+    if keys_id == 0 {
+        return 0;
     }
+    let mut entries = crate::state::state().shapes.entries.borrow_mut();
+    entries
+        .entry(keys_id)
+        .or_insert_with(|| Shape {
+            indexed_len: 0,
+            shape_id: alloc_shape_id(),
+            slots: HashMap::with_capacity(key_count as usize),
+        })
+        .shape_id
 }
 
 /// Build (or extend) the slot map for `keys` covering `key_count` keys.
@@ -92,8 +137,7 @@ pub(crate) unsafe fn shape_slot_lookup(
     build: bool,
 ) -> Option<u32> {
     let keys_id = keys as usize;
-    let table = &crate::state::state().shapes;
-    let mut entries = table.entries.borrow_mut();
+    let mut entries = crate::state::state().shapes.entries.borrow_mut();
     let shape = match entries.get_mut(&keys_id) {
         Some(s) => {
             if s.indexed_len > key_count {
@@ -107,10 +151,9 @@ pub(crate) unsafe fn shape_slot_lookup(
             if !build {
                 return None;
             }
-            let shape_id = table.alloc_shape_id();
             entries.entry(keys_id).or_insert(Shape {
                 indexed_len: 0,
-                shape_id,
+                shape_id: alloc_shape_id(),
                 slots: HashMap::with_capacity(key_count as usize),
             })
         }
@@ -248,13 +291,11 @@ pub(crate) fn test_shape_entry_exists(keys_id: usize) -> bool {
 
 #[cfg(test)]
 pub(crate) fn test_seed_shape_entry(keys_id: usize) {
-    let table = &crate::state::state().shapes;
-    let shape_id = table.alloc_shape_id();
-    table.entries.borrow_mut().insert(
+    crate::state::state().shapes.entries.borrow_mut().insert(
         keys_id,
         Shape {
             indexed_len: 0,
-            shape_id,
+            shape_id: alloc_shape_id(),
             slots: HashMap::new(),
         },
     );
@@ -269,3 +310,76 @@ pub(crate) fn test_shape_id_for_keys(keys_id: usize) -> Option<u32> {
         .get(&keys_id)
         .map(|s| s.shape_id)
 }
+
+#[cfg(test)]
+mod c3c_tests {
+    use super::*;
+
+    fn key(name: &str) -> *mut crate::StringHeader {
+        crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32)
+    }
+
+    /// #6759 C3c: ids come from the dedicated range (disjoint from real and
+    /// builtin class ids), are stable per keys identity, and distinct
+    /// across identities.
+    #[test]
+    fn shape_ids_are_range_disjoint_and_stable() {
+        let _lock = crate::gc::global_side_table_test_lock();
+        let a: usize = 0xC3C0_0000_0000_1000;
+        let b: usize = 0xC3C0_0000_0000_2000;
+        let ida = shape_id_for_keys_ensure(a as *const ArrayHeader, 4);
+        let idb = shape_id_for_keys_ensure(b as *const ArrayHeader, 4);
+        assert!(is_shape_id(ida) && is_shape_id(idb));
+        assert_ne!(ida, idb);
+        assert_eq!(shape_id_for_keys_ensure(a as *const ArrayHeader, 4), ida);
+        // Real class-id space must never classify as a shape id.
+        assert!(!is_shape_id(0));
+        assert!(!is_shape_id(1));
+        assert!(!is_shape_id(0x7FFF_FF30));
+        assert!(!is_shape_id(0xFFFF_0005));
+        shape_drop(a as *const ArrayHeader);
+        shape_drop(b as *const ArrayHeader);
+    }
+
+    /// #6759 C3c stamp invariant on a REAL object through the real
+    /// write/read paths: a read resolution stamps a shape id into the
+    /// plain object's `parent_class_id`; after further appends the stamp
+    /// is either cleared (keys pointer changed) or still equal to the
+    /// current keys' id (in-place append / migrated grow).
+    #[test]
+    fn plain_object_stamp_lifecycle() {
+        let _lock = crate::gc::global_side_table_test_lock();
+        unsafe {
+            let obj = crate::object::js_object_alloc(0, 8);
+            for name in ["c3c_a", "c3c_b", "c3c_c"] {
+                crate::object::js_object_set_field_by_name(obj, key(name), 1.0);
+            }
+            assert_eq!((*obj).class_id, 0, "test premise: plain object");
+            let _ = crate::object::js_object_get_field_by_name(obj, key("c3c_b"));
+            let stamp = (*obj).parent_class_id;
+            assert!(
+                is_shape_id(stamp),
+                "read resolution must stamp a shape id, got {stamp:#x}"
+            );
+
+            crate::object::js_object_set_field_by_name(obj, key("c3c_d"), 2.0);
+            crate::object::js_object_set_field_by_name(obj, key("c3c_e"), 3.0);
+            let stamp2 = (*obj).parent_class_id;
+            if stamp2 != 0 {
+                assert!(is_shape_id(stamp2));
+                let cur = shape_id_for_keys_ensure(
+                    (*obj).keys_array,
+                    crate::array::js_array_length((*obj).keys_array),
+                );
+                assert_eq!(
+                    stamp2, cur,
+                    "a surviving stamp must equal the CURRENT keys' id"
+                );
+            }
+
+            // Reads still resolve correctly through the id-keyed cache.
+            let v = crate::object::js_object_get_field_by_name(obj, key("c3c_d"));
+            assert_eq!(f64::from_bits(v.bits()), 2.0);
+        }
+    }
+}