perf(shapes): append a freshly allocated descriptor id without the family membership scan

changelog.d/keystroke-shape-family-append.md

@@ -0,0 +1,14 @@
+### Fixed
+
+- Interning a shape descriptor no longer scans the keys array's whole
+  descriptor history. `ShapeTableInner::family_push_back` / `facts_push_back`
+  answer "is this id already here?" with a linear scan of the family, and a
+  family accumulates every descriptor ever created for one keys array — so
+  interning the *n*-th descriptor for a keys array cost O(n) and a render that
+  keeps bumping a shape's semantic generation paid quadratic time. The two
+  interning sites append ids that `alloc_shape_id` has just handed out, and
+  that allocator never reuses a value, so the scan was provably dead work:
+  they now use `IdList::append_unchecked`. On the compiled claude-code TUI
+  `IdList::contains` was 6.2 % of main-thread leaf samples during a streamed
+  reply and 5.9 % in the window after it, 95 % of it under
+  `family_push_back`.

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

@@ -264,6 +264,15 @@ impl ShapeTableInner {
         self.families.entry(keys).or_default().push_back(id);
     }
 
+    /// Append a FRESHLY allocated id (see [`IdList::append_unchecked`]): the
+    /// id came from `alloc_shape_id`, which never reuses a value, so the
+    /// membership scan `family_push_back` would run is dead work that is
+    /// linear in the number of descriptors this keys array has ever had.
+    #[inline]
+    fn family_append_fresh(&mut self, keys: u64, id: u32) {
+        self.families.entry(keys).or_default().append_unchecked(id);
+    }
+
     #[inline]
     fn family_push_front(&mut self, keys: u64, id: u32) {
         self.families.entry(keys).or_default().push_front(id);
@@ -287,6 +296,12 @@ impl ShapeTableInner {
         self.by_facts.entry(facts).or_default().push_back(id);
     }
 
+    /// Fresh-id twin of [`ShapeTableInner::facts_push_back`]; same argument.
+    #[inline]
+    fn facts_append_fresh(&mut self, facts: u64, id: u32) {
+        self.by_facts.entry(facts).or_default().append_unchecked(id);
+    }
+
     #[inline]
     fn facts_push_front(&mut self, facts: u64, id: u32) {
         self.by_facts.entry(facts).or_default().push_front(id);
@@ -507,8 +522,12 @@ pub(crate) fn shape_descriptor_ensure_with_holes(
     // complete descriptor.
     // SAFETY: no slab reference is held; `slab()` above went out of scope.
     unsafe { table.slab_mut().insert(id, record) };
-    inner.facts_push_back(facts, id);
-    inner.family_push_back(keys_id, id);
+    // `id` was just handed out by `alloc_shape_id`, which never reuses a
+    // value, so neither accelerator can already hold it: append without the
+    // membership scan, whose cost is linear in this keys array's descriptor
+    // history (see `IdList::append_unchecked`).
+    inner.facts_append_fresh(facts, id);
+    inner.family_append_fresh(keys_id, id);
     Ok(id)
 }
 

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

@@ -401,7 +401,9 @@ pub(crate) unsafe fn rekey_stable_tombstone_shape_after_squeeze(
         .get_mut(&record.keys)
         .is_some_and(|ids| ids.replace(old_id, new_id));
     if !replaced {
-        inner.family_push_back(record.keys, new_id);
+        // `new_id` came from `alloc_shape_id` a few lines above and is in no
+        // list yet (see `IdList::append_unchecked`).
+        inner.family_append_fresh(record.keys, new_id);
     }
     inner.indices.remove(&(record.keys as usize));
     drop(inner);

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

@@ -537,6 +537,27 @@ impl IdList {
         if self.contains(id) {
             return;
         }
+        self.append_unchecked(id);
+    }
+
+    /// Append an id the caller knows is not in this list.
+    ///
+    /// `alloc_shape_id` hands out a strictly increasing counter that is never
+    /// reused (it parks at `SHAPE_ID_END` rather than wrapping), so an id that
+    /// was allocated after this list was built cannot be in it, in this family
+    /// or in any other. The membership scan in [`push_back`] is therefore dead
+    /// work at the two interning sites, and it is not O(1) dead work: a family
+    /// holds every descriptor ever created for one keys array, so the scan is
+    /// linear in the history of that keys array and interning the *n*-th
+    /// descriptor for it costs O(n) — quadratic over a render that keeps
+    /// bumping a shape's semantic generation. `IdList::contains` was 6.2 % of
+    /// main-thread leaf samples on a claude-code streamed reply, 95 % of it
+    /// under `ShapeTableInner::family_push_back`.
+    ///
+    /// Callers that re-file an EXISTING id (the metadata rekey when a keys
+    /// array moves) must keep using [`push_back`]: those ids can already be in
+    /// the destination list.
+    pub(super) fn append_unchecked(&mut self, id: u32) {
         match self {
             IdList::Inline { len, ids } if (*len as usize) < ids.len() => {
                 ids[*len as usize] = id;

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

@@ -551,6 +551,43 @@ mod descriptor_tests_8067 {
         test_drop_shape_descriptors(keys);
     }
 
+    #[test]
+    fn interning_appends_each_new_descriptor_to_the_family_exactly_once() {
+        // `shape_descriptor_ensure` appends a FRESHLY allocated id with
+        // `IdList::append_unchecked`, skipping the membership scan whose cost
+        // is linear in the family's history. The scan is skippable only
+        // because `alloc_shape_id` never reuses a value; this pins the
+        // observable consequence — every distinct descriptor for one keys
+        // array appears in its family exactly once, in birth order — so a
+        // later change that feeds a recycled id through the fresh path fails
+        // here instead of silently duplicating a family entry.
+        let _lock = crate::gc::global_side_table_test_lock();
+        let keys = 0x8067_0000_0000_2900usize;
+        let mut born = Vec::new();
+        for n in 1..=6u32 {
+            born.push(
+                shape_descriptor_ensure(keys as *const ArrayHeader, n, n)
+                    .expect("shape range unexpectedly exhausted"),
+            );
+        }
+        assert_eq!(
+            test_shape_ids_for_keys(keys),
+            born,
+            "each new descriptor is appended once, in birth order"
+        );
+        // Re-interning the same facts must hit the accelerator and add nothing.
+        for (i, n) in (1..=6u32).enumerate() {
+            assert_eq!(
+                shape_descriptor_ensure(keys as *const ArrayHeader, n, n).unwrap(),
+                born[i],
+                "an existing descriptor must be reused, not re-appended"
+            );
+        }
+        assert_eq!(test_shape_ids_for_keys(keys), born);
+
+        test_drop_shape_descriptors(keys);
+    }
+
     #[test]
     fn a_foreign_agent_id_misses_instead_of_aliasing_same_address() {
         let _lock = crate::gc::global_side_table_test_lock();