perf(object): pack the per-shape key index into 4-byte cells

changelog.d/9755-gc-side-table-young-logs.md

@@ -3,9 +3,9 @@
 - **Minor collections no longer walk every runtime side table.** A copying
   minor's three root-scan passes — and a budgeted minor's initial root scan
   and final remark — visited every entry of the closure dynamic-prop tables,
-  the string-keyed descriptor tables, the shape family/slot-index maps, the
-  transition cache and the shape cache on every collection, to discover that
-  nothing in them pointed at the nursery. On the compiled claude-code TUI
+  the string-keyed descriptor tables, the shape family/slot-index maps and
+  the transition cache on every collection, to discover that nothing in them
+  pointed at the nursery. On the compiled claude-code TUI
   that was ~35k shape families, ~120k descriptors and ~13k closure owners
   per walk, 41 minors per streamed reply, all reporting `slots=0`: 34–56 ms
   of scanner time per minor.
@@ -26,6 +26,13 @@
   prints `[gc-young-log]` rows (logged / visited / kept / table size) per
   table and cycle.
 
+  The **shape cache** was measured and deliberately left on its plain walk.
+  Its canonical keys arrays are allocated in the longlived arena, which
+  `addr_is_minor_relevant` must answer `true` for, so no entry ever leaves a
+  log there: on the claude-code TUI the log named 100 % of the table in every
+  one of 107 collections (0 % skipped) and cost **35 % more** than the walk it
+  replaced. The four tables above skip 75–93 %.
+
 - **The post-minor remembered-set coverage restore is proportional to what
   the dirty scan could not cover.** `restore_surviving_dirty_coverage`
   (#5029) re-walked every slot of every object on the pre-cycle dirty pages

crates/perry-runtime/src/gc/tests/young_log_tests.rs

@@ -53,6 +53,11 @@ fn walk(table: &'static str) -> young_log::YoungLogWalk {
     young_log::last_walk(table).unwrap_or_else(|| panic!("no walk recorded for {table}"))
 }
 
+/// For a table that is deliberately NOT young-logged: no walk row at all.
+fn walk_opt(table: &'static str) -> Option<young_log::YoungLogWalk> {
+    young_log::last_walk(table)
+}
+
 // ---------------------------------------------------------------- closures
 
 #[test]
@@ -430,15 +435,18 @@ fn young_transition_key_under_an_old_target_arms_the_log_through_the_writer() {
     );
 }
 
+/// The shape cache is deliberately NOT young-logged (see
+/// `scan_shape_cache_roots_mut`): its keys arrays are longlived, so a log
+/// there names every entry forever and skips nothing. This pins the walk that
+/// replaced it — a young entry reachable only through the cache still moves
+/// and is re-keyed in both the inline slot and the overflow map.
 #[test]
-fn young_shape_cache_entry_is_moved_through_the_log() {
+fn shape_cache_entry_is_moved_by_the_plain_walk() {
     let _guard = CopyingNurseryTestGuard::new(0);
     gc_register_mutable_root_scanner(crate::object::scan_shape_cache_roots_mut);
 
-    // Reachable ONLY through the cache (which roots it). Seeded through the
-    // PRODUCTION writer (`shape_cache_insert`), not a test seam: a seam that
-    // arms the log itself makes this test pass with the writer's own arm site
-    // deleted, which is how #9755 shipped an unenforced rule 1.
+    // Reachable ONLY through the cache (which roots it), seeded through the
+    // PRODUCTION writer (`shape_cache_insert`), not a test seam.
     let keys = unsafe { young_keys_array() };
     let shape_id = 0x9754_0001;
     crate::object::test_shape_cache_insert(shape_id, keys);
@@ -455,9 +463,11 @@ fn young_shape_cache_entry_is_moved_through_the_log() {
         inline, overflow,
         "inline and overflow must agree on the new address"
     );
-    let row = walk("object.shape_cache");
-    assert!(row.partial);
-    assert!(row.visited >= 1, "{row:?}");
+    assert!(
+        walk_opt("object.shape_cache").is_none(),
+        "the shape cache must not report a young-log walk: #9755's log for it \
+         skipped 0 % and cost 35 % more than this walk, and was removed"
+    );
 }
 
 // ---------------------------------------------------------------------------

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

@@ -662,21 +662,6 @@ fn shape_cache_get_with_id(shape_id: u32) -> (*mut ArrayHeader, u32) {
         .unwrap_or((std::ptr::null_mut(), 0))
 }
 
-/// Rule 1 of `gc/young_log.rs` for the shape cache: log `shape_id` BEFORE the
-/// entry naming `keys_array` becomes findable.
-///
-/// Every writer of the cache — the production `shape_cache_insert` and the
-/// `#[cfg(test)]` seed seam — arms through this one function. A seam that
-/// re-implements the predicate is the failure mode this exists to prevent:
-/// the tests then validate an arming rule that is not the one that ships, and
-/// deleting the production arm site stays green.
-#[inline]
-pub(super) fn arm_shape_cache_young(shape_id: u32, keys_array: *mut ArrayHeader) {
-    if crate::gc::young_log::addr_is_minor_relevant(keys_array as usize) {
-        SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().note(shape_id));
-    }
-}
-
 /// Insert a keys_array into the cache. Updates the inline slot
 /// (evicting any prior entry there) and also writes to the overflow
 /// map so misses on the inline cache still find the value.
@@ -706,9 +691,6 @@ fn shape_cache_insert(shape_id: u32, keys_array: *mut ArrayHeader) {
     };
     let st = crate::state::state();
     let slot = (shape_id as usize) & (SHAPE_INLINE_CACHE_SIZE - 1);
-    // #9754 rule 1: log the id BEFORE the entry is published when the keys
-    // array can matter to a minor.
-    arm_shape_cache_young(shape_id, keys_array);
     unsafe {
         // GC_STORE_AUDIT(ROOT): shape_inline_cache entries are scanned by scan_shape_cache_roots_mut.
         let entry = &mut (*st.object_hot.shape_inline_cache.get())[slot];
@@ -822,14 +804,9 @@ crate::perry_thread_local! {
     /// `scan_transition_cache_roots_mut` visits only these.
     static TRANSITION_CACHE_YOUNG: RefCell<crate::gc::young_log::YoungLog<u32>> =
         const { RefCell::new(crate::gc::young_log::YoungLog::new()) };
-    /// #9754: shape-cache ids (inline slot and overflow key alike) whose keys
-    /// array may still be acted on by a minor.
-    static SHAPE_CACHE_YOUNG: RefCell<crate::gc::young_log::YoungLog<u32>> =
-        const { RefCell::new(crate::gc::young_log::YoungLog::new()) };
 }
 
 const TRANSITION_CACHE_YOUNG_LOG_NAME: &str = "object.transition_cache";
-const SHAPE_CACHE_YOUNG_LOG_NAME: &str = "object.shape_cache";
 
 /// Is a transition-cache entry still something a minor can act on?
 #[inline]
@@ -1334,7 +1311,6 @@ pub(crate) fn test_shape_cache_insert(shape_id: u32, keys_array: *mut ArrayHeade
 pub(crate) fn test_seed_shape_cache_root(shape_id: u32, keys_array: *mut ArrayHeader) {
     let st = crate::state::state();
     let slot = (shape_id as usize) & (SHAPE_INLINE_CACHE_SIZE - 1);
-    arm_shape_cache_young(shape_id, keys_array);
     unsafe {
         // GC_STORE_AUDIT(ROOT): test seed mirrors shape_inline_cache roots scanned by scan_shape_cache_roots_mut.
         let entry = &mut (*st.object_hot.shape_inline_cache.get())[slot];

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

@@ -45,7 +45,7 @@ pub(crate) use shapes_slot_list::{
     object_shape_hole_count, publish_object_shape_holes,
     rekey_stable_tombstone_shape_after_squeeze, retire_owned_shape_history,
     shape_index_migrate_after_delete, shape_index_shift_in_place,
-    try_update_stable_tombstone_shape, try_update_stable_tombstone_shape_cached, SlotList,
+    try_update_stable_tombstone_shape, try_update_stable_tombstone_shape_cached, SlotIndex,
 };
 use shapes_store::{
     IdList, ShapeRecord, ShapeSlab, RECORD_FLAG_CACHE_CARRIER, RECORD_FLAG_CARRIED_SEEN,
@@ -68,7 +68,7 @@ pub(crate) struct ShapeIndex {
     /// `bench_populated_delete.ts` — perry's worst object-model gap against
     /// node — `hash_one::<&usize>` plus `sip::Hasher::write` were **14.7% of
     /// self time**, second only to the lookup that performs them.
-    slots: crate::fast_hash::PtrHashMap<u64, SlotList>,
+    slots: SlotIndex,
 }
 
 /// Immutable facts named by one ShapeId, copied out of the table.
@@ -1627,12 +1627,7 @@ unsafe fn index_range(shape: &mut ShapeIndex, keys: *const ArrayHeader, key_coun
         let v = crate::JSValue::from_bits((*slots.add(i as usize)).to_bits());
         if let Some(b) = crate::string::js_string_key_bytes(v, &mut sso) {
             let h = super::key_bytes_hash(b.as_ptr(), b.len());
-            match shape.slots.entry(h) {
-                std::collections::hash_map::Entry::Occupied(mut e) => e.get_mut().push(i),
-                std::collections::hash_map::Entry::Vacant(e) => {
-                    e.insert(SlotList::One(i));
-                }
-            }
+            shape.slots.push(h, i);
         }
     }
     shape.indexed_len = key_count;
@@ -1699,7 +1694,7 @@ pub(crate) unsafe fn shape_slot_lookup_verdict(
             inner.note_young_keys(keys_id as u64);
             inner.indices.entry(keys_id).or_insert(ShapeIndex {
                 indexed_len: 0,
-                slots: crate::fast_hash::new_ptr_hash_map(),
+                slots: SlotIndex::new(),
             })
         }
     };
@@ -1712,12 +1707,9 @@ pub(crate) unsafe fn shape_slot_lookup_verdict(
     } else {
         KeysIndexVerdict::Unindexed
     };
-    let Some(candidates) = shape.slots.get(&key_hash) else {
-        return absent;
-    };
     let mut sso = [0u8; crate::value::SHORT_STRING_MAX_LEN];
     let (slots, slot_len) = super::keys_array_dense_slots(keys);
-    for &i in candidates.iter() {
+    for i in shape.slots.candidates(key_hash) {
         if (i as usize) >= slot_len || i >= key_count {
             continue;
         }
@@ -1746,12 +1738,7 @@ pub(crate) fn shape_note_append(
     if let Some(shape) = inner.indices.get_mut(&(keys as usize)) {
         if shape.indexed_len + 1 == new_count {
             shape.indexed_len = new_count;
-            match shape.slots.entry(key_hash) {
-                std::collections::hash_map::Entry::Occupied(mut e) => e.get_mut().push(slot),
-                std::collections::hash_map::Entry::Vacant(e) => {
-                    e.insert(SlotList::One(slot));
-                }
-            }
+            shape.slots.push(key_hash, slot);
         }
     }
 }
@@ -1761,12 +1748,7 @@ pub(crate) fn shape_note_append(
 pub(crate) fn shape_note_hit(keys: *const ArrayHeader, key_hash: u64, slot: u32) {
     let mut inner = crate::state::state().shapes.inner.borrow_mut();
     if let Some(shape) = inner.indices.get_mut(&(keys as usize)) {
-        match shape.slots.entry(key_hash) {
-            std::collections::hash_map::Entry::Occupied(mut e) => e.get_mut().push(slot),
-            std::collections::hash_map::Entry::Vacant(e) => {
-                e.insert(SlotList::One(slot));
-            }
-        }
+        shape.slots.push(key_hash, slot);
     }
 }
 
@@ -2314,17 +2296,13 @@ pub(crate) fn shrink_shape_tables() {
 /// `PERRY_GC_CENSUS`: the by-id slab, the per-shape key indices, the
 /// exact-facts accelerator and the keys-address family index.
 pub(crate) fn shape_table_census() -> Vec<crate::gc::census::SideTableRow> {
-    use crate::gc::census::{hash_table_bytes, map_bytes};
+    use crate::gc::census::map_bytes;
     let table = &crate::state::state().shapes;
     let inner = table.inner.borrow();
     let slab = table.slab();
     let mut rows = Vec::new();
     rows.push(("shapes.descriptors", slab.len(), slab.estimated_bytes()));
-    let index_inner: usize = inner
-        .indices
-        .values()
-        .map(|ix| hash_table_bytes(ix.slots.capacity(), std::mem::size_of::<(u64, SlotList)>()))
-        .sum();
+    let index_inner: usize = inner.indices.values().map(|ix| ix.slots.heap_bytes()).sum();
     rows.push((
         "shapes.indices",
         inner.indices.len(),