fix(runtime): birth-stamp the class allocators #8009 left lazy, and gate the split population

changelog.d/8010-class-shape-uniformity-gate.md

@@ -0,0 +1,38 @@
+### Birth-stamp the class allocators #8009 left lazy, and gate the split population (#7983)
+
+#8009 (C3 rung 2) stamps a class instance's ShapeId at birth on the **compiled**
+path. Three other class-instance allocators were left on rung 1's lazy
+self-heal — `js_object_alloc_class_with_keys`,
+`js_object_alloc_class_dynamic_parent`, and the `js_object_alloc_class_inline_keys`
+compatibility entry point — so for any class reaching one of them the shape's
+population is still split between stamped and newborn receivers.
+
+That is not a slow start. The emitted read PIC derives its entire cache token
+from the header shape word, so a stamped receiver and a newborn one of the same
+shape compute two different tokens and the site's hit rate is **0% forever**:
+instance #1 misses, is stamped, primes the id token; instance #2 is newborn,
+computes the keys pointer, misses; the handler re-primes the same id; instance
+#3 misses.
+
+This is the defect bisected to `4784d5da7` (#7983). On instructions retired,
+isolated against its own parent: `cycles` +54.3%, `deeplist` +45.2%, `interp`
++28.3%, `pipeline` +23.9%, `iso_miss` +22.9% — while the object-literal
+benchmarks (`churn` +1.2%, `retain` +0.2%) and `fib40` (+0.04%) did not move,
+literals having been birth-stamped since #6804. Isolated further on one program
+and one build: a read pass over 3,000,000 newborn class instances costs 43.6
+instructions per read, a second pass over the same now-stamped instances 15.5.
+
+All three allocators now stamp at birth. The two shape-cached ones read the id
+out of the `ShapeCacheEntry::runtime_shape_id` their existing probe already
+returns; the compatibility entry point mints from its canonical keys array, off
+the compiled hot path.
+
+The gate, `a_fresh_class_instance_computes_the_token_the_miss_handler_primed`,
+asserts the token the miss handler PRIMES equals the token a freshly-allocated
+sibling COMPUTES, comparing against the emitted IR's formula transcribed into
+the test. It fails on `main` as of #8009 and passes here. "A newborn carries a
+stamp" is a presence check that both-stamped and both-unstamped each satisfy —
+only the mixture is the bug, and only a test comparing the two sides can see it.
+This one passes in either uniform state, so it survives a future policy flip.
+
+Working notes, including the full bisect and the two dead ends: `gc-handoff/BISECT-NOTES.md`.

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

@@ -296,16 +296,34 @@ fn object_alloc_class_inline_keys_impl(
 }
 
 /// Compatibility entry point for runtime callers that do not have a
-/// module-init ShapeId. Their first by-name resolve retains rung 1's lazy
-/// self-heal; compiled allocations use the stamped entry point below.
+/// module-init ShapeId.
+///
+/// It mints the id from the canonical keys array instead of receiving it, so
+/// the instance is still stamped AT BIRTH. Leaving it to rung 1's lazy
+/// self-heal would split this class's population between stamped and newborn
+/// receivers, which the emitted PIC cannot tolerate — see
+/// `shapes::birth_stamp_object_shape`. The mint is one shape-table probe and
+/// this is not the compiled hot path (compiled `new C(…)` sites call
+/// `js_object_alloc_class_inline_keys_stamped` with a module-init id).
 #[no_mangle]
 pub extern "C" fn js_object_alloc_class_inline_keys(
     class_id: u32,
     parent_class_id: u32,
     field_count: u32,
     keys_array: *mut ArrayHeader,
 ) -> *mut ObjectHeader {
-    object_alloc_class_inline_keys_impl(class_id, parent_class_id, field_count, keys_array)
+    let ptr =
+        object_alloc_class_inline_keys_impl(class_id, parent_class_id, field_count, keys_array);
+    if !keys_array.is_null() {
+        unsafe {
+            let id = crate::object::shapes::shape_id_for_keys_ensure(
+                keys_array as *const ArrayHeader,
+                (*keys_array).length,
+            );
+            crate::object::shapes::birth_stamp_object_shape(ptr, id);
+        }
+    }
+    ptr
 }
 
 /// The compiled-class allocation entry point after #6759 C3 rung 2.
@@ -462,9 +480,9 @@ pub extern "C" fn js_object_alloc_class_with_keys(
         .wrapping_mul(10007)
         .wrapping_add(field_count.wrapping_mul(100003))
         .wrapping_add(1000000);
-    let cached = shape_cache_get(shape_id);
-    let keys_arr = if !cached.is_null() {
-        cached
+    let (cached, cached_runtime_id) = shape_cache_get_with_id(shape_id);
+    let (keys_arr, runtime_shape_id) = if !cached.is_null() {
+        (cached, cached_runtime_id)
     } else {
         let keys_bytes =
             unsafe { std::slice::from_raw_parts(packed_keys, packed_keys_len as usize) };
@@ -492,11 +510,18 @@ pub extern "C" fn js_object_alloc_class_with_keys(
             }
         }
         shape_cache_insert(shape_id, arr);
-        arr
+        (arr, shape_cache_get_with_id(shape_id).1)
     };
 
     unsafe {
         set_object_keys_array(ptr, keys_arr);
+        // #6759 C3 rung 2, completed: birth-stamp here too. #8009 stamped the
+        // COMPILED entry point (`js_object_alloc_class_inline_keys_stamped`)
+        // and left this one lazily self-healing, which is a SPLIT population
+        // for every class that lands here — and a split population is a
+        // permanent PIC miss, not a slow start. See
+        // `shapes::birth_stamp_object_shape`.
+        crate::object::shapes::birth_stamp_object_shape(ptr, runtime_shape_id);
     }
     remember_class_keys_array(class_id, field_count, keys_arr);
     ptr
@@ -559,9 +584,9 @@ pub extern "C" fn js_object_alloc_class_dynamic_parent(
     // from the own-only shape (`+ 2_000_000`) so it can't collide with the
     // `js_build_class_keys_array` / `js_object_alloc_class_with_keys` shapes.
     let shape_id = class_id.wrapping_mul(10007).wrapping_add(2_000_000);
-    let cached = shape_cache_get(shape_id);
-    let (merged_arr, field_count) = if !cached.is_null() {
-        (cached, unsafe { (*cached).length })
+    let (cached, cached_runtime_id) = shape_cache_get_with_id(shape_id);
+    let (merged_arr, field_count, runtime_shape_id) = if !cached.is_null() {
+        (cached, unsafe { (*cached).length }, cached_runtime_id)
     } else {
         let own_keys: Vec<&[u8]> = if own_packed_keys.is_null() || own_packed_keys_len == 0 {
             Vec::new()
@@ -599,7 +624,7 @@ pub extern "C" fn js_object_alloc_class_dynamic_parent(
             }
         }
         shape_cache_insert(shape_id, arr);
-        (arr, merged_len as u32)
+        (arr, merged_len as u32, shape_cache_get_with_id(shape_id).1)
     };
 
     let header_size = std::mem::size_of::<ObjectHeader>();
@@ -621,6 +646,9 @@ pub extern "C" fn js_object_alloc_class_dynamic_parent(
         }
         set_object_keys_array(ptr, merged_arr);
         crate::gc::layout_init_pointer_free(ptr as *mut u8);
+        // The dynamically-parented subclass shape needs the same birth stamp
+        // as every other class instance, or its sites split the same way.
+        crate::object::shapes::birth_stamp_object_shape(ptr, runtime_shape_id);
     }
     remember_class_keys_array(class_id, field_count, merged_arr);
     ptr

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

@@ -724,20 +724,23 @@ mod shape_transition_tests_6759 {
             }
             let keys_before = (*obj).keys_array;
             assert_eq!((*obj).class_id, CID, "test premise: a class instance");
-            assert!(
-                !is_shape_id((*obj).parent_class_id),
-                "test premise: a FRESH instance carries its allocation-time \
-                 parent_class_id — rung 1's stamp is lazy, not a birth stamp"
-            );
-
-            // Lazy stamp: the first by-name resolve installs a ShapeId over the
-            // (now readerless — rung 0/#7981) inheritance word.
-            let _ = crate::object::js_object_get_field_by_name(obj, key("del6759_y"));
             let before = (*obj).parent_class_id;
             assert!(
                 is_shape_id(before),
-                "a class instance was NOT stamped by its first by-name resolve \
-                 (got {before:#x}) — rung 1's whole subject is inert"
+                "test premise: a class instance is stamped AT BIRTH (got \
+                 {before:#x}). Rung 2 (#8009 for the compiled path, and the \
+                 runtime allocators alongside it) exists because a LAZY stamp \
+                 splits the shape's population — see \
+                 `shapes::birth_stamp_object_shape`"
+            );
+            // A by-name resolve must not change it: the birth stamp is already
+            // the id every later resolve would have minted.
+            let _ = crate::object::js_object_get_field_by_name(obj, key("del6759_y"));
+            assert_eq!(
+                (*obj).parent_class_id,
+                before,
+                "a resolve re-stamped an already-stamped instance with a \
+                 DIFFERENT id — every site holding the birth token would miss"
             );
 
             assert_eq!(js_object_delete_field(obj, key("del6759_x")), 1);
@@ -857,10 +860,14 @@ mod shape_transition_tests_6759 {
             // prelude instead); MID→BASE has no instance here, so register it
             // the way that prelude does.
             crate::object::register_class(MID, BASE);
-            assert_eq!(
-                (*leaf).parent_class_id,
-                MID,
-                "test premise: the header word starts as inheritance data"
+            // Rung 2: the word is a ShapeId from BIRTH, so the parent edge is
+            // already only in the registry before anything below runs. That
+            // makes this test stronger than when the word still started as
+            // inheritance data — there is no window in which it was correct.
+            assert!(
+                is_shape_id((*leaf).parent_class_id),
+                "test premise: the newborn's header word was not clobbered by a \
+                 birth stamp, so the chain is not actually being stressed"
             );
 
             let boxed = crate::value::js_nanbox_pointer(leaf as i64);
@@ -869,11 +876,11 @@ mod shape_transition_tests_6759 {
             assert!(truthy(crate::object::js_instanceof(boxed, MID)));
             assert!(truthy(crate::object::js_instanceof(boxed, BASE)));
 
-            // Clobber the word with a stamp.
+            // …and it stays clobbered across a resolve.
             let _ = crate::object::js_object_get_field_by_name(leaf, key("chain6759_p"));
             assert!(
                 is_shape_id((*leaf).parent_class_id),
-                "test premise: the resolve did not stamp, so nothing is being tested"
+                "test premise: the word stopped being a stamp, so nothing is being tested"
             );
 
             assert!(

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

@@ -1575,6 +1575,106 @@ mod c3c_pic_tests {
             assert_eq!(c_compacted[1], 1, "compacted `c` slot");
         }
     }
+
+    /// The PIC cache token the EMITTED code computes for `obj`, transcribed
+    /// from `perry-codegen/src/expr/property_get/generic_dispatch.rs`:
+    ///
+    /// ```text
+    /// is_stamp = (parent_class_id - 0x8000_0000) u< 0x4000_0000
+    /// token    = is_stamp ? (parent_class_id | 1<<62) : keys_array
+    /// ```
+    ///
+    /// The runtime never calls this; it exists so a test can compare what the
+    /// miss handler PRIMES against what the hit path will COMPUTE, which is
+    /// the only pair whose agreement decides whether a site can ever hit.
+    unsafe fn emitted_pic_token(obj: *const super::ObjectHeader) -> u64 {
+        let word = (*obj).parent_class_id;
+        if crate::object::shapes::is_shape_id(word) {
+            word as u64 | crate::object::shapes::PIC_ID_TOKEN_BIT
+        } else {
+            (*obj).keys_array as u64
+        }
+    }
+
+    /// ★ The invariant #6759 C3 rung 1 broke, asserted where it broke.
+    ///
+    /// A shape's population must be UNIFORMLY stamped: the token the miss
+    /// handler primes from one instance is only useful if a DIFFERENT,
+    /// freshly-allocated instance of the same class computes the same token.
+    /// Rung 1 (#7983) stamped class instances lazily while their allocator
+    /// still wrote the real `parent_class_id`, so instance #1 primed an id
+    /// token and every newborn sibling computed its keys pointer instead —
+    /// `token_eq` failed at every site reading a field of a fresh instance,
+    /// forever. Measured cost before the birth stamp: `cycles` +54%,
+    /// `deeplist` +45%, `interp` +28% in instructions retired.
+    ///
+    /// This is deliberately NOT "the newborn carries a stamp" — that is a
+    /// presence check two different states satisfy (both-stamped and
+    /// both-unstamped are each fine; the mixture is the bug). Comparing the
+    /// primed token against a fresh sibling's COMPUTED token is what fails
+    /// under either half of the split.
+    #[test]
+    fn a_fresh_class_instance_computes_the_token_the_miss_handler_primed() {
+        let _lock = crate::gc::global_side_table_test_lock();
+        unsafe {
+            let packed = b"picbirth_x\0picbirth_y";
+            let mk = || {
+                crate::object::js_object_alloc_class_with_keys(
+                    0x6082,
+                    0,
+                    2,
+                    packed.as_ptr(),
+                    packed.len() as u32,
+                )
+            };
+            let key = crate::string::js_string_from_bytes(b"picbirth_x".as_ptr(), 10);
+
+            let primed_from = mk();
+            crate::object::js_object_set_field(
+                primed_from,
+                0,
+                crate::JSValue::from_bits(5.0f64.to_bits()),
+            );
+            assert_eq!(
+                (*primed_from).class_id,
+                0x6082,
+                "test premise: the receiver is a class instance, not a literal"
+            );
+
+            let mut cache = [0i64; super::PIC_CACHE_WORDS];
+            assert_eq!(
+                super::js_object_get_field_ic_miss(primed_from, key, &mut cache),
+                5.0,
+                "test premise: the miss handler resolved the field"
+            );
+            assert_ne!(
+                cache[0], 0,
+                "test premise: the miss handler primed SOMETHING — a zero token \
+                 never hits, so the comparison below would be vacuous"
+            );
+
+            // The next `new C(...)`. Nothing has resolved a field on it.
+            let fresh = mk();
+            assert_eq!(
+                emitted_pic_token(fresh),
+                cache[0] as u64,
+                "a freshly allocated instance of the SAME class computes a \
+                 different PIC token than the one primed from its sibling, so \
+                 every read of a newborn instance's field misses the cache and \
+                 takes the full miss handler — #7983's split population"
+            );
+
+            // And the same must hold once the fresh one has itself resolved:
+            // priming from either instance is interchangeable.
+            let mut cache2 = [0i64; super::PIC_CACHE_WORDS];
+            super::js_object_get_field_ic_miss(fresh, key, &mut cache2);
+            assert_eq!(
+                cache2[0], cache[0],
+                "two instances of one class primed two different tokens — the \
+                 site thrashes between them"
+            );
+        }
+    }
 }
 
 #[cfg(test)]

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

@@ -215,6 +215,51 @@ pub(crate) unsafe fn stamp_object_shape(
     id
 }
 
+/// Birth-stamp a NEWBORN receiver with an already-minted ShapeId. A zero id —
+/// no shape-cache record yet, or the id range exhausted — leaves the word
+/// alone, which preserves the pre-stamp fallback rather than inventing one.
+///
+/// ★ **A shape's population must be UNIFORMLY stamped or uniformly not.** The
+/// emitted read PIC derives its ENTIRE cache token from this word
+/// (`perry-codegen/src/expr/property_get/generic_dispatch.rs`):
+///
+/// ```text
+/// is_stamp = (parent_class_id - 0x8000_0000) u< 0x4000_0000
+/// token    = is_stamp ? (parent_class_id | 1<<62) : keys_array
+/// ```
+///
+/// so a stamped receiver and an unstamped one OF THE SAME SHAPE compute two
+/// DIFFERENT tokens, and a site that sees both can never hold a hit. It is not
+/// a slow start — it is a permanent 0% hit rate: instance #1 misses, is
+/// stamped, primes the id token; instance #2 is newborn, computes the
+/// keys-pointer token, misses; the handler re-primes the same id; instance #3
+/// misses. Forever.
+///
+/// #6759 C3 rung 1 (#7983) stamped class instances only LAZILY, at the first
+/// by-name resolve, and that is exactly what it cost — measured in
+/// instructions retired, isolated against its own parent: `cycles` +54.3%,
+/// `deeplist` +45.2%, `interp` +28.3%, `pipeline` +23.9%, `iso_miss` +22.9%,
+/// while the object-literal benchmarks (`churn` +1.2%, `retain` +0.2%) and
+/// `fib40` (+0.04%) did not move — literals have been birth-stamped since
+/// #6804, so their population was always uniform.
+///
+/// Rung 2 (#8009) closed the compiled path. **Every OTHER allocator that
+/// installs a shape-cached keys array on a fresh `ObjectHeader` must call this
+/// too**, or its classes keep the split.
+///
+/// No `shape_word_is_writable` check: the callers have just written
+/// `object_type`/`class_id` into a header they allocated, so the receiver is a
+/// genuine `ObjectHeader` and never the `RegExpHeader` alias.
+#[inline]
+pub(crate) unsafe fn birth_stamp_object_shape(
+    obj: *mut crate::object::ObjectHeader,
+    runtime_shape_id: u32,
+) {
+    if is_shape_id(runtime_shape_id) {
+        (*obj).parent_class_id = runtime_shape_id;
+    }
+}
+
 /// Drop the stamp iff the word currently holds one, leaving a real
 /// `parent_class_id` untouched. Returns true when a stamp was cleared.
 ///