From 390796a116e1987a0d883e32a5fb5c4a74c4b9eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 14:01:14 +0200 Subject: [PATCH 1/5] perf(gc): one pass over the per-object layout tables per prune, and a filter that admits when it is outgrown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `prune_dead_per_object_layout_owners` walked every live key three times per collection — `retain`, `layout_addr_filter_rebuild` (which buffered them all into a `Vec` first), then `recount_young_layout_records`. The last two want exactly the survivor set `retain` already visits, so they fold into its closure and the `Vec` disappears; on the compiled claude-code TUI that `Vec` alone allocated 50.6 MB per 400-character reply. The new `PERRY_LAYOUT_DIAG` instrument reports what made this expensive: 162,258 live keys against a 4,096-bit address sketch documented for "one or two entries", with all 4,096 bits set. Every probe answers "may hold", so the early returns the sketch exists to serve never fire, and each rebuild is an O(live keys) walk that restores the all-ones state it started from. Past one eighth of the bits the rebuild now reaches that state in O(1) instead. Widening the sketch is a codegen change — the geometry and hash are mirrored in `emit_gated_forget_object_layout` — and would need ~190 KB of inline TLS per thread to discriminate at this occupancy. `transfer_per_object_descriptor` gains the emptiness test its shared flag cannot express: one `len` load instead of two hashes per evacuated object, for a map that is empty for the whole of a cc turn. `LAYOUT_DIAG` is declared with `crate::perry_thread_local!`, as `scripts/check_thread_locals.py` requires of every new declaration. Claude-Session: https://claude.ai/code/session_014UZWia6L37DpA93VLtNK9m --- changelog.d/9807-layout-prune-single-pass.md | 33 ++++ crates/perry-runtime/src/gc/layout_tables.rs | 182 ++++++++++++++++--- crates/perry-runtime/src/hot_diag.rs | 148 +++++++++++++++ 3 files changed, 334 insertions(+), 29 deletions(-) create mode 100644 changelog.d/9807-layout-prune-single-pass.md diff --git a/changelog.d/9807-layout-prune-single-pass.md b/changelog.d/9807-layout-prune-single-pass.md new file mode 100644 index 0000000000..ab8c8fe72c --- /dev/null +++ b/changelog.d/9807-layout-prune-single-pass.md @@ -0,0 +1,33 @@ +**The per-object layout death-prune walks its tables once instead of three +times, and no longer allocates a `Vec` of every live key** — 50.6 MB of a +compiled claude-code turn's 304 MB in the layout tables (#9792). + +`prune_dead_per_object_layout_owners` visited every surviving key three times +per collection: `retain` to drop the dead owners, then +`layout_addr_filter_rebuild` — which first collected all of them into a +`Vec` — and then `recount_young_layout_records` to re-derive the +nursery-key count. The last two want exactly the survivor set `retain` is +already walking, so both fold into its closure. The `Vec` is gone from the +rebuild's other caller too. + +The measurement that prompted it also found the accelerator these tables sit +behind unable to do its job. `layout_addr_filter_may_hold` is a 4,096-bit +one-hash sketch documented for "one or two entries, ~0.05 % false positives"; +a new `PERRY_LAYOUT_DIAG` instrument reports **162,258 live keys and 4,096 of +4,096 bits set** on one 400-character claude-code reply. Every probe answers +"may hold", so the early returns in `transfer_per_object_descriptor` and +`transfer_per_object_slot_mask` never fire, and each rebuild was an O(live +keys) walk restoring the all-ones state it started from. Past four times the +bit count — 16,384 keys, where the false-positive rate is already 98.2 % — the +rebuild now sets all ones directly, the same conservative answer reached in +O(1); below that the filter keeps exactly the selectivity it has today. The +instrument says so out loud rather than leaving it to be inferred. Widening the sketch is not available +from the runtime: its geometry and hash are mirrored in `perry-codegen`'s +`emit_gated_forget_object_layout`, and discriminating at 162k keys would take +~190 KB of inline thread-local storage per thread. + +`transfer_per_object_descriptor` also gained the emptiness test its shared +flag cannot express: the flag and the filter are common to both per-object +tables, so a full slot-mask table drags every relocation into the typed-layout +map as well — which on cc is permanently empty (typed=0, masks=162,258). One +`len` load replaces two hashes per evacuated object. diff --git a/crates/perry-runtime/src/gc/layout_tables.rs b/crates/perry-runtime/src/gc/layout_tables.rs index e5d71a1c03..420c2b1df0 100644 --- a/crates/perry-runtime/src/gc/layout_tables.rs +++ b/crates/perry-runtime/src/gc/layout_tables.rs @@ -168,21 +168,11 @@ fn note_new_layout_record(user_ptr: usize) { } } -/// Re-derive this thread's young-record count from the live keys and publish -/// the delta. Runs after every death prune (all cycle kinds) and whenever the -/// tables empty, so promotion (a key moving to an old page) and death both -/// bring the count back down. -fn recount_young_layout_records() { - let live = { - let masks = hot_layout_slot_masks().borrow(); - let typed = hot_typed_layouts().borrow(); - masks - .keys() - .chain(typed.keys()) - .filter(|key| layout_key_may_be_nursery(**key)) - .count() - }; - let live = u32::try_from(live).unwrap_or(u32::MAX); +/// Publish this thread's young-record count and the delta to the process +/// total. The count itself is derived by the death prune's single pass over +/// the live keys (all cycle kinds), so promotion (a key moving to an old page) +/// and death both bring it back down without a walk of their own. +fn publish_young_layout_records(live: u32) { let prev = hot_per_object_layout_hint().young_records.replace(live); use std::sync::atomic::Ordering::SeqCst; if live > prev { @@ -204,28 +194,97 @@ pub(in crate::gc) fn prune_dead_per_object_layout_owners(is_dead_owner: &dyn Fn( if !per_object_layouts_maybe_nonempty() { return; } + // ONE pass over each table, not three. The old shape visited every live + // key three times per collection — `retain`, then + // `layout_addr_filter_rebuild` (which first collected them all into a + // `Vec`), then `recount_young_layout_records` — and the survivor + // set the last two want is exactly what `retain` is already walking. On cc + // that was 162k keys x 47 prunes per 400-character reply, with the `Vec` + // alone allocating 50.6 MB of the turn's 304 MB (#9792). + let hint = hot_per_object_layout_hint(); + // A filter this occupancy has outgrown is not worth rebuilding: see + // [`layout_addr_filter_saturating_occupancy`]. Decide from the pre-prune + // size, which bounds the survivor count from above, so the decision is one + // branch captured by the closure rather than a test per key. + let occupancy = hot_layout_slot_masks().borrow().len() + hot_typed_layouts().borrow().len(); + let rebuild_filter = occupancy <= layout_addr_filter_saturating_occupancy(); + if rebuild_filter { + // Cleared FIRST so the bits set below describe survivors only — a key + // that dies in this pass must leave no bit behind, which is the whole + // point of rebuilding. + layout_addr_filter_clear(); + } else { + layout_addr_filter_saturate(); + } + let mut young: u32 = 0; + let mut keep = |key: usize| { + if is_dead_owner(key) { + return false; + } + if rebuild_filter { + let (word, bit) = layout_addr_filter_slot(key); + // SAFETY: the filter is a plain `UnsafeCell` in this thread's own + // hot slot and nothing else holds a reference to it here. This is + // the same single-threaded access every probe makes; the tables + // borrowed around it are different thread-locals. + unsafe { + (*hint.filter.get())[word] |= bit; + } + } + if layout_key_may_be_nursery(key) { + young = young.saturating_add(1); + } + true + }; let masks_emptied = { let mut masks = hot_layout_slot_masks().borrow_mut(); let had = !masks.is_empty(); - masks.retain(|key, _| !is_dead_owner(*key)); + masks.retain(|key, _| keep(*key)); had && masks.is_empty() }; let typed_emptied = { let mut typed = hot_typed_layouts().borrow_mut(); let had = !typed.is_empty(); - typed.retain(|key, _| !is_dead_owner(*key)); + typed.retain(|key, _| keep(*key)); had && typed.is_empty() }; + publish_young_layout_records(young); + // Runs last: when it finds both tables empty it disarms the flag, zeroes + // the young count published above and clears the filter, which is the + // correct end state whichever branch the pass took. refresh_per_object_layouts_flag(masks_emptied || typed_emptied); - if per_object_layouts_maybe_nonempty() { - // Stale filter bits are what the pruned keys leave behind; rebuilding - // from the survivors keeps the runtime probes as selective as the - // tables really are. - layout_addr_filter_rebuild(); - recount_young_layout_records(); + if crate::hot_diag::layout_on() { + layout_diag_note_prune(rebuild_filter); } } +/// `PERRY_LAYOUT_DIAG`'s per-prune sample. Out of line and behind +/// [`crate::hot_diag::layout_on`] so an unarmed build pays one relaxed load. +#[cold] +fn layout_diag_note_prune(rebuilt_filter: bool) { + let (typed_len, masks_len) = ( + hot_typed_layouts().borrow().len(), + hot_layout_slot_masks().borrow().len(), + ); + let hint = hot_per_object_layout_hint(); + // SAFETY: as in the pass above — this thread's own filter, no other + // reference live. + let set = unsafe { + (*hint.filter.get()) + .iter() + .map(|w| w.count_ones() as usize) + .sum::() + }; + crate::hot_diag::layout_note_prune( + typed_len, + masks_len, + set, + LAYOUT_ADDR_FILTER_BITS, + rebuilt_filter, + layout_addr_filter_saturating_occupancy(), + ); +} + #[cfg(test)] pub(in crate::gc) fn test_per_object_layout_present(user_ptr: usize) -> bool { hot_layout_slot_masks().borrow().contains_key(&user_ptr) @@ -366,18 +425,74 @@ pub(in crate::gc) fn layout_addr_filter_note(user_ptr: usize) { /// their own (two keys may share one), so this is what keeps a workload that /// genuinely churns per-object records from saturating the filter forever. fn layout_addr_filter_rebuild() { + let occupancy = hot_layout_slot_masks().borrow().len() + hot_typed_layouts().borrow().len(); + if occupancy > layout_addr_filter_saturating_occupancy() { + // Walking the keys would set almost every bit, so this is where the + // rebuild lands anyway — reached in O(1) instead of O(live keys). + layout_addr_filter_saturate(); + return; + } layout_addr_filter_clear(); - let keys: Vec = { - let masks = hot_layout_slot_masks().borrow(); - let typed = hot_typed_layouts().borrow(); - masks.keys().copied().chain(typed.keys().copied()).collect() - }; let hint = hot_per_object_layout_hint(); - for k in keys { + // Straight from the tables: the `Vec` of every live key that used + // to buffer this walk allocated 50.6 MB per 400-character cc reply, and + // bought nothing — no borrow held here conflicts with the filter, which + // lives in a different thread-local. + let set_bit = |k: usize| { let (word, bit) = layout_addr_filter_slot(k); + // SAFETY: this thread's own filter, no other reference live; the + // tables borrowed around it are different thread-locals. unsafe { (*hint.filter.get())[word] |= bit; } + }; + for k in hot_layout_slot_masks().borrow().keys() { + set_bit(*k); + } + for k in hot_typed_layouts().borrow().keys() { + set_bit(*k); + } + hint.sets.set(0); +} + +/// Live keys past which the 4,096-bit sketch stops being an accelerator. +/// +/// The filter is a one-hash bitmap, so `n` live keys leave it answering +/// "may hold" for about `1 - e^(-n/4096)` of all addresses: 12 % at 512 keys, +/// 63 % at 4,096, and indistinguishable from "always yes" past ~16k. cc holds +/// **162,258** (`PERRY_LAYOUT_DIAG`, one 400-character reply), i.e. every bit +/// set, every probe positive, and every rebuild an O(live keys) walk that +/// restores exactly the all-ones state it started from. +/// +/// Sizing the filter up is not available here: the geometry and hash are +/// mirrored in generated code (`perry-codegen`'s +/// `emit_gated_forget_object_layout`), so widening it is a codegen change, and +/// a sketch that discriminated at 162k keys would need ~1.5 Mbit — 190 KB of +/// inline thread-local storage on every thread, to serve a workload that has +/// already lost the fast path. What is available is to stop *paying* for a +/// gate that cannot pay back: past this occupancy the filter is set to all +/// ones, which is the conservative answer it would have reached anyway, and +/// the walk is skipped. Nothing downstream changes behaviour — `may_hold` is +/// a hint whose `true` every caller already handles. +/// +/// Four times the bit count is deliberately far past the point where the +/// filter merely *degrades*: at 16,384 keys its false-positive rate is 98.2 %, +/// so a rebuilt filter still proves absence for under one address in fifty +/// while costing a walk of every live key. Below that the filter is left +/// exactly as it was — a workload holding a few thousand records keeps the +/// selectivity it has today, and this branch never fires for it. +#[inline] +fn layout_addr_filter_saturating_occupancy() -> usize { + LAYOUT_ADDR_FILTER_BITS * 4 +} + +/// Set every bit: "may hold" for any address. Conservative by construction — +/// the filter's `false` is the only load-bearing answer. +fn layout_addr_filter_saturate() { + let hint = hot_per_object_layout_hint(); + // SAFETY: this thread's own filter, no other reference live. + unsafe { + (*hint.filter.get()).fill(u64::MAX); } hint.sets.set(0); } @@ -809,6 +924,15 @@ pub(in crate::gc) fn transfer_per_object_descriptor(old_user: usize, new_user: u return false; } let mut typed = hot_typed_layouts().borrow_mut(); + // The flag and the filter above are shared with `LAYOUT_SLOT_MASKS`, so a + // full mask table drags every relocation in here even when this map is + // empty — which is cc's steady state (`PERRY_LAYOUT_DIAG`: typed=0, + // masks=162,258). An empty map has nothing to remove at either address, so + // the two hashes below are pure loss; the `len` test that proves it is one + // load. #9792. + if typed.is_empty() { + return false; + } typed.remove(&new_user); match typed.remove(&old_user) { Some(layout) => { diff --git a/crates/perry-runtime/src/hot_diag.rs b/crates/perry-runtime/src/hot_diag.rs index faa49fba65..b4bd5238ee 100644 --- a/crates/perry-runtime/src/hot_diag.rs +++ b/crates/perry-runtime/src/hot_diag.rs @@ -324,6 +324,154 @@ fn ic_sink() -> &'static Option { }) } +// --------------------------------------------------------------------------- +// Per-object layout tables: occupancy vs the address filter that gates them +// --------------------------------------------------------------------------- + +static LAYOUT_SINK: OnceLock> = OnceLock::new(); +static LAYOUT_ON: AtomicBool = AtomicBool::new(false); + +fn layout_sink() -> &'static Option { + LAYOUT_SINK.get_or_init(|| { + let sink = sink_from_env("PERRY_LAYOUT_DIAG"); + LAYOUT_ON.store(sink.is_some(), Ordering::Relaxed); + sink + }) +} + +/// Is the per-object-layout occupancy instrument armed? +#[inline] +pub fn layout_on() -> bool { + if LAYOUT_SINK.get().is_none() { + layout_sink(); + } + LAYOUT_ON.load(Ordering::Relaxed) +} + +/// One collection's view of the per-object layout tables and the 4096-bit +/// address filter that is supposed to keep evacuation off them. +/// +/// The question this exists to settle: `layout_addr_filter_may_hold` is +/// documented for "one or two entries … ~0.05 % false-positive rate", and +/// `transfer_per_object_descriptor` / `transfer_per_object_slot_mask` return +/// early only when it says no. If the tables hold far more keys than the +/// filter has bits, it answers "maybe" for every address, both early returns +/// stop firing, and every evacuated object pays the full two-map hash path — +/// while nothing in the system says so out loud. +/// +/// `keys` is also what a filter rebuild used to cost: one `Vec` of +/// every live key per prune, plus a second full walk to recount the young +/// records. +#[derive(Default)] +pub struct LayoutDiag { + prunes: u64, + typed_len: usize, + masks_len: usize, + typed_max: usize, + masks_max: usize, + /// Set bits in the address filter, and its capacity in bits. + filter_bits_set: usize, + filter_bits_total: usize, + filter_bits_set_max: usize, + /// Live keys past which a rebuild stops producing a selective filter. + useful_keys: usize, + /// Prunes that rebuilt the filter from the survivors, and prunes that + /// found it already outgrown and skipped the walk. + rebuilt: u64, + outgrown: u64, + /// Keys visited by prunes that DID rebuild — the walk that is still paid. + rebuilt_keys: u64, +} + +crate::perry_thread_local! { + static LAYOUT_DIAG: RefCell = RefCell::new(LayoutDiag::default()); +} + +/// Record one death-prune's occupancy. `rebuilt_filter` says whether this +/// prune rebuilt the address filter from its survivors, or found the tables +/// too full for a 4,096-bit sketch to discriminate and saturated it instead. +pub fn layout_note_prune( + typed_len: usize, + masks_len: usize, + filter_bits_set: usize, + filter_bits_total: usize, + rebuilt_filter: bool, + useful_keys: usize, +) { + LAYOUT_DIAG.with(|d| { + let mut d = d.borrow_mut(); + d.prunes += 1; + d.typed_len = typed_len; + d.masks_len = masks_len; + d.typed_max = d.typed_max.max(typed_len); + d.masks_max = d.masks_max.max(masks_len); + d.filter_bits_set = filter_bits_set; + d.filter_bits_total = filter_bits_total; + d.filter_bits_set_max = d.filter_bits_set_max.max(filter_bits_set); + d.useful_keys = useful_keys; + if rebuilt_filter { + d.rebuilt += 1; + d.rebuilt_keys += (typed_len + masks_len) as u64; + } else { + d.outgrown += 1; + } + let text = d.render(); + if let Some(sink) = layout_sink() { + write_sink(sink, &text); + } + }); +} + +impl LayoutDiag { + fn render(&self) -> String { + use std::fmt::Write as _; + let mut out = String::with_capacity(512); + let pct = |n: usize, d: usize| { + if d == 0 { + 0.0 + } else { + 100.0 * n as f64 / d as f64 + } + }; + let keys = self.typed_len + self.masks_len; + let _ = writeln!( + out, + "[layout-diag] prunes={} keys_now={} (typed={} masks={}) keys_max={} \ + (typed={} masks={})", + self.prunes, + keys, + self.typed_len, + self.masks_len, + self.typed_max + self.masks_max, + self.typed_max, + self.masks_max + ); + let _ = writeln!( + out, + " filter: {}/{} bits set ({:.1} %), max {}/{} ({:.1} %); selective up to \ + {} keys, so this table is {}", + self.filter_bits_set, + self.filter_bits_total, + pct(self.filter_bits_set, self.filter_bits_total), + self.filter_bits_set_max, + self.filter_bits_total, + pct(self.filter_bits_set_max, self.filter_bits_total), + self.useful_keys, + if keys > self.useful_keys { + "OUTGROWN it -- every probe answers `may hold`" + } else { + "within it" + } + ); + let _ = writeln!( + out, + " filter rebuilds={} over {} keys walked; outgrown-and-skipped={}", + self.rebuilt, self.rebuilt_keys, self.outgrown + ); + out + } +} + /// Is the IC-miss instrument armed? One relaxed load once initialised. #[inline] pub fn ic_on() -> bool { From 19a6cd201cc285026160faf4be5d33761b4e4824 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 07:30:14 +0200 Subject: [PATCH 2/5] perf(gc): prune the per-object layout tables from a young-entry log on a minor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A minor's death prune for `LAYOUT_SLOT_MASKS` + `TYPED_LAYOUTS` asked "which owner died?" of every key in tables sized by everything the program ever created. Both of a minor's deadness predicates require the owner to be in the nursery — `owner_is_dead_copied_minor_from_space` demands eden or the active survivor half, `PostTraceProbe::owner_is_dead` on a minor demands an in-arena, untenured `HeapGeneration::Nursery` address — so an owner that was old at the last prune is still old and the visit cannot remove anything. Measured on a compiled claude-code streamed reply (`PERRY_GC_DIAG=1`, counter on a measurement-only branch): 6,792,375 entries visited across 109 minor prunes, of which at most 125,367 (1.85 %) could possibly have died; median `young_before/visited` 0.0000; 40 of those 109 minors had NOTHING that could have died while carrying 39 % of all visits. 54x over-visit at 3300 characters, 25x at 400. `dead <= young_before` held on 152/152 minor prunes. So both maps take the young-entry log of #9754 (`gc/young_log.rs`), kept in the existing `PerObjectLayoutHint` hot slot so no new thread-local is declared and every writer arms it with a TLS resolution it has already paid for: * every writer notes a key `layout_key_may_be_nursery` admits BEFORE the entry becomes findable (rule 1) — the two insert wrappers, the in-borrow mask mint in `layout_note_slot`, and both per-object move hooks; * a minor walks the log: a logged key in neither map is stale and drops, a present dead key is removed from both maps, a live key is re-logged only while still young, so a promoted owner leaves the log for good; * a full prune keeps its whole-table walk (old owners DO die in a full trace) and rebuilds the log from the survivors it is already classifying; * under `debug_assertions` the young prune re-derives the candidate set from the maps and panics on any young key the log does not name (rule 2); * `note_walk` records logged/visited/kept/table_len per prune, so `[gc-young-log] table=gc.layout_tables` prints the skip and the tests read it back (rule 3). The address filter is deliberately NOT rebuilt on a minor: the whole-table walk that rebuilt it is what this removes, its `false` is the only load-bearing answer, and a stale set bit is a false positive. The amortised rebuild in `layout_addr_filter_add` and the full prune keep it selective. Why this table pays where #9754's scanners did not: a scanner keeps `addr_is_minor_relevant`, true for `Longlived` by design, and cc allocates its shape-key arrays longlived, so those logs never drain (`kept/logged` median 1.000, and `closure.dynamic_props` is a 2.56x regression there). A prune excludes `Longlived` and `Old` both, and cc promotes every survivor after one survival. Same mechanism, opposite sign, decided by the predicate. `YoungLog::clear` loses its `#[cfg(test)]`: a prune that early-returns on the emptiness proof must drop the log with it, or a workload that repeatedly fills and empties the maps between collections accumulates stale keys for ever. --- changelog.d/9841-layout-prune-young-log.md | 43 ++++ crates/perry-runtime/src/gc/dead_owner.rs | 2 +- crates/perry-runtime/src/gc/layout.rs | 16 +- crates/perry-runtime/src/gc/layout_tables.rs | 230 +++++++++++++++++- .../src/gc/tests/young_log_tests.rs | 131 ++++++++++ crates/perry-runtime/src/gc/young_log.rs | 7 +- 6 files changed, 417 insertions(+), 12 deletions(-) create mode 100644 changelog.d/9841-layout-prune-young-log.md diff --git a/changelog.d/9841-layout-prune-young-log.md b/changelog.d/9841-layout-prune-young-log.md new file mode 100644 index 0000000000..5804e50beb --- /dev/null +++ b/changelog.d/9841-layout-prune-young-log.md @@ -0,0 +1,43 @@ +**A minor's per-object layout death-prune now walks a young-entry log instead +of both tables** — on a compiled claude-code streamed reply it visited +**6,792,375 entries where at most 125,367 (1.85 %) could possibly have died, +and on 37 % of minors nothing could have died at all.** + +`prune_dead_per_object_layout_owners` asks "which owners died?" of every key +in `LAYOUT_SLOT_MASKS` + `TYPED_LAYOUTS`, tables sized by everything the +program ever created (~66k live keys on cc, from a history far larger). But +both of a minor's deadness predicates require the owner to be in the nursery: +`owner_is_dead_copied_minor_from_space` demands eden or the active survivor +half, and `PostTraceProbe::owner_is_dead` on a minor demands an in-arena, +untenured `HeapGeneration::Nursery` address. An owner that was old at the last +prune is still old, so the walk over it cannot remove anything. + +So the two maps get the young-entry log of #9754 (`gc/young_log.rs`): every +writer notes a key whose owner `layout_key_may_be_nursery` admits before the +entry becomes findable, a minor prunes from the log, and a survivor is +re-logged only while it is still young — a promoted owner leaves the log and +no later minor visits it again. A full prune keeps its whole-table walk (old +owners do die in a full trace) and rebuilds the log from the survivors it is +already classifying, at no extra pass. + +**Why this table pays where the scanners of #9754 did not.** Read back per +table on an unmodified binary, that PR's four converted tables are a net 0.78x +on cc and `closure.dynamic_props` is a 2.56x regression, because a scanner +keeps `addr_is_minor_relevant` — true for `Longlived` **by design**, since a +longlived object can point at a young one — and cc allocates its shape-key +arrays longlived, so those logs never drain (`kept/logged` median 1.000). A +prune's predicate is `layout_key_may_be_nursery`, which excludes `Longlived` +**and** `Old`; cc's tenuring promotes every survivor after one survival, so a +key leaves this log after one minor. Same mechanism, opposite sign, decided +entirely by which predicate the walk keeps on. The measured over-visit is 54x +at a 3300-character reply and 25x at 400, with `dead <= young_before` on +152/152 minor prunes — the empirical proof that the log's predicate is a sound +superset of what a minor can kill. + +Rule 2 of the design travels with it: under `debug_assertions` the young prune +re-derives the candidate set from the authoritative maps and panics on any +young key the log does not name, so deleting an arming site is a red test +rather than a dead owner's record surviving in silence. The in-borrow mask +mint in `layout_note_slot` — the dominant insert path on cc, and the one site +that published a young record without counting it — is armed for the first +time here. diff --git a/crates/perry-runtime/src/gc/dead_owner.rs b/crates/perry-runtime/src/gc/dead_owner.rs index 563e404c93..482392c3d2 100644 --- a/crates/perry-runtime/src/gc/dead_owner.rs +++ b/crates/perry-runtime/src/gc/dead_owner.rs @@ -336,7 +336,7 @@ pub(super) const DEAD_KEY_PRUNES: &[DeadKeyPrune] = &[ table: "LAYOUT_SLOT_MASKS + TYPED_LAYOUTS", owner: DeadKeyOwner::Any, prune: crate::gc::layout_tables::prune_dead_per_object_layout_owners, - young_prune: None, + young_prune: Some(crate::gc::layout_tables::prune_dead_per_object_layout_owners_young), }, // Re-keyed by the per-object move hook, not by a metadata visitor. DeadKeyPrune { diff --git a/crates/perry-runtime/src/gc/layout.rs b/crates/perry-runtime/src/gc/layout.rs index 5ee4f87cb7..74ae34248f 100644 --- a/crates/perry-runtime/src/gc/layout.rs +++ b/crates/perry-runtime/src/gc/layout.rs @@ -954,11 +954,23 @@ pub(crate) fn layout_note_slot(parent_user: usize, slot_index: usize, value_bits } else { let mut mask = LayoutSlotMask::Inline(0); mask.set_slot(slot_index); + // The one insert site that holds its own `borrow_mut`, + // so it maintains the address filter, the young log + // and the young-record count inline too. The log lives + // in the hint, not in this map, so arming it here + // takes no second borrow — and it goes BEFORE the + // insert (`gc/young_log.rs` rule 1). Before #9841 this + // site published a young record without counting it; + // on cc it is the DOMINANT insert path (`TYPED_LAYOUTS` + // is empty there), so it is where a missing arm would + // do the most damage. + let young = super::layout_tables::arm_young_layout_key(parent_user); masks.insert(parent_user, mask); mark_per_object_layouts_nonempty(); - // The one insert site that holds its own `borrow_mut`, - // so it maintains the address filter inline too. super::layout_tables::layout_addr_filter_note(parent_user); + if young { + super::layout_tables::count_new_young_layout_record(); + } set_layout_state(header, GC_LAYOUT_SIDE_MASK); } } else { diff --git a/crates/perry-runtime/src/gc/layout_tables.rs b/crates/perry-runtime/src/gc/layout_tables.rs index 420c2b1df0..97da20152f 100644 --- a/crates/perry-runtime/src/gc/layout_tables.rs +++ b/crates/perry-runtime/src/gc/layout_tables.rs @@ -76,6 +76,16 @@ pub(in crate::gc) struct PerObjectLayoutHint { /// on every new nursery-keyed insert; made exact again by /// [`recount_young_layout_records`] after each collection's death prune. pub(in crate::gc) young_records: Cell, + /// #9754-style young-entry log for BOTH per-object maps + /// (`gc/young_log.rs`): the keys whose owner may still sit on a page a + /// minor can act on. A minor's death prune walks this instead of the + /// maps — an owner that was old at the last prune is still old, so only + /// a logged key can be found dead by a minor. + /// + /// It lives here, in the same hot slot as the flag and the filter, so a + /// writer arms it with the thread-local resolution it has already paid + /// for, and so nothing new is declared for `tls_hot::fill` to resolve. + pub(in crate::gc) young_keys: RefCell>, } impl PerObjectLayoutHint { @@ -85,10 +95,14 @@ impl PerObjectLayoutHint { sets: Cell::new(0), filter: std::cell::UnsafeCell::new([0u64; LAYOUT_ADDR_FILTER_WORDS]), young_records: Cell::new(0), + young_keys: RefCell::new(crate::gc::young_log::YoungLog::new()), } } } +/// The `[gc-young-log]` / `young_log::last_walk` row name for the two maps. +pub(in crate::gc) const LAYOUT_YOUNG_LOG_NAME: &str = "gc.layout_tables"; + impl Drop for PerObjectLayoutHint { fn drop(&mut self) { // The ownership bit and its teardown live in this ONE TLS value. The @@ -155,12 +169,28 @@ fn layout_key_may_be_nursery(addr: usize) -> bool { ) } -/// A NEW per-object record was keyed by `user_ptr`. +/// A per-object record is ABOUT to be keyed by `user_ptr`: if the owner sits +/// where a minor could kill it, log the key. Rule 1 of `gc/young_log.rs` — +/// note BEFORE the entry is findable. Returns that youngness so the caller can +/// bump the young-record count once it knows the insert was fresh, without a +/// second classification. #[inline] -fn note_new_layout_record(user_ptr: usize) { +pub(in crate::gc) fn arm_young_layout_key(user_ptr: usize) -> bool { if !layout_key_may_be_nursery(user_ptr) { - return; + return false; } + hot_per_object_layout_hint() + .young_keys + .borrow_mut() + .note(user_ptr); + true +} + +/// A NEW nursery-keyed record was published: keep the inline allocator's gate +/// ([`PERRY_YOUNG_LAYOUT_RECORDS`]) conservative until the next prune makes it +/// exact. +#[inline] +pub(in crate::gc) fn count_new_young_layout_record() { let hint = hot_per_object_layout_hint(); if let Some(next) = hint.young_records.get().checked_add(1) { hint.young_records.set(next); @@ -168,6 +198,33 @@ fn note_new_layout_record(user_ptr: usize) { } } +/// The flag proved BOTH maps empty, so every key the log still names is +/// stale. Dropping them here is what keeps the log bounded: a prune that +/// early-returns on the emptiness proof never drains it, so a workload that +/// repeatedly fills and empties the maps between collections would otherwise +/// accumulate one dead key per insert for ever. +#[cold] +fn drop_stale_young_layout_log() { + hot_per_object_layout_hint().young_keys.borrow_mut().clear(); +} + +/// A record is being re-keyed to `new_user` by the per-object move hook +/// (`transfer_per_object_*`), which runs during evacuation — i.e. BEFORE the +/// copied minor's prune, so the key this notes is one the prune will classify +/// in this very collection. +/// +/// Logged unconditionally: the destination is a to-space survivor (young), a +/// promoted address (old), or mid-evacuation not yet classifiable. Noting it +/// without asking is correct (the prune classifies once and an old key simply +/// drops) and keeps a page-map probe out of the evacuation loop. +#[inline] +fn arm_moved_layout_key(new_user: usize) { + hot_per_object_layout_hint() + .young_keys + .borrow_mut() + .note(new_user); +} + /// Publish this thread's young-record count and the delta to the process /// total. The count itself is derived by the death prune's single pass over /// the live keys (all cycle kinds), so promotion (a key moving to an old page) @@ -192,6 +249,7 @@ fn publish_young_layout_records(live: u32) { /// inline allocator's gate reads that instead of probing. pub(in crate::gc) fn prune_dead_per_object_layout_owners(is_dead_owner: &dyn Fn(usize) -> bool) { if !per_object_layouts_maybe_nonempty() { + drop_stale_young_layout_log(); return; } // ONE pass over each table, not three. The old shape visited every live @@ -217,6 +275,12 @@ pub(in crate::gc) fn prune_dead_per_object_layout_owners(is_dead_owner: &dyn Fn( layout_addr_filter_saturate(); } let mut young: u32 = 0; + // A full walk is authoritative, so it also REBUILDS the young log — from + // the survivors it is classifying anyway, at the cost of one `push` per + // young key and no extra pass (`young_log.rs`: "a full-scope scanner + // walks the whole table as before and REBUILDS the log from what it + // found"). + let mut kept = hint.young_keys.borrow_mut().take_spare(); let mut keep = |key: usize| { if is_dead_owner(key) { return false; @@ -233,6 +297,7 @@ pub(in crate::gc) fn prune_dead_per_object_layout_owners(is_dead_owner: &dyn Fn( } if layout_key_may_be_nursery(key) { young = young.saturating_add(1); + kept.push(key); } true }; @@ -248,6 +313,23 @@ pub(in crate::gc) fn prune_dead_per_object_layout_owners(is_dead_owner: &dyn Fn( typed.retain(|key, _| keep(*key)); had && typed.is_empty() }; + // A full walk is authoritative: rebuild the young log from the tables + // (same shape as the shape/descriptor full scanners). + { + let mut log = hint.young_keys.borrow_mut(); + let _ = log.take_sorted(); + log.extend(kept); + } + crate::gc::young_log::note_walk( + LAYOUT_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: false, + logged: occupancy as u64, + visited: occupancy as u64, + kept: u64::from(young), + table_len: occupancy as u64, + }, + ); publish_young_layout_records(young); // Runs last: when it finds both tables empty it disarms the flag, zeroes // the young count published above and clears the filter, which is the @@ -258,6 +340,134 @@ pub(in crate::gc) fn prune_dead_per_object_layout_owners(is_dead_owner: &dyn Fn( } } +/// [`prune_dead_per_object_layout_owners`] for a MINOR (`DEAD_KEY_PRUNES` +/// `young_prune`). +/// +/// # Why this is sound +/// +/// A minor's two deadness predicates both require the owner to be in the +/// nursery: `owner_is_dead_copied_minor_from_space` demands eden or the active +/// survivor half, and `PostTraceProbe::owner_is_dead` on a minor demands an +/// in-arena, untenured `HeapGeneration::Nursery` address. So the only keys a +/// minor can remove are the ones [`layout_key_may_be_nursery`] admits — which +/// is a strict SUPERSET of both (it also admits an unclassifiable address, and +/// classifies from the same page map). Every writer notes such a key before +/// the entry becomes findable, and every walk re-logs a survivor that is still +/// young, so the log names every candidate and the walk loses nothing. +/// +/// That predicate is the whole difference between this conversion and the +/// scanner conversions of #9754: a scanner keeps `addr_is_minor_relevant`, +/// which admits `Longlived` **by design** (a longlived object can point at a +/// young one), whereas a prune asks who DIED and therefore excludes +/// `Longlived` and `Old` both. +/// +/// A logged key that is in neither map is stale (moved away, removed) and +/// drops; a present key whose owner is dead is removed from both maps; a live +/// key is re-logged iff its owner is still young, so a promoted owner leaves +/// the log and no later minor visits it again. +/// +/// The address filter is NOT rebuilt here — the whole-table walk that rebuilt +/// it is exactly what this replaces. Its `false` is the only load-bearing +/// answer and a stale set bit is a false positive, so leaving bits behind is +/// safe; the amortised rebuild in [`layout_addr_filter_add`] and the full +/// prune keep it selective. +pub(in crate::gc) fn prune_dead_per_object_layout_owners_young( + is_dead_owner: &dyn Fn(usize) -> bool, +) { + if !per_object_layouts_maybe_nonempty() { + drop_stale_young_layout_log(); + return; + } + let hint = hot_per_object_layout_hint(); + let table_len = + (hot_layout_slot_masks().borrow().len() + hot_typed_layouts().borrow().len()) as u64; + // Rule 2 (`gc/young_log.rs`): re-derive the candidate set from the + // authoritative maps and refuse to run a partial walk that would miss one. + // A miss is a writer that published a young-keyed record without arming + // the log, which in release would silently keep a dead owner's record. + #[cfg(debug_assertions)] + { + let relevant: Vec = { + let masks = hot_layout_slot_masks().borrow(); + let typed = hot_typed_layouts().borrow(); + masks + .keys() + .chain(typed.keys()) + .copied() + .filter(|key| layout_key_may_be_nursery(*key)) + .collect() + }; + hint.young_keys + .borrow() + .debug_assert_logged(LAYOUT_YOUNG_LOG_NAME, &relevant); + } + let mut logged = 0u64; + let mut visited = 0u64; + // The record count is per MAP ENTRY, as the full prune counts it: a key + // present in both maps is two records and one log entry. + let mut young: u32 = 0; + let mut kept = hint.young_keys.borrow_mut().take_spare(); + let (masks_emptied, typed_emptied) = { + let mut masks = hot_layout_slot_masks().borrow_mut(); + let mut typed = hot_typed_layouts().borrow_mut(); + let had_masks = !masks.is_empty(); + let had_typed = !typed.is_empty(); + loop { + // Re-drained in a loop so a note made while this walk runs (the + // move hooks fire from inside a collection) is not lost. + let batch = hint.young_keys.borrow_mut().take_sorted(); + if batch.is_empty() { + break; + } + logged += batch.len() as u64; + for key in batch { + let in_masks = masks.contains_key(&key); + let in_typed = typed.contains_key(&key); + if !in_masks && !in_typed { + continue; + } + visited += 1; + if is_dead_owner(key) { + if in_masks { + masks.remove(&key); + } + if in_typed { + typed.remove(&key); + } + continue; + } + if layout_key_may_be_nursery(key) { + young = young + .saturating_add(u32::from(in_masks)) + .saturating_add(u32::from(in_typed)); + kept.push(key); + } + } + } + (had_masks && masks.is_empty(), had_typed && typed.is_empty()) + }; + let kept_len = kept.len() as u64; + hint.young_keys.borrow_mut().extend(kept); + crate::gc::young_log::note_walk( + LAYOUT_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: true, + logged, + visited, + kept: kept_len, + table_len, + }, + ); + publish_young_layout_records(young); + // Runs last, as in the full prune: with both maps empty it disarms the + // flag, zeroes the count published above and clears the filter. + refresh_per_object_layouts_flag(masks_emptied || typed_emptied); + if crate::hot_diag::layout_on() { + // `rebuilt_filter = false`: a young prune never rebuilds it. + layout_diag_note_prune(false); + } +} + /// `PERRY_LAYOUT_DIAG`'s per-prune sample. Out of line and behind /// [`crate::hot_diag::layout_on`] so an unarmed build pays one relaxed load. #[cold] @@ -831,12 +1041,14 @@ pub(in crate::gc) fn refresh_per_object_layouts_flag(touched_map_emptied: bool) pub(in crate::gc) fn typed_layouts_insert(user_ptr: usize, descriptor: TypedLayoutDescriptor) { mark_per_object_layouts_nonempty(); layout_addr_filter_add(user_ptr); + // Armed BEFORE the insert makes the entry findable (young-log rule 1). + let young = arm_young_layout_key(user_ptr); let fresh = hot_typed_layouts() .borrow_mut() .insert(user_ptr, descriptor) .is_none(); - if fresh { - note_new_layout_record(user_ptr); + if fresh && young { + count_new_young_layout_record(); } } @@ -845,12 +1057,14 @@ pub(in crate::gc) fn typed_layouts_insert(user_ptr: usize, descriptor: TypedLayo pub(in crate::gc) fn slot_masks_insert(user_ptr: usize, mask: LayoutSlotMask) { mark_per_object_layouts_nonempty(); layout_addr_filter_add(user_ptr); + // Armed BEFORE the insert makes the entry findable (young-log rule 1). + let young = arm_young_layout_key(user_ptr); let fresh = hot_layout_slot_masks() .borrow_mut() .insert(user_ptr, mask) .is_none(); - if fresh { - note_new_layout_record(user_ptr); + if fresh && young { + count_new_young_layout_record(); } } @@ -936,6 +1150,7 @@ pub(in crate::gc) fn transfer_per_object_descriptor(old_user: usize, new_user: u typed.remove(&new_user); match typed.remove(&old_user) { Some(layout) => { + arm_moved_layout_key(new_user); typed.insert(new_user, layout); drop(typed); layout_addr_filter_add(new_user); @@ -956,6 +1171,7 @@ pub(in crate::gc) fn transfer_per_object_slot_mask(old_user: usize, new_user: us let mut masks = hot_layout_slot_masks().borrow_mut(); masks.remove(&new_user); if let Some(mask) = masks.remove(&old_user) { + arm_moved_layout_key(new_user); masks.insert(new_user, mask); drop(masks); layout_addr_filter_add(new_user); diff --git a/crates/perry-runtime/src/gc/tests/young_log_tests.rs b/crates/perry-runtime/src/gc/tests/young_log_tests.rs index db66fadbf6..40878a6040 100644 --- a/crates/perry-runtime/src/gc/tests/young_log_tests.rs +++ b/crates/perry-runtime/src/gc/tests/young_log_tests.rs @@ -592,3 +592,134 @@ fn installing_an_external_shape_id_arms_the_family_log() { "the family must have followed the keys array" ); } + +// -------------------------------------------------- per-object layout tables +// +// #9841: the DEATH PRUNE of `LAYOUT_SLOT_MASKS + TYPED_LAYOUTS`, not a root +// scanner. Its predicate is `layout_key_may_be_nursery`, which excludes +// `Longlived` AND `Old` — strictly stronger than the scanners' +// `addr_is_minor_relevant` — so an old-keyed record is not merely cheap to +// visit, it is provably impossible for a minor to remove. + +use crate::gc::layout_tables::{test_per_object_layout_present, LAYOUT_YOUNG_LOG_NAME}; + +/// A nursery object whose header says POINTER_FREE and which then takes a +/// pointer store — the mutator path that mints a mask from inside +/// `layout_note_slot`'s own `borrow_mut` (WRITER 3). On cc that is the +/// dominant insert path: `TYPED_LAYOUTS` is empty there and every one of the +/// ~66k live keys is a `LAYOUT_SLOT_MASKS` entry. +fn young_masked_object() -> usize { + let obj = crate::object::js_object_alloc(0, 8); + crate::object::js_object_set_field(obj, 0, crate::value::JSValue::number(1.0)); + crate::object::js_object_set_field(obj, 1, crate::value::JSValue::number(2.0)); + crate::gc::layout_clear_for_ptr(obj as usize); + unsafe { crate::gc::layout_init_pointer_free(obj as *mut u8) }; + let child = crate::string::js_string_from_bytes(b"late-pointer".as_ptr(), 12); + crate::object::js_object_set_field(obj, 1, crate::value::JSValue::string_ptr(child)); + assert!( + test_per_object_layout_present(obj as usize), + "premise: the in-place mask mint published a per-object record" + ); + obj as usize +} + +/// WRITER 3's arming site. Delete `arm_young_layout_key` from +/// `gc/layout.rs`'s in-borrow mint and this goes red: under +/// `debug_assertions` on the log-completeness re-derivation, and in release +/// on the record the young prune can no longer see. +#[test] +fn dead_young_masked_owner_is_pruned_through_the_layout_log() { + let _guard = CopyingNurseryTestGuard::new(1); + // One rooted young object so the minor has real work; the owner is not it. + js_shadow_slot_set(0, string_bits(young_leaf())); + + let dead = young_masked_object(); + + let _ = gc_collect_minor(); + + assert!( + !test_per_object_layout_present(dead), + "the dead young owner's per-object layout record must be pruned from the log" + ); + let row = walk(LAYOUT_YOUNG_LOG_NAME); + assert!( + row.partial, + "a copying minor must take the young-scoped prune: {row:?}" + ); + assert!( + row.visited >= 1, + "the logged key must have been visited: {row:?}" + ); +} + +/// WRITER 4's arming site (`transfer_per_object_slot_mask`, which runs during +/// evacuation and therefore BEFORE this collection's prune). Delete its +/// `arm_moved_layout_key` and the re-derivation panics here on the to-space +/// key. +#[test] +fn surviving_young_masked_owner_is_rekeyed_and_stays_logged() { + let _guard = CopyingNurseryTestGuard::new(1); + + let obj = young_masked_object(); + js_shadow_slot_set(0, ptr_bits(obj)); + + let _ = gc_collect_minor(); + + let after = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + assert_ne!(after, obj, "the rooted owner must have been evacuated"); + assert!( + test_per_object_layout_present(after), + "the mask must follow its owner to the new address" + ); + assert!( + !test_per_object_layout_present(obj), + "the stale from-space key must be gone" + ); + let row = walk(LAYOUT_YOUNG_LOG_NAME); + assert!(row.partial, "{row:?}"); + assert!( + row.visited >= 1, + "the move hook's key must have been logged and visited: {row:?}" + ); + if crate::arena::pointer_in_nursery(after) { + assert!( + row.kept >= 1, + "a survivor still in the nursery must stay logged: {row:?}" + ); + } +} + +/// Rule 3: the skip has to be observable, or a latch that never fires looks +/// landed. An OLD-keyed record cannot be found dead by any minor, so the +/// young prune must not visit it at all. +#[test] +fn old_layout_records_are_skipped_by_a_minor() { + let _guard = CopyingNurseryTestGuard::new(0); + + // Drain whatever this thread's earlier tests left young, so `visited` + // below is about the record installed after it. + let _ = gc_collect_minor(); + + let (owner, _) = unsafe { alloc_old_test_object(2) }; + crate::gc::layout_tables::slot_masks_insert( + owner as usize, + crate::gc::layout::LayoutSlotMask::from_words(&[1]), + ); + + let _ = gc_collect_minor(); + + assert!( + test_per_object_layout_present(owner as usize), + "an old owner's record must survive a minor" + ); + let row = walk(LAYOUT_YOUNG_LOG_NAME); + assert!(row.partial, "{row:?}"); + assert!(row.table_len >= 1, "{row:?}"); + assert_eq!( + row.visited, 0, + "an old-keyed record is not a candidate for any minor and must not be \ + visited: {row:?}" + ); + + crate::gc::layout_clear_for_ptr(owner as usize); +} diff --git a/crates/perry-runtime/src/gc/young_log.rs b/crates/perry-runtime/src/gc/young_log.rs index 368d146149..7cd1917f43 100644 --- a/crates/perry-runtime/src/gc/young_log.rs +++ b/crates/perry-runtime/src/gc/young_log.rs @@ -145,8 +145,11 @@ impl YoungLog { } } - /// Test-only: the table resets (`test_clear_*`) clear their log with them. - #[cfg(test)] + /// Drop every logged key, keeping both buffers' capacity. For a caller + /// that has just PROVED its table empty: every key in the log is then + /// stale, and a walk that early-returns on that proof would otherwise + /// carry them forward for ever (the table resets `test_clear_*` use it + /// for the same reason). pub(crate) fn clear(&mut self) { self.keys.clear(); self.spare.clear(); From 41a8af7daf4d8c9f155ecd30eb7d67a8668fc3aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 17:29:42 +0200 Subject: [PATCH 3/5] =?UTF-8?q?perf(gc):=20take=20closure.dynamic=5Fprops?= =?UTF-8?q?=20off=20its=20young=20walk=20=E2=80=94=20measured=20worse=20th?= =?UTF-8?q?an=20full?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #9754 gave four side tables a minor-scoped root scan. Measured per table on the compiled claude-code TUI, three of them earn it and this one does not. Both arms of a 3-turn 3300-character run on a quiet host (`[gc-young-log]` rows, 273 and 272 minor cycles): | table | skipped | verdict | |---|---|---| | `gc.layout_tables` | 99.0 % (366,347 visited of 36,938,896) | earns it | | `object.descriptors` | 95.7 % | earns it | | `object.transition_cache` | 83 % | earns it | | `shapes.families+indices` | 80 % | earns it | | **`closure.dynamic_props`** | **2.2-2.7 %** | **WORSE THAN FULL on 223/273 and 217/272 cycles** | The cause is one predicate, and it is why the same technique lands so differently on adjacent tables. A SCANNER's keep-test is `addr_is_minor_relevant`, which returns true for `Longlived` BY DESIGN — a longlived object can point at a young one — so the "relevant" set is close to the whole table and the log never drains: `kept/logged` has median 1.000 here. Multi-round re-logging then makes the young walk visit MORE entries than the full walk it replaces. That is the same finding, and the same fix, as the shape-cache young log dropped for skipping 0 % and costing 35 % more. So every pass over this table is a full pass again. The LOG ITSELF STAYS. `prune_dead_closure_side_table_owners_young` uses it, and a PRUNE's predicate is not the scanner's: it asks who DIED, so it excludes `Longlived` and `Old` both. That asymmetry is the whole point — the same mechanism is a 99.0 % skip on one walk and a 2.2 % skip on another, decided entirely by which predicate the walk keeps on. The full scanner rebuilds the log from what it finds, so the prune's candidate set stays complete. Rule 2 moves with it. The log-completeness re-derivation ran at the top of the minor-scoped scanner; that walk is gone, and the prune is now the log's only consumer, so the machine check that catches a writer publishing without arming moves into the prune. Dropping the scanner without moving it would have deleted the only guard on a log a prune still trusts — the failure would not have been a slow walk but a dead owner's entries surviving in silence. Not yet verified: this file has only been compiled with `debug_assertions` OFF, so the moved `#[cfg(debug_assertions)]` call has not been type-checked and the moved rule 2 has not been observed to fire. --- .../src/closure/dynamic_props.rs | 92 +++++++------------ 1 file changed, 34 insertions(+), 58 deletions(-) diff --git a/crates/perry-runtime/src/closure/dynamic_props.rs b/crates/perry-runtime/src/closure/dynamic_props.rs index d3ea542188..fc4e8f5cbe 100644 --- a/crates/perry-runtime/src/closure/dynamic_props.rs +++ b/crates/perry-runtime/src/closure/dynamic_props.rs @@ -341,6 +341,14 @@ pub(crate) fn prune_dead_closure_side_table_owners(is_dead_closure: &dyn Fn(usiz /// young), so the log is the complete candidate set (#9754). pub(crate) fn prune_dead_closure_side_table_owners_young(is_dead_closure: &dyn Fn(usize) -> bool) { super::prune_dead_closure_box_capture_owners(is_dead_closure); + // Rule 2 lives here now. It used to run at the top of the minor-scoped + // SCANNER; that walk was dropped as measured-worse-than-full (#9841), and + // this prune is the log's remaining consumer, so the machine check that + // catches an un-armed writer has to move with it. Deleting the scanner + // without moving this would have removed the only guard on a log a prune + // still trusts. + #[cfg(debug_assertions)] + debug_assert_closure_young_log_complete(); let candidates = CLOSURE_YOUNG_OWNERS.with(|log| log.borrow_mut().take_sorted()); let mut kept = Vec::with_capacity(candidates.len()); for owner in candidates { @@ -450,15 +458,33 @@ pub(crate) fn visit_closure_static_prototype_slot_mut( /// ajv's `validate.errors = [{ msg }]`) had its element objects freed /// behind the still-live array. /// -/// #9754: a minor-scoped pass (`visitor.young_scope()`) visits only the -/// owners in `CLOSURE_YOUNG_OWNERS`; a full pass walks every owner and -/// rebuilds the log. Both go through [`scan_closure_owner`], so the per-entry -/// work is identical and only the candidate set differs. +/// #9754 gave this scanner a minor-scoped variant that visited only the owners +/// in `CLOSURE_YOUNG_OWNERS`. **#9841 withdrew it**: measured, that walk was +/// worse than the full walk it replaced on the great majority of minor cycles +/// (see the comment in the body). Every pass is a full pass again, and it +/// rebuilds the log for the death prune, which is now the log's only consumer +/// and does earn it. pub fn scan_closure_dynamic_props_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { - if visitor.young_scope() { - scan_closure_side_tables_young(visitor); - return; - } + // #9841: this table's young walk is MEASURED WORSE THAN THE FULL WALK and + // is therefore not taken. On the compiled claude-code TUI (3-turn, + // 3300-char, perrymaster, both arms of the FC2 pair) the young-scoped + // walk skipped only 2.2-2.7 % of the entries a full walk visits and came + // out worse than full on 223 of 273 minor cycles in one arm and 217 of + // 272 in the other. The three tables that keep their young walk in the + // same capture skip 80-99 %. + // + // The reason is one predicate: a SCANNER keeps `addr_is_minor_relevant`, + // which returns true for `Longlived` BY DESIGN (a longlived object can + // point at a young one), so the "relevant" set is close to the whole + // table and the log cannot drain — `kept/logged` has median 1.000 here. + // Multi-round re-logging then makes the young walk visit MORE than the + // full walk it replaces. Same conclusion, same evidence, as the + // shape-cache log dropped for skipping 0 % and costing 35 % more. + // + // The LOG ITSELF STAYS: `prune_dead_closure_side_table_owners_young` uses + // it, and a prune's predicate excludes `Longlived` and `Old` both, which + // is exactly the asymmetry #9841 is about. This full walk rebuilds the log + // from what it finds, so the prune's candidate set stays complete. let mut owners: Vec = Vec::new(); if let Ok(props) = get_closure_props().lock() { owners.extend(props.keys().copied()); @@ -498,56 +524,6 @@ pub fn scan_closure_dynamic_props_roots_mut(visitor: &mut crate::gc::RuntimeRoot ); } -/// The minor-scoped walk: only logged owners. Rounds repeat while visits -/// trigger owner-move hooks that log new keys (`note_young_closure_owner_rekeyed`), -/// which is also what closes the pre-#9754 gap where an entry re-keyed -/// mid-walk was skipped by the mark pass and only rewritten later. -fn scan_closure_side_tables_young(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { - let table_len = { - let props = get_closure_props().lock().map(|m| m.len()).unwrap_or(0); - let prototypes = get_closure_prototypes() - .lock() - .map(|m| m.len()) - .unwrap_or(0); - let deleted = get_closure_deleted_keys() - .lock() - .map(|m| m.len()) - .unwrap_or(0); - (props + prototypes + deleted) as u64 - }; - #[cfg(debug_assertions)] - debug_assert_closure_young_log_complete(); - let mut logged = 0u64; - let mut visited = 0u64; - let mut kept = CLOSURE_YOUNG_OWNERS.with(|log| log.borrow_mut().take_spare()); - loop { - let batch = CLOSURE_YOUNG_OWNERS.with(|log| log.borrow_mut().take_sorted()); - if batch.is_empty() { - break; - } - logged += batch.len() as u64; - for owner in batch { - visited += 1; - let (new_owner, relevant) = scan_closure_owner(visitor, owner); - if relevant { - kept.push(new_owner); - } - } - } - let kept_len = kept.len() as u64; - CLOSURE_YOUNG_OWNERS.with(|log| log.borrow_mut().extend(kept)); - crate::gc::young_log::note_walk( - CLOSURE_YOUNG_LOG_NAME, - crate::gc::young_log::YoungLogWalk { - partial: true, - logged, - visited, - kept: kept_len, - table_len, - }, - ); -} - /// Rule 2 of `gc/young_log.rs`: re-derive the relevant owners from the three /// tables and require the log to name each one. #[cfg(debug_assertions)] From bdd1fc003adbf71426f8bec5cf22a9fa22eac503 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 17:40:49 +0200 Subject: [PATCH 4/5] test(gc): re-point closure.dynamic_props' walk tests at the full-walk contract, and prove the moved rule-2 guard can fail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two problems with the previous commit, both mine. **1. It broke three tests and I did not run them.** `41a8af7da` reversed the walk policy for `closure.dynamic_props` without grepping for the tests that assert it. On a debug-assertions run: 3225 passed, 3 failed — `young_closure_prop_value_is_moved_through_the_log`, `young_value_under_an_old_closure_owner_is_logged_by_the_value` and `old_closure_entries_are_skipped_by_a_minor`, each asserting `row.partial`, i.e. exactly the policy that commit reverses. They fail in release too. Re-pointed at the current contract rather than deleted, because in every case the property under test survives and only the walk that delivers it changed: * `..._is_traced_and_moved_by_a_minor` — a value reachable ONLY through the side table is still traced, evacuated and re-keyed. Now asserts the FULL walk, and that it rebuilds the log for the death prune. * `..._is_traced_by_a_minor` — a young value and prototype under an OLD owner are still traced. Renamed off "is_logged_by_the_value": the log is still armed by the value, but its only consumer is now the prune, which keys on OWNERS, so value-based arming has no reader left. Flagged in the test, not removed — dropping it is a separate change needing its own measurement. * `old_closure_entries_survive_a_minor_full_walk` — was "are skipped by a minor", asserting `visited == 0`. There is no skip left to observe for this table, which IS the finding; it now asserts the full walk and the surviving correctness property. **2. The rule-2 guard I moved had been exercised but never seen to fail.** The layout-prune tests drive the prune and passed, so the moved `debug_assert_closure_young_log_complete` compiles and runs — but "did not fire" is not "can fire", and a check that cannot fail is documentation. `dropping_a_logged_closure_owner_trips_the_prune_rule2_check` drops every logged owner while LEAVING the three tables populated — precisely what a writer publishing without arming leaves behind — and requires the prune to panic with its own named message. Clearing the tables as well would empty the relevant set and the guard would return early, i.e. prove nothing; the new `test_drop_closure_young_log` hook exists to avoid exactly that. It runs in a child process: the panic is raised inside a collection and can cross an `extern "C"` frame and abort rather than unwind, which `#[should_panic]` cannot catch. Asserting on the child's status and stderr is robust to both, and is the isolation pattern `test_armed_per_object_layout_thread_exit_disarms_global_count` already uses. It is `#[cfg(debug_assertions)]`, since a release build has no guard to trip and the child would exit 0. NOT COMPILED. Disk on this box is at 9 GB against a 12 GB floor, so this commit has not been type-checked, formatted-checked beyond `cargo fmt`, or run. It needs a debug `cargo test -p perry-runtime --lib` before it is believed. --- .../src/closure/dynamic_props.rs | 11 ++ crates/perry-runtime/src/closure/mod.rs | 2 + .../src/gc/tests/young_log_tests.rs | 138 ++++++++++++++++-- 3 files changed, 138 insertions(+), 13 deletions(-) diff --git a/crates/perry-runtime/src/closure/dynamic_props.rs b/crates/perry-runtime/src/closure/dynamic_props.rs index fc4e8f5cbe..56516b048c 100644 --- a/crates/perry-runtime/src/closure/dynamic_props.rs +++ b/crates/perry-runtime/src/closure/dynamic_props.rs @@ -1050,6 +1050,17 @@ pub fn closure_delete_own_dynamic_prop(ptr: usize, prop: &str) -> bool { false } +/// Sabotage hook for the rule-2 guard in +/// [`prune_dead_closure_side_table_owners_young`]: drop every logged owner +/// while LEAVING the three tables populated, so the prune's re-derivation +/// finds a minor-relevant owner the log does not name. Clearing the tables +/// too (as `test_clear_closure_side_tables` does) would make the relevant set +/// empty and the check would return early — i.e. it would test nothing. +#[cfg(test)] +pub(crate) fn test_drop_closure_young_log() { + CLOSURE_YOUNG_OWNERS.with(|log| log.borrow_mut().clear()); +} + #[cfg(test)] pub(crate) fn test_clear_closure_side_tables() { CLOSURE_YOUNG_OWNERS.with(|log| log.borrow_mut().clear()); diff --git a/crates/perry-runtime/src/closure/mod.rs b/crates/perry-runtime/src/closure/mod.rs index 20666f8b15..3f7af108f4 100644 --- a/crates/perry-runtime/src/closure/mod.rs +++ b/crates/perry-runtime/src/closure/mod.rs @@ -75,6 +75,8 @@ pub(crate) use box_captures::{ }; #[cfg(test)] pub(crate) use dynamic_props::test_clear_closure_side_tables; +#[cfg(test)] +pub(crate) use dynamic_props::test_drop_closure_young_log; pub(crate) use dynamic_props::{ clear_closure_side_tables_for_dead_ptr, clone_closure_rebind_this, closure_dynamic_props_owner_moved, closure_dynamic_side_tables_nonempty, diff --git a/crates/perry-runtime/src/gc/tests/young_log_tests.rs b/crates/perry-runtime/src/gc/tests/young_log_tests.rs index 40878a6040..3ee73264cc 100644 --- a/crates/perry-runtime/src/gc/tests/young_log_tests.rs +++ b/crates/perry-runtime/src/gc/tests/young_log_tests.rs @@ -56,7 +56,7 @@ fn walk(table: &'static str) -> young_log::YoungLogWalk { // ---------------------------------------------------------------- closures #[test] -fn young_closure_prop_value_is_moved_through_the_log() { +fn young_closure_prop_value_is_traced_and_moved_by_a_minor() { let _guard = CopyingNurseryTestGuard::new(1); gc_register_mutable_root_scanner(crate::closure::scan_closure_dynamic_props_roots_mut); @@ -87,23 +87,30 @@ fn young_closure_prop_value_is_moved_through_the_log() { crate::closure::closure_get_own_dynamic_prop(owner, "memo").is_none(), "the stale owner key must be gone" ); + // #9841 re-pointed this at the CURRENT contract: `closure.dynamic_props` + // takes the FULL walk on a copying minor, because its young walk measured + // worse than full on 223/273 minor cycles. The rooting property above is + // what this test is really for and is unchanged — the value reachable + // only through the side table is still traced and evacuated. What changed + // is which walk does it. let row = walk("closure.dynamic_props"); assert!( - row.partial, - "a copying minor must take the young-scoped walk" + !row.partial, + "closure.dynamic_props takes the FULL walk since #9841: {row:?}" ); - assert!( - row.visited >= 1, - "the logged owner must have been visited: {row:?}" + assert_eq!( + row.visited, row.table_len, + "a full walk visits every owner: {row:?}" ); assert!( row.kept >= 1, - "a survivor still young must stay logged: {row:?}" + "the full walk must REBUILD the log for the death prune, which is now \ + its only consumer: {row:?}" ); } #[test] -fn young_value_under_an_old_closure_owner_is_logged_by_the_value() { +fn young_value_under_an_old_closure_owner_is_traced_by_a_minor() { let _guard = CopyingNurseryTestGuard::new(0); gc_register_mutable_root_scanner(crate::closure::scan_closure_dynamic_props_roots_mut); @@ -125,11 +132,21 @@ fn young_value_under_an_old_closure_owner_is_logged_by_the_value() { & POINTER_MASK) as usize; assert_ne!(proto_after, proto); assert!(crate::arena::pointer_in_nursery(proto_after)); - assert!(walk("closure.dynamic_props").partial); + // The value/prototype rooting above is the property under test and holds + // unchanged. The walk is now the full one (#9841); the log this test was + // named for is still ARMED by the value (`note_young_closure_owner`), but + // its only consumer is now the death prune, which keys on OWNERS, so + // value-based arming has no reader left. Flagged rather than removed + // here: dropping it is a separate change with its own measurement. + let row = walk("closure.dynamic_props"); + assert!( + !row.partial, + "closure.dynamic_props takes the FULL walk since #9841: {row:?}" + ); } #[test] -fn old_closure_entries_are_skipped_by_a_minor() { +fn old_closure_entries_survive_a_minor_full_walk() { let _guard = CopyingNurseryTestGuard::new(0); gc_register_mutable_root_scanner(crate::closure::scan_closure_dynamic_props_roots_mut); @@ -144,12 +161,22 @@ fn old_closure_entries_are_skipped_by_a_minor() { Some(42.0) ); assert!(crate::closure::closure_is_key_deleted(owner, "name")); + // Was: "must not be visited by a minor", asserting the young walk's skip. + // #9841 withdrew that skip for this table — it was measured at 2.2-2.7 % + // and worse than full on the great majority of minor cycles — so the + // contract is now the opposite and is asserted as such. The surviving + // property is the one above: an old owner's entries are intact after a + // minor. There is no skip left to observe for this table, which is + // exactly the finding. let row = walk("closure.dynamic_props"); - assert!(row.partial); + assert!( + !row.partial, + "closure.dynamic_props takes the FULL walk since #9841: {row:?}" + ); assert!(row.table_len >= 2, "{row:?}"); assert_eq!( - row.visited, 0, - "an old owner with no heap values must not be visited by a minor: {row:?}" + row.visited, row.table_len, + "a full walk visits every owner, old ones included: {row:?}" ); } @@ -723,3 +750,88 @@ fn old_layout_records_are_skipped_by_a_minor() { crate::gc::layout_clear_for_ptr(owner as usize); } + +// ------------------------------------------------- rule 2, proved able to fail +// +// #9841 moved `closure.dynamic_props`'s log-completeness re-derivation out of +// the minor-scoped SCANNER (withdrawn) and into +// `prune_dead_closure_side_table_owners_young`, which is now the log's only +// consumer. A guard that has been EXERCISED and never fired is not yet a +// guard: the campaign's own rule is that a check which cannot fail is +// documentation. This drives it to failure deliberately. +// +// It runs in a CHILD PROCESS because the expected outcome is a panic raised +// inside a collection. Depending on the frames between the prune and the test +// body that panic can cross an `extern "C"` boundary and abort the process +// ("panic in a function that cannot unwind"), which `#[should_panic]` cannot +// catch. Asserting on the child's exit status and stderr is robust to both +// outcomes, and is the same isolation pattern +// `test_armed_per_object_layout_thread_exit_disarms_global_count` uses. + +#[cfg(debug_assertions)] +const RULE2_SABOTAGE_ENV: &str = "PERRY_TEST_CLOSURE_YOUNG_LOG_SABOTAGE_CHILD"; +#[cfg(debug_assertions)] +const RULE2_SABOTAGE_TEST: &str = + "gc::tests::young_log_tests::dropping_a_logged_closure_owner_trips_the_prune_rule2_check"; + +// Rule 2 is `#[cfg(debug_assertions)]`, so in a release test build there is no +// guard to trip and the child would exit 0. Gating the test the same way keeps +// it honest instead of red on the release rotation. +#[cfg(debug_assertions)] +#[test] +fn dropping_a_logged_closure_owner_trips_the_prune_rule2_check() { + if std::env::var_os(RULE2_SABOTAGE_ENV).is_some() { + let _guard = CopyingNurseryTestGuard::new(1); + crate::closure::test_clear_closure_side_tables(); + // One rooted young object so the minor has real work to do. + js_shadow_slot_set(0, string_bits(young_leaf())); + + // A YOUNG closure owner with an entry: `addr_is_minor_relevant` holds + // for it, so the prune's re-derivation puts it in `relevant` and the + // log is required to name it. + let owner = young_closure(); + crate::closure::closure_set_dynamic_prop(owner, "memo", 42.0); + + // THE SABOTAGE: drop the log while leaving the tables populated — + // exactly what a writer that publishes without arming would leave + // behind. Clearing the tables as well would empty `relevant` and the + // check would return early, i.e. prove nothing. + crate::closure::test_drop_closure_young_log(); + + let _ = gc_collect_minor(); + + // Unreachable when the guard works. + panic!( + "RULE 2 DID NOT FIRE: the prune accepted a log missing a \ + minor-relevant closure owner" + ); + } + + let out = std::process::Command::new(std::env::current_exe().expect("current test binary")) + .arg(RULE2_SABOTAGE_TEST) + .arg("--exact") + .arg("--nocapture") + .env(RULE2_SABOTAGE_ENV, "1") + .output() + .expect("launch the isolated rule-2 sabotage child"); + + let combined = format!( + "{}{}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); + + assert!( + !out.status.success(), + "the child must FAIL: dropping a logged owner has to trip rule 2.\n{combined}" + ); + assert!( + combined.contains("young log for closure.dynamic_props does not name"), + "the child must fail through the RULE 2 assertion, by its own message, \ + not through some unrelated crash.\n{combined}" + ); + assert!( + !combined.contains("RULE 2 DID NOT FIRE"), + "the prune ran to completion with an incomplete log.\n{combined}" + ); +} From 0a427a39f2d3ad08c0be0c57b27e0d329f525c80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 17:51:44 +0200 Subject: [PATCH 5/5] test(gc): assert the full walk's own table_len, not the young walk's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `old_closure_entries_survive_a_minor_full_walk` failed on `assert!(row.table_len >= 2)` with `YoungLogWalk { partial: false, logged: 1, visited: 1, kept: 0, table_len: 1 }`. It is the assertion, not the semantics, and the reason is a field-name collision I carried over without checking. **`table_len` means two different things in this one table's two walks.** The young walk computed `props.len() + prototypes.len() + deleted.len()`; this fixture's single owner appears in `props` AND `deleted_keys`, so it counted 2. The full walk builds a deduped OWNER vec, so the same state counts 1. I moved the assertion from one walk to the other and kept the old number. **`kept=0` with `logged=1` is correct and is not a stale log entry.** On a FULL row `logged` is set to `table_len` by construction (`logged: table_len, visited: table_len`), so it means "one owner considered", not "one entry still logged". The log after the walk holds `kept` entries, and 0 is right: an old owner whose only value is a number and whose only other state is a deleted key has nothing a minor can act on, so it must not be re-logged. That property is now asserted (`kept < table_len`) instead of being an unexplained field in a failure message. Test-only. No product code changes. The collision cuts the safe way for the verdict that motivated the revert, and that is worth stating rather than leaving to be rechecked. `young_log_an.py` derives `skipped = table_len * passes - visited` and flags `visited > table_len * passes` as worse-than-full. In young mode `table_len` is the INFLATED sum-of-three-maps, so it makes worse-than-full HARDER to trigger and makes the reported skip percentage LARGER than the truth. The measured `closure.dynamic_props` result — 2.2-2.7 % skipped, worse than full on 223/273 and 217/272 cycles — is therefore conservative in both directions, and the real case for taking that table off its young walk is stronger, not weaker. --- .../src/gc/tests/young_log_tests.rs | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/crates/perry-runtime/src/gc/tests/young_log_tests.rs b/crates/perry-runtime/src/gc/tests/young_log_tests.rs index 3ee73264cc..1bbc26b37a 100644 --- a/crates/perry-runtime/src/gc/tests/young_log_tests.rs +++ b/crates/perry-runtime/src/gc/tests/young_log_tests.rs @@ -173,11 +173,30 @@ fn old_closure_entries_survive_a_minor_full_walk() { !row.partial, "closure.dynamic_props takes the FULL walk since #9841: {row:?}" ); - assert!(row.table_len >= 2, "{row:?}"); + // `table_len >= 2` here was inherited from the YOUNG walk and is wrong for + // the full one: the two walks of this same table compute `table_len` + // differently. The young walk used `props.len() + prototypes.len() + + // deleted.len()` — this fixture's single owner appears in two of those + // maps, so it counted 2. The full walk builds a deduped OWNER vec, so the + // same state counts 1. One owner, one entry. + assert!(row.table_len >= 1, "{row:?}"); assert_eq!( row.visited, row.table_len, "a full walk visits every owner, old ones included: {row:?}" ); + // The property worth pinning, and the one the `kept=0` in the failing row + // was actually reporting: an OLD owner whose only value is a number and + // whose only other state is a deleted key has nothing a minor can act on, + // so the full walk must NOT re-log it. `kept` counts what goes back into + // the log for the death prune; this owner must not be in it. + // + // (`logged` on a FULL row is not "still logged" — the full walk sets + // `logged: table_len` by construction, so `logged=1` here means "one owner + // considered", not "one entry left behind". Nothing stale is implied.) + assert!( + row.kept < row.table_len, + "an old owner with no minor-relevant value must not be re-logged: {row:?}" + ); } #[test]