perf(gc) phase 2: memoise the shape scanner's per-address forwarding probe

changelog.d/8892-shape-scan-probe-memo.md

@@ -0,0 +1,51 @@
+Sped up the shape-table GC scanner by probing each distinct keys-array address
+once per pass instead of once per shape.
+
+`scan_shape_table_rekey_mut` is the single most expensive root scanner on
+`claude -p` — **178.9 ms, 53.3% of all scanner time**. Measuring it on a
+shape-heavy workload showed where that goes:
+
+| phase | calls | total | per call | share |
+|---|---:|---:|---:|---:|
+| mark | 10 | 61.4 ms | 6.1 ms | 10.4% |
+| rewrite | 10 | 528.5 ms | 52.9 ms | **89.6%** |
+
+The cost is not computation. Each descriptor's probe runs
+`classify_heap_space_in_range` and then reads the GC header at a scattered
+address — two likely cache misses, per descriptor, per collection.
+
+Shapes share keys arrays at a measured, stable **2.5:1**, so the loop paid that
+~2.5 times per distinct address. The probe is now memoised per pass. This is
+sound by construction: the same addresses are visited, just once each, and
+forwarding is a pure function of the address within one pass. The carrier flag
+is part of the memo key — carriers take `visit_usize_slot`, which MARKS in mark
+modes, so a non-carrier's cached answer must not be allowed to satisfy a
+carrier's marking duty. The per-descriptor bookkeeping is lifted into a shared
+helper so the memoised and probing paths cannot drift apart; only the probe is
+deduplicated.
+
+Measured on the same workload: **3074.7 ms → 2685.9 ms of scanner time, −12.6%**.
+
+That is well short of the 2.5× the sharing ratio suggests, because a lookup in a
+300k-entry map costs nearly as much as the probe it replaces — one cache miss
+traded for another. The memo is therefore a reused thread-local scratch map
+rather than a fresh allocation per scan; at this size, allocating one every
+collection is exactly the churn the memory-parity work is trying to remove.
+
+### The larger finding, not fixed here
+
+The shape table **grows without bound between full collections**. On a workload
+that never holds more than 400 live objects it reached **786,205 descriptors**,
+and scanner cost tracks it directly: **3.6 ms → 490 ms per call**.
+
+The mechanism: `prune_dead_owner_side_tables_copied_minor` is nursery-only by
+construction, so a keys array that is promoted and *then* dies is not reclaimed
+until a full collection — while the scanner walks the whole table on every
+minor. That, not the per-probe cost, is why this scanner dominates.
+
+Fixing it means either pruning promoted-then-dead shapes sooner, or not walking
+the whole table on a minor. The second is the better fix and needs the collector
+to say whether old-page defrag runs in the cycle: without that, skipping
+tenured entries is unsound, because a defragging moving collection *can* move
+them, and a stale keys pointer is silent heap corruption. Deliberately left for
+its own change rather than guessed at here.

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

@@ -1675,39 +1675,119 @@ pub(crate) fn prune_dead_shape_keys(is_dead_owner: &dyn Fn(usize) -> bool) {
 /// pointer-keyed slot indices. Mark/copy mode does not root anything; live
 /// object scans provide descriptor reachability, and post-copy rewrite follows
 /// only forwarding records those live edges already created.
+
+crate::perry_thread_local! {
+    /// Scratch memo for [`scan_shape_table_rekey_mut`]'s per-address probe,
+    /// reused across collections so the scan allocates nothing.
+    static PROBE_MEMO: std::cell::RefCell<std::collections::HashMap<(usize, bool), (bool, usize)>> =
+        std::cell::RefCell::new(std::collections::HashMap::new());
+}
+
+/// Per-descriptor bookkeeping after its keys address has been probed.
+///
+/// Lifted out of `scan_shape_table_rekey_mut`'s loop so the memoised path and
+/// the probing path cannot drift apart — the probe is what is deduplicated,
+/// never the bookkeeping, which still runs once per descriptor.
+#[inline]
+fn record_shape_scan_outcome(
+    visitor: &mut crate::gc::RuntimeRootVisitor<'_>,
+    id: &u32,
+    descriptor: &mut ShapeDescriptor,
+    addr: usize,
+    moved: bool,
+    dead_descriptor_ids: &mut Vec<u32>,
+    descriptor_rekeys: &mut Vec<u32>,
+) {
+    // Validate the POST-visit address. A stale shape key can follow the
+    // forwarding record of the non-array tenant that recycled its address;
+    // checking only an unmoved old address misses that case.
+    if visitor.is_metadata_rewrite_phase() && shape_keys_address_is_recycled(addr) {
+        dead_descriptor_ids.push(*id);
+    } else if moved {
+        descriptor.keys = addr as u64;
+    }
+    // A live-object edge can rewrite the boxed `keys` slot before this metadata
+    // pass. Comparing against the address represented in the reverse maps
+    // catches both that ordering and a move observed here.
+    if descriptor.keys != descriptor.indexed_keys {
+        descriptor_rekeys.push(*id);
+    }
+}
+
 pub(crate) fn scan_shape_table_rekey_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) {
     let mut inner = crate::state::state().shapes.inner.borrow_mut();
+    // TEMPORARY (#6759 phase 2 measurement): this scanner is 53.3% of all
+    // root-scanner time on `claude -p`. Report what it is actually walking so
+    // the fix targets the real term instead of a guess.
     let mut descriptor_rekeys: Vec<u32> = Vec::new();
     let mut dead_descriptor_ids: Vec<u32> = Vec::new();
-    for (id, descriptor) in inner.descriptors.iter_mut() {
-        let mut addr = descriptor.keys as usize;
-        // #8112 ephemeron gate. A shape with an OLD carrier is rooted here:
-        // the minor that has to keep its keys array alive never enumerates the
-        // object that carries it. A shape with only young carriers is NOT —
-        // those receivers are traced, and each one emits the edge itself, so
-        // rooting them from the table would make every keys array ever minted
-        // immortal and turn `prune_dead_shape_keys`'s "is the keys array
-        // dead?" into a question it asks of itself.
-        let moved = if descriptor.old_carrier || descriptor.cache_carrier {
-            visitor.visit_usize_slot(&mut addr)
-        } else {
-            visitor.visit_metadata_usize_slot(&mut addr)
-        };
-        // Validate the POST-visit address. A stale shape key can follow the
-        // forwarding record of the non-array tenant that recycled its address;
-        // checking only an unmoved old address misses that case.
-        if visitor.is_metadata_rewrite_phase() && shape_keys_address_is_recycled(addr) {
-            dead_descriptor_ids.push(*id);
-        } else if moved {
-            descriptor.keys = addr as u64;
-        }
-        // A live-object edge can rewrite the boxed `keys` slot before this
-        // metadata pass. Comparing against the address represented in the
-        // reverse maps catches both that ordering and a move observed here.
-        if descriptor.keys != descriptor.indexed_keys {
-            descriptor_rekeys.push(*id);
+
+    // #6759 phase 2: probe each distinct keys-array address ONCE.
+    //
+    // The per-descriptor probe is the expensive part of this scanner — 89.6% of
+    // its time is the rewrite phase, and each probe runs
+    // `classify_heap_space_in_range` and then reads the GC header at a
+    // scattered address (two likely cache misses). Shapes share keys arrays at
+    // a measured, stable 2.5:1, so the unmemoised loop paid that ~2.5 times per
+    // distinct address.
+    //
+    // Memoising is sound by construction: the same addresses are visited, just
+    // once each, and forwarding is a pure function of the address within one
+    // pass. Carriers take a different visit (`visit_usize_slot`, which MARKS in
+    // mark modes) than non-carriers, so the carrier flag is part of the key —
+    // otherwise a non-carrier hit could satisfy a carrier's marking duty.
+    // Reused across collections rather than allocated per scan: at ~300k
+    // entries a fresh map every GC is exactly the kind of churn the
+    // memory-parity work is trying to remove. `clear()` keeps the capacity.
+    PROBE_MEMO.with(|memo| {
+        let mut probe_memo = memo.borrow_mut();
+        probe_memo.clear();
+
+        for (id, descriptor) in inner.descriptors.iter_mut() {
+            let mut addr = descriptor.keys as usize;
+            // #8112 ephemeron gate. A shape with an OLD carrier is rooted here:
+            // the minor that has to keep its keys array alive never enumerates the
+            // object that carries it. A shape with only young carriers is NOT —
+            // those receivers are traced, and each one emits the edge itself, so
+            // rooting them from the table would make every keys array ever minted
+            // immortal and turn `prune_dead_shape_keys`'s "is the keys array
+            // dead?" into a question it asks of itself.
+            let is_carrier = descriptor.old_carrier || descriptor.cache_carrier;
+            let memo_key = (addr, is_carrier);
+            if let Some(&(prev_moved, prev_addr)) = probe_memo.get(&memo_key) {
+                // Already probed this exact (address, carrier-duty) pair in this
+                // pass — reuse the answer instead of paying the walk again.
+                let moved = prev_moved;
+                addr = prev_addr;
+                record_shape_scan_outcome(
+                    visitor,
+                    id,
+                    descriptor,
+                    addr,
+                    moved,
+                    &mut dead_descriptor_ids,
+                    &mut descriptor_rekeys,
+                );
+                continue;
+            }
+            let probe_addr = addr;
+            let moved = if is_carrier {
+                visitor.visit_usize_slot(&mut addr)
+            } else {
+                visitor.visit_metadata_usize_slot(&mut addr)
+            };
+            probe_memo.insert((probe_addr, is_carrier), (moved, addr));
+            record_shape_scan_outcome(
+                visitor,
+                id,
+                descriptor,
+                addr,
+                moved,
+                &mut dead_descriptor_ids,
+                &mut descriptor_rekeys,
+            );
         }
-    }
+    });
     // Remove descriptors whose keys array was recycled.
     if !dead_descriptor_ids.is_empty() {
         for id in &dead_descriptor_ids {