From 019318d0bcaff3b5c7b4731698cb7cd30177a0b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 27 Aug 2026 23:34:41 +0200 Subject: [PATCH] perf(gc): memoise the shape scanner's per-address forwarding probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scan_shape_table_rekey_mut is 53.3% of all root-scanner time on claude -p. Measured split: the rewrite phase is 89.6% of it, and the cost is cache misses, not computation — each descriptor's probe runs classify_heap_space_in_range and then reads a GC header at a scattered address. Shapes share keys arrays 2.5:1 (stable across a run), so the loop paid that ~2.5x per distinct address. Probing is now memoised per pass. Sound by construction: same addresses, once each, and forwarding is a pure function of the address within a 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 satisfy a carrier's marking duty. Per-descriptor bookkeeping moved to a shared helper so the memoised and probing paths cannot drift; only the probe is deduplicated. Measured: 3074.7ms -> 2685.9ms of scanner time (-12.6%). Short of the 2.5x the ratio suggests, because a 300k-entry map lookup costs nearly as much as the probe it replaces. The memo is a reused thread-local scratch map, not a per-scan allocation. Larger finding, deliberately NOT fixed here: the shape table grows unboundedly between full collections — 786,205 descriptors on a workload holding <400 live objects, with scanner cost tracking it 3.6ms -> 490ms per call. The copied-minor prune is nursery-only, so promoted-then-dead keys arrays survive to the next full GC while every minor walks the whole table. The better fix is to not walk it all on a minor, which needs the collector to report whether old-page defrag runs that cycle; without that signal, skipping tenured entries is unsound. Suite 2751 passed. --- changelog.d/8892-shape-scan-probe-memo.md | 51 ++++++++ crates/perry-runtime/src/object/shapes.rs | 136 +++++++++++++++++----- 2 files changed, 159 insertions(+), 28 deletions(-) create mode 100644 changelog.d/8892-shape-scan-probe-memo.md diff --git a/changelog.d/8892-shape-scan-probe-memo.md b/changelog.d/8892-shape-scan-probe-memo.md new file mode 100644 index 0000000000..06ac3733e7 --- /dev/null +++ b/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. diff --git a/crates/perry-runtime/src/object/shapes.rs b/crates/perry-runtime/src/object/shapes.rs index 93c3708ba8..3506f13a23 100644 --- a/crates/perry-runtime/src/object/shapes.rs +++ b/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::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, + descriptor_rekeys: &mut Vec, +) { + // 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 = Vec::new(); let mut dead_descriptor_ids: Vec = 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 {