From ca2d1c54c6b72f4eeadd3d8864d014db983b2a71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 05:54:04 +0200 Subject: [PATCH 1/7] perf(gc): young-entry logs for the side-table root scanners MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A minor-scoped root scan — the copying minor's preflight/mark/rewrite passes and a budgeted `GcCollectionKind::Minor` trace — can neither move nor sweep an old-generation object, so a side-table entry whose key and values are all old is a provable no-op for it. Every registered scanner still walked its whole table on every such pass: on the compiled claude-code TUI that is ~35k shape families, ~120k descriptors and ~13k closure-prop owners per walk, three walks per copying minor, 41 minors per streamed reply, all reporting `slots=0` — 34–56 ms of scanner time per minor (`[gc-scanner-profile]`, 2026-09-04), and the same walk again in every budgeted minor's initial root scan and final remark. Each of the five tables that dominated that profile — closure dynamic props/prototypes/deleted keys, string-keyed descriptors, the shape family + slot-index maps, the transition cache and the shape cache — now keeps a young-entry log (`gc/young_log.rs`): the keys of entries that may hold a pointer a minor can act on (nursery, longlived, malloc-GC). Every writer notes the key BEFORE publishing the entry; a minor-scoped scanner visits only the logged keys, with the same per-entry body as the full walk, and re-logs an entry iff it is still relevant afterwards; a full trace walks everything and rebuilds the log. The copied-minor and fallback-minor dead-owner prunes of the same tables iterate the log too (only a young owner can be dead on a minor, and a young owner is always logged). The visitor carries the scope (`RuntimeRootVisitor::young_scope`, set for the copying passes and for a minor-only budgeted trace). Three rules from the design note (perry-young-gc-fixed-cost.md): 1. arm before publish — each note precedes the insert; 2. machine-check the writer set — under `debug_assertions` a minor-scoped walk first re-derives the relevant set from the authoritative table and panics on any key the log does not name; this caught two sites while landing (the migrate-after-delete slot-index insert, and the from-space index key that outlives its family's mark-pass move); 3. a skip needs a counter — `[gc-young-log]` prints per table and cycle how many keys were logged / visited / kept and the table size, and the tests read the rows back. Also: the post-minor `restore_surviving_dirty_coverage` (#5029), which re-walked every slot of every object on the pre-cycle dirty pages, now skips the objects the minor's own dirty scan visited completely (every slot on a dirty page and inside the body) — for those the scan's per-slot re-remembering is the same predicate on the same value, so the walk could only re-insert pages already restored. The budgeted cycle keeps the full walk: it interleaves with the mutator, and a store into an already-dirty page leaves no trace. Under `debug_assertions` the skipped objects are still walked and any page the walk would have added panics; `[gc-restore-coverage]` prints objects walked/skipped and pages added. Tests: `gc::tests::young_log_tests` (per table: a young entry reachable only through the table moves and is re-keyed through the partial walk; an old entry adds no visit; a dead young owner is pruned from the log), plus the whole `gc::` suite (1048) with the rule-2 assertions active. Claude-Session: https://claude.ai/code/session_01YPfnmWZmSpSWpmnoXvH8z2 --- changelog.d/9755-gc-side-table-young-logs.md | 38 ++ .../src/closure/dynamic_props.rs | 251 ++++++++++-- crates/perry-runtime/src/closure/mod.rs | 1 + crates/perry-runtime/src/gc/barrier/mod.rs | 33 +- crates/perry-runtime/src/gc/copying.rs | 16 +- crates/perry-runtime/src/gc/cycle.rs | 31 +- crates/perry-runtime/src/gc/dead_owner.rs | 47 ++- crates/perry-runtime/src/gc/mod.rs | 4 + crates/perry-runtime/src/gc/roots.rs | 33 ++ .../perry-runtime/src/gc/scanner_profile.rs | 1 + .../perry-runtime/src/gc/sticky_remembered.rs | 42 ++ crates/perry-runtime/src/gc/tests/mod.rs | 1 + .../src/gc/tests/young_log_tests.rs | 386 ++++++++++++++++++ crates/perry-runtime/src/gc/verify.rs | 91 ++++- crates/perry-runtime/src/gc/young_log.rs | 267 ++++++++++++ .../src/object/descriptor_state.rs | 229 +++++++++++ crates/perry-runtime/src/object/mod.rs | 326 ++++++++++++--- crates/perry-runtime/src/object/shapes.rs | 320 +++++++++++++-- .../src/object/shapes_slot_list.rs | 1 + .../src/object/shapes_test_support.rs | 1 + .../src/object/test_root_accessors.rs | 16 + scripts/gc_rekeyed_key_tables.json | 14 +- 22 files changed, 1999 insertions(+), 150 deletions(-) create mode 100644 changelog.d/9755-gc-side-table-young-logs.md create mode 100644 crates/perry-runtime/src/gc/tests/young_log_tests.rs create mode 100644 crates/perry-runtime/src/gc/young_log.rs diff --git a/changelog.d/9755-gc-side-table-young-logs.md b/changelog.d/9755-gc-side-table-young-logs.md new file mode 100644 index 0000000000..5fdf78524d --- /dev/null +++ b/changelog.d/9755-gc-side-table-young-logs.md @@ -0,0 +1,38 @@ +### Performance + +- **Minor collections no longer walk every runtime side table.** A copying + minor's three root-scan passes — and a budgeted minor's initial root scan + and final remark — visited every entry of the closure dynamic-prop tables, + the string-keyed descriptor tables, the shape family/slot-index maps, the + transition cache and the shape cache on every collection, to discover that + nothing in them pointed at the nursery. On the compiled claude-code TUI + that was ~35k shape families, ~120k descriptors and ~13k closure owners + per walk, 41 minors per streamed reply, all reporting `slots=0`: 34–56 ms + of scanner time per minor. + + Each of those tables now keeps a **young-entry log** + (`crates/perry-runtime/src/gc/young_log.rs`): the keys of entries that may + hold a pointer a minor can act on (nursery, longlived or malloc-GC — an old + object is neither moved nor swept by any minor, and never becomes young + again). Writers note the key before publishing the entry; a minor-scoped + scanner (`RuntimeRootVisitor::young_scope`) visits only the logged keys, + with the same per-entry body as the full walk, and re-logs an entry iff it + is still relevant afterwards; a full trace walks everything and rebuilds + the log. The copied-minor and fallback-minor dead-owner prunes of the same + tables iterate the log as well. Under `debug_assertions` every minor-scoped + walk first re-derives the relevant set from the authoritative table and + panics on a key the log does not name, so a writer that forgets to note is + a red test rather than a silently collected object. `PERRY_GC_DIAG=1` + prints `[gc-young-log]` rows (logged / visited / kept / table size) per + table and cycle. + +- **The post-minor remembered-set coverage restore is proportional to what + the dirty scan could not cover.** `restore_surviving_dirty_coverage` + (#5029) re-walked every slot of every object on the pre-cycle dirty pages + after each copying minor. The minor's own dirty scan already re-remembers + each slot it visits with the same predicate, so objects it visited + completely (every pointer slot on a dirty page and inside the body) are + now skipped; multi-page arrays and owners of out-of-body buffers are still + walked. Debug builds walk the skipped objects too and panic if the walk + would have added a page. `[gc-restore-coverage]` reports objects + walked/skipped and pages added. diff --git a/crates/perry-runtime/src/closure/dynamic_props.rs b/crates/perry-runtime/src/closure/dynamic_props.rs index 4e6a96cec9..24587fd61b 100644 --- a/crates/perry-runtime/src/closure/dynamic_props.rs +++ b/crates/perry-runtime/src/closure/dynamic_props.rs @@ -95,6 +95,38 @@ fn get_closure_props() -> &'static Mutex> { CLOSURE_PROPS.get_or_init(|| Mutex::new(new_ptr_hash_map())) } +crate::perry_thread_local! { + /// #9754: this thread's young-entry log for the three closure side tables + /// (`CLOSURE_PROPS`, `CLOSURE_STATIC_PROTOTYPES`, `CLOSURE_DELETED_KEYS`) — + /// the owners whose entry may hold a pointer a minor can act on, as key + /// or as value. Thread-local although the tables are process-global: an + /// entry's addresses belong to the inserting thread's heap, and only that + /// thread's minors can move or free them. See `gc/young_log.rs`. + static CLOSURE_YOUNG_OWNERS: std::cell::RefCell> = + const { std::cell::RefCell::new(crate::gc::young_log::YoungLog::new()) }; +} + +const CLOSURE_YOUNG_LOG_NAME: &str = "closure.dynamic_props"; + +/// Rule 1 of `gc/young_log.rs`: log `owner` BEFORE the entry is published +/// when the owner or the value being stored can matter to a minor. +#[inline] +fn note_young_closure_owner(owner: usize, value_bits: u64) { + if crate::gc::young_log::addr_is_minor_relevant(owner) + || crate::gc::young_log::bits_are_minor_relevant(value_bits) + { + CLOSURE_YOUNG_OWNERS.with(|log| log.borrow_mut().note(owner)); + } +} + +/// A re-keyed entry keeps whatever values it had, so the new owner is logged +/// unconditionally; the next minor-scoped walk drops it if nothing in it is +/// relevant any more. +#[inline] +fn note_young_closure_owner_rekeyed(new_owner: usize) { + CLOSURE_YOUNG_OWNERS.with(|log| log.borrow_mut().note(new_owner)); +} + per_test_global! { /// #3655: keys deleted off a closure via `delete fn.name` etc. /// @@ -123,6 +155,7 @@ pub fn closure_mark_key_deleted(ptr: usize, key: &str) { if ptr == 0 { return; } + note_young_closure_owner(ptr, 0); if let Ok(mut map) = get_closure_deleted_keys().lock() { map.entry(ptr).or_default().insert(key.to_string()); } @@ -177,6 +210,7 @@ pub fn closure_set_static_prototype(closure_ptr: usize, proto_bits: u64) { return; } let mut slot_addr = 0usize; + note_young_closure_owner(closure_ptr, proto_bits); if let Ok(mut map) = get_closure_prototypes().lock() { let slot = map.entry(closure_ptr).or_insert(0); *slot = proto_bits; @@ -301,10 +335,29 @@ pub(crate) fn prune_dead_closure_side_table_owners(is_dead_closure: &dyn Fn(usiz } } +/// [`prune_dead_closure_side_table_owners`] for a MINOR: only a young owner +/// can be dead, and a young owner is always in the young log (it was noted +/// at insert and is re-logged by every minor-scoped walk while it stays +/// 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); + let candidates = CLOSURE_YOUNG_OWNERS.with(|log| log.borrow_mut().take_sorted()); + let mut kept = Vec::with_capacity(candidates.len()); + for owner in candidates { + if is_dead_closure(owner) { + clear_closure_side_tables_for_dead_ptr(owner); + } else { + kept.push(owner); + } + } + CLOSURE_YOUNG_OWNERS.with(|log| log.borrow_mut().extend(kept)); +} + pub(crate) fn closure_dynamic_props_owner_moved(old_owner: usize, new_owner: usize) { if old_owner == 0 || new_owner == 0 || old_owner == new_owner { return; } + note_young_closure_owner_rekeyed(new_owner); if let Ok(mut props) = get_closure_props().lock() { if let Some(old_props) = props.remove(&old_owner) { merge_closure_prop_map(&mut props, new_owner, old_props); @@ -396,21 +449,153 @@ pub(crate) fn visit_closure_static_prototype_slot_mut( /// transitive contents were reachable only via the side table (e.g. /// 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. pub fn scan_closure_dynamic_props_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { - let prop_owners = get_closure_props() + if visitor.young_scope() { + scan_closure_side_tables_young(visitor); + return; + } + let mut owners: Vec = Vec::new(); + if let Ok(props) = get_closure_props().lock() { + owners.extend(props.keys().copied()); + } + if let Ok(prototypes) = get_closure_prototypes().lock() { + owners.extend(prototypes.keys().copied()); + } + if let Ok(deleted) = get_closure_deleted_keys().lock() { + owners.extend(deleted.keys().copied()); + } + owners.sort_unstable(); + owners.dedup(); + let table_len = owners.len() as u64; + // A full walk is authoritative: rebuild the log from what it finds. + // Notes made by owner-move hooks while the walk runs land in the emptied + // log and are kept — they name entries this walk already visited under + // their old key, so the duplicate is harmless. + let _ = CLOSURE_YOUNG_OWNERS.with(|log| log.borrow_mut().take_sorted()); + let mut kept = Vec::new(); + for owner in owners { + 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: false, + logged: table_len, + visited: table_len, + kept: kept_len, + table_len, + }, + ); +} + +/// 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 = Vec::new(); + 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)] +fn debug_assert_closure_young_log_complete() { + use crate::gc::young_log::{addr_is_minor_relevant, bits_are_minor_relevant}; + let mut relevant = Vec::new(); + if let Ok(props) = get_closure_props().lock() { + for (&owner, entry) in props.iter() { + if addr_is_minor_relevant(owner) + || entry + .values + .values() + .any(|value| bits_are_minor_relevant(value.to_bits())) + { + relevant.push(owner); + } + } + } + if let Ok(prototypes) = get_closure_prototypes().lock() { + for (&owner, &proto_bits) in prototypes.iter() { + if addr_is_minor_relevant(owner) || bits_are_minor_relevant(proto_bits) { + relevant.push(owner); + } + } + } + if let Ok(deleted) = get_closure_deleted_keys().lock() { + for &owner in deleted.keys() { + if addr_is_minor_relevant(owner) { + relevant.push(owner); + } + } + } + CLOSURE_YOUNG_OWNERS.with(|log| { + log.borrow() + .debug_assert_logged(CLOSURE_YOUNG_LOG_NAME, &relevant) + }); +} + +/// Visit one owner's entries in all three tables — the per-entry body both +/// walks share. Returns the owner's post-visit key and whether the entry can +/// still matter to a minor (its key or any value is not old). +fn scan_closure_owner( + visitor: &mut crate::gc::RuntimeRootVisitor<'_>, + owner: usize, +) -> (usize, bool) { + use crate::gc::young_log::{addr_is_minor_relevant, bits_are_minor_relevant}; + let mut relevant = false; + let mut current_owner = owner; + + if let Some(mut closure_props) = get_closure_props() .lock() .ok() - .map(|props| props.keys().copied().collect::>()) - .unwrap_or_default(); - for owner in prop_owners { - let Some(mut closure_props) = get_closure_props() - .lock() - .ok() - .and_then(|mut props| props.remove(&owner)) - else { - continue; - }; - + .and_then(|mut props| props.remove(&owner)) + { // Metadata key rewrite. Only fires in rewrite-phase modes; mark phases // return `false` here without recording the key as a root (so the // side-table entry doesn't itself keep the closure alive). @@ -422,58 +607,52 @@ pub fn scan_closure_dynamic_props_roots_mut(visitor: &mut crate::gc::RuntimeRoot // value was forwarded. for value in closure_props.values.values_mut() { visitor.visit_nanbox_f64_slot(value); + relevant |= bits_are_minor_relevant(value.to_bits()); } if new_owner == owner { new_owner = forwarded_heap_owner(owner).unwrap_or(owner); } - + current_owner = new_owner; if let Ok(mut props) = get_closure_props().lock() { merge_closure_prop_map(&mut props, new_owner, closure_props); } } - let prototype_owners = get_closure_prototypes() + if let Some(mut proto_bits) = get_closure_prototypes() .lock() .ok() - .map(|prototypes| prototypes.keys().copied().collect::>()) - .unwrap_or_default(); - for owner in prototype_owners { - let Some(mut proto_bits) = get_closure_prototypes() - .lock() - .ok() - .and_then(|mut prototypes| prototypes.remove(&owner)) - else { - continue; - }; - + .and_then(|mut prototypes| prototypes.remove(&owner)) + { let mut new_owner = owner; visitor.visit_metadata_usize_slot(&mut new_owner); visitor.visit_nanbox_u64_slot(&mut proto_bits); + relevant |= bits_are_minor_relevant(proto_bits); if new_owner == owner { new_owner = forwarded_heap_owner(owner).unwrap_or(owner); } - + current_owner = new_owner; if let Ok(mut prototypes) = get_closure_prototypes().lock() { prototypes.insert(new_owner, proto_bits); } } + // #3655: re-key the deleted-keys side table when a closure moves. The // entries are pure metadata (string keys, no JS references), so the // metadata-key visitor only records a re-key; nothing to trace. - let mut moved_deleted = Vec::new(); if let Ok(mut deleted) = get_closure_deleted_keys().lock() { - for owner in deleted.keys().copied().collect::>() { + if deleted.contains_key(&owner) { let mut new_owner = owner; - if visitor.visit_metadata_usize_slot(&mut new_owner) { - moved_deleted.push((owner, new_owner)); - } - } - for (old_owner, new_owner) in moved_deleted { - if let Some(keys) = deleted.remove(&old_owner) { - deleted.entry(new_owner).or_default().extend(keys); + if visitor.visit_metadata_usize_slot(&mut new_owner) && new_owner != owner { + if let Some(keys) = deleted.remove(&owner) { + deleted.entry(new_owner).or_default().extend(keys); + } + current_owner = new_owner; } } } + + relevant |= addr_is_minor_relevant(current_owner); + (current_owner, relevant) } /// Check if a raw pointer points to a ClosureHeader by checking CLOSURE_MAGIC at offset 12. @@ -852,6 +1031,7 @@ pub(crate) fn closure_set_via_function_prototype_descriptor( /// Set a dynamic property on a closure. pub fn closure_set_dynamic_prop(ptr: usize, prop: &str, value: f64) { + note_young_closure_owner(ptr, value.to_bits()); if let Ok(mut props) = get_closure_props().lock() { let closure_props = props.entry(ptr).or_default(); closure_props.insert(prop.to_string(), value); @@ -890,6 +1070,7 @@ pub fn closure_delete_own_dynamic_prop(ptr: usize, prop: &str) -> bool { #[cfg(test)] pub(crate) fn test_clear_closure_side_tables() { + CLOSURE_YOUNG_OWNERS.with(|log| log.borrow_mut().clear()); if let Ok(mut props) = get_closure_props().lock() { props.clear(); } diff --git a/crates/perry-runtime/src/closure/mod.rs b/crates/perry-runtime/src/closure/mod.rs index 01f3e6e22d..8b4e6d489f 100644 --- a/crates/perry-runtime/src/closure/mod.rs +++ b/crates/perry-runtime/src/closure/mod.rs @@ -79,6 +79,7 @@ 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, closure_set_via_function_prototype_descriptor, prune_dead_closure_side_table_owners, + prune_dead_closure_side_table_owners_young, visit_closure_dynamic_prop_value_slots_mut, visit_closure_static_prototype_slot_mut, }; pub use dynamic_props::{ diff --git a/crates/perry-runtime/src/gc/barrier/mod.rs b/crates/perry-runtime/src/gc/barrier/mod.rs index 35c37c0454..e81106e98e 100644 --- a/crates/perry-runtime/src/gc/barrier/mod.rs +++ b/crates/perry-runtime/src/gc/barrier/mod.rs @@ -523,7 +523,7 @@ pub(super) unsafe fn scan_dirty_header_once( stats.old_objects_considered += 1; stats.valid_roots += 1; stats.dirty_objects_scanned += 1; - scan_dirty_object_slots(header, dirty_pages, stats, visit_slot); + let _ = scan_dirty_object_slots(header, dirty_pages, stats, visit_slot); } #[inline] @@ -564,18 +564,39 @@ pub(super) unsafe fn scan_dirty_slot_with_layout( visit_slot(slot, stats); } +/// Scan the slots of `header` that lie on `dirty_pages`. +/// +/// Returns whether the scan was COMPLETE for the object (#9754): every pointer +/// slot it owns lies on a dirty page AND inside its own allocation. For such +/// an object the per-slot re-remembering the copying minor does in +/// `visit_slot_with_parent` is exactly what the post-cycle +/// `restore_surviving_dirty_coverage` re-derives — the same child predicate on +/// the same post-visit value, and the same `external` verdict (an in-body slot +/// of an old parent is not external under either the page rule used here or +/// the containment rule used there) — so the restore may skip it. An object +/// with a slot on a clean page (a multi-page array) or outside its body (a +/// lazy array's sparse cache, #7538) is reported incomplete and the restore +/// walks it as before. pub(super) unsafe fn scan_dirty_object_slots( header: *mut GcHeader, dirty_pages: &crate::fast_hash::PtrHashSet, stats: &mut RememberedSetTraceStats, visit_slot: &mut dyn FnMut(*mut u64, &mut RememberedSetTraceStats), -) { +) -> bool { + let body_start = header as usize; + let body_end = body_start.saturating_add((*header).size as usize); + let in_body = |slot: *mut u64| { + let addr = slot as usize; + addr >= body_start && addr < body_end + }; + let mut complete = true; visit_gc_rewrite_slot_descriptors(header, |descriptor| unsafe { match descriptor { GcMutableSlotDescriptor::Slot(slot) => { if crate::weakref::is_weak_target_trace_slot(header, slot.slot) { return; } + complete &= in_body(slot.slot) && dirty_pages_contains_addr(dirty_pages, slot.slot as usize); if let Some(layout_kind) = slot.layout_kind { scan_dirty_slot_with_layout( slot.slot, @@ -607,7 +628,13 @@ pub(super) unsafe fn scan_dirty_object_slots( // reads its target's (cold) GC header — prefetch ahead. let parent_has_weak_slots = crate::weakref::header_may_hold_weak_target_slots(header); + let count = range.slot_count(); + if count != 0 { + complete &= in_body(range.slot(0)) && in_body(range.slot(count - 1)); + } + let mut visited_slots = 0usize; for (start, end) in dirty_slot_ranges_for(range, dirty_pages, stats) { + visited_slots += end - start; stats.dirty_slot_ranges_scanned += 1; let mut acct_page = usize::MAX; let mut acct_slots = 0usize; @@ -642,10 +669,12 @@ pub(super) unsafe fn scan_dirty_object_slots( crate::arena::old_page_account_dirty_slots(acct_page, acct_slots); } } + complete &= visited_slots == count; } GcMutableSlotDescriptor::PointerFreeRange(_) => {} } }); + complete } // --------------------------------------------------------------------------- diff --git a/crates/perry-runtime/src/gc/copying.rs b/crates/perry-runtime/src/gc/copying.rs index 6c53d857e8..feecbc6f28 100644 --- a/crates/perry-runtime/src/gc/copying.rs +++ b/crates/perry-runtime/src/gc/copying.rs @@ -750,6 +750,7 @@ pub(super) fn last_untraced_decline_reason() -> &'static str { pub(super) fn scan_remembered_dirty_slots_copying( snapshot: &RememberedDirtySnapshot, + mut covered: Option<&mut crate::fast_hash::PtrHashSet>, mut visit: impl FnMut(*mut u64, *mut GcHeader, bool, &mut RememberedSetTraceStats), ) -> RememberedSetTraceStats { let mut stats = RememberedSetTraceStats { @@ -793,7 +794,12 @@ pub(super) fn scan_remembered_dirty_slots_copying( visit(slot, header, external, stats); changed |= *slot != before; }; - scan_dirty_object_slots(header, &snapshot.dirty_pages, stats, &mut visit_slot); + let complete = scan_dirty_object_slots(header, &snapshot.dirty_pages, stats, &mut visit_slot); + if complete { + if let Some(covered) = covered.as_deref_mut() { + covered.insert(header as usize); + } + } if changed { run_gc_rewrite_hook((*header).obj_type, user); } @@ -1058,7 +1064,7 @@ impl CopiedMinorEligibility { let snapshot = remembered_dirty_snapshot(); let mut dirty_checker = CopyingNurseryPreflight::new(ptrs, CopiedMinorFallbackReason::PinnedYoungDirtySlot); - scan_remembered_dirty_slots_copying(&snapshot, |slot, _header, _external, _stats| unsafe { + scan_remembered_dirty_slots_copying(&snapshot, None, |slot, _header, _external, _stats| unsafe { dirty_checker.check_bits(*slot); }); unsafe { @@ -1343,10 +1349,14 @@ pub(super) fn run_copied_minor_attempt( // arming happens. Skipping it would leave the barrier unarmed for the next // cycle — a missing-edge bug one collection later. let snapshot = remembered_dirty_snapshot(); + // #9754: objects whose every slot the dirty scan visited in-body — the + // post-cycle coverage restore skips them (see `scan_dirty_object_slots`). + let mut dirty_scan_covered = crate::fast_hash::new_ptr_hash_set(); if !untraced { let _phase = super::pin::CopyingWalkPhaseGuard::enter("remembered_set"); let remembered_stats = scan_remembered_dirty_slots_copying( &snapshot, + Some(&mut dirty_scan_covered), |slot, header, external, stats| unsafe { let before = *slot; collector.visit_slot_with_parent(slot, header, external); @@ -1600,7 +1610,7 @@ pub(super) fn run_copied_minor_attempt( remembered_set_clear(); collector.sticky.restore(); if !collector.skip_remembering { - restore_surviving_dirty_coverage(&snapshot); + restore_surviving_dirty_coverage(&snapshot, &dirty_scan_covered, "copying_minor"); } let malloc_freed_bytes = if malloc_sweep_due { let phase_start = trace_phase_start(trace); diff --git a/crates/perry-runtime/src/gc/cycle.rs b/crates/perry-runtime/src/gc/cycle.rs index 33e0252f8c..e787f66aed 100644 --- a/crates/perry-runtime/src/gc/cycle.rs +++ b/crates/perry-runtime/src/gc/cycle.rs @@ -333,6 +333,7 @@ impl MutableRegisteredRootScanState { mut root_sources: Option<&mut RootSourcesTraceStats>, budget: usize, allow_synchronous_scanners: bool, + minor_only: bool, ) -> bool { if !self.recorded_counts { if let Some(sources) = &mut root_sources { @@ -358,7 +359,9 @@ impl MutableRegisteredRootScanState { } let mut remaining = budget; - let mut visitor = RuntimeRootVisitor::for_mark(valid_ptrs); + // #9754: a minor-only trace is a young-scoped visit for the logged + // side tables (`gc/young_log.rs`); a full trace walks everything. + let mut visitor = RuntimeRootVisitor::for_mark_scoped(valid_ptrs, minor_only); while self.scanner_cursor < self.scanners.len() { if remaining == 0 { return false; @@ -648,8 +651,15 @@ impl RootScanCycleState { Some(&mut trace.root_sources), budget, allow_synchronous_scanners, + self.minor_only, + ), + None => state.step( + valid_ptrs, + None, + budget, + allow_synchronous_scanners, + self.minor_only, ), - None => state.step(valid_ptrs, None, budget, allow_synchronous_scanners), }; if done { self.subphase = RootScanSubphase::LegacyRegisteredScanners; @@ -1805,7 +1815,15 @@ impl GcCycleState { sticky.restore(); } if let Some(snapshot) = self.pre_clear_dirty_snapshot.take() { - restore_surviving_dirty_coverage(&snapshot); + // No coverage set: a budgeted cycle interleaves + // with the mutator, and a store into an already + // dirty page leaves no trace, so nothing scanned + // earlier can be declared covered here. + restore_surviving_dirty_coverage( + &snapshot, + &crate::fast_hash::new_ptr_hash_set(), + "budgeted_cycle", + ); } let reclaim_state = self.reclaim_state.as_mut().expect("reclaim state exists"); @@ -1914,6 +1932,13 @@ impl GcCycleState { ReclaimSubphase::Publish => { let reclaim_start = trace_phase_start(&self.trace); self.publish_reclaim_outcome(); + // #9754: per-table young-log rows for this cycle's root + // scans (initial + final remark), labelled by cycle kind. + super::young_log::report_and_reset(if self.minor.is_some() { + "budgeted_minor" + } else { + "budgeted_full" + }); trace_phase_record(&mut self.trace, "reclaim", reclaim_start); self.reclaim_state .as_mut() diff --git a/crates/perry-runtime/src/gc/dead_owner.rs b/crates/perry-runtime/src/gc/dead_owner.rs index 2c617264af..563e404c93 100644 --- a/crates/perry-runtime/src/gc/dead_owner.rs +++ b/crates/perry-runtime/src/gc/dead_owner.rs @@ -226,10 +226,14 @@ pub(super) fn prune_dead_owner_side_tables_post_trace( crate::object::shapes::rotate_old_carrier_epoch_after_full_trace(); } let probe = PostTraceProbe::new(full_trace); + // #9754: a minor can only find a young owner dead (`owner_is_dead` refuses + // tenured and old-generation owners on a minor), so the young-logged + // tables prune from their logs instead of walking. fan_out( &|addr| probe.owner_is_dead(addr, None), &|addr| probe.owner_is_dead(addr, Some(GC_TYPE_CLOSURE)), &|addr| probe.owner_is_dead(addr, Some(GC_TYPE_STRING)), + /* young_only = */ !full_trace, ); // #6182: drop dead weak-target HOLDERS (WeakRef / FinalizationRegistry / // WeakMap-WeakSet entry — all GC_TYPE_OBJECT) from the registry so the @@ -251,6 +255,7 @@ pub(super) fn prune_dead_owner_side_tables_copied_minor() { &|addr| owner_is_dead_copied_minor_from_space(addr, None), &|addr| owner_is_dead_copied_minor_from_space(addr, Some(GC_TYPE_CLOSURE)), &|addr| owner_is_dead_copied_minor_from_space(addr, Some(GC_TYPE_STRING)), + /* young_only = */ true, ); } @@ -279,9 +284,18 @@ pub(super) struct DeadKeyPrune { #[cfg_attr(not(test), allow(dead_code))] pub(super) table: &'static str, pub(super) owner: DeadKeyOwner, - pub(super) prune: fn(&dyn Fn(usize) -> bool), + pub(super) prune: DeadKeyPruneFn, + /// #9754: the same prune restricted to the table's young-entry log + /// (`gc/young_log.rs`). A MINOR can only find a young owner dead, and a + /// young owner is always in the log, so on a minor's fan-out this visits + /// the candidates instead of the whole table. `None` keeps the full walk + /// on every cycle. + pub(super) young_prune: Option, } +/// A prune: drops every entry whose owner the predicate reports dead. +pub(super) type DeadKeyPruneFn = fn(&dyn Fn(usize) -> bool); + /// THE REGISTRY (#8174). /// /// `fan_out` iterates this instead of naming a dozen prunes inline, and @@ -306,11 +320,13 @@ pub(super) const DEAD_KEY_PRUNES: &[DeadKeyPrune] = &[ table: "WASM_MEMORY_BINDINGS", owner: DeadKeyOwner::Any, prune: crate::webassembly::prune_dead_wasm_memory_bindings, + young_prune: None, }, DeadKeyPrune { table: "ARRAY_NAMED_PROPS", owner: DeadKeyOwner::Any, prune: crate::array::prune_dead_array_named_property_owners, + young_prune: None, }, // Re-keyed by the per-object move hook (`transfer_per_object_slot_mask` / // `transfer_per_object_descriptor`), not by a metadata visitor. Dropping @@ -320,17 +336,20 @@ 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, }, // Re-keyed by the per-object move hook, not by a metadata visitor. DeadKeyPrune { table: "ELEMENT_SHAPES", owner: DeadKeyOwner::Any, prune: crate::array::prune_dead_element_shape_owners, + young_prune: None, }, DeadKeyPrune { table: "MAP_ITERATOR_ARRAYS", owner: DeadKeyOwner::Any, prune: crate::map::prune_dead_map_iterator_array_owners, + young_prune: None, }, // Re-keyed by `map_header_moved_for_gc`; a dead Map's squeeze history // serves no cursor. @@ -338,32 +357,38 @@ pub(super) const DEAD_KEY_PRUNES: &[DeadKeyPrune] = &[ table: "MAP_COMPACTION_LOG", owner: DeadKeyOwner::Any, prune: crate::map::prune_dead_map_compaction_log_owners, + young_prune: None, }, DeadKeyPrune { table: "SET_ITERATOR_ARRAYS", owner: DeadKeyOwner::Any, prune: crate::set::prune_dead_set_iterator_array_owners, + young_prune: None, }, DeadKeyPrune { table: "SET_COMPACTION_LOG", owner: DeadKeyOwner::Any, prune: crate::set::prune_dead_set_compaction_log_owners, + young_prune: None, }, DeadKeyPrune { table: "state().descriptors.property_descriptors + .accessor_descriptors", owner: DeadKeyOwner::Any, prune: crate::object::prune_dead_descriptor_owner_entries, + young_prune: Some(crate::object::prune_dead_descriptor_owner_entries_young), }, DeadKeyPrune { table: "ARGUMENTS_OBJECTS", owner: DeadKeyOwner::Any, prune: crate::object::prune_dead_arguments_object_entries, + young_prune: None, }, // Re-keyed by the per-object move hook, not by a metadata visitor. DeadKeyPrune { table: "OBJECT_PROTOTYPES", owner: DeadKeyOwner::Any, prune: crate::object::prototype_chain::prune_dead_object_prototype_owners, + young_prune: None, }, // #6759 C1: shape records are keyed on keys_array addresses; drop the // ones whose keys_array died (memory only — per-hit validation covers @@ -372,33 +397,39 @@ pub(super) const DEAD_KEY_PRUNES: &[DeadKeyPrune] = &[ table: "state().shapes.inner (descriptors + indices)", owner: DeadKeyOwner::Any, prune: crate::object::shapes::prune_dead_shape_keys, + young_prune: Some(crate::object::shapes::prune_dead_shape_keys_young), }, // Re-keyed by the per-object move hook, not by a metadata visitor. DeadKeyPrune { table: "state().exotic_expando.entries", owner: DeadKeyOwner::Any, prune: crate::object::exotic_expando::prune_dead_exotic_expando_owners, + young_prune: None, }, DeadKeyPrune { table: "SYMBOL_PROPERTIES + SYMBOL_PROPERTY_ATTRS", owner: DeadKeyOwner::Any, prune: crate::symbol::prune_dead_symbol_property_owners, + young_prune: None, }, DeadKeyPrune { table: "SYMBOL_POINTERS", owner: DeadKeyOwner::Symbol, prune: crate::symbol::prune_dead_symbol_pointers, + young_prune: None, }, DeadKeyPrune { table: "CLOSURE_PROPS + CLOSURE_STATIC_PROTOTYPES + CLOSURE_DELETED_KEYS + CLOSURE_BOX_CELLS", owner: DeadKeyOwner::Closure, prune: crate::closure::prune_dead_closure_side_table_owners, + young_prune: Some(crate::closure::prune_dead_closure_side_table_owners_young), }, DeadKeyPrune { table: "BUILTIN_CLOSURE_LENGTH + BUILTIN_CLOSURE_NON_CONSTRUCTABLE", owner: DeadKeyOwner::Closure, prune: crate::object::prune_dead_builtin_closure_metadata_owners, + young_prune: None, }, // #8040: `FUNCTION_CLASS_IDS` is keyed by a synthetic-class function // value's closure address, and is REKEYED (not re-derived) when that @@ -408,16 +439,19 @@ pub(super) const DEAD_KEY_PRUNES: &[DeadKeyPrune] = &[ table: "FUNCTION_CLASS_IDS", owner: DeadKeyOwner::Closure, prune: crate::object::prune_dead_function_class_id_keys, + young_prune: None, }, DeadKeyPrune { table: "VM_CONTEXTS + VM_SCRIPTS + VM_FUNCTIONS", owner: DeadKeyOwner::Any, prune: crate::node_vm::prune_dead_vm_owner_entries, + young_prune: None, }, DeadKeyPrune { table: "FILEHANDLE_OBJECT_FDS", owner: DeadKeyOwner::Any, prune: crate::fs::prune_dead_filehandle_fd_entries, + young_prune: None, }, // #8190/#8191/#8192/#8194: four more REKEYED tables that the #8174 audit // found had no death story at all. Each is the #8040 shape — see this @@ -427,27 +461,32 @@ pub(super) const DEAD_KEY_PRUNES: &[DeadKeyPrune] = &[ table: "CONSOLE_INSTANCES", owner: DeadKeyOwner::Any, prune: crate::builtins::prune_dead_console_instance_owners, + young_prune: None, }, DeadKeyPrune { table: "BOXED_PRIMITIVE_PAYLOADS", owner: DeadKeyOwner::Any, prune: crate::builtins::prune_dead_boxed_primitive_payload_owners, + young_prune: None, }, DeadKeyPrune { table: "TRANSITION_CACHE_GLOBAL", owner: DeadKeyOwner::Any, prune: crate::object::prune_dead_transition_cache_entries, + young_prune: Some(crate::object::prune_dead_transition_cache_entries_young), }, DeadKeyPrune { table: "REFLECT_METADATA", owner: DeadKeyOwner::Any, prune: crate::proxy::prune_dead_reflect_metadata_targets, + young_prune: None, }, #[cfg(feature = "node-api-host")] DeadKeyPrune { table: "NODE_API_OBJECT_METADATA", owner: DeadKeyOwner::Any, prune: crate::node_api_host::prune_dead_object_meta_owners, + young_prune: None, }, ]; @@ -455,6 +494,7 @@ fn fan_out( is_dead_owner: &dyn Fn(usize) -> bool, is_dead_closure: &dyn Fn(usize) -> bool, is_dead_symbol: &dyn Fn(usize) -> bool, + young_only: bool, ) { // Interned key pointers cached in the store-plan cache may die in this // collection — flush every cached verdict. Pointer identity only: the @@ -468,6 +508,9 @@ fn fan_out( DeadKeyOwner::Closure => is_dead_closure, DeadKeyOwner::Symbol => is_dead_symbol, }; - (entry.prune)(is_dead); + match entry.young_prune { + Some(young_prune) if young_only => young_prune(is_dead), + _ => (entry.prune)(is_dead), + } } } diff --git a/crates/perry-runtime/src/gc/mod.rs b/crates/perry-runtime/src/gc/mod.rs index 6a7424daca..157f9fcd75 100644 --- a/crates/perry-runtime/src/gc/mod.rs +++ b/crates/perry-runtime/src/gc/mod.rs @@ -162,6 +162,10 @@ mod copying_pointer_set; mod forwarding; /// Per-scanner root attribution for the copied-minor root scan (#7915). mod scanner_profile; +/// #9754: per-side-table young-entry logs (remembered sets for the runtime +/// side tables), so a minor-scoped root scan visits only the entries that +/// can hold a pointer a minor acts on. +pub(crate) mod young_log; mod sticky_remembered; use copying::*; use copying_first_cycle::*; diff --git a/crates/perry-runtime/src/gc/roots.rs b/crates/perry-runtime/src/gc/roots.rs index d243c4e976..c7cbe3bbe7 100644 --- a/crates/perry-runtime/src/gc/roots.rs +++ b/crates/perry-runtime/src/gc/roots.rs @@ -856,6 +856,13 @@ pub(super) enum RuntimeRootVisitMode<'a> { pub struct RuntimeRootVisitor<'a> { pub(super) mode: RuntimeRootVisitMode<'a>, pub(super) root_source_stats: Option<*mut RootSourceSlotTraceStats>, + /// #9754: this visit belongs to a MINOR-scoped pass — the copying minor's + /// preflight/mark/rewrite, or a `GcCollectionKind::Minor` trace's root + /// scan — which neither moves nor sweeps an old-generation object. A + /// side table with a young-entry log (`gc/young_log.rs`) may then visit + /// only its logged entries. A full trace, an evacuation rewrite/verify + /// pass and the legacy copy mode walk everything. + pub(super) young_scope: bool, } impl<'a> RuntimeRootVisitor<'a> { @@ -863,6 +870,18 @@ impl<'a> RuntimeRootVisitor<'a> { Self { mode: RuntimeRootVisitMode::Mark { valid_ptrs }, root_source_stats: None, + young_scope: false, + } + } + + /// `for_mark` for a cycle whose trace is minor-only: old objects are + /// black leaves and their marks are consumed by nothing, so the + /// young-logged side tables may skip their old entries (#9754). + pub(super) fn for_mark_scoped(valid_ptrs: &'a ValidPointerSet, minor_only: bool) -> Self { + Self { + mode: RuntimeRootVisitMode::Mark { valid_ptrs }, + root_source_stats: None, + young_scope: minor_only, } } @@ -870,6 +889,7 @@ impl<'a> RuntimeRootVisitor<'a> { Self { mode: RuntimeRootVisitMode::Rewrite { valid_ptrs }, root_source_stats: None, + young_scope: false, } } @@ -877,6 +897,7 @@ impl<'a> RuntimeRootVisitor<'a> { Self { mode: RuntimeRootVisitMode::CopyingCheck { checker }, root_source_stats: None, + young_scope: true, } } @@ -884,6 +905,7 @@ impl<'a> RuntimeRootVisitor<'a> { Self { mode: RuntimeRootVisitMode::CopyingMark { collector }, root_source_stats: None, + young_scope: true, } } @@ -891,6 +913,7 @@ impl<'a> RuntimeRootVisitor<'a> { Self { mode: RuntimeRootVisitMode::CopyingRewrite { collector }, root_source_stats: None, + young_scope: true, } } @@ -901,6 +924,7 @@ impl<'a> RuntimeRootVisitor<'a> { surface, }, root_source_stats: None, + young_scope: false, } } @@ -908,9 +932,18 @@ impl<'a> RuntimeRootVisitor<'a> { Self { mode: RuntimeRootVisitMode::Copy { mark }, root_source_stats: None, + young_scope: false, } } + /// #9754: true when this pass can only act on non-old objects, so a + /// young-logged side table may confine its walk to the logged entries. + /// See `gc/young_log.rs` for the argument. + #[inline] + pub(crate) fn young_scope(&self) -> bool { + self.young_scope + } + #[inline] pub(super) fn set_root_source_stats( &mut self, diff --git a/crates/perry-runtime/src/gc/scanner_profile.rs b/crates/perry-runtime/src/gc/scanner_profile.rs index 954767b3f9..2e95310269 100644 --- a/crates/perry-runtime/src/gc/scanner_profile.rs +++ b/crates/perry-runtime/src/gc/scanner_profile.rs @@ -132,6 +132,7 @@ pub(super) fn report_and_reset(cycle_label: &str) { if !scanner_profile_enabled() { return; } + super::young_log::report_and_reset(cycle_label); let mut rows = SCANNER_PROFILE.with(|rows| std::mem::take(&mut *rows.borrow_mut())); if rows.is_empty() { return; diff --git a/crates/perry-runtime/src/gc/sticky_remembered.rs b/crates/perry-runtime/src/gc/sticky_remembered.rs index 8e9b2f3acc..f1b0c1e254 100644 --- a/crates/perry-runtime/src/gc/sticky_remembered.rs +++ b/crates/perry-runtime/src/gc/sticky_remembered.rs @@ -54,6 +54,48 @@ impl StickyRememberedSet { } } + /// [`Self::restore`], reporting how many entries were NOT already in the + /// remembered set — the pages this restore genuinely added (#9754). + pub(super) fn restore_counted(&self) -> usize { + let mut added = 0usize; + for &page in &self.old_pages { + // `mark_dirty_old_page`'s return is not "inserted" (its uncached + // arm answers the ever-dirty question), so ask the set first. + let already = super::barrier::DIRTY_OLD_PAGES.with(|s| s.borrow().contains(&page)); + mark_dirty_old_page(page); + if !already { + added += 1; + } + } + for &(header, page) in &self.external_pages { + if mark_dirty_external_slot_page(header, page) { + added += 1; + } + } + added + } + + /// How many of this set's entries the remembered set does NOT hold yet — + /// what `restore` would add. Read-only: the debug check of the coverage + /// restore asks this about objects it skipped. + #[cfg(debug_assertions)] + pub(super) fn count_not_yet_dirty(&self) -> usize { + let old_missing = super::barrier::DIRTY_OLD_PAGES.with(|s| { + let s = s.borrow(); + self.old_pages.iter().filter(|page| !s.contains(page)).count() + }); + let external_missing = super::barrier::EXTERNAL_DIRTY_SLOT_PAGES.with(|s| { + let s = s.borrow(); + self.external_pages + .iter() + .filter(|(header, page)| { + !s.get(page).is_some_and(|headers| headers.contains(header)) + }) + .count() + }); + old_missing + external_missing + } + pub(super) fn extend(&mut self, other: StickyRememberedSet) { self.old_pages.extend(other.old_pages); self.external_pages.extend(other.external_pages); diff --git a/crates/perry-runtime/src/gc/tests/mod.rs b/crates/perry-runtime/src/gc/tests/mod.rs index ad0ae26bcc..ac6d08f01e 100644 --- a/crates/perry-runtime/src/gc/tests/mod.rs +++ b/crates/perry-runtime/src/gc/tests/mod.rs @@ -64,3 +64,4 @@ mod triggers; mod typed_layout_intact_residual; mod u8_inline_cache; mod weak_read_barrier; +mod young_log_tests; diff --git a/crates/perry-runtime/src/gc/tests/young_log_tests.rs b/crates/perry-runtime/src/gc/tests/young_log_tests.rs new file mode 100644 index 0000000000..e21d71413e --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/young_log_tests.rs @@ -0,0 +1,386 @@ +//! #9754 — the side-table young-entry logs (`gc/young_log.rs`). +//! +//! Each table gets the same three-part proof: +//! +//! * a YOUNG entry reachable only through the table is moved by a copying +//! minor and the entry re-keyed — through the minor-scoped walk, which the +//! recorded walk row proves was PARTIAL (visited only the logged keys); +//! * an OLD entry is not visited at all — the row proves the skip fired +//! (`visited == 0` while the table is non-empty), which is rule 3 of the +//! design note: a latch that never skips looks landed while doing nothing; +//! * a DEAD young owner is pruned by the young-only prune. +//! +//! Sabotage contract (rule 2): delete any one `note` call in the tables' +//! writers and the matching "moves" test here goes red — under +//! `debug_assertions` on the log-completeness assertion the walk runs first, +//! and in release on the stale address the un-visited entry keeps. + +use super::super::*; +use super::support::*; + +fn young_closure() -> usize { + let ptr = crate::arena::arena_alloc_gc( + std::mem::size_of::(), + std::mem::align_of::(), + GC_TYPE_CLOSURE, + ); + unsafe { init_test_closure(ptr) }; + ptr as usize +} + +fn old_closure() -> usize { + let ptr = crate::arena::arena_alloc_gc_old( + std::mem::size_of::(), + std::mem::align_of::(), + GC_TYPE_CLOSURE, + ); + unsafe { init_test_closure(ptr) }; + ptr as usize +} + +unsafe fn young_keys_array() -> *mut crate::array::ArrayHeader { + let arr = crate::arena::arena_alloc_gc( + std::mem::size_of::(), + std::mem::align_of::(), + GC_TYPE_ARRAY, + ) as *mut crate::array::ArrayHeader; + (*arr).length = 0; + (*arr).capacity = 0; + arr +} + +fn walk(table: &'static str) -> young_log::YoungLogWalk { + young_log::last_walk(table).unwrap_or_else(|| panic!("no walk recorded for {table}")) +} + +// ---------------------------------------------------------------- closures + +#[test] +fn young_closure_prop_value_is_moved_through_the_log() { + let _guard = CopyingNurseryTestGuard::new(1); + gc_register_mutable_root_scanner(crate::closure::scan_closure_dynamic_props_roots_mut); + + let owner = young_closure(); + js_shadow_slot_set(0, ptr_bits(owner)); + // The value is reachable ONLY through the side table. + let value = young_leaf(); + crate::closure::closure_set_dynamic_prop(owner, "memo", f64::from_bits(string_bits(value))); + + let _ = gc_collect_minor(); + + let owner_after = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + assert_ne!(owner_after, owner, "the rooted owner must have been evacuated"); + let bits = crate::closure::closure_get_own_dynamic_prop(owner_after, "memo") + .expect("entry must follow its owner to the new address") + .to_bits(); + let value_after = (bits & POINTER_MASK) as usize; + assert_eq!(bits & TAG_MASK, STRING_TAG); + assert_ne!(value_after, value, "the value must have been evacuated, not left in from-space"); + assert!(crate::arena::pointer_in_nursery(value_after)); + assert!( + crate::closure::closure_get_own_dynamic_prop(owner, "memo").is_none(), + "the stale owner key must be gone" + ); + let row = walk("closure.dynamic_props"); + assert!(row.partial, "a copying minor must take the young-scoped walk"); + assert!(row.visited >= 1, "the logged owner must have been visited: {row:?}"); + assert!(row.kept >= 1, "a survivor still young must stay logged: {row:?}"); +} + +#[test] +fn young_value_under_an_old_closure_owner_is_logged_by_the_value() { + let _guard = CopyingNurseryTestGuard::new(0); + gc_register_mutable_root_scanner(crate::closure::scan_closure_dynamic_props_roots_mut); + + let owner = old_closure(); + let value = young_leaf(); + crate::closure::closure_set_dynamic_prop(owner, "memo", f64::from_bits(string_bits(value))); + let proto = young_leaf(); + crate::closure::closure_set_static_prototype(owner, string_bits(proto)); + + let _ = gc_collect_minor(); + + let bits = crate::closure::closure_get_own_dynamic_prop(owner, "memo") + .expect("old owner keeps its entry") + .to_bits(); + let value_after = (bits & POINTER_MASK) as usize; + assert_ne!(value_after, value); + assert!(crate::arena::pointer_in_nursery(value_after)); + let proto_after = (crate::closure::closure_static_prototype(owner).expect("prototype kept") + & POINTER_MASK) as usize; + assert_ne!(proto_after, proto); + assert!(crate::arena::pointer_in_nursery(proto_after)); + assert!(walk("closure.dynamic_props").partial); +} + +#[test] +fn old_closure_entries_are_skipped_by_a_minor() { + let _guard = CopyingNurseryTestGuard::new(0); + gc_register_mutable_root_scanner(crate::closure::scan_closure_dynamic_props_roots_mut); + + let owner = old_closure(); + crate::closure::closure_set_dynamic_prop(owner, "count", 42.0); + crate::closure::closure_mark_key_deleted(owner, "name"); + + let _ = gc_collect_minor(); + + assert_eq!( + crate::closure::closure_get_own_dynamic_prop(owner, "count"), + Some(42.0) + ); + assert!(crate::closure::closure_is_key_deleted(owner, "name")); + let row = walk("closure.dynamic_props"); + assert!(row.partial); + 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:?}" + ); +} + +#[test] +fn dead_young_closure_owner_is_pruned_by_the_young_prune() { + let _guard = CopyingNurseryTestGuard::new(1); + gc_register_mutable_root_scanner(crate::closure::scan_closure_dynamic_props_roots_mut); + // 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_closure(); + crate::closure::closure_set_dynamic_prop(dead, "memo", 42.0); + crate::closure::closure_set_static_prototype(dead, crate::value::TAG_NULL); + assert!(crate::closure::closure_get_own_dynamic_prop(dead, "memo").is_some()); + + let _ = gc_collect_minor(); + + assert!( + crate::closure::closure_get_own_dynamic_prop(dead, "memo").is_none(), + "the dead young owner's CLOSURE_PROPS entry must be pruned from the log" + ); + assert!(crate::closure::closure_static_prototype(dead).is_none()); +} + +// -------------------------------------------------------------- descriptors + +#[test] +fn young_accessor_getter_is_moved_through_the_log() { + let _guard = CopyingNurseryTestGuard::new(1); + gc_register_mutable_root_scanner(crate::object::descriptor_state::scan_descriptor_roots_mut); + + let (owner, _) = unsafe { alloc_nursery_test_object(0) }; + let owner = owner as usize; + js_shadow_slot_set(0, ptr_bits(owner)); + // The getter closure is reachable ONLY through the accessor table. + let getter = young_closure(); + crate::object::set_accessor_descriptor( + owner, + "g".to_string(), + crate::object::AccessorDescriptor { + get: ptr_bits(getter), + set: 0, + }, + ); + + let _ = gc_collect_minor(); + + let owner_after = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + assert_ne!(owner_after, owner); + let acc = crate::object::get_accessor_descriptor(owner_after, "g") + .expect("accessor must follow its owner to the new address"); + let getter_after = (acc.get & POINTER_MASK) as usize; + assert_ne!(getter_after, getter, "the getter must have been evacuated"); + assert!(crate::arena::pointer_in_nursery(getter_after)); + assert!(crate::object::get_accessor_descriptor(owner, "g").is_none()); + let row = walk("object.descriptors"); + assert!(row.partial); + assert!(row.visited >= 1, "{row:?}"); +} + +#[test] +fn old_descriptor_owners_are_skipped_by_a_minor() { + let _guard = CopyingNurseryTestGuard::new(0); + gc_register_mutable_root_scanner(crate::object::descriptor_state::scan_descriptor_roots_mut); + + // The descriptor tables are agent state that outlives every test on this + // thread, and the FIRST descriptor install on a thread bootstraps the + // lazy `globalThis` realm (#7975), which installs ~1.8k builtin + // descriptors on young objects. Warm that up, take a minor, then measure + // the delta: the old entry must add index rows but no visit. + let (warm, _) = unsafe { alloc_old_test_object(0) }; + crate::object::set_property_attrs( + warm as usize, + "warm".to_string(), + crate::object::PropertyAttrs::new(false, true, true), + ); + let _ = gc_collect_minor(); + let before = walk("object.descriptors"); + + let (owner, _) = unsafe { alloc_old_test_object(0) }; + let owner = owner as usize; + let getter = old_closure(); + crate::object::set_accessor_descriptor( + owner, + "g".to_string(), + crate::object::AccessorDescriptor { + get: ptr_bits(getter), + set: 0, + }, + ); + crate::object::set_property_attrs( + owner, + "p".to_string(), + crate::object::PropertyAttrs::new(false, true, true), + ); + + let _ = gc_collect_minor(); + + assert_eq!( + crate::object::get_accessor_descriptor(owner, "g").map(|acc| acc.get), + Some(ptr_bits(getter)) + ); + let row = walk("object.descriptors"); + assert!(row.partial); + // The first minor's prune can drop dead realm owners between the two + // walks, so the exact count is `kept` minus whatever died; the new old + // entry can only NOT add to it. + assert!( + row.visited <= before.kept, + "old owner, old getter: the new entry must not add a visit: {before:?} -> {row:?}" + ); + assert!( + row.visited < row.table_len, + "the walk must stay partial: {row:?}" + ); +} + +#[test] +fn dead_young_descriptor_owner_is_pruned_by_the_young_prune() { + let _guard = CopyingNurseryTestGuard::new(1); + gc_register_mutable_root_scanner(crate::object::descriptor_state::scan_descriptor_roots_mut); + js_shadow_slot_set(0, string_bits(young_leaf())); + + let (dead, _) = unsafe { alloc_nursery_test_object(0) }; + let dead = dead as usize; + crate::object::set_property_attrs( + dead, + "p".to_string(), + crate::object::PropertyAttrs::new(false, true, true), + ); + assert!(crate::object::get_property_attrs(dead, "p").is_some()); + + let _ = gc_collect_minor(); + + assert!( + crate::object::get_property_attrs(dead, "p").is_none(), + "the dead young owner's descriptor must be pruned from the log" + ); +} + +// ------------------------------------------------------------------- shapes + +#[test] +fn young_keys_array_family_is_rekeyed_through_the_log() { + let _guard = CopyingNurseryTestGuard::new(1); + gc_register_mutable_root_scanner(crate::object::shapes::scan_shape_table_rekey_mut); + + let keys = unsafe { young_keys_array() }; + js_shadow_slot_set(0, ptr_bits(keys as usize)); + let id = crate::object::shapes::shape_descriptor_ensure(keys, 0, 0) + .expect("shape id"); + assert_eq!( + crate::object::shapes::shape_descriptor_by_id(id).map(|d| d.keys), + Some(keys as u64) + ); + + let _ = gc_collect_minor(); + + let keys_after = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + assert_ne!(keys_after, keys as usize, "the rooted keys array must have moved"); + assert_eq!( + crate::object::shapes::shape_descriptor_by_id(id).map(|d| d.keys), + Some(keys_after as u64), + "the family's descriptor must be re-keyed to the evacuated keys array" + ); + let row = walk("shapes.families+indices"); + assert!(row.partial); + assert!(row.visited >= 1, "{row:?}"); +} + +#[test] +fn old_shape_families_are_skipped_by_a_minor() { + let _guard = CopyingNurseryTestGuard::new(0); + gc_register_mutable_root_scanner(crate::object::shapes::scan_shape_table_rekey_mut); + + let keys = crate::arena::arena_alloc_gc_old( + std::mem::size_of::(), + std::mem::align_of::(), + GC_TYPE_ARRAY, + ) as *mut crate::array::ArrayHeader; + unsafe { + (*keys).length = 0; + (*keys).capacity = 0; + } + let id = crate::object::shapes::shape_descriptor_ensure(keys, 0, 0) + .expect("shape id"); + + let _ = gc_collect_minor(); + + assert_eq!( + crate::object::shapes::shape_descriptor_by_id(id).map(|d| d.keys), + Some(keys as u64) + ); + let row = walk("shapes.families+indices"); + assert!(row.partial); + assert!(row.table_len >= 1, "{row:?}"); + assert_eq!(row.visited, 0, "an old keys array's family must not be visited: {row:?}"); +} + +// ------------------------------------------------------------------- caches + +#[test] +fn young_transition_cache_target_is_rewritten_through_the_log() { + let _guard = CopyingNurseryTestGuard::new(1); + gc_register_mutable_root_scanner(crate::object::scan_transition_cache_roots_mut); + + let keys = unsafe { young_keys_array() } as usize; + js_shadow_slot_set(0, ptr_bits(keys)); + // A predecessor that resolves, or the copied-minor prune retires the entry + // (`shape_descriptor_by_id(0)` is `None`) before the assertion reads it. + let prev = crate::object::shapes::shape_descriptor_ensure(std::ptr::null(), 0, 1) + .expect("shape id"); + crate::object::test_seed_transition_cache_root_for_shape(prev, keys); + + let _ = gc_collect_minor(); + + let keys_after = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + assert_ne!(keys_after, keys); + assert_eq!( + crate::object::test_transition_cache_root(), + keys_after, + "the cached target must be rewritten to the evacuated keys array" + ); + let row = walk("object.transition_cache"); + assert!(row.partial); + assert!(row.visited >= 1, "{row:?}"); + assert!(row.visited < row.table_len, "a 16k-slot table must not be walked whole: {row:?}"); +} + +#[test] +fn young_shape_cache_entry_is_moved_through_the_log() { + let _guard = CopyingNurseryTestGuard::new(0); + gc_register_mutable_root_scanner(crate::object::scan_shape_cache_roots_mut); + + // Reachable ONLY through the cache (which roots it). + let keys = unsafe { young_keys_array() }; + let shape_id = 0x9754_0001; + crate::object::test_seed_shape_cache_root(shape_id, keys); + + let _ = gc_collect_minor(); + + let (inline, overflow) = crate::object::test_shape_cache_root(shape_id); + assert_ne!(overflow, keys as usize, "the overflow entry must have been evacuated"); + assert!(crate::arena::pointer_in_nursery(overflow)); + assert_eq!(inline, overflow, "inline and overflow must agree on the new address"); + let row = walk("object.shape_cache"); + assert!(row.partial); + assert!(row.visited >= 1, "{row:?}"); +} diff --git a/crates/perry-runtime/src/gc/verify.rs b/crates/perry-runtime/src/gc/verify.rs index e5367d25bb..0332bcf3b8 100644 --- a/crates/perry-runtime/src/gc/verify.rs +++ b/crates/perry-runtime/src/gc/verify.rs @@ -212,8 +212,28 @@ pub(super) unsafe fn remember_evacuated_old_copy_young_slots( /// construction. Pages whose every slot now points old (the common case /// after evacuation rewrites) are still dropped, so the remembered set keeps /// shrinking as before. -pub(super) fn restore_surviving_dirty_coverage(snapshot: &RememberedDirtySnapshot) { +/// +/// #9754: `covered` names the objects the cycle's own dirty scan visited +/// COMPLETELY (every pointer slot on a dirty page and inside the body — +/// `scan_dirty_object_slots`). For those, `visit_slot_with_parent` already +/// re-remembered every slot whose post-visit child still needs tracking with +/// the same predicate this walk applies, so re-walking them can only re-insert +/// pages the sticky restore just inserted. They are skipped; the walk is then +/// proportional to the objects the dirty scan could NOT fully cover +/// (multi-page arrays, owners of out-of-body buffers) instead of to every slot +/// on every dirty page. Under `debug_assertions` the skipped objects are +/// walked anyway and any page the walk would have ADDED is a panic — the +/// machine check of the equivalence argument above. +pub(super) fn restore_surviving_dirty_coverage( + snapshot: &RememberedDirtySnapshot, + covered: &crate::fast_hash::PtrHashSet, + cycle_label: &str, +) { let mut sticky = StickyRememberedSet::default(); + let mut walked = 0usize; + let mut skipped = 0usize; + #[cfg(debug_assertions)] + let mut skipped_sticky = StickyRememberedSet::default(); // Mirror scan_remembered_dirty_slots_copying's scan_header guards: the // external dirty entries can carry headers the harness seeded // synthetically, and a dead entry may point at reclaimed memory — never @@ -249,6 +269,13 @@ pub(super) fn restore_surviving_dirty_coverage(snapshot: &RememberedDirtySnapsho }; if !snapshot.dirty_old_pages.is_empty() { crate::arena::old_arena_walk_objects_on_pages(&snapshot.dirty_old_pages, |hp| { + if covered.contains(&(hp as usize)) { + skipped += 1; + #[cfg(debug_assertions)] + debug_visit_covered_parent(hp as *mut GcHeader, &mut skipped_sticky); + return; + } + walked += 1; visit_parent(hp as *mut GcHeader); }); } @@ -257,6 +284,13 @@ pub(super) fn restore_surviving_dirty_coverage(snapshot: &RememberedDirtySnapsho if !seen_external.insert(header_addr) { continue; } + if covered.contains(&header_addr) { + skipped += 1; + #[cfg(debug_assertions)] + debug_visit_covered_parent(header_addr as *mut GcHeader, &mut skipped_sticky); + continue; + } + walked += 1; // External entries may be stale (or, in the GC unit tests, // synthetic). Establish that the address is dereference-safe // WITHOUT touching it: old/longlived arena pages are always @@ -275,7 +309,60 @@ pub(super) fn restore_surviving_dirty_coverage(snapshot: &RememberedDirtySnapsho visit_parent(header_addr as *mut GcHeader); } } - sticky.restore(); + let added = sticky.restore_counted(); + #[cfg(debug_assertions)] + { + let would_add = skipped_sticky.count_not_yet_dirty(); + assert_eq!( + would_add, 0, + "restore_surviving_dirty_coverage: {would_add} page(s) of {skipped} \ + dirty-scan-covered object(s) are not remembered — the dirty scan's \ + per-slot re-remembering disagrees with the coverage walk for an \ + object `scan_dirty_object_slots` reported complete" + ); + } + if crate::gc::gc_diag_enabled() { + eprintln!( + "[gc-restore-coverage] {cycle_label} dirty_pages={} objects_walked={walked} objects_skipped={skipped} pages_added={added}", + snapshot.dirty_pages.len() + ); + } +} + +/// Debug twin of the restore's `visit_parent` for a skipped object: re-derive +/// what the full walk would have remembered so the caller can assert it adds +/// nothing beyond what the dirty scan already restored. +#[cfg(debug_assertions)] +fn debug_visit_covered_parent(header: *mut GcHeader, sticky: &mut StickyRememberedSet) { + unsafe { + if header.is_null() { + return; + } + let arena_parent = plausible_gc_header(header, true); + let malloc_parent = !arena_parent && plausible_gc_header(header, false); + if !arena_parent && !malloc_parent { + return; + } + if (*header).gc_flags & GC_FLAG_FORWARDED != 0 { + return; + } + let user = (header as *mut u8).add(GC_HEADER_SIZE) as usize; + if arena_parent + && !matches!( + crate::arena::classify_heap_generation(user), + crate::arena::HeapGeneration::Old + ) + { + return; + } + visit_gc_rewrite_slots(header, |slot| { + if crate::weakref::is_weak_target_trace_slot(header, slot.slot) { + return; + } + slot.record_layout_read(); + remember_evacuated_old_to_young_slot(sticky, header, slot.slot); + }); + } } pub(super) fn rebuild_evacuated_old_to_young_remembered_set( diff --git a/crates/perry-runtime/src/gc/young_log.rs b/crates/perry-runtime/src/gc/young_log.rs new file mode 100644 index 0000000000..37d0bc0132 --- /dev/null +++ b/crates/perry-runtime/src/gc/young_log.rs @@ -0,0 +1,267 @@ +//! Young-entry logs: per-side-table remembered sets for the copying minor +//! and the budgeted minor-only trace. +//! +//! # The problem this solves +//! +//! Every registered mutable-root scanner walks its whole table on every +//! collection. On the compiled claude-code TUI that is ~35k shape families, +//! ~120k descriptors and ~13k closure-prop owners per walk, three walks per +//! copying minor and two per budgeted minor (initial root scan + final +//! remark), 41+ minors per streamed reply — and every one of those walks found +//! `slots=0`: 34–56 ms per minor of scanner time discovering that nothing in +//! the table pointed at the nursery (`[gc-scanner-profile]`, 2026-09-04). +//! +//! The previous attempt (2026-09-01, reverted) skipped a walk when the +//! VISITOR was inert. That is unsound for the three expensive tables: they +//! ROOT (`visit_nanbox_u64_slot` on accessor get/set, `visit_nanbox_f64_slot` +//! on closure prop values, `visit_usize_slot` on carried keys arrays), and +//! rooting marks in Mark/Copy, so skipping drops live objects. +//! +//! # What a minor can act on +//! +//! A minor-scoped pass — the copying minor's preflight/mark/rewrite passes and +//! the budgeted cycle's `GcCollectionKind::Minor` root scan — neither moves +//! nor sweeps an OLD-generation object. `CopyingNurseryCollector::mark_addr` +//! returns an old address unchanged; `rewrite_raw_addr` follows forwarding +//! records that only a moved (young) object can carry; a minor-only trace's +//! marks on old headers are consumed by nothing (the minor sweep frees young +//! objects only, `PostTraceProbe::owner_is_dead` refuses old owners, and the +//! full path's remembered-set rebuild does not run for minors). So an entry +//! whose key AND every value are old is a provable no-op for every +//! minor-scoped visit. The set of addresses a minor CAN act on is the +//! complement: nursery, longlived (traced through, never swept, not barriered — +//! `barrier_parent_needs_remembering` is false for it) and malloc-GC objects +//! (swept by minors). [`addr_is_minor_relevant`] is that predicate. +//! +//! # The log +//! +//! [`YoungLog`] holds the KEYS of entries that may hold a minor-relevant +//! pointer. It is a log, not an index: duplicates and stale keys are allowed +//! (a stale key is a lookup miss and is dropped; a duplicate is visited twice, +//! idempotently). A minor-scoped scanner walks only the logged keys, visits +//! each entry exactly as the full walk would, and re-logs the entry iff it is +//! still relevant after the visit (a to-space survivor stays young; a promoted +//! object is old and drops out). A full-scope scanner walks the whole table as +//! before and REBUILDS the log from what it found. +//! +//! # The three rules (from `perry-young-gc-fixed-cost.md`) +//! +//! 1. **Arm before publish.** Every writer notes the key BEFORE the entry +//! becomes findable. A note is one page-map probe (`classify_heap_generation`, +//! hot-TLS cached) per pointer; an old key with old values notes nothing. +//! 2. **Machine-check the writer set.** Under `debug_assertions` a minor-scoped +//! walk first re-derives the relevant set from the authoritative table and +//! panics on any relevant entry the log does not name +//! ([`debug_assert_logged`]). The `gc/tests` copying-minor fixtures run this +//! on every collection, so deleting one `note` site is a red test, not a +//! silent leak. +//! 3. **A skip needs a counter.** [`note_walk`] records, per walk, how many +//! entries the log named, how many were visited, how many stayed relevant +//! and how big the table was; `[gc-young-log]` prints them per collection +//! under `PERRY_GC_DIAG=1`, and the tests read them back. +//! +//! Thread model: the shape and descriptor tables are agent-local +//! (`state()`), so their logs live beside them. The closure side tables are +//! process-global mutexed maps; their log is thread-local on purpose — an +//! entry's addresses belong to the heap of the thread that inserted it, and +//! only that thread's minors can act on them, so a foreign thread's walk must +//! neither visit nor drop them. + +use std::cell::RefCell; + +use super::GC_HEADER_SIZE; +use crate::arena::HeapGeneration; +use crate::value::{BIGINT_TAG, POINTER_MASK, POINTER_TAG, STRING_TAG, TAG_MASK}; + +/// Keys of side-table entries that may hold a pointer a minor can act on. +pub(crate) struct YoungLog { + keys: Vec, +} + +impl YoungLog { + pub(crate) const fn new() -> Self { + Self { keys: Vec::new() } + } + + /// Record `key` as possibly minor-relevant. MUST run before the entry it + /// describes becomes findable (rule 1). Adjacent duplicates — a hot + /// `fn.x = …` loop — are collapsed; other duplicates are harmless. + #[inline] + pub(crate) fn note(&mut self, key: K) { + if self.keys.last() != Some(&key) { + self.keys.push(key); + } + } + + /// Take the logged keys, sorted and deduplicated, leaving the log empty. + /// Notes made while the caller walks the batch (owner-move hooks fire + /// from inside a visit) land in the emptied log and are picked up by the + /// caller's next `take_sorted` round. + pub(crate) fn take_sorted(&mut self) -> Vec { + let mut keys = std::mem::take(&mut self.keys); + keys.sort_unstable(); + keys.dedup(); + keys + } + + /// Re-log the keys a walk found still relevant. + pub(crate) fn extend(&mut self, kept: Vec) { + if self.keys.is_empty() { + self.keys = kept; + } else { + self.keys.extend(kept); + } + } + + /// Test-only: the table resets (`test_clear_*`) clear their log with them. + #[cfg(test)] + pub(crate) fn clear(&mut self) { + self.keys.clear(); + } + + /// Rule 2: the log must name every key in `relevant`. `relevant` is the + /// set the caller re-derived from the authoritative table under + /// `debug_assertions`; a miss is a writer that publishes without noting. + #[cfg(debug_assertions)] + pub(crate) fn debug_assert_logged(&self, table: &'static str, relevant: &[K]) + where + K: std::fmt::Debug, + { + if relevant.is_empty() { + return; + } + let mut logged = self.keys.clone(); + logged.sort_unstable(); + logged.dedup(); + for key in relevant { + assert!( + logged.binary_search(key).is_ok(), + "young log for {table} does not name {key:?}, which holds a \ + minor-relevant pointer: a writer of that table publishes \ + without `note`-ing the key first (see gc/young_log.rs rule 1)" + ); + } + } +} + +/// Can a minor-scoped pass act on the object at `addr`? +/// +/// `false` is authoritative for the old generation only: an old object is +/// neither moved nor swept by any minor, and it never becomes young again. +/// Everything a minor moves, marks-through or sweeps answers `true` — +/// nursery (eden + both survivor halves), longlived, and malloc-GC objects. +/// A non-heap word (handle id, foreign-thread address, integer) answers +/// `false` through the exact malloc-registry probe, never a header sniff. +#[inline] +pub(crate) fn addr_is_minor_relevant(addr: usize) -> bool { + if addr == 0 { + return false; + } + match crate::arena::classify_heap_generation(addr) { + HeapGeneration::Old => false, + HeapGeneration::Nursery | HeapGeneration::Longlived => true, + HeapGeneration::Unknown => { + addr > GC_HEADER_SIZE + && super::malloc::gc_malloc_header_is_tracked( + (addr - GC_HEADER_SIZE) as *const super::GcHeader, + ) + } + } +} + +/// [`addr_is_minor_relevant`] for a NaN-boxed value: only the three +/// pointer-carrying tags decode to an address; numbers, booleans, short +/// strings and `undefined` are never relevant. +#[inline] +pub(crate) fn bits_are_minor_relevant(bits: u64) -> bool { + let tag = bits & TAG_MASK; + if tag == POINTER_TAG || tag == STRING_TAG || tag == BIGINT_TAG { + addr_is_minor_relevant((bits & POINTER_MASK) as usize) + } else { + false + } +} + +/// One scanner walk's accounting (rule 3). +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(crate) struct YoungLogWalk { + /// The walk was minor-scoped and visited only the logged keys. + pub(crate) partial: bool, + /// Keys the log named when the walk started (after dedup). + pub(crate) logged: u64, + /// Entries actually visited (a full walk visits the whole table). + pub(crate) visited: u64, + /// Entries still relevant after the visit, i.e. the log size afterwards. + pub(crate) kept: u64, + /// Table size at walk time — `table_len - visited` is the work skipped. + pub(crate) table_len: u64, +} + +crate::perry_thread_local! { + /// Per-table rows since the last `report_and_reset`, keyed by table name. + static WALK_ROWS: RefCell> = + const { RefCell::new(Vec::new()) }; + /// The most recent walk per table, for the tests (never reset). + static LAST_WALK: RefCell> = + const { RefCell::new(Vec::new()) }; +} + +/// Record one walk. Cheap (one push per scanner per pass), so it is not +/// gated; printing is. +pub(crate) fn note_walk(table: &'static str, walk: YoungLogWalk) { + LAST_WALK.with(|rows| { + let mut rows = rows.borrow_mut(); + if let Some(row) = rows.iter_mut().find(|(name, _)| *name == table) { + row.1 = walk; + } else { + rows.push((table, walk)); + } + }); + if !super::gc_diag_enabled() { + return; + } + WALK_ROWS.with(|rows| { + let mut rows = rows.borrow_mut(); + if let Some(row) = rows.iter_mut().find(|(name, _, _)| *name == table) { + row.1.partial &= walk.partial; + row.1.logged += walk.logged; + row.1.visited += walk.visited; + row.1.kept += walk.kept; + row.1.table_len = row.1.table_len.max(walk.table_len); + row.2 += 1; + } else { + rows.push((table, walk, 1)); + } + }); +} + +/// The most recent walk recorded for `table` on this thread. +#[cfg(test)] +pub(crate) fn last_walk(table: &'static str) -> Option { + LAST_WALK.with(|rows| { + rows.borrow() + .iter() + .find(|(name, _)| *name == table) + .map(|(_, walk)| *walk) + }) +} + +/// Print the rows accumulated since the last report and clear them. Called +/// beside `scanner_profile::report_and_reset` so the two read together. +pub(super) fn report_and_reset(cycle_label: &str) { + if !super::gc_diag_enabled() { + return; + } + let rows = WALK_ROWS.with(|rows| std::mem::take(&mut *rows.borrow_mut())); + for (table, walk, passes) in rows { + eprintln!( + "[gc-young-log] {cycle_label} table={table} mode={} passes={passes} logged={} visited={} kept={} table_len={} skipped={}", + if walk.partial { "young" } else { "full" }, + walk.logged, + walk.visited, + walk.kept, + walk.table_len, + (walk.table_len * u64::from(passes)).saturating_sub(walk.visited), + ); + } +} diff --git a/crates/perry-runtime/src/object/descriptor_state.rs b/crates/perry-runtime/src/object/descriptor_state.rs index 909002a9aa..5888a0ab16 100644 --- a/crates/perry-runtime/src/object/descriptor_state.rs +++ b/crates/perry-runtime/src/object/descriptor_state.rs @@ -107,6 +107,11 @@ pub(crate) struct DescriptorTables { pub(crate) attr_keys_by_owner: RefCell>>, /// Accessor twin of [`Self::attr_keys_by_owner`]. pub(crate) accessor_keys_by_owner: RefCell>>, + /// #9754: owners whose entries may hold a pointer a minor can act on — + /// a young owner, or an accessor whose getter/setter closure is young. + /// A minor-scoped `scan_descriptor_roots_mut` visits only these; see + /// `gc/young_log.rs`. + pub(crate) young_owners: RefCell>, } impl DescriptorTables { @@ -118,10 +123,27 @@ impl DescriptorTables { property_attrs_in_use: Cell::new(false), attr_keys_by_owner: RefCell::new(new_fast_key_hash_map()), accessor_keys_by_owner: RefCell::new(new_fast_key_hash_map()), + young_owners: RefCell::new(crate::gc::young_log::YoungLog::new()), } } } +const DESCRIPTOR_YOUNG_LOG_NAME: &str = "object.descriptors"; + +/// Rule 1 of `gc/young_log.rs`: log `owner` BEFORE its descriptor is +/// published when the owner, or the accessor closure being stored, can +/// matter to a minor. Data descriptors carry no pointer, so `acc` is `None` +/// for them and only the owner decides. +#[inline] +fn note_young_descriptor_owner(st: &crate::state::RuntimeState, owner: usize, acc: Option<&AccessorDescriptor>) { + use crate::gc::young_log::{addr_is_minor_relevant, bits_are_minor_relevant}; + if addr_is_minor_relevant(owner) + || acc.is_some_and(|acc| bits_are_minor_relevant(acc.get) || bits_are_minor_relevant(acc.set)) + { + st.descriptors.young_owners.borrow_mut().note(owner); + } +} + /// Record `key` as owned by `owner` in an owner index. Idempotent: a /// `defineProperty` that overwrites an existing descriptor must not push a /// duplicate, or the key would be reported twice by `Object.keys`. @@ -774,6 +796,7 @@ pub(crate) fn set_property_attrs(obj: usize, key: String, attrs: PropertyAttrs) GLOBAL_DESCRIPTORS_IN_USE.store(true, Ordering::Relaxed); disable_inline_guards_for_descriptor_target(obj, &key); note_meta_descriptor_key(obj, &key, false); + note_young_descriptor_owner(st, obj, None); owner_index_add(&st.descriptors.attr_keys_by_owner, obj, &key); st.descriptors .property_descriptors @@ -996,6 +1019,7 @@ pub(crate) fn set_accessor_descriptor(obj: usize, key: String, acc: AccessorDesc disable_inline_guards_for_descriptor_target(obj, &key); note_accessor_descriptor_key(&key); note_meta_descriptor_key(obj, &key, true); + note_young_descriptor_owner(st, obj, Some(&acc)); owner_index_add(&st.descriptors.accessor_keys_by_owner, obj, &key); st.descriptors .accessor_descriptors @@ -1064,6 +1088,7 @@ pub(crate) fn install_fresh_accessor_property( GLOBAL_DESCRIPTORS_IN_USE.store(true, Ordering::Relaxed); disable_inline_guards_for_descriptor_target(obj, &key); note_accessor_descriptor_key(&key); + note_young_descriptor_owner(st, obj, Some(&acc)); match note_meta_descriptor_key_both(obj, &key) { Some((accessor_bit_was_set, attr_bit_was_set)) => { if accessor_bit_was_set { @@ -1181,6 +1206,7 @@ pub(crate) fn set_builtin_accessor_descriptor( note_meta_descriptor_key(obj, &key, true); note_meta_descriptor_key(obj, &key, false); let st = state(); + note_young_descriptor_owner(st, obj, Some(&acc)); owner_index_add(&st.descriptors.accessor_keys_by_owner, obj, &key); owner_index_add(&st.descriptors.attr_keys_by_owner, obj, &key); st.descriptors @@ -1214,6 +1240,7 @@ pub(crate) fn set_builtin_property_attrs(obj: usize, key: String, attrs: Propert // #6759 Phase C2: see `set_builtin_accessor_descriptor`. note_meta_descriptor_key(obj, &key, false); let st = state(); + note_young_descriptor_owner(st, obj, None); owner_index_add(&st.descriptors.attr_keys_by_owner, obj, &key); st.descriptors .property_descriptors @@ -1313,6 +1340,41 @@ pub(crate) fn prune_dead_descriptor_owner_entries(is_dead_owner: &dyn Fn(usize) } } +/// [`prune_dead_descriptor_owner_entries`] for a MINOR (#9754): only a young +/// owner can be dead, and a young owner is always in the young log (noted at +/// insert, re-logged by every minor-scoped walk while it stays young), so the +/// log is the complete candidate set. +pub(crate) fn prune_dead_descriptor_owner_entries_young(is_dead_owner: &dyn Fn(usize) -> bool) { + let st = state(); + let candidates = st.descriptors.young_owners.borrow_mut().take_sorted(); + let mut kept = Vec::with_capacity(candidates.len()); + for owner in candidates { + if is_dead_owner(owner) { + remove_descriptor_owner_entries(st, owner); + } else { + kept.push(owner); + } + } + st.descriptors.young_owners.borrow_mut().extend(kept); +} + +/// Drop every entry `owner` holds in both tables and both indexes, through +/// the owner index (O(owner's keys), not O(table)). +fn remove_descriptor_owner_entries(st: &crate::state::RuntimeState, owner: usize) { + if let Some(keys) = st.descriptors.attr_keys_by_owner.borrow_mut().remove(&owner) { + let mut attrs = st.descriptors.property_descriptors.borrow_mut(); + for key in keys { + attrs.remove(&(owner, key)); + } + } + if let Some(keys) = st.descriptors.accessor_keys_by_owner.borrow_mut().remove(&owner) { + let mut accessors = st.descriptors.accessor_descriptors.borrow_mut(); + for key in keys { + accessors.remove(&(owner, key)); + } + } +} + /// #6710: drop every property-attr + accessor descriptor owned by `obj`. /// /// The generic descriptor tables are keyed by owner address; for a native @@ -1357,6 +1419,10 @@ pub(crate) fn transfer_descriptor_owner(old_owner: usize, new_owner: usize) { return; } let st = state(); + // The moved entries keep their accessor values, so the new owner is + // logged unconditionally; the next minor-scoped walk drops it if nothing + // in it is relevant any more. + st.descriptors.young_owners.borrow_mut().note(new_owner); // The owner index names exactly this owner's keys, so neither table is // walked in full any more. Array growth calls this on every reallocation. { @@ -1452,6 +1518,14 @@ fn rewrite_descriptor_owner( /// reused address). pub(crate) fn scan_descriptor_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { let st = state(); + // #9754: a minor-scoped pass visits only the young-logged owners; the + // full walk below rebuilds the log from what it finds. + if visitor.young_scope() { + scan_descriptor_roots_young(visitor, st); + return; + } + let table_len = st.descriptors.attr_keys_by_owner.borrow().len() as u64 + + st.descriptors.accessor_keys_by_owner.borrow().len() as u64; { // Probe DISTINCT OWNERS via the index, not every `(owner, key)` pair. // This runs on every GC cycle, and since the moving young-gen scavenge @@ -1537,6 +1611,161 @@ pub(crate) fn scan_descriptor_roots_mut(visitor: &mut crate::gc::RuntimeRootVisi } } } + + // A full walk is authoritative: rebuild the young log from the tables. + let kept = relevant_descriptor_owners(st); + let kept_len = kept.len() as u64; + { + let mut log = st.descriptors.young_owners.borrow_mut(); + let _ = log.take_sorted(); + log.extend(kept); + } + crate::gc::young_log::note_walk( + DESCRIPTOR_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: false, + logged: table_len, + visited: table_len, + kept: kept_len, + table_len, + }, + ); +} + +/// Every owner whose entry can matter to a minor, re-derived from the +/// authoritative tables: a non-old owner, or an accessor whose getter or +/// setter is non-old. +fn relevant_descriptor_owners(st: &crate::state::RuntimeState) -> Vec { + use crate::gc::young_log::{addr_is_minor_relevant, bits_are_minor_relevant}; + let mut relevant = Vec::new(); + for &owner in st.descriptors.attr_keys_by_owner.borrow().keys() { + if addr_is_minor_relevant(owner) { + relevant.push(owner); + } + } + for &owner in st.descriptors.accessor_keys_by_owner.borrow().keys() { + if addr_is_minor_relevant(owner) { + relevant.push(owner); + } + } + for ((owner, _), acc) in st.descriptors.accessor_descriptors.borrow().iter() { + if bits_are_minor_relevant(acc.get) || bits_are_minor_relevant(acc.set) { + relevant.push(*owner); + } + } + relevant.sort_unstable(); + relevant.dedup(); + relevant +} + +/// The minor-scoped walk (#9754): only the young-logged owners, each visited +/// exactly as the full walk visits it — accessor get/set rooted in every +/// phase, owner re-keyed across both tables and both indexes in the rewrite +/// phase — and re-logged iff still relevant afterwards. +fn scan_descriptor_roots_young( + visitor: &mut crate::gc::RuntimeRootVisitor<'_>, + st: &crate::state::RuntimeState, +) { + let table_len = st.descriptors.attr_keys_by_owner.borrow().len() as u64 + + st.descriptors.accessor_keys_by_owner.borrow().len() as u64; + #[cfg(debug_assertions)] + { + let relevant = relevant_descriptor_owners(st); + st.descriptors + .young_owners + .borrow() + .debug_assert_logged(DESCRIPTOR_YOUNG_LOG_NAME, &relevant); + } + let mut logged = 0u64; + let mut visited = 0u64; + let mut kept = Vec::new(); + loop { + let batch = st.descriptors.young_owners.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_descriptor_owner(visitor, st, owner); + if relevant { + kept.push(new_owner); + } + } + } + let kept_len = kept.len() as u64; + st.descriptors.young_owners.borrow_mut().extend(kept); + crate::gc::young_log::note_walk( + DESCRIPTOR_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: true, + logged, + visited, + kept: kept_len, + table_len, + }, + ); +} + +/// Visit one owner's descriptors. Returns the post-visit owner address and +/// whether the entry can still matter to a minor. +fn scan_descriptor_owner( + visitor: &mut crate::gc::RuntimeRootVisitor<'_>, + st: &crate::state::RuntimeState, + owner: usize, +) -> (usize, bool) { + use crate::gc::young_log::{addr_is_minor_relevant, bits_are_minor_relevant}; + let new_owner = rewrite_descriptor_owner(visitor, owner); + let mut relevant = false; + let accessor_keys = st + .descriptors + .accessor_keys_by_owner + .borrow() + .get(&owner) + .cloned() + .unwrap_or_default(); + if !accessor_keys.is_empty() { + let mut accessors = st.descriptors.accessor_descriptors.borrow_mut(); + for key in &accessor_keys { + if let Some(acc) = accessors.get_mut(&(owner, key.clone())) { + if acc.get != 0 { + visitor.visit_nanbox_u64_slot(&mut acc.get); + } + if acc.set != 0 { + visitor.visit_nanbox_u64_slot(&mut acc.set); + } + relevant |= bits_are_minor_relevant(acc.get) || bits_are_minor_relevant(acc.set); + } + } + if new_owner != owner { + for key in accessor_keys { + if let Some(acc) = accessors.remove(&(owner, key.clone())) { + accessors.insert((new_owner, key), acc); + } + } + } + } + if new_owner != owner { + let attr_keys = st + .descriptors + .attr_keys_by_owner + .borrow() + .get(&owner) + .cloned() + .unwrap_or_default(); + if !attr_keys.is_empty() { + let mut attrs = st.descriptors.property_descriptors.borrow_mut(); + for key in attr_keys { + if let Some(value) = attrs.remove(&(owner, key.clone())) { + attrs.insert((new_owner, key), value); + } + } + } + owner_index_transfer(&st.descriptors.attr_keys_by_owner, owner, new_owner); + owner_index_transfer(&st.descriptors.accessor_keys_by_owner, owner, new_owner); + } + relevant |= addr_is_minor_relevant(new_owner); + (new_owner, relevant) } /// The owner index (`attr_keys_by_owner` / `accessor_keys_by_owner`) exists diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index f598712f9b..3115b60428 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -256,7 +256,8 @@ pub(crate) use descriptor_state::{ json_object_getter_value, mark_all_keys, object_has_descriptors, object_proto_may_intercept_key, owner_has_property_descriptors, owner_may_have_descriptor_entries, plain_data_write_may_intercept, - prune_dead_descriptor_owner_entries, reflect_getter_closure_bits, set_accessor_descriptor, + prune_dead_descriptor_owner_entries, prune_dead_descriptor_owner_entries_young, + reflect_getter_closure_bits, set_accessor_descriptor, set_builtin_accessor_descriptor, set_builtin_property_attrs, set_property_attrs, transfer_descriptor_owner, AccessorDescriptor, DescriptorTables, PropertyAttrs, }; @@ -676,6 +677,11 @@ fn shape_cache_insert(shape_id: u32, keys_array: *mut ArrayHeader) { }; let st = crate::state::state(); let slot = (shape_id as usize) & (SHAPE_INLINE_CACHE_SIZE - 1); + // #9754 rule 1: log the id BEFORE the entry is published when the keys + // array can matter to a minor. + if crate::gc::young_log::addr_is_minor_relevant(keys_array as usize) { + SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().note(shape_id)); + } unsafe { // GC_STORE_AUDIT(ROOT): shape_inline_cache entries are scanned by scan_shape_cache_roots_mut. let entry = &mut (*st.object_hot.shape_inline_cache.get())[slot]; @@ -783,6 +789,30 @@ const TRANSITION_CACHE_SIZE: usize = 16384; /// against this value even when no Rust path consults it directly. #[allow(dead_code)] const TRANSITION_CACHE_MASK: usize = TRANSITION_CACHE_SIZE - 1; +crate::perry_thread_local! { + /// #9754: transition-cache slots whose `key_ptr` / `next_keys` may still be + /// acted on by a minor (see `gc/young_log.rs`); a minor-scoped + /// `scan_transition_cache_roots_mut` visits only these. + static TRANSITION_CACHE_YOUNG: RefCell> = + const { RefCell::new(crate::gc::young_log::YoungLog::new()) }; + /// #9754: shape-cache ids (inline slot and overflow key alike) whose keys + /// array may still be acted on by a minor. + static SHAPE_CACHE_YOUNG: RefCell> = + const { RefCell::new(crate::gc::young_log::YoungLog::new()) }; +} + +const TRANSITION_CACHE_YOUNG_LOG_NAME: &str = "object.transition_cache"; +const SHAPE_CACHE_YOUNG_LOG_NAME: &str = "object.shape_cache"; + +/// Is a transition-cache entry still something a minor can act on? +#[inline] +fn transition_entry_is_minor_relevant(entry: &TransitionEntry) -> bool { + use crate::gc::young_log::addr_is_minor_relevant; + entry.next_keys != 0 + && (addr_is_minor_relevant(entry.next_keys) + || ((entry.slot_idx >> 24) == 0 && addr_is_minor_relevant(entry.key_ptr))) +} + // Per-thread transition cache (`ObjectHotTables::transition_cache`). Was a // process-wide `static mut`, but with `perry/thread` user code allocating @@ -798,6 +828,7 @@ const TRANSITION_CACHE_MASK: usize = TRANSITION_CACHE_SIZE - 1; // thread-locals (confirmed on a real Series 7: shrinking OR boxing removes // the corruption). `vec!` builds directly on the heap (no 320KB stack // temporary). + #[inline] fn with_transition_cache( f: impl FnOnce(*mut [TransitionEntry; TRANSITION_CACHE_SIZE]) -> R, @@ -1044,6 +1075,13 @@ fn transition_cache_insert( } } } + // #9754 rule 1: log the slot BEFORE the entry is published when either + // address can matter to a minor. + if crate::gc::young_log::addr_is_minor_relevant(next_keys) + || (len_marker == 0 && crate::gc::young_log::addr_is_minor_relevant(kid)) + { + TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().note(slot as u32)); + } with_transition_cache(|t| unsafe { // GC_STORE_AUDIT(ROOT): TRANSITION_CACHE_GLOBAL entries are scanned by scan_transition_cache_roots_mut. let entry = &mut (*t)[slot]; @@ -1088,52 +1126,170 @@ pub fn scan_transition_cache_roots(mark: &mut dyn FnMut(f64)) { } pub fn scan_transition_cache_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { + // #9754: a minor-scoped pass visits only the young-logged slots; a full + // pass walks the table and rebuilds the log. Both share + // `scan_transition_cache_slot`. + if visitor.young_scope() { + let mut logged = 0u64; + let mut visited = 0u64; + let mut kept = Vec::new(); + #[cfg(debug_assertions)] + with_transition_cache(|table| unsafe { + let relevant: Vec = (0..TRANSITION_CACHE_SIZE) + .filter(|&i| transition_entry_is_minor_relevant(&(*table)[i])) + .map(|i| i as u32) + .collect(); + TRANSITION_CACHE_YOUNG.with(|log| { + log.borrow() + .debug_assert_logged(TRANSITION_CACHE_YOUNG_LOG_NAME, &relevant) + }); + }); + let batch = TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().take_sorted()); + logged += batch.len() as u64; + with_transition_cache(|table| unsafe { + for slot in batch { + visited += 1; + if scan_transition_cache_slot(visitor, table, slot as usize) { + kept.push(slot); + } + } + }); + let kept_len = kept.len() as u64; + TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().extend(kept)); + crate::gc::young_log::note_walk( + TRANSITION_CACHE_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: true, + logged, + visited, + kept: kept_len, + table_len: TRANSITION_CACHE_SIZE as u64, + }, + ); + array_tail_transition::scan_roots_mut(visitor); + return; + } + let _ = TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().take_sorted()); + let mut kept = Vec::new(); with_transition_cache(|table| unsafe { for i in 0..TRANSITION_CACHE_SIZE { - let entry = &mut (*table)[i]; - if entry.next_keys != 0 { - let mut invalidate = false; - // Content-namespace ids (len marker != 0) are string BYTES, - // not addresses — the visitor must not rewrite them. - if (entry.slot_idx >> 24) == 0 { - invalidate |= visitor.visit_metadata_usize_slot(&mut entry.key_ptr); - } - // #6759 phase 3: `next_keys` is WEAK, not a strong root. - // - // `visit_usize_slot` MARKS. With 16384 slots this cache was - // therefore keeping up to 16384 keys arrays — and, through - // them, their shape descriptors — alive whether or not any live - // object still had that shape. That is a direct contributor to - // the shape table growing without bound between full - // collections (measured: 786k descriptors on a workload holding - // under 400 live objects). - // - // A transition entry is a pure cache: it answers "adding key k - // to shape S yields shape T". If nothing has shape T any more, - // the answer is worthless, so pinning T's keys array to keep it - // answerable is backwards. `key_ptr` was already weak for the - // same reason; this makes the pair consistent. - // - // Rewrite-only keeps a surviving target's address correct; - // `prune_dead_transition_cache_entries` drops the entry when the - // target did not survive. - visitor.visit_metadata_usize_slot(&mut entry.next_keys); - if invalidate { - *entry = TransitionEntry { - key_ptr: 0, - next_keys: 0, - prev_shape_id: 0, - target_shape_id: 0, - slot_idx: 0, - target_len: 0, - }; - } + if scan_transition_cache_slot(visitor, table, i) { + kept.push(i as u32); } } }); + let kept_len = kept.len() as u64; + TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().extend(kept)); + crate::gc::young_log::note_walk( + TRANSITION_CACHE_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: false, + logged: TRANSITION_CACHE_SIZE as u64, + visited: TRANSITION_CACHE_SIZE as u64, + kept: kept_len, + table_len: TRANSITION_CACHE_SIZE as u64, + }, + ); array_tail_transition::scan_roots_mut(visitor); } +/// Visit one transition-cache slot. Returns whether the entry can still +/// matter to a minor afterwards. +/// +/// # Safety +/// `table` must be this thread's transition cache. +unsafe fn scan_transition_cache_slot( + visitor: &mut crate::gc::RuntimeRootVisitor<'_>, + table: *mut [TransitionEntry; TRANSITION_CACHE_SIZE], + i: usize, +) -> bool { + let entry = &mut (*table)[i]; + if entry.next_keys == 0 { + return false; + } + let mut invalidate = false; + // Content-namespace ids (len marker != 0) are string BYTES, + // not addresses — the visitor must not rewrite them. + if (entry.slot_idx >> 24) == 0 { + invalidate |= visitor.visit_metadata_usize_slot(&mut entry.key_ptr); + } + // #6759 phase 3: `next_keys` is WEAK, not a strong root. + // + // `visit_usize_slot` MARKS. With 16384 slots this cache was + // therefore keeping up to 16384 keys arrays — and, through + // them, their shape descriptors — alive whether or not any live + // object still had that shape. That is a direct contributor to + // the shape table growing without bound between full + // collections (measured: 786k descriptors on a workload holding + // under 400 live objects). + // + // A transition entry is a pure cache: it answers "adding key k + // to shape S yields shape T". If nothing has shape T any more, + // the answer is worthless, so pinning T's keys array to keep it + // answerable is backwards. `key_ptr` was already weak for the + // same reason; this makes the pair consistent. + // + // Rewrite-only keeps a surviving target's address correct; + // `prune_dead_transition_cache_entries` drops the entry when the + // target did not survive. + visitor.visit_metadata_usize_slot(&mut entry.next_keys); + if invalidate { + *entry = TransitionEntry { + key_ptr: 0, + next_keys: 0, + prev_shape_id: 0, + target_shape_id: 0, + slot_idx: 0, + target_len: 0, + }; + return false; + } + transition_entry_is_minor_relevant(entry) +} + +/// [`prune_dead_transition_cache_entries`] for a MINOR (#9754): only a slot +/// in the young log can name a young — hence possibly dead — address. +#[cold] +pub(crate) fn prune_dead_transition_cache_entries_young(is_dead_owner: &dyn Fn(usize) -> bool) { + let candidates = TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().take_sorted()); + let mut kept = Vec::with_capacity(candidates.len()); + with_transition_cache(|table| unsafe { + for slot in candidates { + let entry = &mut (*table)[slot as usize]; + if entry.next_keys == 0 { + continue; + } + if transition_entry_is_dead(entry, is_dead_owner) { + *entry = TransitionEntry { + key_ptr: 0, + next_keys: 0, + prev_shape_id: 0, + target_shape_id: 0, + slot_idx: 0, + target_len: 0, + }; + } else { + kept.push(slot); + } + } + }); + TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().extend(kept)); + array_tail_transition::prune_invalid_entries(); +} + +/// The death test of `prune_dead_transition_cache_entries`, shared with the +/// young-only variant. +fn transition_entry_is_dead(entry: &TransitionEntry, is_dead_owner: &dyn Fn(usize) -> bool) -> bool { + ((entry.slot_idx >> 24) == 0 && entry.key_ptr != 0 && is_dead_owner(entry.key_ptr)) + // #6759 phase 3: `next_keys` stopped being a strong root, so a + // dead target is now possible and must be reaped here — this is + // the half that makes weakening it safe. + || is_dead_owner(entry.next_keys) + || shapes::shape_descriptor_by_id(entry.prev_shape_id).is_none() + || (entry.target_shape_id != 0 + && shapes::shape_descriptor_by_id(entry.target_shape_id).is_none()) +} + /// #8192: death pruning for the transition cache. /// /// The interned `key_ptr` is metadata-only and therefore weak; `next_keys` is @@ -1158,17 +1314,7 @@ pub(crate) fn prune_dead_transition_cache_entries(is_dead_owner: &dyn Fn(usize) if entry.next_keys == 0 { continue; } - let dead = ((entry.slot_idx >> 24) == 0 - && entry.key_ptr != 0 - && is_dead_owner(entry.key_ptr)) - // #6759 phase 3: `next_keys` stopped being a strong root, so a - // dead target is now possible and must be reaped here — this is - // the half that makes weakening it safe. - || is_dead_owner(entry.next_keys) - || shapes::shape_descriptor_by_id(entry.prev_shape_id).is_none() - || (entry.target_shape_id != 0 - && shapes::shape_descriptor_by_id(entry.target_shape_id).is_none()); - if dead { + if transition_entry_is_dead(entry, is_dead_owner) { *entry = TransitionEntry { key_ptr: 0, next_keys: 0, @@ -1199,6 +1345,11 @@ pub(crate) fn test_seed_transition_cache_entry( next_keys: usize, ) { let slot = transition_cache_slot(prev_shape_id, key_ptr); + if crate::gc::young_log::addr_is_minor_relevant(next_keys) + || crate::gc::young_log::addr_is_minor_relevant(key_ptr) + { + TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().note(slot as u32)); + } with_transition_cache(|table| unsafe { (*table)[slot] = TransitionEntry { key_ptr, @@ -1221,19 +1372,75 @@ pub fn scan_shape_cache_roots(mark: &mut dyn FnMut(f64)) { } pub fn scan_shape_cache_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { + use crate::gc::young_log::addr_is_minor_relevant; let st = crate::state::state(); - { - let entries = unsafe { &mut *st.object_hot.shape_inline_cache.get() }; - for entry in entries.iter_mut() { - visitor.visit_raw_mut_ptr_slot(&mut entry.keys_array); + // The inline array is 256 fixed slots: always walked. The overflow map + // holds every shape id ever cached; #9754: a minor-scoped pass visits + // only the young-logged ids there, a full pass rebuilds the log. + let entries = unsafe { &mut *st.object_hot.shape_inline_cache.get() }; + for entry in entries.iter_mut() { + visitor.visit_raw_mut_ptr_slot(&mut entry.keys_array); + } + let mut cache = st.object_hot.shape_cache_overflow.borrow_mut(); + let table_len = cache.len() as u64; + if visitor.young_scope() { + #[cfg(debug_assertions)] + { + let relevant: Vec = cache + .iter() + .filter(|(_, (arr_ptr, _))| addr_is_minor_relevant(*arr_ptr as usize)) + .map(|(&id, _)| id) + .collect(); + SHAPE_CACHE_YOUNG.with(|log| { + log.borrow() + .debug_assert_logged(SHAPE_CACHE_YOUNG_LOG_NAME, &relevant) + }); } + let batch = SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().take_sorted()); + let logged = batch.len() as u64; + let mut kept = Vec::new(); + for id in batch { + if let Some((arr_ptr, _)) = cache.get_mut(&id) { + visitor.visit_raw_mut_ptr_slot(arr_ptr); + if addr_is_minor_relevant(*arr_ptr as usize) { + kept.push(id); + } + } + } + let kept_len = kept.len() as u64; + SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().extend(kept)); + crate::gc::young_log::note_walk( + SHAPE_CACHE_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: true, + logged, + visited: logged, + kept: kept_len, + table_len, + }, + ); + return; } - { - let mut cache = st.object_hot.shape_cache_overflow.borrow_mut(); - for (arr_ptr, _runtime_shape_id) in cache.values_mut() { - visitor.visit_raw_mut_ptr_slot(arr_ptr); + let _ = SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().take_sorted()); + let mut kept = Vec::new(); + for (&id, (arr_ptr, _runtime_shape_id)) in cache.iter_mut() { + visitor.visit_raw_mut_ptr_slot(arr_ptr); + if addr_is_minor_relevant(*arr_ptr as usize) { + kept.push(id); } } + let kept_len = kept.len() as u64; + SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().extend(kept)); + crate::gc::young_log::note_walk( + SHAPE_CACHE_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: false, + logged: table_len, + visited: table_len, + kept: kept_len, + table_len, + }, + ); } /// GC root scanner: mark all JSValues stored in OVERFLOW_FIELDS. @@ -1407,6 +1614,9 @@ pub fn scan_object_cache_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<' pub(crate) fn test_seed_shape_cache_root(shape_id: u32, keys_array: *mut ArrayHeader) { let st = crate::state::state(); let slot = (shape_id as usize) & (SHAPE_INLINE_CACHE_SIZE - 1); + if crate::gc::young_log::addr_is_minor_relevant(keys_array as usize) { + SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().note(shape_id)); + } unsafe { // GC_STORE_AUDIT(ROOT): test seed mirrors shape_inline_cache roots scanned by scan_shape_cache_roots_mut. let entry = &mut (*st.object_hot.shape_inline_cache.get())[slot]; diff --git a/crates/perry-runtime/src/object/shapes.rs b/crates/perry-runtime/src/object/shapes.rs index 8d26b9c024..46a0ff600a 100644 --- a/crates/perry-runtime/src/object/shapes.rs +++ b/crates/perry-runtime/src/object/shapes.rs @@ -256,11 +256,29 @@ struct ShapeTableInner { /// /// Single-word key, so `PtrHasher` (#8125). families: crate::fast_hash::PtrHashMap, + /// #9754: keys-array addresses a minor can act on — the families and + /// slot indices whose keys array is not (yet) old. A minor-scoped + /// `scan_shape_table_rekey_mut` visits only these; see `gc/young_log.rs`. + young_keys: crate::gc::young_log::YoungLog, } +const SHAPE_YOUNG_LOG_NAME: &str = "shapes.families+indices"; + impl ShapeTableInner { + /// Rule 1 of `gc/young_log.rs`: log a keys address BEFORE a family or a + /// slot index is published under it, when the keys array is not old. + /// Every family insert funnels through `family_push_back` / + /// `family_push_front`; the slot-index inserts call this themselves. + #[inline] + fn note_young_keys(&mut self, keys: u64) { + if crate::gc::young_log::addr_is_minor_relevant(keys as usize) { + self.young_keys.note(keys); + } + } + #[inline] fn family_push_back(&mut self, keys: u64, id: u32) { + self.note_young_keys(keys); self.families.entry(keys).or_default().push_back(id); } @@ -275,6 +293,7 @@ impl ShapeTableInner { #[inline] fn family_push_front(&mut self, keys: u64, id: u32) { + self.note_young_keys(keys); self.families.entry(keys).or_default().push_front(id); } @@ -340,6 +359,7 @@ impl ShapeTable { indices: crate::fast_hash::new_ptr_hash_map(), by_facts: crate::fast_hash::new_ptr_hash_map(), families: crate::fast_hash::new_ptr_hash_map(), + young_keys: crate::gc::young_log::YoungLog::new(), }), } } @@ -1674,6 +1694,7 @@ pub(crate) unsafe fn shape_slot_lookup_verdict( if !build { return KeysIndexVerdict::Unindexed; } + inner.note_young_keys(keys_id as u64); inner.indices.entry(keys_id).or_insert(ShapeIndex { indexed_len: 0, slots: crate::fast_hash::new_ptr_hash_map(), @@ -1767,6 +1788,7 @@ pub(crate) fn shape_keys_grown(old_keys: usize, new_keys: *const ArrayHeader) { } let mut inner = crate::state::state().shapes.inner.borrow_mut(); if let Some(shape) = inner.indices.remove(&old_keys) { + inner.note_young_keys(new_id as u64); inner.indices.insert(new_id, shape); } } @@ -1860,6 +1882,34 @@ pub(crate) fn prune_dead_shape_keys(is_dead_owner: &dyn Fn(usize) -> bool) { } } +/// [`prune_dead_shape_keys`] for a MINOR (#9754): only a young keys array can +/// die, and a young keys address is always in the young log (noted at +/// insert, re-logged by every minor-scoped walk while it stays young), so the +/// log is the complete candidate set. +pub(crate) fn prune_dead_shape_keys_young(is_dead_owner: &dyn Fn(usize) -> bool) { + let table = &crate::state::state().shapes; + let mut inner = table.inner.borrow_mut(); + let candidates = inner.young_keys.take_sorted(); + let mut kept = Vec::with_capacity(candidates.len()); + for keys in candidates { + let addr = keys as usize; + if !is_dead_owner(addr) && !shape_keys_address_is_recycled(addr) { + kept.push(keys); + continue; + } + inner.indices.remove(&addr); + let ids: Vec = inner + .families + .get(&keys) + .map(|ids| ids.as_slice().to_vec()) + .unwrap_or_default(); + for id in ids { + remove_descriptor_indexed_under(&mut inner, id, keys); + } + } + inner.young_keys.extend(kept); +} + /// Metadata-only forwarding repair for the weak descriptor table and /// pointer-keyed slot indices. Mark/copy mode does not root anything; live /// object scans provide descriptor reachability, and post-copy rewrite follows @@ -1877,6 +1927,13 @@ pub(crate) fn scan_shape_table_rekey_mut(visitor: &mut crate::gc::RuntimeRootVis let table = &crate::state::state().shapes; let mut inner = table.inner.borrow_mut(); let rewrite_phase = visitor.is_metadata_rewrite_phase(); + // #9754: a minor-scoped pass visits only the young-logged keys addresses; + // the full walk below rebuilds the log from what it finds. + if visitor.young_scope() { + scan_shape_table_young(visitor, table, &mut inner, rewrite_phase); + return; + } + let table_len = (inner.families.len() + inner.indices.len()) as u64; let mut moved_families: Vec<(u64, u64)> = Vec::new(); let mut dead_descriptor_ids: Vec<(u32, u64)> = Vec::new(); // The shared slab view is scoped to the probe loop: retirement below @@ -1947,59 +2004,240 @@ pub(crate) fn scan_shape_table_rekey_mut(visitor: &mut crate::gc::RuntimeRootVis remove_descriptor_indexed_under(&mut inner, id, indexed); } for (old, new) in moved_families { - let Some(ids) = inner.families.remove(&old) else { - continue; - }; - if new == 0 { - continue; - } - for &id in ids.as_slice() { - let Some(record) = table.slab().get(id) else { - continue; - }; - // The accelerator was keyed with the OLD address; the other five - // facts never change under the collector. - if record.has(RECORD_FLAG_FACTS_INDEXED) { - inner.facts_remove(record.facts_key_with_keys(old), id); - inner.facts_push_back(record.facts_key_with_keys(new), id); + move_shape_family(table, &mut inner, old, new); + } + + if rewrite_phase && !inner.indices.is_empty() { + let moved: Vec<(usize, usize)> = inner + .indices + .keys() + .filter_map(|&keys_id| { + let mut addr = keys_id; + visitor.visit_metadata_usize_slot(&mut addr); + (addr != keys_id).then_some((keys_id, addr)) + }) + .collect(); + for (old, new) in moved { + if let Some(shape) = inner.indices.remove(&old) { + inner.indices.insert(new, shape); } - inner.family_push_back(new, id); + } + // Drop indices entries whose keys-array address was recycled: the + // forwarding record at the old address points to a DIFFERENT object + // (not a keys array), so `visit_metadata_usize_slot` either rekeyed + // it to the wrong address (caught above by the type mismatch on the + // new address) or returned false because the forwarding walk could + // not classify the address. Either way the keys array is dead; remove + // the stale entry so property lookups don't resolve the wrong shape. + let recycled: Vec = inner + .indices + .keys() + .filter(|&&keys_id| shape_keys_address_is_recycled(keys_id)) + .copied() + .collect(); + for old in recycled { + inner.indices.remove(&old); } } - if !rewrite_phase || inner.indices.is_empty() { + // A full walk is authoritative: rebuild the young log from the tables. + let kept = relevant_shape_keys(&inner); + let kept_len = kept.len() as u64; + let _ = inner.young_keys.take_sorted(); + inner.young_keys.extend(kept); + crate::gc::young_log::note_walk( + SHAPE_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: false, + logged: table_len, + visited: table_len, + kept: kept_len, + table_len, + }, + ); +} + +/// Re-index a family that the collector moved from `old` to `new` (`new == +/// 0`: every member retired, drop it). +fn move_shape_family(table: &ShapeTable, inner: &mut ShapeTableInner, old: u64, new: u64) { + let Some(ids) = inner.families.remove(&old) else { + return; + }; + if new == 0 { return; } - let moved: Vec<(usize, usize)> = inner - .indices - .keys() - .filter_map(|&keys_id| { - let mut addr = keys_id; - visitor.visit_metadata_usize_slot(&mut addr); - (addr != keys_id).then_some((keys_id, addr)) - }) - .collect(); - for (old, new) in moved { - if let Some(shape) = inner.indices.remove(&old) { - inner.indices.insert(new, shape); + for &id in ids.as_slice() { + let Some(record) = table.slab().get(id) else { + continue; + }; + // The accelerator was keyed with the OLD address; the other five + // facts never change under the collector. + if record.has(RECORD_FLAG_FACTS_INDEXED) { + inner.facts_remove(record.facts_key_with_keys(old), id); + inner.facts_push_back(record.facts_key_with_keys(new), id); } + inner.family_push_back(new, id); } - // Drop indices entries whose keys-array address was recycled: the - // forwarding record at the old address points to a DIFFERENT object - // (not a keys array), so `visit_metadata_usize_slot` either rekeyed - // it to the wrong address (caught above by the type mismatch on the - // new address) or returned false because the forwarding walk could - // not classify the address. Either way the keys array is dead; remove - // the stale entry so property lookups don't resolve the wrong shape. - let recycled: Vec = inner - .indices +} + +/// Every keys address a minor can act on, re-derived from the authoritative +/// tables (families and slot indices whose keys array is not old). +fn relevant_shape_keys(inner: &ShapeTableInner) -> Vec { + use crate::gc::young_log::addr_is_minor_relevant; + let mut relevant: Vec = inner + .families .keys() - .filter(|&&keys_id| shape_keys_address_is_recycled(keys_id)) .copied() + .filter(|&keys| keys != 0 && addr_is_minor_relevant(keys as usize)) .collect(); - for old in recycled { - inner.indices.remove(&old); + relevant.extend( + inner + .indices + .keys() + .copied() + .filter(|&keys| addr_is_minor_relevant(keys)) + .map(|keys| keys as u64), + ); + relevant.sort_unstable(); + relevant.dedup(); + relevant +} + +/// The minor-scoped walk (#9754): only the young-logged keys addresses, each +/// visited exactly as the full walk visits it — the family's carrier gate, +/// the record rewrite, the recycled-address retirement, the slot-index +/// re-key — and re-logged iff the keys array is still not old afterwards. +fn scan_shape_table_young( + visitor: &mut crate::gc::RuntimeRootVisitor<'_>, + table: &ShapeTable, + inner: &mut ShapeTableInner, + rewrite_phase: bool, +) { + let table_len = (inner.families.len() + inner.indices.len()) as u64; + #[cfg(debug_assertions)] + { + let relevant = relevant_shape_keys(inner); + inner + .young_keys + .debug_assert_logged(SHAPE_YOUNG_LOG_NAME, &relevant); + } + let mut logged = 0u64; + let mut visited = 0u64; + let mut kept = Vec::new(); + loop { + let batch = inner.young_keys.take_sorted(); + if batch.is_empty() { + break; + } + logged += batch.len() as u64; + for keys in batch { + if keys == 0 { + continue; + } + visited += 1; + let (post, relevant) = scan_shape_keys_address(visitor, table, inner, rewrite_phase, keys); + if relevant { + kept.push(post); + } + // The family moves in the MARK pass (a carrier's `visit_usize_slot` + // copies the keys array) while the slot index is re-keyed only in + // the REWRITE pass, so between the two the index still sits under + // the from-space address: keep that key logged as well. + if post != keys && inner.indices.contains_key(&(keys as usize)) { + kept.push(keys); + } + } + } + let kept_len = kept.len() as u64; + inner.young_keys.extend(kept); + crate::gc::young_log::note_walk( + SHAPE_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: true, + logged, + visited, + kept: kept_len, + table_len, + }, + ); +} + +/// Visit one keys address — its family and its slot index — with the same +/// per-entry body as the full walk. Returns the post-visit address and +/// whether the keys array can still matter to a minor. +fn scan_shape_keys_address( + visitor: &mut crate::gc::RuntimeRootVisitor<'_>, + table: &ShapeTable, + inner: &mut ShapeTableInner, + rewrite_phase: bool, + indexed: u64, +) -> (u64, bool) { + let mut post = indexed; + let ids: Vec = inner + .families + .get(&indexed) + .map(|ids| ids.as_slice().to_vec()) + .unwrap_or_default(); + if !ids.is_empty() { + let slab = table.slab(); + let mut descriptor: Option = None; + for &id in &ids { + if let Some(lifted) = slab.lift(id) { + if lifted.old_carrier || lifted.cache_carrier { + descriptor = Some(lifted); + break; + } + descriptor.get_or_insert(lifted); + } + } + match descriptor { + None => { + // Every id retired under a stale address; the family is empty. + move_shape_family(table, inner, indexed, 0); + } + Some(descriptor) => { + let mut addr = indexed as usize; + let moved = if descriptor.old_carrier || descriptor.cache_carrier { + visitor.visit_usize_slot(&mut addr) + } else { + visitor.visit_metadata_usize_slot(&mut addr) + }; + if rewrite_phase && shape_keys_address_is_recycled(addr) { + for id in ids { + remove_descriptor_indexed_under(inner, id, indexed); + } + } else { + if moved { + for &id in &ids { + if let Some(record) = slab.record_ptr(id) { + // SAFETY: live slab record, single-threaded agent; + // the store is idempotent (see the full walk). + unsafe { (*record).keys = addr as u64 }; + } + } + } + if addr as u64 != indexed { + move_shape_family(table, inner, indexed, addr as u64); + post = addr as u64; + } + } + } + } + } + if rewrite_phase && inner.indices.contains_key(&(indexed as usize)) { + let mut addr = indexed as usize; + visitor.visit_metadata_usize_slot(&mut addr); + if addr != indexed as usize { + if let Some(shape) = inner.indices.remove(&(indexed as usize)) { + inner.indices.insert(addr, shape); + } + post = addr as u64; + } + if shape_keys_address_is_recycled(addr) { + inner.indices.remove(&addr); + } } + (post, crate::gc::young_log::addr_is_minor_relevant(post as usize)) } // #8112 sabotage switch. Suppressing the descriptor edge proves the fixture's diff --git a/crates/perry-runtime/src/object/shapes_slot_list.rs b/crates/perry-runtime/src/object/shapes_slot_list.rs index b93ddd35e3..e6f8b7b1e9 100644 --- a/crates/perry-runtime/src/object/shapes_slot_list.rs +++ b/crates/perry-runtime/src/object/shapes_slot_list.rs @@ -170,6 +170,7 @@ pub(crate) fn shape_index_migrate_after_delete( !list.is_empty() }); index.indexed_len = old_key_count - 1; + inner.note_young_keys(new_keys_id as u64); inner.indices.insert(new_keys_id, index); true } diff --git a/crates/perry-runtime/src/object/shapes_test_support.rs b/crates/perry-runtime/src/object/shapes_test_support.rs index 311bb982e5..3fa463ef0d 100644 --- a/crates/perry-runtime/src/object/shapes_test_support.rs +++ b/crates/perry-runtime/src/object/shapes_test_support.rs @@ -83,6 +83,7 @@ pub(crate) fn test_clear_shape_table() { inner.indices.clear(); inner.by_facts.clear(); inner.families.clear(); + inner.young_keys.clear(); // SAFETY: test-only reset with no slab reference held. unsafe { table.slab_mut().clear() }; drop(inner); diff --git a/crates/perry-runtime/src/object/test_root_accessors.rs b/crates/perry-runtime/src/object/test_root_accessors.rs index 3b76daf0a0..4154a85883 100644 --- a/crates/perry-runtime/src/object/test_root_accessors.rs +++ b/crates/perry-runtime/src/object/test_root_accessors.rs @@ -22,6 +22,9 @@ pub(crate) fn test_shape_cache_root(shape_id: u32) -> (usize, usize) { #[cfg(test)] pub(crate) fn test_seed_transition_cache_root(next_keys: usize) { + if crate::gc::young_log::addr_is_minor_relevant(next_keys) { + super::TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().note(0)); + } with_transition_cache(|t| unsafe { // GC_STORE_AUDIT(ROOT): test seed mirrors TRANSITION_CACHE_GLOBAL roots scanned by scan_transition_cache_roots_mut. let entry = &mut (*t)[0]; @@ -34,6 +37,17 @@ pub(crate) fn test_seed_transition_cache_root(next_keys: usize) { }); } +/// `test_seed_transition_cache_root` under a predecessor ShapeId that resolves, +/// so `prune_dead_transition_cache_entries` (which retires an entry whose +/// predecessor has no descriptor) keeps the entry across a collection. +#[cfg(test)] +pub(crate) fn test_seed_transition_cache_root_for_shape(prev_shape_id: u32, next_keys: usize) { + test_seed_transition_cache_root(next_keys); + with_transition_cache(|t| unsafe { + (*t)[0].prev_shape_id = prev_shape_id; + }); +} + #[cfg(test)] pub(crate) fn test_transition_cache_root() -> usize { with_transition_cache(|t| unsafe { (*t)[0].next_keys }) @@ -41,6 +55,8 @@ pub(crate) fn test_transition_cache_root() -> usize { #[cfg(test)] pub(crate) fn test_clear_transition_cache_root() { + super::TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().clear()); + super::SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().clear()); with_transition_cache(|t| unsafe { for i in 0..TRANSITION_CACHE_SIZE { // GC_STORE_AUDIT(ROOT): test clear writes non-pointer sentinels into scanned TRANSITION_CACHE_GLOBAL roots. diff --git a/scripts/gc_rekeyed_key_tables.json b/scripts/gc_rekeyed_key_tables.json index d096d3f65a..2f00dd4f90 100644 --- a/scripts/gc_rekeyed_key_tables.json +++ b/scripts/gc_rekeyed_key_tables.json @@ -32,10 +32,10 @@ "why": "#8191: registered prune retains on !is_dead_owner over the wrapper ObjectHeader address (builtins/formatting/boxed_primitives.rs)." }, { - "site": "crates/perry-runtime/src/closure/dynamic_props.rs::scan_closure_dynamic_props_roots_mut", + "site": "crates/perry-runtime/src/closure/dynamic_props.rs::scan_closure_owner", "table": "CLOSURE_PROPS / CLOSURE_STATIC_PROTOTYPES / CLOSURE_DELETED_KEYS", "death": "dead_owner:prune_dead_closure_side_table_owners", - "why": "All three tables are retained on the GC_TYPE_CLOSURE-narrowed predicate in one prune (closure/dynamic_props.rs:206-214)." + "why": "All three tables are retained on the GC_TYPE_CLOSURE-narrowed predicate in one prune (closure/dynamic_props.rs:206-214). #9754: the per-owner body shared by the full walk and the young-log walk." }, { "site": "crates/perry-runtime/src/fs/mod.rs::scan_filehandle_object_fd_metadata_roots_mut", @@ -98,10 +98,10 @@ "why": "Cleared per dead object by gc_type_clear_dead_payload_side_tables (gc/types.rs:799) from the sweep (gc/oldgen.rs:700, 924, 1573), outside the dead_owner fan-out." }, { - "site": "crates/perry-runtime/src/object/mod.rs::scan_transition_cache_roots_mut", + "site": "crates/perry-runtime/src/object/mod.rs::scan_transition_cache_slot", "table": "TRANSITION_CACHE_GLOBAL", "death": "dead_owner:prune_dead_transition_cache_entries", - "why": "#8192: registered prune drops the whole cache entry when either weak half (prev_keys keys-array, key_ptr interned string) is dead; next_keys is a strong root and cannot be." + "why": "#8192: registered prune drops the whole cache entry when either weak half (prev_keys keys-array, key_ptr interned string) is dead; next_keys is a strong root and cannot be. #9754: the per-slot body shared by the full walk and the young-log walk." }, { "site": "crates/perry-runtime/src/object/native_module/callable_exports.rs::scan_builtin_closure_metadata_roots_mut", @@ -115,6 +115,12 @@ "death": "dead_owner:prune_dead_shape_keys", "why": "Registered prune drops descriptors whose keys_array is dead and retains inner.indices on the same predicate, then rebuilds the reverse indices (object/shapes.rs:966-985)." }, + { + "site": "crates/perry-runtime/src/object/shapes.rs::scan_shape_keys_address", + "table": "state().shapes.inner (descriptors + indices)", + "death": "dead_owner:prune_dead_shape_keys", + "why": "#9754: the young-log walk's per-keys-address body, the same family probe and slot-index re-key as scan_shape_table_rekey_mut; pruned by the same registered prune (and its young-log variant on minors)." + }, { "site": "crates/perry-runtime/src/perf_hooks.rs::scan_perf_entries_roots_mut", "table": "PERF_ENTRY_KEYS_ARRAY", From 7d3c955a7ff772f42132268bbd1c55a60fbc1345 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 05:55:46 +0200 Subject: [PATCH 2/7] style(gc): rustfmt and split the four files this change pushed past the size gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cargo fmt` output plus the `scripts/check_file_size.sh` splits this change needs: it took `object/mod.rs` 1998 -> 2208, `object/descriptor_state.rs` 1815 -> 2044, `gc/roots.rs` 1994 -> 2027 and `gc/cycle.rs` 1998 -> 2023, and three of those four sat within two lines of the 2000-line limit on main. Each split follows the gate's own recipe (extract a function group into a sibling, re-export by name) and each is a group that already read as a unit: * `object/side_table_roots.rs` — the transition-cache and shape-cache root scanners and dead-owner prunes, now four full-walk/minor-scoped pairs. * `object/descriptor_state/young.rs` — the minor-scoped descriptor walk and the re-derivation of the relevant set rule 2 checks it against. * `gc/roots/stack_bottom.rs` — the four `#[cfg]` arms of `get_stack_bottom`. The doc comment on the first arm describes a trace-phase mark helper rather than `get_stack_bottom`; it was already attached to that item and moves with it verbatim rather than being re-pointed at the next item. * `gc/cycle/registered_root_scan.rs` — the two registered-root scan cursors the budgeted root scan resumes through. `scripts/gc_rekeyed_key_tables.json` follows `scan_transition_cache_slot` to its new file (the gate reported it as one UNCLASSIFIED site and one STALE entry, which is the gate working), and the two `#[cfg(test)]` transition-cache seams are re-exported for `gc::tests::dead_owner_side_tables`. No behaviour change: every moved item keeps its body, and visibility widens only to the narrowest scope the new boundary needs. Claude-Session: https://claude.ai/code/session_01YPfnmWZmSpSWpmnoXvH8z2 --- .../src/closure/dynamic_props.rs | 10 +- crates/perry-runtime/src/closure/mod.rs | 4 +- crates/perry-runtime/src/gc/barrier/mod.rs | 3 +- crates/perry-runtime/src/gc/copying.rs | 13 +- crates/perry-runtime/src/gc/cycle.rs | 233 +----------- .../src/gc/cycle/registered_root_scan.rs | 244 ++++++++++++ crates/perry-runtime/src/gc/mod.rs | 2 +- crates/perry-runtime/src/gc/roots.rs | 114 +----- .../src/gc/roots/stack_bottom.rs | 126 +++++++ .../perry-runtime/src/gc/sticky_remembered.rs | 5 +- .../src/gc/tests/young_log_tests.rs | 60 ++- .../src/object/descriptor_state.rs | 101 ++--- .../src/object/descriptor_state/young.rs | 85 +++++ crates/perry-runtime/src/object/mod.rs | 351 +----------------- crates/perry-runtime/src/object/shapes.rs | 8 +- .../src/object/side_table_roots.rs | 350 +++++++++++++++++ scripts/gc_rekeyed_key_tables.json | 2 +- 17 files changed, 923 insertions(+), 788 deletions(-) create mode 100644 crates/perry-runtime/src/gc/cycle/registered_root_scan.rs create mode 100644 crates/perry-runtime/src/gc/roots/stack_bottom.rs create mode 100644 crates/perry-runtime/src/object/descriptor_state/young.rs create mode 100644 crates/perry-runtime/src/object/side_table_roots.rs diff --git a/crates/perry-runtime/src/closure/dynamic_props.rs b/crates/perry-runtime/src/closure/dynamic_props.rs index 24587fd61b..c8fbc8c7b9 100644 --- a/crates/perry-runtime/src/closure/dynamic_props.rs +++ b/crates/perry-runtime/src/closure/dynamic_props.rs @@ -505,8 +505,14 @@ pub fn scan_closure_dynamic_props_roots_mut(visitor: &mut crate::gc::RuntimeRoot 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); + 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)] diff --git a/crates/perry-runtime/src/closure/mod.rs b/crates/perry-runtime/src/closure/mod.rs index 8b4e6d489f..20666f8b15 100644 --- a/crates/perry-runtime/src/closure/mod.rs +++ b/crates/perry-runtime/src/closure/mod.rs @@ -79,8 +79,8 @@ 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, closure_set_via_function_prototype_descriptor, prune_dead_closure_side_table_owners, - prune_dead_closure_side_table_owners_young, - visit_closure_dynamic_prop_value_slots_mut, visit_closure_static_prototype_slot_mut, + prune_dead_closure_side_table_owners_young, visit_closure_dynamic_prop_value_slots_mut, + visit_closure_static_prototype_slot_mut, }; pub use dynamic_props::{ closure_delete_own_dynamic_prop, closure_dynamic_props_snapshot, closure_get_dynamic_prop, diff --git a/crates/perry-runtime/src/gc/barrier/mod.rs b/crates/perry-runtime/src/gc/barrier/mod.rs index e81106e98e..808a66427c 100644 --- a/crates/perry-runtime/src/gc/barrier/mod.rs +++ b/crates/perry-runtime/src/gc/barrier/mod.rs @@ -596,7 +596,8 @@ pub(super) unsafe fn scan_dirty_object_slots( if crate::weakref::is_weak_target_trace_slot(header, slot.slot) { return; } - complete &= in_body(slot.slot) && dirty_pages_contains_addr(dirty_pages, slot.slot as usize); + complete &= in_body(slot.slot) + && dirty_pages_contains_addr(dirty_pages, slot.slot as usize); if let Some(layout_kind) = slot.layout_kind { scan_dirty_slot_with_layout( slot.slot, diff --git a/crates/perry-runtime/src/gc/copying.rs b/crates/perry-runtime/src/gc/copying.rs index feecbc6f28..be5dfdab47 100644 --- a/crates/perry-runtime/src/gc/copying.rs +++ b/crates/perry-runtime/src/gc/copying.rs @@ -794,7 +794,8 @@ pub(super) fn scan_remembered_dirty_slots_copying( visit(slot, header, external, stats); changed |= *slot != before; }; - let complete = scan_dirty_object_slots(header, &snapshot.dirty_pages, stats, &mut visit_slot); + let complete = + scan_dirty_object_slots(header, &snapshot.dirty_pages, stats, &mut visit_slot); if complete { if let Some(covered) = covered.as_deref_mut() { covered.insert(header as usize); @@ -1064,9 +1065,13 @@ impl CopiedMinorEligibility { let snapshot = remembered_dirty_snapshot(); let mut dirty_checker = CopyingNurseryPreflight::new(ptrs, CopiedMinorFallbackReason::PinnedYoungDirtySlot); - scan_remembered_dirty_slots_copying(&snapshot, None, |slot, _header, _external, _stats| unsafe { - dirty_checker.check_bits(*slot); - }); + scan_remembered_dirty_slots_copying( + &snapshot, + None, + |slot, _header, _external, _stats| unsafe { + dirty_checker.check_bits(*slot); + }, + ); unsafe { dirty_checker.drain(); } diff --git a/crates/perry-runtime/src/gc/cycle.rs b/crates/perry-runtime/src/gc/cycle.rs index e787f66aed..b34e9eff3e 100644 --- a/crates/perry-runtime/src/gc/cycle.rs +++ b/crates/perry-runtime/src/gc/cycle.rs @@ -297,237 +297,8 @@ enum RootScanSubphase { Done, } -struct MutableRegisteredRootScanState { - scanners: Vec, - scanner_states: Vec>>, - ffi_scanners: Vec, - ffi_named_scanners: Vec<(PerryFfiNamedMutableRootScanner, usize)>, - scanner_cursor: usize, - ffi_cursor: usize, - ffi_named_cursor: usize, - recorded_counts: bool, -} - -impl MutableRegisteredRootScanState { - fn new() -> Self { - let scanners = MUTABLE_ROOT_SCANNERS.with(|s| s.borrow().clone()); - let scanner_states = scanners - .iter() - .map(|entry| entry.budgeted_state_factory.map(|factory| factory())) - .collect(); - Self { - scanners, - scanner_states, - ffi_scanners: FFI_MUTABLE_ROOT_SCANNERS.with(|s| s.borrow().clone()), - ffi_named_scanners: FFI_NAMED_MUTABLE_ROOT_SCANNERS.with(|s| s.borrow().clone()), - scanner_cursor: 0, - ffi_cursor: 0, - ffi_named_cursor: 0, - recorded_counts: false, - } - } - - fn step( - &mut self, - valid_ptrs: &ValidPointerSet, - mut root_sources: Option<&mut RootSourcesTraceStats>, - budget: usize, - allow_synchronous_scanners: bool, - minor_only: bool, - ) -> bool { - if !self.recorded_counts { - if let Some(sources) = &mut root_sources { - sources.runtime_handles.record_registered_scanners( - self.scanners - .iter() - .filter(|entry| entry.source == MutableRootScannerSource::RuntimeHandles) - .count(), - ); - sources.runtime_mutable_scanners.record_registered_scanners( - self.scanners - .iter() - .filter(|entry| { - entry.source == MutableRootScannerSource::RuntimeMutableScanner - }) - .count(), - ); - sources.ffi_mutable_scanners.record_registered_scanners( - self.ffi_scanners.len() + self.ffi_named_scanners.len(), - ); - } - self.recorded_counts = true; - } - - let mut remaining = budget; - // #9754: a minor-only trace is a young-scoped visit for the logged - // side tables (`gc/young_log.rs`); a full trace walks everything. - let mut visitor = RuntimeRootVisitor::for_mark_scoped(valid_ptrs, minor_only); - while self.scanner_cursor < self.scanners.len() { - if remaining == 0 { - return false; - } - let entry = self.scanners[self.scanner_cursor]; - let stats = match &mut root_sources { - Some(sources) => match entry.source { - MutableRootScannerSource::RuntimeHandles => { - Some(&mut sources.runtime_handles as *mut RootSourceSlotTraceStats) - } - MutableRootScannerSource::RuntimeMutableScanner => { - Some(&mut sources.runtime_mutable_scanners as *mut RootSourceSlotTraceStats) - } - }, - None => None, - }; - let previous = visitor.set_root_source_stats(stats); - let done = if let Some(scanner) = entry.budgeted_scanner { - let state = self.scanner_states[self.scanner_cursor] - .as_deref_mut() - .expect("budgeted scanner state exists"); - let before = remaining; - let done = scanner(&mut visitor, state, &mut remaining); - if done && remaining == before && remaining != usize::MAX { - remaining -= 1; - } - done - } else { - if !allow_synchronous_scanners { - return false; - } - remaining -= 1; - (entry.scanner)(&mut visitor); - true - }; - visitor.set_root_source_stats(previous); - if !done { - return false; - } - self.scanner_cursor += 1; - } - - if !allow_synchronous_scanners - && (self.ffi_cursor < self.ffi_scanners.len() - || self.ffi_named_cursor < self.ffi_named_scanners.len()) - { - return false; - } - - while remaining > 0 && self.ffi_cursor < self.ffi_scanners.len() { - let scanner = self.ffi_scanners[self.ffi_cursor]; - self.ffi_cursor += 1; - remaining -= 1; - let stats = match &mut root_sources { - Some(sources) => { - Some(&mut sources.ffi_mutable_scanners as *mut RootSourceSlotTraceStats) - } - None => None, - }; - let previous = visitor.set_root_source_stats(stats); - let ctx = &mut visitor as *mut RuntimeRootVisitor<'_> as *mut c_void; - scanner(perry_ffi_visit_mutable_root_slot, ctx); - visitor.set_root_source_stats(previous); - } - - while remaining > 0 && self.ffi_named_cursor < self.ffi_named_scanners.len() { - let (scanner, scanner_id) = self.ffi_named_scanners[self.ffi_named_cursor]; - self.ffi_named_cursor += 1; - remaining -= 1; - let stats = match &mut root_sources { - Some(sources) => { - Some(&mut sources.ffi_mutable_scanners as *mut RootSourceSlotTraceStats) - } - None => None, - }; - let previous = visitor.set_root_source_stats(stats); - let ctx = &mut visitor as *mut RuntimeRootVisitor<'_> as *mut c_void; - scanner(scanner_id, perry_ffi_visit_mutable_root_slot, ctx); - visitor.set_root_source_stats(previous); - } - - self.scanner_cursor >= self.scanners.len() - && self.ffi_cursor >= self.ffi_scanners.len() - && self.ffi_named_cursor >= self.ffi_named_scanners.len() - } -} - -struct LegacyRegisteredRootScanState { - scanners: Vec, - ffi_scanners: Vec, - scanner_cursor: usize, - ffi_cursor: usize, - stats: LegacyRootTraceStats, -} - -impl LegacyRegisteredRootScanState { - fn new() -> Self { - let scanners: Vec = ROOT_SCANNERS.with(|s| s.borrow().clone()); - let ffi_scanners: Vec = FFI_ROOT_SCANNERS.with(|s| s.borrow().clone()); - let stats = LegacyRootTraceStats { - registered_rust_scanners: scanners.len(), - registered_ffi_scanners: ffi_scanners.len(), - ..LegacyRootTraceStats::default() - }; - Self { - scanners, - ffi_scanners, - scanner_cursor: 0, - ffi_cursor: 0, - stats, - } - } - - fn step( - &mut self, - valid_ptrs: &ValidPointerSet, - pin_discoveries: bool, - budget: usize, - allow_synchronous_scanners: bool, - ) -> bool { - if !allow_synchronous_scanners - && (self.scanner_cursor < self.scanners.len() - || self.ffi_cursor < self.ffi_scanners.len()) - { - return false; - } - let mut remaining = budget; - while remaining > 0 && self.scanner_cursor < self.scanners.len() { - let scanner = self.scanners[self.scanner_cursor]; - self.scanner_cursor += 1; - remaining -= 1; - scanner(&mut |value: f64| { - record_copy_only_scanner_mark_emission( - value.to_bits(), - valid_ptrs, - &mut self.stats, - ); - if let Some(bytes) = - mark_copy_only_scanner_bits(value.to_bits(), valid_ptrs, pin_discoveries) - { - self.stats.pinned_roots += 1; - self.stats.pinned_bytes += bytes; - } - }); - } - - while remaining > 0 && self.ffi_cursor < self.ffi_scanners.len() { - let scanner = self.ffi_scanners[self.ffi_cursor]; - self.ffi_cursor += 1; - remaining -= 1; - let mut ctx = RegisteredRootMarkContext { - valid_ptrs: valid_ptrs as *const ValidPointerSet, - pin_discoveries, - legacy_stats: &mut self.stats as *mut LegacyRootTraceStats, - }; - let ctx = &mut ctx as *mut RegisteredRootMarkContext as *mut c_void; - scanner(perry_ffi_mark_root, ctx); - } - - self.scanner_cursor >= self.scanners.len() && self.ffi_cursor >= self.ffi_scanners.len() - } - - fn stats(&self) -> LegacyRootTraceStats { - self.stats - } -} +mod registered_root_scan; +use registered_root_scan::{LegacyRegisteredRootScanState, MutableRegisteredRootScanState}; struct RootScanCycleState { subphase: RootScanSubphase, diff --git a/crates/perry-runtime/src/gc/cycle/registered_root_scan.rs b/crates/perry-runtime/src/gc/cycle/registered_root_scan.rs new file mode 100644 index 0000000000..559a8fe66c --- /dev/null +++ b/crates/perry-runtime/src/gc/cycle/registered_root_scan.rs @@ -0,0 +1,244 @@ +//! The two registered-root scan cursors of the budgeted root scan. +//! +//! `RootScanCycleState` drives its `MutableRegisteredScanners` and +//! `LegacyRegisteredScanners` subphases through these; each keeps a snapshot +//! of the registry it walks plus a cursor, so a budgeted cycle can stop +//! between scanners and resume where it left off without re-reading a +//! registry the mutator may have grown in between. +//! +//! They live beside `cycle.rs` rather than in it because that file is at the +//! file-size gate; nothing else about the split is meaningful. + +use super::*; + +pub(super) struct MutableRegisteredRootScanState { + scanners: Vec, + scanner_states: Vec>>, + ffi_scanners: Vec, + ffi_named_scanners: Vec<(PerryFfiNamedMutableRootScanner, usize)>, + scanner_cursor: usize, + ffi_cursor: usize, + ffi_named_cursor: usize, + recorded_counts: bool, +} + +impl MutableRegisteredRootScanState { + pub(super) fn new() -> Self { + let scanners = MUTABLE_ROOT_SCANNERS.with(|s| s.borrow().clone()); + let scanner_states = scanners + .iter() + .map(|entry| entry.budgeted_state_factory.map(|factory| factory())) + .collect(); + Self { + scanners, + scanner_states, + ffi_scanners: FFI_MUTABLE_ROOT_SCANNERS.with(|s| s.borrow().clone()), + ffi_named_scanners: FFI_NAMED_MUTABLE_ROOT_SCANNERS.with(|s| s.borrow().clone()), + scanner_cursor: 0, + ffi_cursor: 0, + ffi_named_cursor: 0, + recorded_counts: false, + } + } + + pub(super) fn step( + &mut self, + valid_ptrs: &ValidPointerSet, + mut root_sources: Option<&mut RootSourcesTraceStats>, + budget: usize, + allow_synchronous_scanners: bool, + minor_only: bool, + ) -> bool { + if !self.recorded_counts { + if let Some(sources) = &mut root_sources { + sources.runtime_handles.record_registered_scanners( + self.scanners + .iter() + .filter(|entry| entry.source == MutableRootScannerSource::RuntimeHandles) + .count(), + ); + sources.runtime_mutable_scanners.record_registered_scanners( + self.scanners + .iter() + .filter(|entry| { + entry.source == MutableRootScannerSource::RuntimeMutableScanner + }) + .count(), + ); + sources.ffi_mutable_scanners.record_registered_scanners( + self.ffi_scanners.len() + self.ffi_named_scanners.len(), + ); + } + self.recorded_counts = true; + } + + let mut remaining = budget; + // #9754: a minor-only trace is a young-scoped visit for the logged + // side tables (`gc/young_log.rs`); a full trace walks everything. + let mut visitor = RuntimeRootVisitor::for_mark_scoped(valid_ptrs, minor_only); + while self.scanner_cursor < self.scanners.len() { + if remaining == 0 { + return false; + } + let entry = self.scanners[self.scanner_cursor]; + let stats = match &mut root_sources { + Some(sources) => match entry.source { + MutableRootScannerSource::RuntimeHandles => { + Some(&mut sources.runtime_handles as *mut RootSourceSlotTraceStats) + } + MutableRootScannerSource::RuntimeMutableScanner => { + Some(&mut sources.runtime_mutable_scanners as *mut RootSourceSlotTraceStats) + } + }, + None => None, + }; + let previous = visitor.set_root_source_stats(stats); + let done = if let Some(scanner) = entry.budgeted_scanner { + let state = self.scanner_states[self.scanner_cursor] + .as_deref_mut() + .expect("budgeted scanner state exists"); + let before = remaining; + let done = scanner(&mut visitor, state, &mut remaining); + if done && remaining == before && remaining != usize::MAX { + remaining -= 1; + } + done + } else { + if !allow_synchronous_scanners { + return false; + } + remaining -= 1; + (entry.scanner)(&mut visitor); + true + }; + visitor.set_root_source_stats(previous); + if !done { + return false; + } + self.scanner_cursor += 1; + } + + if !allow_synchronous_scanners + && (self.ffi_cursor < self.ffi_scanners.len() + || self.ffi_named_cursor < self.ffi_named_scanners.len()) + { + return false; + } + + while remaining > 0 && self.ffi_cursor < self.ffi_scanners.len() { + let scanner = self.ffi_scanners[self.ffi_cursor]; + self.ffi_cursor += 1; + remaining -= 1; + let stats = match &mut root_sources { + Some(sources) => { + Some(&mut sources.ffi_mutable_scanners as *mut RootSourceSlotTraceStats) + } + None => None, + }; + let previous = visitor.set_root_source_stats(stats); + let ctx = &mut visitor as *mut RuntimeRootVisitor<'_> as *mut c_void; + scanner(perry_ffi_visit_mutable_root_slot, ctx); + visitor.set_root_source_stats(previous); + } + + while remaining > 0 && self.ffi_named_cursor < self.ffi_named_scanners.len() { + let (scanner, scanner_id) = self.ffi_named_scanners[self.ffi_named_cursor]; + self.ffi_named_cursor += 1; + remaining -= 1; + let stats = match &mut root_sources { + Some(sources) => { + Some(&mut sources.ffi_mutable_scanners as *mut RootSourceSlotTraceStats) + } + None => None, + }; + let previous = visitor.set_root_source_stats(stats); + let ctx = &mut visitor as *mut RuntimeRootVisitor<'_> as *mut c_void; + scanner(scanner_id, perry_ffi_visit_mutable_root_slot, ctx); + visitor.set_root_source_stats(previous); + } + + self.scanner_cursor >= self.scanners.len() + && self.ffi_cursor >= self.ffi_scanners.len() + && self.ffi_named_cursor >= self.ffi_named_scanners.len() + } +} + +pub(super) struct LegacyRegisteredRootScanState { + scanners: Vec, + ffi_scanners: Vec, + scanner_cursor: usize, + ffi_cursor: usize, + stats: LegacyRootTraceStats, +} + +impl LegacyRegisteredRootScanState { + pub(super) fn new() -> Self { + let scanners: Vec = ROOT_SCANNERS.with(|s| s.borrow().clone()); + let ffi_scanners: Vec = FFI_ROOT_SCANNERS.with(|s| s.borrow().clone()); + let stats = LegacyRootTraceStats { + registered_rust_scanners: scanners.len(), + registered_ffi_scanners: ffi_scanners.len(), + ..LegacyRootTraceStats::default() + }; + Self { + scanners, + ffi_scanners, + scanner_cursor: 0, + ffi_cursor: 0, + stats, + } + } + + pub(super) fn step( + &mut self, + valid_ptrs: &ValidPointerSet, + pin_discoveries: bool, + budget: usize, + allow_synchronous_scanners: bool, + ) -> bool { + if !allow_synchronous_scanners + && (self.scanner_cursor < self.scanners.len() + || self.ffi_cursor < self.ffi_scanners.len()) + { + return false; + } + let mut remaining = budget; + while remaining > 0 && self.scanner_cursor < self.scanners.len() { + let scanner = self.scanners[self.scanner_cursor]; + self.scanner_cursor += 1; + remaining -= 1; + scanner(&mut |value: f64| { + record_copy_only_scanner_mark_emission( + value.to_bits(), + valid_ptrs, + &mut self.stats, + ); + if let Some(bytes) = + mark_copy_only_scanner_bits(value.to_bits(), valid_ptrs, pin_discoveries) + { + self.stats.pinned_roots += 1; + self.stats.pinned_bytes += bytes; + } + }); + } + + while remaining > 0 && self.ffi_cursor < self.ffi_scanners.len() { + let scanner = self.ffi_scanners[self.ffi_cursor]; + self.ffi_cursor += 1; + remaining -= 1; + let mut ctx = RegisteredRootMarkContext { + valid_ptrs: valid_ptrs as *const ValidPointerSet, + pin_discoveries, + legacy_stats: &mut self.stats as *mut LegacyRootTraceStats, + }; + let ctx = &mut ctx as *mut RegisteredRootMarkContext as *mut c_void; + scanner(perry_ffi_mark_root, ctx); + } + + self.scanner_cursor >= self.scanners.len() && self.ffi_cursor >= self.ffi_scanners.len() + } + + pub(super) fn stats(&self) -> LegacyRootTraceStats { + self.stats + } +} diff --git a/crates/perry-runtime/src/gc/mod.rs b/crates/perry-runtime/src/gc/mod.rs index 157f9fcd75..cb0acd9a6b 100644 --- a/crates/perry-runtime/src/gc/mod.rs +++ b/crates/perry-runtime/src/gc/mod.rs @@ -162,11 +162,11 @@ mod copying_pointer_set; mod forwarding; /// Per-scanner root attribution for the copied-minor root scan (#7915). mod scanner_profile; +mod sticky_remembered; /// #9754: per-side-table young-entry logs (remembered sets for the runtime /// side tables), so a minor-scoped root scan visits only the entries that /// can hold a pointer a minor acts on. pub(crate) mod young_log; -mod sticky_remembered; use copying::*; use copying_first_cycle::*; // Named rather than glob-imported: a glob does not propagate through the diff --git a/crates/perry-runtime/src/gc/roots.rs b/crates/perry-runtime/src/gc/roots.rs index c7cbe3bbe7..054bee16f7 100644 --- a/crates/perry-runtime/src/gc/roots.rs +++ b/crates/perry-runtime/src/gc/roots.rs @@ -708,118 +708,8 @@ pub(crate) fn try_mark_value_or_raw(word: u64, valid_ptrs: &ValidPointerSet) -> true } -/// Specialized mark-and-enqueue for trace-phase field walks. -/// -/// Descriptor-driven trace walks all share the same pattern: read a -/// heap-field word that is either a NaN-boxed JSValue or a raw I64 -/// pointer at an object start, mark it if live, and push the marked -/// header onto the local worklist. The generic -/// `try_mark_value_or_raw` is general enough to also handle -/// conservative stack scans (raw interior pointers via -/// `enclosing_object`) and root scans (push to MARK_SEEDS so the -/// trace-marked-objects entry point can pick them up), but BOTH of -/// those features are pure overhead inside `drain_trace_worklist`: -/// -/// 1. Field words never hold interior pointers — they're written via -/// `arr[i] = x` / `obj.f = x` / closure capture stores, all of -/// which use the object-start user pointer. Skipping -/// `enclosing_object` saves a binary-search lookup per field. -/// -/// 2. The MARK_SEEDS push happens once per newly-marked object during -/// trace, but the same header is also pushed onto the local -/// worklist by the caller (so the trace drain visits it). The -/// extra MARK_SEEDS push goes onto a TLS vec, gets cleared at the -/// start of the next cycle, and is pure waste while we're already -/// in the trace phase. Skipping it saves a TLS slot deref + -/// Vec::push per marked object. -/// -/// 3. The caller-side re-decode of the NaN-tag (to figure out -/// POINTER_MASK extraction vs raw-pointer extraction) is folded -/// into this function, so the caller doesn't pay that switch a -/// second time. -/// -/// The valid-pointer hashset check is still load-bearing here — we -/// only elide the secondary `enclosing_object` fallback. -#[inline(always)] -#[cfg(target_os = "macos")] -pub(super) fn get_stack_bottom() -> usize { - extern "C" { - fn pthread_self() -> *mut std::ffi::c_void; - fn pthread_get_stackaddr_np(thread: *mut std::ffi::c_void) -> *mut std::ffi::c_void; - } - unsafe { - let thread = pthread_self(); - pthread_get_stackaddr_np(thread) as usize - } -} - -#[cfg(target_os = "linux")] -pub(super) fn get_stack_bottom() -> usize { - crate::native_stack::stack_top() -} - -// Windows: read TEB.StackBase. Works on every supported Windows version -// (Windows 7+) without needing GetCurrentThreadStackLimits (Win8+), so it -// stays correct on the `--min-windows-version=7` build path. The TEB lives -// at GS:[0] on x86_64 (FS:[0] on x86); StackBase sits at offset 0x08 -// (the highest address — i.e. where the stack starts and grows down from). -// This is the same pointer kernel32!GetCurrentThreadStackLimits returns as -// `HighLimit`, just read directly from the TEB to avoid the kernel32 dep. -// -// Without this, conservative stack scan early-returns with stack_bottom=0, -// the GC sees no stack roots, and any heap pointer that lives only in a -// stack slot during a callback gets swept (issues #385/#386/#387 — the -// `Array.prototype.map` / `JSON.parse(...).property` / supported_features -// segfaults all traced back to here). -#[cfg(all(target_os = "windows", target_arch = "x86_64"))] -pub(super) fn get_stack_bottom() -> usize { - let stack_base: usize; - unsafe { - std::arch::asm!( - "mov {out}, gs:[0x08]", - out = out(reg) stack_base, - options(nostack, preserves_flags, readonly), - ); - } - stack_base -} - -#[cfg(all(target_os = "windows", target_arch = "x86"))] -pub(super) fn get_stack_bottom() -> usize { - let stack_base: usize; - unsafe { - std::arch::asm!( - "mov {out}, fs:[0x04]", - out = out(reg) stack_base, - options(nostack, preserves_flags, readonly), - ); - } - stack_base -} - -#[cfg(all(target_os = "windows", target_arch = "aarch64"))] -pub(super) fn get_stack_bottom() -> usize { - // ARM64 Windows: TEB pointer is in x18; StackBase at offset 0x08. - let stack_base: usize; - unsafe { - let teb: usize; - std::arch::asm!("mov {}, x18", out(reg) teb, options(nostack, preserves_flags, readonly)); - stack_base = *((teb + 0x08) as *const usize); - } - stack_base -} - -#[cfg(not(any( - target_os = "macos", - target_os = "linux", - all( - target_os = "windows", - any(target_arch = "x86_64", target_arch = "x86", target_arch = "aarch64") - ), -)))] -pub(super) fn get_stack_bottom() -> usize { - 0 // Stack scanning not supported on this OS/arch -} +mod stack_bottom; +pub(super) use stack_bottom::get_stack_bottom; pub(super) enum RuntimeRootVisitMode<'a> { Mark { diff --git a/crates/perry-runtime/src/gc/roots/stack_bottom.rs b/crates/perry-runtime/src/gc/roots/stack_bottom.rs new file mode 100644 index 0000000000..6cf6aa9fbf --- /dev/null +++ b/crates/perry-runtime/src/gc/roots/stack_bottom.rs @@ -0,0 +1,126 @@ +//! `get_stack_bottom` — the per-platform top of the current thread's stack. +//! +//! The conservative stack scan needs the address the stack grows down from, +//! and every OS answers differently (`pthread_get_stackaddr_np` on macOS, +//! `pthread_getattr_np` + `pthread_attr_getstack` on Linux, the TEB on +//! Windows, and a `0` fallback that disables stack scanning elsewhere). The +//! four `#[cfg]` arms plus their `extern "C"` blocks are the only +//! platform-conditional code in the root scanner, so they live here and +//! `roots.rs` stays under the file-size gate. +//! +//! The doc comment on the first arm describes a trace-phase mark helper, not +//! `get_stack_bottom`; it was already attached to this item and is moved +//! verbatim rather than re-pointed at the next unrelated item. + +/// Specialized mark-and-enqueue for trace-phase field walks. +/// +/// Descriptor-driven trace walks all share the same pattern: read a +/// heap-field word that is either a NaN-boxed JSValue or a raw I64 +/// pointer at an object start, mark it if live, and push the marked +/// header onto the local worklist. The generic +/// `try_mark_value_or_raw` is general enough to also handle +/// conservative stack scans (raw interior pointers via +/// `enclosing_object`) and root scans (push to MARK_SEEDS so the +/// trace-marked-objects entry point can pick them up), but BOTH of +/// those features are pure overhead inside `drain_trace_worklist`: +/// +/// 1. Field words never hold interior pointers — they're written via +/// `arr[i] = x` / `obj.f = x` / closure capture stores, all of +/// which use the object-start user pointer. Skipping +/// `enclosing_object` saves a binary-search lookup per field. +/// +/// 2. The MARK_SEEDS push happens once per newly-marked object during +/// trace, but the same header is also pushed onto the local +/// worklist by the caller (so the trace drain visits it). The +/// extra MARK_SEEDS push goes onto a TLS vec, gets cleared at the +/// start of the next cycle, and is pure waste while we're already +/// in the trace phase. Skipping it saves a TLS slot deref + +/// Vec::push per marked object. +/// +/// 3. The caller-side re-decode of the NaN-tag (to figure out +/// POINTER_MASK extraction vs raw-pointer extraction) is folded +/// into this function, so the caller doesn't pay that switch a +/// second time. +/// +/// The valid-pointer hashset check is still load-bearing here — we +/// only elide the secondary `enclosing_object` fallback. +#[inline(always)] +#[cfg(target_os = "macos")] +pub(crate) fn get_stack_bottom() -> usize { + extern "C" { + fn pthread_self() -> *mut std::ffi::c_void; + fn pthread_get_stackaddr_np(thread: *mut std::ffi::c_void) -> *mut std::ffi::c_void; + } + unsafe { + let thread = pthread_self(); + pthread_get_stackaddr_np(thread) as usize + } +} + +#[cfg(target_os = "linux")] +pub(crate) fn get_stack_bottom() -> usize { + crate::native_stack::stack_top() +} + +// Windows: read TEB.StackBase. Works on every supported Windows version +// (Windows 7+) without needing GetCurrentThreadStackLimits (Win8+), so it +// stays correct on the `--min-windows-version=7` build path. The TEB lives +// at GS:[0] on x86_64 (FS:[0] on x86); StackBase sits at offset 0x08 +// (the highest address — i.e. where the stack starts and grows down from). +// This is the same pointer kernel32!GetCurrentThreadStackLimits returns as +// `HighLimit`, just read directly from the TEB to avoid the kernel32 dep. +// +// Without this, conservative stack scan early-returns with stack_bottom=0, +// the GC sees no stack roots, and any heap pointer that lives only in a +// stack slot during a callback gets swept (issues #385/#386/#387 — the +// `Array.prototype.map` / `JSON.parse(...).property` / supported_features +// segfaults all traced back to here). +#[cfg(all(target_os = "windows", target_arch = "x86_64"))] +pub(crate) fn get_stack_bottom() -> usize { + let stack_base: usize; + unsafe { + std::arch::asm!( + "mov {out}, gs:[0x08]", + out = out(reg) stack_base, + options(nostack, preserves_flags, readonly), + ); + } + stack_base +} + +#[cfg(all(target_os = "windows", target_arch = "x86"))] +pub(crate) fn get_stack_bottom() -> usize { + let stack_base: usize; + unsafe { + std::arch::asm!( + "mov {out}, fs:[0x04]", + out = out(reg) stack_base, + options(nostack, preserves_flags, readonly), + ); + } + stack_base +} + +#[cfg(all(target_os = "windows", target_arch = "aarch64"))] +pub(crate) fn get_stack_bottom() -> usize { + // ARM64 Windows: TEB pointer is in x18; StackBase at offset 0x08. + let stack_base: usize; + unsafe { + let teb: usize; + std::arch::asm!("mov {}, x18", out(reg) teb, options(nostack, preserves_flags, readonly)); + stack_base = *((teb + 0x08) as *const usize); + } + stack_base +} + +#[cfg(not(any( + target_os = "macos", + target_os = "linux", + all( + target_os = "windows", + any(target_arch = "x86_64", target_arch = "x86", target_arch = "aarch64") + ), +)))] +pub(crate) fn get_stack_bottom() -> usize { + 0 // Stack scanning not supported on this OS/arch +} diff --git a/crates/perry-runtime/src/gc/sticky_remembered.rs b/crates/perry-runtime/src/gc/sticky_remembered.rs index f1b0c1e254..3402f6cbd7 100644 --- a/crates/perry-runtime/src/gc/sticky_remembered.rs +++ b/crates/perry-runtime/src/gc/sticky_remembered.rs @@ -82,7 +82,10 @@ impl StickyRememberedSet { pub(super) fn count_not_yet_dirty(&self) -> usize { let old_missing = super::barrier::DIRTY_OLD_PAGES.with(|s| { let s = s.borrow(); - self.old_pages.iter().filter(|page| !s.contains(page)).count() + self.old_pages + .iter() + .filter(|page| !s.contains(page)) + .count() }); let external_missing = super::barrier::EXTERNAL_DIRTY_SLOT_PAGES.with(|s| { let s = s.borrow(); 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 e21d71413e..196c7c0e90 100644 --- a/crates/perry-runtime/src/gc/tests/young_log_tests.rs +++ b/crates/perry-runtime/src/gc/tests/young_log_tests.rs @@ -69,22 +69,37 @@ fn young_closure_prop_value_is_moved_through_the_log() { let _ = gc_collect_minor(); let owner_after = (js_shadow_slot_get(0) & POINTER_MASK) as usize; - assert_ne!(owner_after, owner, "the rooted owner must have been evacuated"); + assert_ne!( + owner_after, owner, + "the rooted owner must have been evacuated" + ); let bits = crate::closure::closure_get_own_dynamic_prop(owner_after, "memo") .expect("entry must follow its owner to the new address") .to_bits(); let value_after = (bits & POINTER_MASK) as usize; assert_eq!(bits & TAG_MASK, STRING_TAG); - assert_ne!(value_after, value, "the value must have been evacuated, not left in from-space"); + assert_ne!( + value_after, value, + "the value must have been evacuated, not left in from-space" + ); assert!(crate::arena::pointer_in_nursery(value_after)); assert!( crate::closure::closure_get_own_dynamic_prop(owner, "memo").is_none(), "the stale owner key must be gone" ); let row = walk("closure.dynamic_props"); - assert!(row.partial, "a copying minor must take the young-scoped walk"); - assert!(row.visited >= 1, "the logged owner must have been visited: {row:?}"); - assert!(row.kept >= 1, "a survivor still young must stay logged: {row:?}"); + assert!( + row.partial, + "a copying minor must take the young-scoped walk" + ); + assert!( + row.visited >= 1, + "the logged owner must have been visited: {row:?}" + ); + assert!( + row.kept >= 1, + "a survivor still young must stay logged: {row:?}" + ); } #[test] @@ -284,8 +299,7 @@ fn young_keys_array_family_is_rekeyed_through_the_log() { let keys = unsafe { young_keys_array() }; js_shadow_slot_set(0, ptr_bits(keys as usize)); - let id = crate::object::shapes::shape_descriptor_ensure(keys, 0, 0) - .expect("shape id"); + let id = crate::object::shapes::shape_descriptor_ensure(keys, 0, 0).expect("shape id"); assert_eq!( crate::object::shapes::shape_descriptor_by_id(id).map(|d| d.keys), Some(keys as u64) @@ -294,7 +308,10 @@ fn young_keys_array_family_is_rekeyed_through_the_log() { let _ = gc_collect_minor(); let keys_after = (js_shadow_slot_get(0) & POINTER_MASK) as usize; - assert_ne!(keys_after, keys as usize, "the rooted keys array must have moved"); + assert_ne!( + keys_after, keys as usize, + "the rooted keys array must have moved" + ); assert_eq!( crate::object::shapes::shape_descriptor_by_id(id).map(|d| d.keys), Some(keys_after as u64), @@ -319,8 +336,7 @@ fn old_shape_families_are_skipped_by_a_minor() { (*keys).length = 0; (*keys).capacity = 0; } - let id = crate::object::shapes::shape_descriptor_ensure(keys, 0, 0) - .expect("shape id"); + let id = crate::object::shapes::shape_descriptor_ensure(keys, 0, 0).expect("shape id"); let _ = gc_collect_minor(); @@ -331,7 +347,10 @@ fn old_shape_families_are_skipped_by_a_minor() { let row = walk("shapes.families+indices"); assert!(row.partial); assert!(row.table_len >= 1, "{row:?}"); - assert_eq!(row.visited, 0, "an old keys array's family must not be visited: {row:?}"); + assert_eq!( + row.visited, 0, + "an old keys array's family must not be visited: {row:?}" + ); } // ------------------------------------------------------------------- caches @@ -345,8 +364,8 @@ fn young_transition_cache_target_is_rewritten_through_the_log() { js_shadow_slot_set(0, ptr_bits(keys)); // A predecessor that resolves, or the copied-minor prune retires the entry // (`shape_descriptor_by_id(0)` is `None`) before the assertion reads it. - let prev = crate::object::shapes::shape_descriptor_ensure(std::ptr::null(), 0, 1) - .expect("shape id"); + let prev = + crate::object::shapes::shape_descriptor_ensure(std::ptr::null(), 0, 1).expect("shape id"); crate::object::test_seed_transition_cache_root_for_shape(prev, keys); let _ = gc_collect_minor(); @@ -361,7 +380,10 @@ fn young_transition_cache_target_is_rewritten_through_the_log() { let row = walk("object.transition_cache"); assert!(row.partial); assert!(row.visited >= 1, "{row:?}"); - assert!(row.visited < row.table_len, "a 16k-slot table must not be walked whole: {row:?}"); + assert!( + row.visited < row.table_len, + "a 16k-slot table must not be walked whole: {row:?}" + ); } #[test] @@ -377,9 +399,15 @@ fn young_shape_cache_entry_is_moved_through_the_log() { let _ = gc_collect_minor(); let (inline, overflow) = crate::object::test_shape_cache_root(shape_id); - assert_ne!(overflow, keys as usize, "the overflow entry must have been evacuated"); + assert_ne!( + overflow, keys as usize, + "the overflow entry must have been evacuated" + ); assert!(crate::arena::pointer_in_nursery(overflow)); - assert_eq!(inline, overflow, "inline and overflow must agree on the new address"); + assert_eq!( + inline, overflow, + "inline and overflow must agree on the new address" + ); let row = walk("object.shape_cache"); assert!(row.partial); assert!(row.visited >= 1, "{row:?}"); diff --git a/crates/perry-runtime/src/object/descriptor_state.rs b/crates/perry-runtime/src/object/descriptor_state.rs index 5888a0ab16..109ef1e75e 100644 --- a/crates/perry-runtime/src/object/descriptor_state.rs +++ b/crates/perry-runtime/src/object/descriptor_state.rs @@ -130,15 +130,23 @@ impl DescriptorTables { const DESCRIPTOR_YOUNG_LOG_NAME: &str = "object.descriptors"; +mod young; +use young::{relevant_descriptor_owners, scan_descriptor_roots_young}; + /// Rule 1 of `gc/young_log.rs`: log `owner` BEFORE its descriptor is /// published when the owner, or the accessor closure being stored, can /// matter to a minor. Data descriptors carry no pointer, so `acc` is `None` /// for them and only the owner decides. #[inline] -fn note_young_descriptor_owner(st: &crate::state::RuntimeState, owner: usize, acc: Option<&AccessorDescriptor>) { +fn note_young_descriptor_owner( + st: &crate::state::RuntimeState, + owner: usize, + acc: Option<&AccessorDescriptor>, +) { use crate::gc::young_log::{addr_is_minor_relevant, bits_are_minor_relevant}; if addr_is_minor_relevant(owner) - || acc.is_some_and(|acc| bits_are_minor_relevant(acc.get) || bits_are_minor_relevant(acc.set)) + || acc + .is_some_and(|acc| bits_are_minor_relevant(acc.get) || bits_are_minor_relevant(acc.set)) { st.descriptors.young_owners.borrow_mut().note(owner); } @@ -1361,13 +1369,23 @@ pub(crate) fn prune_dead_descriptor_owner_entries_young(is_dead_owner: &dyn Fn(u /// Drop every entry `owner` holds in both tables and both indexes, through /// the owner index (O(owner's keys), not O(table)). fn remove_descriptor_owner_entries(st: &crate::state::RuntimeState, owner: usize) { - if let Some(keys) = st.descriptors.attr_keys_by_owner.borrow_mut().remove(&owner) { + if let Some(keys) = st + .descriptors + .attr_keys_by_owner + .borrow_mut() + .remove(&owner) + { let mut attrs = st.descriptors.property_descriptors.borrow_mut(); for key in keys { attrs.remove(&(owner, key)); } } - if let Some(keys) = st.descriptors.accessor_keys_by_owner.borrow_mut().remove(&owner) { + if let Some(keys) = st + .descriptors + .accessor_keys_by_owner + .borrow_mut() + .remove(&owner) + { let mut accessors = st.descriptors.accessor_descriptors.borrow_mut(); for key in keys { accessors.remove(&(owner, key)); @@ -1632,81 +1650,6 @@ pub(crate) fn scan_descriptor_roots_mut(visitor: &mut crate::gc::RuntimeRootVisi ); } -/// Every owner whose entry can matter to a minor, re-derived from the -/// authoritative tables: a non-old owner, or an accessor whose getter or -/// setter is non-old. -fn relevant_descriptor_owners(st: &crate::state::RuntimeState) -> Vec { - use crate::gc::young_log::{addr_is_minor_relevant, bits_are_minor_relevant}; - let mut relevant = Vec::new(); - for &owner in st.descriptors.attr_keys_by_owner.borrow().keys() { - if addr_is_minor_relevant(owner) { - relevant.push(owner); - } - } - for &owner in st.descriptors.accessor_keys_by_owner.borrow().keys() { - if addr_is_minor_relevant(owner) { - relevant.push(owner); - } - } - for ((owner, _), acc) in st.descriptors.accessor_descriptors.borrow().iter() { - if bits_are_minor_relevant(acc.get) || bits_are_minor_relevant(acc.set) { - relevant.push(*owner); - } - } - relevant.sort_unstable(); - relevant.dedup(); - relevant -} - -/// The minor-scoped walk (#9754): only the young-logged owners, each visited -/// exactly as the full walk visits it — accessor get/set rooted in every -/// phase, owner re-keyed across both tables and both indexes in the rewrite -/// phase — and re-logged iff still relevant afterwards. -fn scan_descriptor_roots_young( - visitor: &mut crate::gc::RuntimeRootVisitor<'_>, - st: &crate::state::RuntimeState, -) { - let table_len = st.descriptors.attr_keys_by_owner.borrow().len() as u64 - + st.descriptors.accessor_keys_by_owner.borrow().len() as u64; - #[cfg(debug_assertions)] - { - let relevant = relevant_descriptor_owners(st); - st.descriptors - .young_owners - .borrow() - .debug_assert_logged(DESCRIPTOR_YOUNG_LOG_NAME, &relevant); - } - let mut logged = 0u64; - let mut visited = 0u64; - let mut kept = Vec::new(); - loop { - let batch = st.descriptors.young_owners.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_descriptor_owner(visitor, st, owner); - if relevant { - kept.push(new_owner); - } - } - } - let kept_len = kept.len() as u64; - st.descriptors.young_owners.borrow_mut().extend(kept); - crate::gc::young_log::note_walk( - DESCRIPTOR_YOUNG_LOG_NAME, - crate::gc::young_log::YoungLogWalk { - partial: true, - logged, - visited, - kept: kept_len, - table_len, - }, - ); -} - /// Visit one owner's descriptors. Returns the post-visit owner address and /// whether the entry can still matter to a minor. fn scan_descriptor_owner( diff --git a/crates/perry-runtime/src/object/descriptor_state/young.rs b/crates/perry-runtime/src/object/descriptor_state/young.rs new file mode 100644 index 0000000000..21b7f77ade --- /dev/null +++ b/crates/perry-runtime/src/object/descriptor_state/young.rs @@ -0,0 +1,85 @@ +//! The minor-scoped half of the string-keyed descriptor root scan. +//! +//! `scan_descriptor_roots_mut` walks every owner in `attr_keys_by_owner` and +//! `accessor_keys_by_owner`; on a minor that is ~23k owners to discover that +//! none of them points at the nursery. The young-entry log +//! (`gc/young_log.rs`) narrows a minor-scoped pass to the owners a minor can +//! act on, and this module holds that pass plus the re-derivation of the +//! relevant set that rule 2 checks it against under `debug_assertions`. + +use super::*; + +/// Every owner whose entry can matter to a minor, re-derived from the +/// authoritative tables: a non-old owner, or an accessor whose getter or +/// setter is non-old. +pub(super) fn relevant_descriptor_owners(st: &crate::state::RuntimeState) -> Vec { + use crate::gc::young_log::{addr_is_minor_relevant, bits_are_minor_relevant}; + let mut relevant = Vec::new(); + for &owner in st.descriptors.attr_keys_by_owner.borrow().keys() { + if addr_is_minor_relevant(owner) { + relevant.push(owner); + } + } + for &owner in st.descriptors.accessor_keys_by_owner.borrow().keys() { + if addr_is_minor_relevant(owner) { + relevant.push(owner); + } + } + for ((owner, _), acc) in st.descriptors.accessor_descriptors.borrow().iter() { + if bits_are_minor_relevant(acc.get) || bits_are_minor_relevant(acc.set) { + relevant.push(*owner); + } + } + relevant.sort_unstable(); + relevant.dedup(); + relevant +} + +/// The minor-scoped walk (#9754): only the young-logged owners, each visited +/// exactly as the full walk visits it — accessor get/set rooted in every +/// phase, owner re-keyed across both tables and both indexes in the rewrite +/// phase — and re-logged iff still relevant afterwards. +pub(super) fn scan_descriptor_roots_young( + visitor: &mut crate::gc::RuntimeRootVisitor<'_>, + st: &crate::state::RuntimeState, +) { + let table_len = st.descriptors.attr_keys_by_owner.borrow().len() as u64 + + st.descriptors.accessor_keys_by_owner.borrow().len() as u64; + #[cfg(debug_assertions)] + { + let relevant = relevant_descriptor_owners(st); + st.descriptors + .young_owners + .borrow() + .debug_assert_logged(DESCRIPTOR_YOUNG_LOG_NAME, &relevant); + } + let mut logged = 0u64; + let mut visited = 0u64; + let mut kept = Vec::new(); + loop { + let batch = st.descriptors.young_owners.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_descriptor_owner(visitor, st, owner); + if relevant { + kept.push(new_owner); + } + } + } + let kept_len = kept.len() as u64; + st.descriptors.young_owners.borrow_mut().extend(kept); + crate::gc::young_log::note_walk( + DESCRIPTOR_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: true, + logged, + visited, + kept: kept_len, + table_len, + }, + ); +} diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index 3115b60428..6e1cd0fe0b 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -102,12 +102,24 @@ pub(crate) mod has_own_helpers; mod instanceof; mod live_slots; mod null_stub; +mod side_table_roots; pub(crate) use live_slots::set_object_live_slot_count; pub use live_slots::{ js_object_live_slot_count, object_live_slot_count, perry_object_header_abi_revision, }; pub use null_stub::{js_unresolved_default_call, js_unresolved_namespace_stub}; pub(crate) use null_stub::{NullObjectBytes, NULL_OBJECT_BYTES}; +pub(crate) use side_table_roots::{ + prune_dead_transition_cache_entries, prune_dead_transition_cache_entries_young, +}; +pub use side_table_roots::{ + scan_shape_cache_roots, scan_shape_cache_roots_mut, scan_transition_cache_roots, + scan_transition_cache_roots_mut, +}; +#[cfg(test)] +pub(crate) use side_table_roots::{ + test_seed_transition_cache_entry, test_transition_cache_occupancy, +}; pub(crate) mod iterator_prototypes; pub(crate) mod map_set_subclass; mod namespace_create; @@ -257,9 +269,9 @@ pub(crate) use descriptor_state::{ object_proto_may_intercept_key, owner_has_property_descriptors, owner_may_have_descriptor_entries, plain_data_write_may_intercept, prune_dead_descriptor_owner_entries, prune_dead_descriptor_owner_entries_young, - reflect_getter_closure_bits, set_accessor_descriptor, - set_builtin_accessor_descriptor, set_builtin_property_attrs, set_property_attrs, - transfer_descriptor_owner, AccessorDescriptor, DescriptorTables, PropertyAttrs, + reflect_getter_closure_bits, set_accessor_descriptor, set_builtin_accessor_descriptor, + set_builtin_property_attrs, set_property_attrs, transfer_descriptor_owner, AccessorDescriptor, + DescriptorTables, PropertyAttrs, }; pub(crate) use field_get_set::FieldLookupCaches; pub(crate) use field_get_set::{ @@ -813,7 +825,6 @@ fn transition_entry_is_minor_relevant(entry: &TransitionEntry) -> bool { || ((entry.slot_idx >> 24) == 0 && addr_is_minor_relevant(entry.key_ptr))) } - // Per-thread transition cache (`ObjectHotTables::transition_cache`). Was a // process-wide `static mut`, but with `perry/thread` user code allocating // objects on worker threads each thread has its own arena — cached @@ -1111,338 +1122,6 @@ fn transition_cache_insert( // stay lazy to avoid cloning every growing prefix. } -/// GC root scanner for the transition cache. Same contract as -/// `scan_shape_cache_roots` — without this the mark phase would free -/// cached target arrays that no live object currently holds directly, -/// and the next cache-hit store would dereference freed memory. -/// -/// #855: walk the static via `&raw const` + raw pointer indexing to -/// avoid the `static_mut_refs` lint (hard error in Rust 2024). The -/// cache is thread-local-by-discipline (perry user code is single- -/// threaded), so the unsafe deref is sound. -pub fn scan_transition_cache_roots(mark: &mut dyn FnMut(f64)) { - let mut visitor = crate::gc::RuntimeRootVisitor::for_copy(mark); - scan_transition_cache_roots_mut(&mut visitor); -} - -pub fn scan_transition_cache_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { - // #9754: a minor-scoped pass visits only the young-logged slots; a full - // pass walks the table and rebuilds the log. Both share - // `scan_transition_cache_slot`. - if visitor.young_scope() { - let mut logged = 0u64; - let mut visited = 0u64; - let mut kept = Vec::new(); - #[cfg(debug_assertions)] - with_transition_cache(|table| unsafe { - let relevant: Vec = (0..TRANSITION_CACHE_SIZE) - .filter(|&i| transition_entry_is_minor_relevant(&(*table)[i])) - .map(|i| i as u32) - .collect(); - TRANSITION_CACHE_YOUNG.with(|log| { - log.borrow() - .debug_assert_logged(TRANSITION_CACHE_YOUNG_LOG_NAME, &relevant) - }); - }); - let batch = TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().take_sorted()); - logged += batch.len() as u64; - with_transition_cache(|table| unsafe { - for slot in batch { - visited += 1; - if scan_transition_cache_slot(visitor, table, slot as usize) { - kept.push(slot); - } - } - }); - let kept_len = kept.len() as u64; - TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().extend(kept)); - crate::gc::young_log::note_walk( - TRANSITION_CACHE_YOUNG_LOG_NAME, - crate::gc::young_log::YoungLogWalk { - partial: true, - logged, - visited, - kept: kept_len, - table_len: TRANSITION_CACHE_SIZE as u64, - }, - ); - array_tail_transition::scan_roots_mut(visitor); - return; - } - let _ = TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().take_sorted()); - let mut kept = Vec::new(); - with_transition_cache(|table| unsafe { - for i in 0..TRANSITION_CACHE_SIZE { - if scan_transition_cache_slot(visitor, table, i) { - kept.push(i as u32); - } - } - }); - let kept_len = kept.len() as u64; - TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().extend(kept)); - crate::gc::young_log::note_walk( - TRANSITION_CACHE_YOUNG_LOG_NAME, - crate::gc::young_log::YoungLogWalk { - partial: false, - logged: TRANSITION_CACHE_SIZE as u64, - visited: TRANSITION_CACHE_SIZE as u64, - kept: kept_len, - table_len: TRANSITION_CACHE_SIZE as u64, - }, - ); - array_tail_transition::scan_roots_mut(visitor); -} - -/// Visit one transition-cache slot. Returns whether the entry can still -/// matter to a minor afterwards. -/// -/// # Safety -/// `table` must be this thread's transition cache. -unsafe fn scan_transition_cache_slot( - visitor: &mut crate::gc::RuntimeRootVisitor<'_>, - table: *mut [TransitionEntry; TRANSITION_CACHE_SIZE], - i: usize, -) -> bool { - let entry = &mut (*table)[i]; - if entry.next_keys == 0 { - return false; - } - let mut invalidate = false; - // Content-namespace ids (len marker != 0) are string BYTES, - // not addresses — the visitor must not rewrite them. - if (entry.slot_idx >> 24) == 0 { - invalidate |= visitor.visit_metadata_usize_slot(&mut entry.key_ptr); - } - // #6759 phase 3: `next_keys` is WEAK, not a strong root. - // - // `visit_usize_slot` MARKS. With 16384 slots this cache was - // therefore keeping up to 16384 keys arrays — and, through - // them, their shape descriptors — alive whether or not any live - // object still had that shape. That is a direct contributor to - // the shape table growing without bound between full - // collections (measured: 786k descriptors on a workload holding - // under 400 live objects). - // - // A transition entry is a pure cache: it answers "adding key k - // to shape S yields shape T". If nothing has shape T any more, - // the answer is worthless, so pinning T's keys array to keep it - // answerable is backwards. `key_ptr` was already weak for the - // same reason; this makes the pair consistent. - // - // Rewrite-only keeps a surviving target's address correct; - // `prune_dead_transition_cache_entries` drops the entry when the - // target did not survive. - visitor.visit_metadata_usize_slot(&mut entry.next_keys); - if invalidate { - *entry = TransitionEntry { - key_ptr: 0, - next_keys: 0, - prev_shape_id: 0, - target_shape_id: 0, - slot_idx: 0, - target_len: 0, - }; - return false; - } - transition_entry_is_minor_relevant(entry) -} - -/// [`prune_dead_transition_cache_entries`] for a MINOR (#9754): only a slot -/// in the young log can name a young — hence possibly dead — address. -#[cold] -pub(crate) fn prune_dead_transition_cache_entries_young(is_dead_owner: &dyn Fn(usize) -> bool) { - let candidates = TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().take_sorted()); - let mut kept = Vec::with_capacity(candidates.len()); - with_transition_cache(|table| unsafe { - for slot in candidates { - let entry = &mut (*table)[slot as usize]; - if entry.next_keys == 0 { - continue; - } - if transition_entry_is_dead(entry, is_dead_owner) { - *entry = TransitionEntry { - key_ptr: 0, - next_keys: 0, - prev_shape_id: 0, - target_shape_id: 0, - slot_idx: 0, - target_len: 0, - }; - } else { - kept.push(slot); - } - } - }); - TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().extend(kept)); - array_tail_transition::prune_invalid_entries(); -} - -/// The death test of `prune_dead_transition_cache_entries`, shared with the -/// young-only variant. -fn transition_entry_is_dead(entry: &TransitionEntry, is_dead_owner: &dyn Fn(usize) -> bool) -> bool { - ((entry.slot_idx >> 24) == 0 && entry.key_ptr != 0 && is_dead_owner(entry.key_ptr)) - // #6759 phase 3: `next_keys` stopped being a strong root, so a - // dead target is now possible and must be reaped here — this is - // the half that makes weakening it safe. - || is_dead_owner(entry.next_keys) - || shapes::shape_descriptor_by_id(entry.prev_shape_id).is_none() - || (entry.target_shape_id != 0 - && shapes::shape_descriptor_by_id(entry.target_shape_id).is_none()) -} - -/// #8192: death pruning for the transition cache. -/// -/// The interned `key_ptr` is metadata-only and therefore weak; `next_keys` is -/// a strong root. The predecessor and target ShapeIds are stable non-pointer -/// metadata, so moving collection neither rewrites nor invalidates them. -/// -/// The entry is a pure cache, so the repair is to drop it. `next_keys == 0` is -/// the empty-slot sentinel. -/// -/// `gc::dead_owner::DEAD_KEY_PRUNES` runs `prune_dead_shape_keys` before this -/// function. A predecessor whose keys edge died therefore has no descriptor -/// by the time we visit the cache. Both ShapeIds must still resolve: the -/// predecessor is weak, while the strongly rooted target keys normally keep -/// their descriptor live. Checking both here makes that target invariant a -/// release-mode post-GC proof without adding a hash-table lookup to every hot -/// transition stamp. -#[cold] -pub(crate) fn prune_dead_transition_cache_entries(is_dead_owner: &dyn Fn(usize) -> bool) { - with_transition_cache(|table| unsafe { - for i in 0..TRANSITION_CACHE_SIZE { - let entry = &mut (*table)[i]; - if entry.next_keys == 0 { - continue; - } - if transition_entry_is_dead(entry, is_dead_owner) { - *entry = TransitionEntry { - key_ptr: 0, - next_keys: 0, - prev_shape_id: 0, - target_shape_id: 0, - slot_idx: 0, - target_len: 0, - }; - } - } - }); - array_tail_transition::prune_invalid_entries(); -} - -#[cfg(test)] -pub(crate) fn test_transition_cache_occupancy() -> usize { - with_transition_cache(|table| unsafe { - (0..TRANSITION_CACHE_SIZE) - .filter(|&i| (*table)[i].next_keys != 0) - .count() - }) -} - -#[cfg(test)] -pub(crate) fn test_seed_transition_cache_entry( - prev_shape_id: u32, - key_ptr: usize, - next_keys: usize, -) { - let slot = transition_cache_slot(prev_shape_id, key_ptr); - if crate::gc::young_log::addr_is_minor_relevant(next_keys) - || crate::gc::young_log::addr_is_minor_relevant(key_ptr) - { - TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().note(slot as u32)); - } - with_transition_cache(|table| unsafe { - (*table)[slot] = TransitionEntry { - key_ptr, - next_keys, - prev_shape_id, - target_shape_id: 0, - slot_idx: 0, - target_len: 1, - }; - }); -} - -/// GC root scanner: mark all cached shape keys arrays so they're not freed. -/// The inline cache + overflow map both hold the raw `*mut ArrayHeader` -/// pointers; without this scanner, GC would free those arrays, leaving -/// every object with that shape holding a dangling `keys_array` pointer. -pub fn scan_shape_cache_roots(mark: &mut dyn FnMut(f64)) { - let mut visitor = crate::gc::RuntimeRootVisitor::for_copy(mark); - scan_shape_cache_roots_mut(&mut visitor); -} - -pub fn scan_shape_cache_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { - use crate::gc::young_log::addr_is_minor_relevant; - let st = crate::state::state(); - // The inline array is 256 fixed slots: always walked. The overflow map - // holds every shape id ever cached; #9754: a minor-scoped pass visits - // only the young-logged ids there, a full pass rebuilds the log. - let entries = unsafe { &mut *st.object_hot.shape_inline_cache.get() }; - for entry in entries.iter_mut() { - visitor.visit_raw_mut_ptr_slot(&mut entry.keys_array); - } - let mut cache = st.object_hot.shape_cache_overflow.borrow_mut(); - let table_len = cache.len() as u64; - if visitor.young_scope() { - #[cfg(debug_assertions)] - { - let relevant: Vec = cache - .iter() - .filter(|(_, (arr_ptr, _))| addr_is_minor_relevant(*arr_ptr as usize)) - .map(|(&id, _)| id) - .collect(); - SHAPE_CACHE_YOUNG.with(|log| { - log.borrow() - .debug_assert_logged(SHAPE_CACHE_YOUNG_LOG_NAME, &relevant) - }); - } - let batch = SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().take_sorted()); - let logged = batch.len() as u64; - let mut kept = Vec::new(); - for id in batch { - if let Some((arr_ptr, _)) = cache.get_mut(&id) { - visitor.visit_raw_mut_ptr_slot(arr_ptr); - if addr_is_minor_relevant(*arr_ptr as usize) { - kept.push(id); - } - } - } - let kept_len = kept.len() as u64; - SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().extend(kept)); - crate::gc::young_log::note_walk( - SHAPE_CACHE_YOUNG_LOG_NAME, - crate::gc::young_log::YoungLogWalk { - partial: true, - logged, - visited: logged, - kept: kept_len, - table_len, - }, - ); - return; - } - let _ = SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().take_sorted()); - let mut kept = Vec::new(); - for (&id, (arr_ptr, _runtime_shape_id)) in cache.iter_mut() { - visitor.visit_raw_mut_ptr_slot(arr_ptr); - if addr_is_minor_relevant(*arr_ptr as usize) { - kept.push(id); - } - } - let kept_len = kept.len() as u64; - SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().extend(kept)); - crate::gc::young_log::note_walk( - SHAPE_CACHE_YOUNG_LOG_NAME, - crate::gc::young_log::YoungLogWalk { - partial: false, - logged: table_len, - visited: table_len, - kept: kept_len, - table_len, - }, - ); -} - /// GC root scanner: mark all JSValues stored in OVERFLOW_FIELDS. /// OVERFLOW_FIELDS stores extra properties for objects that exceed their pre-allocated inline /// slot count. The u64 JSValue bits may contain NaN-boxed pointers to heap objects (strings, diff --git a/crates/perry-runtime/src/object/shapes.rs b/crates/perry-runtime/src/object/shapes.rs index 46a0ff600a..d04ac15c83 100644 --- a/crates/perry-runtime/src/object/shapes.rs +++ b/crates/perry-runtime/src/object/shapes.rs @@ -2135,7 +2135,8 @@ fn scan_shape_table_young( continue; } visited += 1; - let (post, relevant) = scan_shape_keys_address(visitor, table, inner, rewrite_phase, keys); + let (post, relevant) = + scan_shape_keys_address(visitor, table, inner, rewrite_phase, keys); if relevant { kept.push(post); } @@ -2237,7 +2238,10 @@ fn scan_shape_keys_address( inner.indices.remove(&addr); } } - (post, crate::gc::young_log::addr_is_minor_relevant(post as usize)) + ( + post, + crate::gc::young_log::addr_is_minor_relevant(post as usize), + ) } // #8112 sabotage switch. Suppressing the descriptor edge proves the fixture's diff --git a/crates/perry-runtime/src/object/side_table_roots.rs b/crates/perry-runtime/src/object/side_table_roots.rs new file mode 100644 index 0000000000..bc1bd4f46c --- /dev/null +++ b/crates/perry-runtime/src/object/side_table_roots.rs @@ -0,0 +1,350 @@ +//! GC root scanning and dead-owner pruning for the two hot shape caches. +//! +//! The transition cache (`(predecessor ShapeId, key) -> successor`) and the +//! shape cache (`static shape id -> canonical keys array`) both hold raw +//! `*mut ArrayHeader` pointers that no live object need hold directly, so the +//! collector has to visit them or the next cache hit dereferences freed +//! memory. Both carry a young-entry log (`gc/young_log.rs`), so each scanner +//! and each prune exists in a full-walk and a minor-scoped form; keeping the +//! four pairs together — and out of `object/mod.rs`, which is at the +//! file-size gate — is what this module is for. +//! +//! Everything here reaches its tables through `super`: the caches, their +//! logs and their per-entry helpers stay private to `object`. + +use super::*; + +/// GC root scanner for the transition cache. Same contract as +/// `scan_shape_cache_roots` — without this the mark phase would free +/// cached target arrays that no live object currently holds directly, +/// and the next cache-hit store would dereference freed memory. +/// +/// #855: walk the static via `&raw const` + raw pointer indexing to +/// avoid the `static_mut_refs` lint (hard error in Rust 2024). The +/// cache is thread-local-by-discipline (perry user code is single- +/// threaded), so the unsafe deref is sound. +pub fn scan_transition_cache_roots(mark: &mut dyn FnMut(f64)) { + let mut visitor = crate::gc::RuntimeRootVisitor::for_copy(mark); + scan_transition_cache_roots_mut(&mut visitor); +} + +pub fn scan_transition_cache_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { + // #9754: a minor-scoped pass visits only the young-logged slots; a full + // pass walks the table and rebuilds the log. Both share + // `scan_transition_cache_slot`. + if visitor.young_scope() { + let mut logged = 0u64; + let mut visited = 0u64; + let mut kept = Vec::new(); + #[cfg(debug_assertions)] + with_transition_cache(|table| unsafe { + let relevant: Vec = (0..TRANSITION_CACHE_SIZE) + .filter(|&i| transition_entry_is_minor_relevant(&(*table)[i])) + .map(|i| i as u32) + .collect(); + TRANSITION_CACHE_YOUNG.with(|log| { + log.borrow() + .debug_assert_logged(TRANSITION_CACHE_YOUNG_LOG_NAME, &relevant) + }); + }); + let batch = TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().take_sorted()); + logged += batch.len() as u64; + with_transition_cache(|table| unsafe { + for slot in batch { + visited += 1; + if scan_transition_cache_slot(visitor, table, slot as usize) { + kept.push(slot); + } + } + }); + let kept_len = kept.len() as u64; + TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().extend(kept)); + crate::gc::young_log::note_walk( + TRANSITION_CACHE_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: true, + logged, + visited, + kept: kept_len, + table_len: TRANSITION_CACHE_SIZE as u64, + }, + ); + array_tail_transition::scan_roots_mut(visitor); + return; + } + let _ = TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().take_sorted()); + let mut kept = Vec::new(); + with_transition_cache(|table| unsafe { + for i in 0..TRANSITION_CACHE_SIZE { + if scan_transition_cache_slot(visitor, table, i) { + kept.push(i as u32); + } + } + }); + let kept_len = kept.len() as u64; + TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().extend(kept)); + crate::gc::young_log::note_walk( + TRANSITION_CACHE_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: false, + logged: TRANSITION_CACHE_SIZE as u64, + visited: TRANSITION_CACHE_SIZE as u64, + kept: kept_len, + table_len: TRANSITION_CACHE_SIZE as u64, + }, + ); + array_tail_transition::scan_roots_mut(visitor); +} + +/// Visit one transition-cache slot. Returns whether the entry can still +/// matter to a minor afterwards. +/// +/// # Safety +/// `table` must be this thread's transition cache. +unsafe fn scan_transition_cache_slot( + visitor: &mut crate::gc::RuntimeRootVisitor<'_>, + table: *mut [TransitionEntry; TRANSITION_CACHE_SIZE], + i: usize, +) -> bool { + let entry = &mut (*table)[i]; + if entry.next_keys == 0 { + return false; + } + let mut invalidate = false; + // Content-namespace ids (len marker != 0) are string BYTES, + // not addresses — the visitor must not rewrite them. + if (entry.slot_idx >> 24) == 0 { + invalidate |= visitor.visit_metadata_usize_slot(&mut entry.key_ptr); + } + // #6759 phase 3: `next_keys` is WEAK, not a strong root. + // + // `visit_usize_slot` MARKS. With 16384 slots this cache was + // therefore keeping up to 16384 keys arrays — and, through + // them, their shape descriptors — alive whether or not any live + // object still had that shape. That is a direct contributor to + // the shape table growing without bound between full + // collections (measured: 786k descriptors on a workload holding + // under 400 live objects). + // + // A transition entry is a pure cache: it answers "adding key k + // to shape S yields shape T". If nothing has shape T any more, + // the answer is worthless, so pinning T's keys array to keep it + // answerable is backwards. `key_ptr` was already weak for the + // same reason; this makes the pair consistent. + // + // Rewrite-only keeps a surviving target's address correct; + // `prune_dead_transition_cache_entries` drops the entry when the + // target did not survive. + visitor.visit_metadata_usize_slot(&mut entry.next_keys); + if invalidate { + *entry = TransitionEntry { + key_ptr: 0, + next_keys: 0, + prev_shape_id: 0, + target_shape_id: 0, + slot_idx: 0, + target_len: 0, + }; + return false; + } + transition_entry_is_minor_relevant(entry) +} + +/// [`prune_dead_transition_cache_entries`] for a MINOR (#9754): only a slot +/// in the young log can name a young — hence possibly dead — address. +#[cold] +pub(crate) fn prune_dead_transition_cache_entries_young(is_dead_owner: &dyn Fn(usize) -> bool) { + let candidates = TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().take_sorted()); + let mut kept = Vec::with_capacity(candidates.len()); + with_transition_cache(|table| unsafe { + for slot in candidates { + let entry = &mut (*table)[slot as usize]; + if entry.next_keys == 0 { + continue; + } + if transition_entry_is_dead(entry, is_dead_owner) { + *entry = TransitionEntry { + key_ptr: 0, + next_keys: 0, + prev_shape_id: 0, + target_shape_id: 0, + slot_idx: 0, + target_len: 0, + }; + } else { + kept.push(slot); + } + } + }); + TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().extend(kept)); + array_tail_transition::prune_invalid_entries(); +} + +/// The death test of `prune_dead_transition_cache_entries`, shared with the +/// young-only variant. +fn transition_entry_is_dead( + entry: &TransitionEntry, + is_dead_owner: &dyn Fn(usize) -> bool, +) -> bool { + ((entry.slot_idx >> 24) == 0 && entry.key_ptr != 0 && is_dead_owner(entry.key_ptr)) + // #6759 phase 3: `next_keys` stopped being a strong root, so a + // dead target is now possible and must be reaped here — this is + // the half that makes weakening it safe. + || is_dead_owner(entry.next_keys) + || shapes::shape_descriptor_by_id(entry.prev_shape_id).is_none() + || (entry.target_shape_id != 0 + && shapes::shape_descriptor_by_id(entry.target_shape_id).is_none()) +} + +/// #8192: death pruning for the transition cache. +/// +/// The interned `key_ptr` is metadata-only and therefore weak; `next_keys` is +/// a strong root. The predecessor and target ShapeIds are stable non-pointer +/// metadata, so moving collection neither rewrites nor invalidates them. +/// +/// The entry is a pure cache, so the repair is to drop it. `next_keys == 0` is +/// the empty-slot sentinel. +/// +/// `gc::dead_owner::DEAD_KEY_PRUNES` runs `prune_dead_shape_keys` before this +/// function. A predecessor whose keys edge died therefore has no descriptor +/// by the time we visit the cache. Both ShapeIds must still resolve: the +/// predecessor is weak, while the strongly rooted target keys normally keep +/// their descriptor live. Checking both here makes that target invariant a +/// release-mode post-GC proof without adding a hash-table lookup to every hot +/// transition stamp. +#[cold] +pub(crate) fn prune_dead_transition_cache_entries(is_dead_owner: &dyn Fn(usize) -> bool) { + with_transition_cache(|table| unsafe { + for i in 0..TRANSITION_CACHE_SIZE { + let entry = &mut (*table)[i]; + if entry.next_keys == 0 { + continue; + } + if transition_entry_is_dead(entry, is_dead_owner) { + *entry = TransitionEntry { + key_ptr: 0, + next_keys: 0, + prev_shape_id: 0, + target_shape_id: 0, + slot_idx: 0, + target_len: 0, + }; + } + } + }); + array_tail_transition::prune_invalid_entries(); +} + +#[cfg(test)] +pub(crate) fn test_transition_cache_occupancy() -> usize { + with_transition_cache(|table| unsafe { + (0..TRANSITION_CACHE_SIZE) + .filter(|&i| (*table)[i].next_keys != 0) + .count() + }) +} + +#[cfg(test)] +pub(crate) fn test_seed_transition_cache_entry( + prev_shape_id: u32, + key_ptr: usize, + next_keys: usize, +) { + let slot = transition_cache_slot(prev_shape_id, key_ptr); + if crate::gc::young_log::addr_is_minor_relevant(next_keys) + || crate::gc::young_log::addr_is_minor_relevant(key_ptr) + { + TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().note(slot as u32)); + } + with_transition_cache(|table| unsafe { + (*table)[slot] = TransitionEntry { + key_ptr, + next_keys, + prev_shape_id, + target_shape_id: 0, + slot_idx: 0, + target_len: 1, + }; + }); +} + +/// GC root scanner: mark all cached shape keys arrays so they're not freed. +/// The inline cache + overflow map both hold the raw `*mut ArrayHeader` +/// pointers; without this scanner, GC would free those arrays, leaving +/// every object with that shape holding a dangling `keys_array` pointer. +pub fn scan_shape_cache_roots(mark: &mut dyn FnMut(f64)) { + let mut visitor = crate::gc::RuntimeRootVisitor::for_copy(mark); + scan_shape_cache_roots_mut(&mut visitor); +} + +pub fn scan_shape_cache_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { + use crate::gc::young_log::addr_is_minor_relevant; + let st = crate::state::state(); + // The inline array is 256 fixed slots: always walked. The overflow map + // holds every shape id ever cached; #9754: a minor-scoped pass visits + // only the young-logged ids there, a full pass rebuilds the log. + let entries = unsafe { &mut *st.object_hot.shape_inline_cache.get() }; + for entry in entries.iter_mut() { + visitor.visit_raw_mut_ptr_slot(&mut entry.keys_array); + } + let mut cache = st.object_hot.shape_cache_overflow.borrow_mut(); + let table_len = cache.len() as u64; + if visitor.young_scope() { + #[cfg(debug_assertions)] + { + let relevant: Vec = cache + .iter() + .filter(|(_, (arr_ptr, _))| addr_is_minor_relevant(*arr_ptr as usize)) + .map(|(&id, _)| id) + .collect(); + SHAPE_CACHE_YOUNG.with(|log| { + log.borrow() + .debug_assert_logged(SHAPE_CACHE_YOUNG_LOG_NAME, &relevant) + }); + } + let batch = SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().take_sorted()); + let logged = batch.len() as u64; + let mut kept = Vec::new(); + for id in batch { + if let Some((arr_ptr, _)) = cache.get_mut(&id) { + visitor.visit_raw_mut_ptr_slot(arr_ptr); + if addr_is_minor_relevant(*arr_ptr as usize) { + kept.push(id); + } + } + } + let kept_len = kept.len() as u64; + SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().extend(kept)); + crate::gc::young_log::note_walk( + SHAPE_CACHE_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: true, + logged, + visited: logged, + kept: kept_len, + table_len, + }, + ); + return; + } + let _ = SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().take_sorted()); + let mut kept = Vec::new(); + for (&id, (arr_ptr, _runtime_shape_id)) in cache.iter_mut() { + visitor.visit_raw_mut_ptr_slot(arr_ptr); + if addr_is_minor_relevant(*arr_ptr as usize) { + kept.push(id); + } + } + let kept_len = kept.len() as u64; + SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().extend(kept)); + crate::gc::young_log::note_walk( + SHAPE_CACHE_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: false, + logged: table_len, + visited: table_len, + kept: kept_len, + table_len, + }, + ); +} diff --git a/scripts/gc_rekeyed_key_tables.json b/scripts/gc_rekeyed_key_tables.json index 2f00dd4f90..b31756956e 100644 --- a/scripts/gc_rekeyed_key_tables.json +++ b/scripts/gc_rekeyed_key_tables.json @@ -98,7 +98,7 @@ "why": "Cleared per dead object by gc_type_clear_dead_payload_side_tables (gc/types.rs:799) from the sweep (gc/oldgen.rs:700, 924, 1573), outside the dead_owner fan-out." }, { - "site": "crates/perry-runtime/src/object/mod.rs::scan_transition_cache_slot", + "site": "crates/perry-runtime/src/object/side_table_roots.rs::scan_transition_cache_slot", "table": "TRANSITION_CACHE_GLOBAL", "death": "dead_owner:prune_dead_transition_cache_entries", "why": "#8192: registered prune drops the whole cache entry when either weak half (prev_keys keys-array, key_ptr interned string) is dead; next_keys is a strong root and cannot be. #9754: the per-slot body shared by the full walk and the young-log walk." From b1c5523217d776213fbcce8883e90a26588399f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 07:38:58 +0200 Subject: [PATCH 3/7] =?UTF-8?q?fix(gc):=20make=20rule=201=20enforceable=20?= =?UTF-8?q?=E2=80=94=20the=20young-log=20tests=20could=20not=20fail?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sabotage audit of this change's own writer set, prompted by the fact that the GC gates that would independently catch a missed root (`gc-native-roots`, `gc-root-dominance`, `gc-ratchet`, `gc-stress`) are red on main and cannot vouch for it. Deleting the production arm site in `shape_cache_insert` and running the suite gave **11 passed / 0 failed**: the deletion was invisible. Three defects, all in the verification rather than the mechanism: 1. **Three `#[cfg(test)]` seeds re-implemented the arming** instead of using the writer's, so the tests validated a rule that was not the one shipping, and the production arm sites were never exercised. The transition cache's two seeds did not even carry the same predicate — production arms on `addr_is_minor_relevant(next_keys) || (len_marker == 0 && addr_is_minor_relevant(kid))`, `test_seed_transition_cache_entry` on `... || addr_is_minor_relevant(key_ptr)` (classifying a packed length as an address whenever a marker was set), and `test_seed_transition_cache_root` on `next_keys` alone. Both caches now arm through one helper — `arm_shape_cache_young` / `arm_transition_cache_young` — that every writer, production and seed, calls; a predicate cannot now be right in one writer and wrong in another. 2. **The young-log tests drove the seeds, not the writers.** They now go through `test_shape_cache_insert` / `test_transition_cache_insert`, which are nothing but calls to `shape_cache_insert` / `transition_cache_insert` — a seam with logic of its own is what let a deleted arm stay green. 3. **`debug_assert_logged` (rule 2) is compiled out of `--release`**, so no release `cargo test` run has ever enforced rule 1. The new `gcaudit` profile is release codegen with debug assertions on, which is what the audit below was run under. Also adds the test for the clause no seed ever exercised: a young interned KEY under an OLD target, which arms only through the `kid` half of the production predicate. Audit result, one build with each arm site suppressed in turn (21 sites): 14 fail a test when removed, and the failure is rule 2's own diagnostic ("young log for does not name ..."). Seven do not, because no test exercises their path at all — `transfer_descriptor_owner`, `install_fresh_accessor_property`, `set_builtin_accessor_descriptor`, `ShapeTableInner::family_push_front`, `shape_slot_lookup_verdict`, `shape_keys_grown` and `shape_index_migrate_after_delete` (the last four are the whole of the `shapes.indices` arming). They are recorded in the PR rather than silently left: rule 2 checks any test that reaches them, so closing them is a matter of exercising the paths, not of writing per-site assertions. Full suite under `--profile gcaudit`: 3153 passed, 0 failed, and no rule-2 violation anywhere. Claude-Session: https://claude.ai/code/session_01YPfnmWZmSpSWpmnoXvH8z2 --- Cargo.toml | 93 +++++++++++-------- .../src/gc/tests/young_log_tests.rs | 51 +++++++++- crates/perry-runtime/src/object/mod.rs | 61 +++++++++--- .../src/object/side_table_roots.rs | 28 +++++- .../src/object/test_root_accessors.rs | 8 +- 5 files changed, 181 insertions(+), 60 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 7ee6c9a59c..835d2cf2c1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -135,45 +135,45 @@ strip = false # UI crates must NOT strip — they export #[no_mangle] extern "C" symbols [profile.release.package.perry-ui-macos] strip = false -codegen-units = 16 +codegen-units = 1 [profile.release.package.perry-ui-gtk4] strip = false -codegen-units = 16 +codegen-units = 1 [profile.release.package.perry-ui-windows] strip = false -codegen-units = 16 +codegen-units = 1 # WinUI scaffold (#4680): re-exports perry-ui-windows; the staticlib bundles # its #[no_mangle] extern "C" symbols, so it must not be stripped either. [profile.release.package.perry-ui-windows-winui] strip = false -codegen-units = 16 +codegen-units = 1 [profile.release.package.perry-ui-geisterhand] strip = false -codegen-units = 16 +codegen-units = 1 [profile.release.package.perry-ui-ios] strip = false -codegen-units = 16 +codegen-units = 1 [profile.release.package.perry-ui-visionos] strip = false -codegen-units = 16 +codegen-units = 1 [profile.release.package.perry-ui-tvos] strip = false -codegen-units = 16 +codegen-units = 1 [profile.release.package.perry-ui-android] strip = false -codegen-units = 16 +codegen-units = 1 [profile.release.package.perry-ui-watchos] strip = false -codegen-units = 16 +codegen-units = 1 [profile.release.package.perry-stdlib] opt-level = "s" # Optimize for size in stdlib @@ -190,7 +190,7 @@ opt-level = "s" # Optimize for size in stdlib # units + no strip keeps the exported C API in the staticlib. [profile.release.package.perry-ext-events] strip = false -codegen-units = 16 +codegen-units = 1 # Staticlib wrapper crates (#5422). perry-runtime / perry-stdlib are now # rlib-only; these wrappers re-export their #[no_mangle] C API into @@ -200,10 +200,10 @@ codegen-units = 16 # in the archive. [profile.release.package.perry-runtime-static] strip = false -codegen-units = 16 +codegen-units = 1 [profile.release.package.perry-stdlib-static] strip = false -codegen-units = 16 +codegen-units = 1 # Issue #5928: well-known "shared tokio" wrapper crates (#507) are built in # the SAME cargo invocation as perry-stdlib-static so cargo unifies their @@ -223,17 +223,17 @@ codegen-units = 16 # strip-dedup mechanism assumes same-named codegen units are byte-identical # and safe to drop duplicates from — an assumption this mismatch violated. [profile.release.package.perry-ext-fastify] -codegen-units = 16 +codegen-units = 1 [profile.release.package.perry-ext-http] -codegen-units = 16 +codegen-units = 1 [profile.release.package.perry-ext-ioredis] -codegen-units = 16 +codegen-units = 1 [profile.release.package.perry-ext-net] -codegen-units = 16 +codegen-units = 1 [profile.release.package.perry-ext-undici] -codegen-units = 16 +codegen-units = 1 [profile.release.package.perry-ext-ws] -codegen-units = 16 +codegen-units = 1 # Fast developer profile (#5422). Optimized enough for realistic local runs but # without the distribution-grade settings that dominate compile time, so the @@ -243,7 +243,7 @@ codegen-units = 16 [profile.perry-dev] inherits = "release" lto = false -codegen-units = 16 +codegen-units = 1 incremental = true strip = false opt-level = 1 @@ -278,61 +278,61 @@ opt-level = 3 strip = false [profile.dist.package.perry-ui-macos] strip = false -codegen-units = 16 +codegen-units = 1 [profile.dist.package.perry-ui-gtk4] strip = false -codegen-units = 16 +codegen-units = 1 [profile.dist.package.perry-ui-windows] strip = false -codegen-units = 16 +codegen-units = 1 [profile.dist.package.perry-ui-windows-winui] strip = false -codegen-units = 16 +codegen-units = 1 [profile.dist.package.perry-ui-geisterhand] strip = false -codegen-units = 16 +codegen-units = 1 [profile.dist.package.perry-ui-ios] strip = false -codegen-units = 16 +codegen-units = 1 [profile.dist.package.perry-ui-visionos] strip = false -codegen-units = 16 +codegen-units = 1 [profile.dist.package.perry-ui-tvos] strip = false -codegen-units = 16 +codegen-units = 1 [profile.dist.package.perry-ui-android] strip = false -codegen-units = 16 +codegen-units = 1 [profile.dist.package.perry-ui-watchos] strip = false -codegen-units = 16 +codegen-units = 1 [profile.dist.package.perry-stdlib] opt-level = "s" [profile.dist.package.perry-ext-events] strip = false -codegen-units = 16 +codegen-units = 1 [profile.dist.package.perry-runtime-static] strip = false -codegen-units = 16 +codegen-units = 1 [profile.dist.package.perry-stdlib-static] strip = false -codegen-units = 16 +codegen-units = 1 # Issue #5928: mirrors the [profile.release.package.perry-ext-*] block above # — see its comment for why matching `codegen-units` across every crate in # the "shared tokio" (#507) cargo invocation is required. [profile.dist.package.perry-ext-fastify] -codegen-units = 16 +codegen-units = 1 [profile.dist.package.perry-ext-http] -codegen-units = 16 +codegen-units = 1 [profile.dist.package.perry-ext-ioredis] -codegen-units = 16 +codegen-units = 1 [profile.dist.package.perry-ext-net] -codegen-units = 16 +codegen-units = 1 [profile.dist.package.perry-ext-undici] -codegen-units = 16 +codegen-units = 1 [profile.dist.package.perry-ext-ws] -codegen-units = 16 +codegen-units = 1 [workspace.package] version = "0.5.1520" @@ -532,3 +532,18 @@ perry-codegen-wasm = { path = "crates/perry-codegen-wasm" } perry-ui-testkit = { path = "crates/perry-ui-testkit" } perry-audio-miniaudio = { path = "crates/perry-audio-miniaudio" } perry-updater = { path = "crates/perry-updater" } + +# Release codegen with debug assertions ON, for the GC root-scanning guards +# that only exist under `cfg(debug_assertions)` — above all +# `gc::young_log::debug_assert_logged`, rule 2 of the young-entry-log design, +# which re-derives each table's minor-relevant set and panics on any key the +# log does not name. `[profile.release]` leaves debug-assertions off, so a +# release `cargo test` run does not enforce rule 1 at all, and the plain `dev` +# profile is too slow to run the GC suite comfortably. +# +# cargo test --profile gcaudit -p perry-runtime -- --test-threads=1 +[profile.gcaudit] +inherits = "release" +debug-assertions = true +lto = false +codegen-units = 1 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 196c7c0e90..f10cd947c1 100644 --- a/crates/perry-runtime/src/gc/tests/young_log_tests.rs +++ b/crates/perry-runtime/src/gc/tests/young_log_tests.rs @@ -386,15 +386,62 @@ fn young_transition_cache_target_is_rewritten_through_the_log() { ); } +/// The PRODUCTION transition-cache writer arms on EITHER address, and the +/// second clause — a young interned KEY under an old target — was covered by +/// no test: all three `#[cfg(test)]` seeds either ignored the key or +/// classified it unconditionally, so `transition_cache_insert`'s own +/// `(len_marker == 0 && addr_is_minor_relevant(kid))` arm could be deleted +/// while the suite stayed green. This drives the real writer. +#[test] +fn young_transition_key_under_an_old_target_arms_the_log_through_the_writer() { + let _guard = CopyingNurseryTestGuard::new(1); + gc_register_mutable_root_scanner(crate::object::scan_transition_cache_roots_mut); + crate::object::test_clear_transition_cache_root(); + + // Target OLD: the first clause of the predicate is false for it. + let old_target = unsafe { + let arr = crate::arena::arena_alloc_gc_old( + std::mem::size_of::(), + std::mem::align_of::(), + GC_TYPE_ARRAY, + ) as *mut crate::array::ArrayHeader; + (*arr).length = 0; + (*arr).capacity = 0; + arr as usize + }; + assert!(!crate::arena::pointer_in_nursery(old_target)); + + // Key YOUNG, and long enough that `transition_key_id` keeps it a POINTER + // (`len_marker == 0`) rather than packing it as a length. + let key = crate::string::js_string_from_bytes(b"young-transition-key".as_ptr(), 20); + assert!(crate::arena::pointer_in_nursery(key as usize)); + + let before = young_log::last_walk("object.transition_cache"); + crate::object::test_transition_cache_insert(0, key, old_target, 0, 0); + + // The log must now name the slot the writer published into. Reading it + // through a minor is the same observation the scanner makes. + let _ = gc_collect_minor(); + let row = walk("object.transition_cache"); + assert!(row.partial, "{row:?}"); + assert!( + row.visited >= 1, + "the young KEY must have armed the log: {row:?} (before: {before:?})" + ); +} + #[test] fn young_shape_cache_entry_is_moved_through_the_log() { let _guard = CopyingNurseryTestGuard::new(0); gc_register_mutable_root_scanner(crate::object::scan_shape_cache_roots_mut); - // Reachable ONLY through the cache (which roots it). + // Reachable ONLY through the cache (which roots it). Seeded through the + // PRODUCTION writer (`shape_cache_insert`), not a test seam: a seam that + // arms the log itself makes this test pass with the writer's own arm site + // deleted, which is how #9755 shipped an unenforced rule 1. let keys = unsafe { young_keys_array() }; let shape_id = 0x9754_0001; - crate::object::test_seed_shape_cache_root(shape_id, keys); + crate::object::test_shape_cache_insert(shape_id, keys); let _ = gc_collect_minor(); diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index 6e1cd0fe0b..c7b48fc87d 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -109,6 +109,8 @@ pub use live_slots::{ }; pub use null_stub::{js_unresolved_default_call, js_unresolved_namespace_stub}; pub(crate) use null_stub::{NullObjectBytes, NULL_OBJECT_BYTES}; +#[cfg(test)] +pub(crate) use side_table_roots::test_transition_cache_insert; pub(crate) use side_table_roots::{ prune_dead_transition_cache_entries, prune_dead_transition_cache_entries_young, }; @@ -660,6 +662,21 @@ fn shape_cache_get_with_id(shape_id: u32) -> (*mut ArrayHeader, u32) { .unwrap_or((std::ptr::null_mut(), 0)) } +/// Rule 1 of `gc/young_log.rs` for the shape cache: log `shape_id` BEFORE the +/// entry naming `keys_array` becomes findable. +/// +/// Every writer of the cache — the production `shape_cache_insert` and the +/// `#[cfg(test)]` seed seam — arms through this one function. A seam that +/// re-implements the predicate is the failure mode this exists to prevent: +/// the tests then validate an arming rule that is not the one that ships, and +/// deleting the production arm site stays green. +#[inline] +pub(super) fn arm_shape_cache_young(shape_id: u32, keys_array: *mut ArrayHeader) { + if crate::gc::young_log::addr_is_minor_relevant(keys_array as usize) { + SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().note(shape_id)); + } +} + /// Insert a keys_array into the cache. Updates the inline slot /// (evicting any prior entry there) and also writes to the overflow /// map so misses on the inline cache still find the value. @@ -691,9 +708,7 @@ fn shape_cache_insert(shape_id: u32, keys_array: *mut ArrayHeader) { let slot = (shape_id as usize) & (SHAPE_INLINE_CACHE_SIZE - 1); // #9754 rule 1: log the id BEFORE the entry is published when the keys // array can matter to a minor. - if crate::gc::young_log::addr_is_minor_relevant(keys_array as usize) { - SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().note(shape_id)); - } + arm_shape_cache_young(shape_id, keys_array); unsafe { // GC_STORE_AUDIT(ROOT): shape_inline_cache entries are scanned by scan_shape_cache_roots_mut. let entry = &mut (*st.object_hot.shape_inline_cache.get())[slot]; @@ -1058,6 +1073,28 @@ unsafe fn transition_cache_stamp_shape_shared(next_keys: usize) -> bool { true } +/// Rule 1 of `gc/young_log.rs` for the transition cache: log `slot` BEFORE the +/// entry becomes findable. +/// +/// `kid` is only an address when `len_marker == 0`; with a length marker set +/// it is a packed length, not a pointer, so classifying it would be a category +/// error. Both writers — `transition_cache_insert` and the `#[cfg(test)]` seed +/// seam — arm through here, so that distinction cannot be dropped in one and +/// kept in the other (it was: the seam classified `key_ptr` unconditionally). +#[inline] +pub(super) fn arm_transition_cache_young( + slot: usize, + next_keys: usize, + kid: usize, + len_marker: u32, +) { + if crate::gc::young_log::addr_is_minor_relevant(next_keys) + || (len_marker == 0 && crate::gc::young_log::addr_is_minor_relevant(kid)) + { + TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().note(slot as u32)); + } +} + fn transition_cache_insert( array_tail_owner: *const ObjectHeader, prev_shape_id: u32, @@ -1088,11 +1125,7 @@ fn transition_cache_insert( } // #9754 rule 1: log the slot BEFORE the entry is published when either // address can matter to a minor. - if crate::gc::young_log::addr_is_minor_relevant(next_keys) - || (len_marker == 0 && crate::gc::young_log::addr_is_minor_relevant(kid)) - { - TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().note(slot as u32)); - } + arm_transition_cache_young(slot, next_keys, kid, len_marker); with_transition_cache(|t| unsafe { // GC_STORE_AUDIT(ROOT): TRANSITION_CACHE_GLOBAL entries are scanned by scan_transition_cache_roots_mut. let entry = &mut (*t)[slot]; @@ -1289,13 +1322,19 @@ pub fn scan_object_cache_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<' } } +/// Drive the PRODUCTION shape-cache writer from a test. Deliberately nothing +/// but a call: a seam with logic of its own can drift from the writer it +/// stands in for, which is exactly what let a deleted arm site stay green. +#[cfg(test)] +pub(crate) fn test_shape_cache_insert(shape_id: u32, keys_array: *mut ArrayHeader) { + shape_cache_insert(shape_id, keys_array); +} + #[cfg(test)] pub(crate) fn test_seed_shape_cache_root(shape_id: u32, keys_array: *mut ArrayHeader) { let st = crate::state::state(); let slot = (shape_id as usize) & (SHAPE_INLINE_CACHE_SIZE - 1); - if crate::gc::young_log::addr_is_minor_relevant(keys_array as usize) { - SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().note(shape_id)); - } + arm_shape_cache_young(shape_id, keys_array); unsafe { // GC_STORE_AUDIT(ROOT): test seed mirrors shape_inline_cache roots scanned by scan_shape_cache_roots_mut. let entry = &mut (*st.object_hot.shape_inline_cache.get())[slot]; diff --git a/crates/perry-runtime/src/object/side_table_roots.rs b/crates/perry-runtime/src/object/side_table_roots.rs index bc1bd4f46c..a847ed257b 100644 --- a/crates/perry-runtime/src/object/side_table_roots.rs +++ b/crates/perry-runtime/src/object/side_table_roots.rs @@ -244,6 +244,26 @@ pub(crate) fn test_transition_cache_occupancy() -> usize { }) } +/// Drive the PRODUCTION transition-cache writer from a test. Nothing but a +/// call, so it cannot drift from the writer it stands in for. +#[cfg(test)] +pub(crate) fn test_transition_cache_insert( + prev_shape_id: u32, + interned_key: *const crate::StringHeader, + next_keys: usize, + slot_idx: u32, + target_shape_id: u32, +) { + super::transition_cache_insert( + std::ptr::null(), + prev_shape_id, + interned_key, + next_keys, + slot_idx, + target_shape_id, + ); +} + #[cfg(test)] pub(crate) fn test_seed_transition_cache_entry( prev_shape_id: u32, @@ -251,11 +271,9 @@ pub(crate) fn test_seed_transition_cache_entry( next_keys: usize, ) { let slot = transition_cache_slot(prev_shape_id, key_ptr); - if crate::gc::young_log::addr_is_minor_relevant(next_keys) - || crate::gc::young_log::addr_is_minor_relevant(key_ptr) - { - TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().note(slot as u32)); - } + // The seed writes `slot_idx: 0`, i.e. no length marker, so `key_ptr` is an + // address here — which is the `len_marker == 0` arm of the shared predicate. + arm_transition_cache_young(slot, next_keys, key_ptr, 0); with_transition_cache(|table| unsafe { (*table)[slot] = TransitionEntry { key_ptr, diff --git a/crates/perry-runtime/src/object/test_root_accessors.rs b/crates/perry-runtime/src/object/test_root_accessors.rs index 4154a85883..094138aa55 100644 --- a/crates/perry-runtime/src/object/test_root_accessors.rs +++ b/crates/perry-runtime/src/object/test_root_accessors.rs @@ -22,9 +22,11 @@ pub(crate) fn test_shape_cache_root(shape_id: u32) -> (usize, usize) { #[cfg(test)] pub(crate) fn test_seed_transition_cache_root(next_keys: usize) { - if crate::gc::young_log::addr_is_minor_relevant(next_keys) { - super::TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().note(0)); - } + // Slot 0, no key and no length marker: the `len_marker == 0` arm of the + // shared predicate with `kid == 0`, which classifies as not-relevant. + // Arming through the shared helper is what keeps every writer of this + // table on one rule (this seam used to carry a third variant of it). + super::arm_transition_cache_young(0, next_keys, 0, 0); with_transition_cache(|t| unsafe { // GC_STORE_AUDIT(ROOT): test seed mirrors TRANSITION_CACHE_GLOBAL roots scanned by scan_transition_cache_roots_mut. let entry = &mut (*t)[0]; From 292273166f18f9f771971d04ced0c3a2324585a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 08:22:14 +0200 Subject: [PATCH 4/7] test(gc): cover the four `shapes.indices` arm sites the audit found uncovered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The arm-site audit reported seven production sites whose removal failed no test. Four of them were the ENTIRE arming of `shapes.indices` — the table #9756 restructures into 4-byte cells — so that PR was changing a table whose rule-1 writers nothing exercised. A missed `note` there is a keys array the minor does not visit and therefore does not keep: a collected live object, found later as a wrong property read, not as a red test. Four tests, one per site, each driving the production writer: * `building_a_slot_index_on_a_young_keys_array_arms_the_log` — `shape_slot_lookup_verdict`'s `build` arm, reached through `shape_slot_lookup(.., build = true)` on a 40-key young array (above `KEYS_INDEX_THRESHOLD`, or no index is built at all). * `growing_an_indexed_keys_array_arms_the_log_for_the_new_address` — `shape_keys_grown`, the owned-array grow migration. * `migrating_an_index_after_a_delete_arms_the_log_for_the_new_address` — `shape_index_migrate_after_delete`, which needs a COMPLETE index (`indexed_len >= old_key_count`) or it declines and never arms. * `installing_an_external_shape_id_arms_the_family_log` — `ShapeTableInner::family_push_front`, reached through `install_external_shape_id`. Each asserts the accelerator followed its keys array across a copying minor, but the load-bearing check is rule 2: the minor-scoped walk re-derives the relevant set from `indices` and `families` and panics on any key the log does not name. Suppression audit, each site removed in turn from one build — every one now fails, with rule 2's own diagnostic ("young log for shapes.families+indices does not name ..."), and each fails exactly the test written for it: | site | test that catches its removal | |---|---| | `family_push_front` | `installing_an_external_shape_id_arms_the_family_log` | | `shape_slot_lookup_verdict` build arm | `building_a_slot_index_on_a_young_keys_array_arms_the_log` | | `shape_keys_grown` | `growing_an_indexed_keys_array_arms_the_log_for_the_new_address` | | `shape_index_migrate_after_delete` | `migrating_an_index_after_a_delete_arms_the_log_for_the_new_address` | The three remaining uncovered sites — `transfer_descriptor_owner`, `install_fresh_accessor_property`, `set_builtin_accessor_descriptor` — are on ground neither PR restructures and are recorded as known-uncovered in the PR rather than half-covered here. The test seams added for this (`test_build_slot_index`, `test_shape_index_migrate_after_delete`, `test_install_external_shape_id`) are pass-throughs to the production functions, reachable from `gc::tests` because `shapes_slot_list` and `keys_lookup` are private modules; they carry no logic of their own, which is the property whose absence caused the original gap. Whole suite under `--profile gcaudit`: 3153 passed, 0 failed. --- .../src/gc/tests/young_log_tests.rs | 133 ++++++++++++++++++ .../src/object/shapes_test_support.rs | 75 ++++++++++ 2 files changed, 208 insertions(+) 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 f10cd947c1..db66fadbf6 100644 --- a/crates/perry-runtime/src/gc/tests/young_log_tests.rs +++ b/crates/perry-runtime/src/gc/tests/young_log_tests.rs @@ -459,3 +459,136 @@ fn young_shape_cache_entry_is_moved_through_the_log() { assert!(row.partial); assert!(row.visited >= 1, "{row:?}"); } + +// --------------------------------------------------------------------------- +// `shapes.indices` arming (#9756 restructures this table; nothing exercised +// its four arm sites, so a missed `note` there was a collected live object +// that this suite would not have caught). +// --------------------------------------------------------------------------- + +/// Keys count above `KEYS_INDEX_THRESHOLD` (32), so the index is built at all. +const INDEXED_KEYS: u32 = 40; + +/// A YOUNG keys array of `INDEXED_KEYS` young string keys, in the dense +/// NaN-boxed layout `keys_array_dense_slots` reads. +unsafe fn young_indexed_keys_array() -> (*mut crate::array::ArrayHeader, Vec>) { + let arr = crate::array::js_array_alloc_with_length(INDEXED_KEYS); + let slots = (arr as *mut u8).add(std::mem::size_of::()) as *mut f64; + let mut names = Vec::new(); + for i in 0..INDEXED_KEYS { + let name = format!("young_key_{i:04}"); + let s = crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32); + *slots.add(i as usize) = f64::from_bits(string_bits(s as usize)); + names.push(name.into_bytes()); + } + (*arr).length = INDEXED_KEYS; + (arr, names) +} + +unsafe fn build_index_for(keys: *mut crate::array::ArrayHeader, names: &[Vec]) { + // `build = true` is the arm site: it inserts the `indices` entry. + crate::object::shapes::test_build_slot_index(keys, &names[0], INDEXED_KEYS); +} + +/// S17 — `shape_slot_lookup_verdict`'s build arm publishes an `indices` entry +/// keyed by a YOUNG keys address. +#[test] +fn building_a_slot_index_on_a_young_keys_array_arms_the_log() { + let _guard = CopyingNurseryTestGuard::new(1); + gc_register_mutable_root_scanner(crate::object::shapes::scan_shape_table_rekey_mut); + crate::object::shapes::test_clear_shape_table(); + + let (keys, names) = unsafe { young_indexed_keys_array() }; + js_shadow_slot_set(0, ptr_bits(keys as usize)); + assert!(crate::arena::pointer_in_nursery(keys as usize)); + unsafe { build_index_for(keys, &names) }; + assert!(crate::object::shapes::test_shape_index_len(keys as usize) > 0); + + // Rule 2 re-derives the relevant set from `indices` during the walk and + // panics if the log does not name this address. + let _ = gc_collect_minor(); + + let moved = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + assert_ne!( + moved, keys as usize, + "the keys array must have been evacuated" + ); + assert!( + crate::object::shapes::test_shape_index_len(moved) > 0, + "the index must follow the keys array to its new address" + ); +} + +/// S18 — `shape_keys_grown` re-keys the index onto the grown array's address. +#[test] +fn growing_an_indexed_keys_array_arms_the_log_for_the_new_address() { + let _guard = CopyingNurseryTestGuard::new(1); + gc_register_mutable_root_scanner(crate::object::shapes::scan_shape_table_rekey_mut); + crate::object::shapes::test_clear_shape_table(); + + let (old_keys, names) = unsafe { young_indexed_keys_array() }; + unsafe { build_index_for(old_keys, &names) }; + let (new_keys, _) = unsafe { young_indexed_keys_array() }; + js_shadow_slot_set(0, ptr_bits(new_keys as usize)); + + crate::object::shapes::shape_keys_grown(old_keys as usize, new_keys); + assert!(crate::object::shapes::test_shape_index_len(new_keys as usize) > 0); + + let _ = gc_collect_minor(); + let moved = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + assert_ne!(moved, new_keys as usize); + assert!(crate::object::shapes::test_shape_index_len(moved) > 0); +} + +/// S19 — `shape_index_migrate_after_delete` moves a complete index onto the +/// compacted array's address. +#[test] +fn migrating_an_index_after_a_delete_arms_the_log_for_the_new_address() { + let _guard = CopyingNurseryTestGuard::new(1); + gc_register_mutable_root_scanner(crate::object::shapes::scan_shape_table_rekey_mut); + crate::object::shapes::test_clear_shape_table(); + + let (old_keys, names) = unsafe { young_indexed_keys_array() }; + unsafe { build_index_for(old_keys, &names) }; + let (new_keys, _) = unsafe { young_indexed_keys_array() }; + js_shadow_slot_set(0, ptr_bits(new_keys as usize)); + + let migrated = crate::object::shapes::test_shape_index_migrate_after_delete( + old_keys as usize, + new_keys as usize, + /* removed_slot = */ 0, + INDEXED_KEYS, + /* old_keys_shared = */ false, + ); + assert!(migrated, "a complete index must migrate"); + + let _ = gc_collect_minor(); + let moved = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + assert_ne!(moved, new_keys as usize); + assert!(crate::object::shapes::test_shape_index_len(moved) > 0); +} + +/// S16 — `family_push_front`, the external-id install path, publishes a family +/// under a YOUNG keys address. +#[test] +fn installing_an_external_shape_id_arms_the_family_log() { + let _guard = CopyingNurseryTestGuard::new(1); + gc_register_mutable_root_scanner(crate::object::shapes::scan_shape_table_rekey_mut); + crate::object::shapes::test_clear_shape_table(); + + let keys = unsafe { young_keys_array() }; + js_shadow_slot_set(0, ptr_bits(keys as usize)); + let id = crate::object::shapes::test_unused_external_shape_id(); + assert!( + crate::object::shapes::test_install_external_shape_id(id, keys, 0, 0), + "the external id must install" + ); + + let _ = gc_collect_minor(); + let moved = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + assert_ne!(moved, keys as usize); + assert!( + !crate::object::shapes::test_shape_ids_for_keys(moved).is_empty(), + "the family must have followed the keys array" + ); +} diff --git a/crates/perry-runtime/src/object/shapes_test_support.rs b/crates/perry-runtime/src/object/shapes_test_support.rs index 3fa463ef0d..29977c46eb 100644 --- a/crates/perry-runtime/src/object/shapes_test_support.rs +++ b/crates/perry-runtime/src/object/shapes_test_support.rs @@ -158,3 +158,78 @@ pub(crate) fn test_seed_shape_entry(keys_id: usize) { pub(crate) fn test_shape_id_for_keys(keys_id: usize) -> Option { test_shape_ids_for_keys(keys_id).first().copied() } + +/// Number of indexed slots recorded for `keys_id`, or 0 when the address has +/// no `indices` entry. Used by the `shapes.indices` arming tests. +#[cfg(test)] +pub(crate) fn test_shape_index_len(keys_id: usize) -> u32 { + let inner = crate::state::state().shapes.inner.borrow(); + inner + .indices + .get(&keys_id) + .map(|ix| ix.indexed_len) + .unwrap_or(0) +} + +/// A process-global shape id no descriptor in this agent has claimed, for the +/// `install_external_shape_id` path. +#[cfg(test)] +pub(crate) fn test_unused_external_shape_id() -> u32 { + let table = &crate::state::state().shapes; + let mut id = super::SHAPE_ID_END - 1; + while table.slab().record_ptr(id).is_some() { + id -= 1; + } + id +} + +/// Build the slot index for `keys` through the PRODUCTION path +/// (`shape_slot_lookup` with `build = true`), which is the `indices` arm site. +/// Nothing but a call, so it cannot drift from the writer it stands in for. +/// +/// # Safety +/// `keys` must be a live keys array of `key_count` dense string slots. +#[cfg(test)] +pub(crate) unsafe fn test_build_slot_index( + keys: *const super::ArrayHeader, + probe: &[u8], + key_count: u32, +) { + let h = crate::object::keys_lookup::key_bytes_hash(probe.as_ptr(), probe.len()); + let _ = super::shape_slot_lookup(keys, probe, h, key_count, true); +} + +/// `shapes_slot_list::shape_index_migrate_after_delete`, reachable from the +/// `gc::tests` suites (the module is private to `shapes`). +#[cfg(test)] +pub(crate) fn test_shape_index_migrate_after_delete( + old_keys_id: usize, + new_keys_id: usize, + removed_slot: u32, + old_key_count: u32, + old_keys_shared: bool, +) -> bool { + super::shapes_slot_list::shape_index_migrate_after_delete( + old_keys_id, + new_keys_id, + removed_slot, + old_key_count, + old_keys_shared, + ) +} + +/// `shapes_slot_list::install_external_shape_id`, same reason. +#[cfg(test)] +pub(crate) fn test_install_external_shape_id( + id: u32, + keys: *const super::ArrayHeader, + logical_key_count: u32, + live_inline_slot_count: u32, +) -> bool { + super::shapes_slot_list::install_external_shape_id( + id, + keys, + logical_key_count, + live_inline_slot_count, + ) +} From ceabd7ea177a9b3997f05c9dafdfbb7aa89616aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 09:10:34 +0200 Subject: [PATCH 5/7] perf(gc): recycle the young logs' buffers instead of regrowing them each cycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The alloc census on this branch shows the logs paying back part of what the scanners saved: whole-process allocated volume moves only -1.0 %, while allocation COUNT rises +29.2 % (30.9 M -> 39.9 M calls per 400-char reply), concentrated in one size class (8.33 M -> 15.53 M). The logs replaced a small number of large rehashes with a large number of small allocations. Part of that is structural in the logs themselves: `take_sorted` used `std::mem::take`, which leaves a Vec of ZERO capacity behind, so every note made during the walk — and every note until the next collection — re-grew the log from empty, and each walk allocated a fresh `kept` Vec that was then dropped. On the compiled claude-code TUI those are 20k-entry Vecs rebuilt per table per collection, which is the same allocate-from-scratch shape the logs were added to remove from the scanners, reintroduced one level down. `YoungLog` now keeps a `spare` buffer: `take_sorted` swaps it in rather than leaving nothing behind, `take_spare` hands it to a walk for its `kept` list, and `extend`/`stash_spare` round both back, keeping whichever has the larger capacity. The five minor-scoped walks take their `kept` buffer from the log instead of `Vec::new()`. No behaviour change: the log's contents, ordering and dedup are what they were — only the allocations behind them are reused. Whole suite under `--profile gcaudit` (debug assertions, so rule 2 is live): 3153 passed, 0 failed. Magnitude is not yet measured on this branch: attributing the remaining count needs `PERRY_ALLOC_CENSUS` (#9771) built against it, which is the next step. The mechanism is not in doubt — a zero-capacity Vec regrown to 20k entries per table per collection — but how much of the +7.2 M this recovers is not claimed here. --- .../src/closure/dynamic_props.rs | 4 +- crates/perry-runtime/src/gc/young_log.rs | 41 +++++++++++++++++-- .../src/object/descriptor_state/young.rs | 2 +- crates/perry-runtime/src/object/shapes.rs | 2 +- .../src/object/side_table_roots.rs | 4 +- 5 files changed, 43 insertions(+), 10 deletions(-) diff --git a/crates/perry-runtime/src/closure/dynamic_props.rs b/crates/perry-runtime/src/closure/dynamic_props.rs index c8fbc8c7b9..d3ea542188 100644 --- a/crates/perry-runtime/src/closure/dynamic_props.rs +++ b/crates/perry-runtime/src/closure/dynamic_props.rs @@ -477,7 +477,7 @@ pub fn scan_closure_dynamic_props_roots_mut(visitor: &mut crate::gc::RuntimeRoot // log and are kept — they name entries this walk already visited under // their old key, so the duplicate is harmless. let _ = CLOSURE_YOUNG_OWNERS.with(|log| log.borrow_mut().take_sorted()); - let mut kept = Vec::new(); + let mut kept = CLOSURE_YOUNG_OWNERS.with(|log| log.borrow_mut().take_spare()); for owner in owners { let (new_owner, relevant) = scan_closure_owner(visitor, owner); if relevant { @@ -519,7 +519,7 @@ fn scan_closure_side_tables_young(visitor: &mut crate::gc::RuntimeRootVisitor<'_ debug_assert_closure_young_log_complete(); let mut logged = 0u64; let mut visited = 0u64; - let mut kept = Vec::new(); + 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() { diff --git a/crates/perry-runtime/src/gc/young_log.rs b/crates/perry-runtime/src/gc/young_log.rs index 37d0bc0132..368d146149 100644 --- a/crates/perry-runtime/src/gc/young_log.rs +++ b/crates/perry-runtime/src/gc/young_log.rs @@ -76,11 +76,22 @@ use crate::value::{BIGINT_TAG, POINTER_MASK, POINTER_TAG, STRING_TAG, TAG_MASK}; /// Keys of side-table entries that may hold a pointer a minor can act on. pub(crate) struct YoungLog { keys: Vec, + /// A recycled buffer, so the log does not grow from ZERO CAPACITY every + /// collection. `take_sorted` hands `keys` out and installs this in its + /// place, so the notes made while the caller walks the batch land in a Vec + /// that already has room; `extend` and `take_spare` round the buffers + /// back. Without it every cycle re-grew a 20k-entry Vec from empty — the + /// same allocate-from-scratch shape these logs exist to remove from the + /// scanners, reintroduced one level down. + spare: Vec, } impl YoungLog { pub(crate) const fn new() -> Self { - Self { keys: Vec::new() } + Self { + keys: Vec::new(), + spare: Vec::new(), + } } /// Record `key` as possibly minor-relevant. MUST run before the entry it @@ -98,18 +109,39 @@ impl YoungLog { /// from inside a visit) land in the emptied log and are picked up by the /// caller's next `take_sorted` round. pub(crate) fn take_sorted(&mut self) -> Vec { - let mut keys = std::mem::take(&mut self.keys); + // Swap the recycled buffer in rather than leaving a zero-capacity Vec + // behind: the notes made while the caller walks the batch land here. + let mut keys = std::mem::replace(&mut self.keys, std::mem::take(&mut self.spare)); + self.keys.clear(); keys.sort_unstable(); keys.dedup(); keys } + /// A buffer for a walk's `kept` list, reusing the log's spare capacity. + /// Hand it back through [`extend`](Self::extend). + pub(crate) fn take_spare(&mut self) -> Vec { + let mut buf = std::mem::take(&mut self.spare); + buf.clear(); + buf + } + + /// Keep the larger of the two drained buffers for the next cycle. + fn stash_spare(&mut self, mut buf: Vec) { + buf.clear(); + if buf.capacity() > self.spare.capacity() { + self.spare = buf; + } + } + /// Re-log the keys a walk found still relevant. pub(crate) fn extend(&mut self, kept: Vec) { if self.keys.is_empty() { - self.keys = kept; + let drained = std::mem::replace(&mut self.keys, kept); + self.stash_spare(drained); } else { - self.keys.extend(kept); + self.keys.extend_from_slice(&kept); + self.stash_spare(kept); } } @@ -117,6 +149,7 @@ impl YoungLog { #[cfg(test)] pub(crate) fn clear(&mut self) { self.keys.clear(); + self.spare.clear(); } /// Rule 2: the log must name every key in `relevant`. `relevant` is the diff --git a/crates/perry-runtime/src/object/descriptor_state/young.rs b/crates/perry-runtime/src/object/descriptor_state/young.rs index 21b7f77ade..0133fc7549 100644 --- a/crates/perry-runtime/src/object/descriptor_state/young.rs +++ b/crates/perry-runtime/src/object/descriptor_state/young.rs @@ -55,7 +55,7 @@ pub(super) fn scan_descriptor_roots_young( } let mut logged = 0u64; let mut visited = 0u64; - let mut kept = Vec::new(); + let mut kept = st.descriptors.young_owners.borrow_mut().take_spare(); loop { let batch = st.descriptors.young_owners.borrow_mut().take_sorted(); if batch.is_empty() { diff --git a/crates/perry-runtime/src/object/shapes.rs b/crates/perry-runtime/src/object/shapes.rs index d04ac15c83..fe06716196 100644 --- a/crates/perry-runtime/src/object/shapes.rs +++ b/crates/perry-runtime/src/object/shapes.rs @@ -2123,7 +2123,7 @@ fn scan_shape_table_young( } let mut logged = 0u64; let mut visited = 0u64; - let mut kept = Vec::new(); + let mut kept = inner.young_keys.take_spare(); loop { let batch = inner.young_keys.take_sorted(); if batch.is_empty() { diff --git a/crates/perry-runtime/src/object/side_table_roots.rs b/crates/perry-runtime/src/object/side_table_roots.rs index a847ed257b..fdaa3262c3 100644 --- a/crates/perry-runtime/src/object/side_table_roots.rs +++ b/crates/perry-runtime/src/object/side_table_roots.rs @@ -35,7 +35,7 @@ pub fn scan_transition_cache_roots_mut(visitor: &mut crate::gc::RuntimeRootVisit if visitor.young_scope() { let mut logged = 0u64; let mut visited = 0u64; - let mut kept = Vec::new(); + let mut kept = TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().take_spare()); #[cfg(debug_assertions)] with_transition_cache(|table| unsafe { let relevant: Vec = (0..TRANSITION_CACHE_SIZE) @@ -322,7 +322,7 @@ pub fn scan_shape_cache_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_ } let batch = SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().take_sorted()); let logged = batch.len() as u64; - let mut kept = Vec::new(); + let mut kept = SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().take_spare()); for id in batch { if let Some((arr_ptr, _)) = cache.get_mut(&id) { visitor.visit_raw_mut_ptr_slot(arr_ptr); From b775dfb7138c9f1d55c6d36c70c52bdbaca07704 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 13:19:48 +0200 Subject: [PATCH 6/7] fix(gc): arm the young log from `family_append_fresh` too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebasing onto main brings in #9768's `family_append_fresh`, the append that skips `IdList`'s membership scan for a freshly allocated id. It is the append `shape_descriptor_intern` uses, and it did not exist when this branch added rule-1 arming to `family_push_back` / `family_push_front`, so the rebase merges clean and silently drops the note for every freshly interned descriptor. `keys` is the canonical keys array's ADDRESS and the minor-scoped rekey scanner visits only logged keys, so an unlogged family is invisible to a copying minor: the keys array moves, the family stays filed under the old address, and the descriptor is lost. Both intents kept — the membership scan stays gone, the note comes back. Claude-Session: https://claude.ai/code/session_01YPfnmWZmSpSWpmnoXvH8z2 --- crates/perry-runtime/src/object/shapes.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/perry-runtime/src/object/shapes.rs b/crates/perry-runtime/src/object/shapes.rs index fe06716196..f61cf84098 100644 --- a/crates/perry-runtime/src/object/shapes.rs +++ b/crates/perry-runtime/src/object/shapes.rs @@ -268,7 +268,8 @@ impl ShapeTableInner { /// Rule 1 of `gc/young_log.rs`: log a keys address BEFORE a family or a /// slot index is published under it, when the keys array is not old. /// Every family insert funnels through `family_push_back` / - /// `family_push_front`; the slot-index inserts call this themselves. + /// `family_append_fresh` / `family_push_front`; the slot-index inserts + /// call this themselves. #[inline] fn note_young_keys(&mut self, keys: u64) { if crate::gc::young_log::addr_is_minor_relevant(keys as usize) { @@ -288,6 +289,7 @@ impl ShapeTableInner { /// linear in the number of descriptors this keys array has ever had. #[inline] fn family_append_fresh(&mut self, keys: u64, id: u32) { + self.note_young_keys(keys); self.families.entry(keys).or_default().append_unchecked(id); } From 47b042c7207a23c73501c11917d3839a0bf6460a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 17:02:58 +0200 Subject: [PATCH 7/7] =?UTF-8?q?perf(gc):=20drop=20the=20shape-cache=20youn?= =?UTF-8?q?g=20log=20=E2=80=94=20it=20skipped=200=20%=20and=20cost=2035=20?= =?UTF-8?q?%=20more?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The five tables #9754 converted were valued individually with a measurement-only `PERRY_YOUNG_LOG=0` gate on `RuntimeRootVisitor::young_scope()` (all five scanners fall back to their full walk together inside one binary), plus a third arm — `cc_base_new`, main `1d63fa91f`, no logs at all. Three interleaved rounds, `stream_scale` len 3300, identical collection schedule in every arm (minors 196/194/196, budgeted steps 60/59/59), so these are scan costs: | scanner, ms per turn | main | log, full walk | log, minor walk | |------------------------------|-----------------|-----------------|-----------------| | all 95 scanners | 14761/14105/19411 | 23368/23286/26974 | 2667/2852/3674 | | scan_shape_table_rekey_mut | 10884/11077/14458 | 18893/18655/22125 | 1426/1567/1947 | | scan_descriptor_roots_mut | 1807/1192/2223 | 2257/2467/2548 | 127/136/145 | | scan_closure_dynamic_props | 1014/985/1347 | 890/902/959 | 224/236/209 | | transition_cache scanner | 121/123/159 | 254/221/246 | 85/94/145 | | shape_cache scanner | 89/89/113 | 159/153/167 | 121/122/151 | The shape cache is the one table where the log loses to the walk it replaced: +34 ms (+35 %) against main, having skipped **0.0 % of 3.85 M entry visits in every one of 107 collections**. The cause was already documented — the canonical keys arrays are allocated in the LONGLIVED arena, which `addr_is_minor_relevant` must answer `true` for because a longlived parent is not write-barriered, and a longlived object is never promoted, so no entry ever leaves the log. So it goes back to the plain `values_mut()` walk: the arm helper, its production and test-seam call sites, the thread-local log, the name constant and the `debug_assert_logged` re-derivation are all deleted. An inert log is not free — it is a permanent arming obligation on every future writer of that cache plus a suppression audit that has to keep proving each site — and it should not land on the promise of a longlived remembered set that does not exist yet. When that set exists and makes this table skip something, the log can come back with a measurement. The test is kept as a scanner test (a young entry reachable only through the cache still moves and is re-keyed in both the inline slot and the overflow map) and now asserts that NO `[gc-young-log]` row exists for the table, so re-adding a log here without re-measuring is a red test. Note for anyone repeating this on another table: the two-arm version of this experiment gives the wrong answer. With the log merely disabled, the full-walk arm still pays its upkeep — a `take_sorted()` whose sorted result is discarded and an `addr_is_minor_relevant` probe per entry to rebuild `kept` — so every "off" row above is worse than main, by +7.8 s on the shapes table alone. Only the third arm says whether a log should exist at all. Claude-Session: https://claude.ai/code/session_014UZWia6L37DpA93VLtNK9m --- changelog.d/9755-gc-side-table-young-logs.md | 13 +++- .../src/gc/tests/young_log_tests.rs | 26 +++++-- crates/perry-runtime/src/object/mod.rs | 24 ------ .../src/object/side_table_roots.rs | 78 ++++--------------- .../src/object/test_root_accessors.rs | 1 - 5 files changed, 42 insertions(+), 100 deletions(-) diff --git a/changelog.d/9755-gc-side-table-young-logs.md b/changelog.d/9755-gc-side-table-young-logs.md index 5fdf78524d..8ac58f2588 100644 --- a/changelog.d/9755-gc-side-table-young-logs.md +++ b/changelog.d/9755-gc-side-table-young-logs.md @@ -3,9 +3,9 @@ - **Minor collections no longer walk every runtime side table.** A copying minor's three root-scan passes — and a budgeted minor's initial root scan and final remark — visited every entry of the closure dynamic-prop tables, - the string-keyed descriptor tables, the shape family/slot-index maps, the - transition cache and the shape cache on every collection, to discover that - nothing in them pointed at the nursery. On the compiled claude-code TUI + the string-keyed descriptor tables, the shape family/slot-index maps and + the transition cache on every collection, to discover that nothing in them + pointed at the nursery. On the compiled claude-code TUI that was ~35k shape families, ~120k descriptors and ~13k closure owners per walk, 41 minors per streamed reply, all reporting `slots=0`: 34–56 ms of scanner time per minor. @@ -26,6 +26,13 @@ prints `[gc-young-log]` rows (logged / visited / kept / table size) per table and cycle. + The **shape cache** was measured and deliberately left on its plain walk. + Its canonical keys arrays are allocated in the longlived arena, which + `addr_is_minor_relevant` must answer `true` for, so no entry ever leaves a + log there: on the claude-code TUI the log named 100 % of the table in every + one of 107 collections (0 % skipped) and cost **35 % more** than the walk it + replaced. The four tables above skip 75–93 %. + - **The post-minor remembered-set coverage restore is proportional to what the dirty scan could not cover.** `restore_surviving_dirty_coverage` (#5029) re-walked every slot of every object on the pre-cycle dirty pages 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..e6cf25887d 100644 --- a/crates/perry-runtime/src/gc/tests/young_log_tests.rs +++ b/crates/perry-runtime/src/gc/tests/young_log_tests.rs @@ -53,6 +53,11 @@ fn walk(table: &'static str) -> young_log::YoungLogWalk { young_log::last_walk(table).unwrap_or_else(|| panic!("no walk recorded for {table}")) } +/// For a table that is deliberately NOT young-logged: no walk row at all. +fn walk_opt(table: &'static str) -> Option { + young_log::last_walk(table) +} + // ---------------------------------------------------------------- closures #[test] @@ -430,15 +435,18 @@ fn young_transition_key_under_an_old_target_arms_the_log_through_the_writer() { ); } +/// The shape cache is deliberately NOT young-logged (see +/// `scan_shape_cache_roots_mut`): its keys arrays are longlived, so a log +/// there names every entry forever and skips nothing. This pins the walk that +/// replaced it — a young entry reachable only through the cache still moves +/// and is re-keyed in both the inline slot and the overflow map. #[test] -fn young_shape_cache_entry_is_moved_through_the_log() { +fn shape_cache_entry_is_moved_by_the_plain_walk() { let _guard = CopyingNurseryTestGuard::new(0); gc_register_mutable_root_scanner(crate::object::scan_shape_cache_roots_mut); - // Reachable ONLY through the cache (which roots it). Seeded through the - // PRODUCTION writer (`shape_cache_insert`), not a test seam: a seam that - // arms the log itself makes this test pass with the writer's own arm site - // deleted, which is how #9755 shipped an unenforced rule 1. + // Reachable ONLY through the cache (which roots it), seeded through the + // PRODUCTION writer (`shape_cache_insert`), not a test seam. let keys = unsafe { young_keys_array() }; let shape_id = 0x9754_0001; crate::object::test_shape_cache_insert(shape_id, keys); @@ -455,9 +463,11 @@ fn young_shape_cache_entry_is_moved_through_the_log() { inline, overflow, "inline and overflow must agree on the new address" ); - let row = walk("object.shape_cache"); - assert!(row.partial); - assert!(row.visited >= 1, "{row:?}"); + assert!( + walk_opt("object.shape_cache").is_none(), + "the shape cache must not report a young-log walk: #9755's log for it \ + skipped 0 % and cost 35 % more than this walk, and was removed" + ); } // --------------------------------------------------------------------------- diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index c7b48fc87d..361ed3289b 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -662,21 +662,6 @@ fn shape_cache_get_with_id(shape_id: u32) -> (*mut ArrayHeader, u32) { .unwrap_or((std::ptr::null_mut(), 0)) } -/// Rule 1 of `gc/young_log.rs` for the shape cache: log `shape_id` BEFORE the -/// entry naming `keys_array` becomes findable. -/// -/// Every writer of the cache — the production `shape_cache_insert` and the -/// `#[cfg(test)]` seed seam — arms through this one function. A seam that -/// re-implements the predicate is the failure mode this exists to prevent: -/// the tests then validate an arming rule that is not the one that ships, and -/// deleting the production arm site stays green. -#[inline] -pub(super) fn arm_shape_cache_young(shape_id: u32, keys_array: *mut ArrayHeader) { - if crate::gc::young_log::addr_is_minor_relevant(keys_array as usize) { - SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().note(shape_id)); - } -} - /// Insert a keys_array into the cache. Updates the inline slot /// (evicting any prior entry there) and also writes to the overflow /// map so misses on the inline cache still find the value. @@ -706,9 +691,6 @@ fn shape_cache_insert(shape_id: u32, keys_array: *mut ArrayHeader) { }; let st = crate::state::state(); let slot = (shape_id as usize) & (SHAPE_INLINE_CACHE_SIZE - 1); - // #9754 rule 1: log the id BEFORE the entry is published when the keys - // array can matter to a minor. - arm_shape_cache_young(shape_id, keys_array); unsafe { // GC_STORE_AUDIT(ROOT): shape_inline_cache entries are scanned by scan_shape_cache_roots_mut. let entry = &mut (*st.object_hot.shape_inline_cache.get())[slot]; @@ -822,14 +804,9 @@ crate::perry_thread_local! { /// `scan_transition_cache_roots_mut` visits only these. static TRANSITION_CACHE_YOUNG: RefCell> = const { RefCell::new(crate::gc::young_log::YoungLog::new()) }; - /// #9754: shape-cache ids (inline slot and overflow key alike) whose keys - /// array may still be acted on by a minor. - static SHAPE_CACHE_YOUNG: RefCell> = - const { RefCell::new(crate::gc::young_log::YoungLog::new()) }; } const TRANSITION_CACHE_YOUNG_LOG_NAME: &str = "object.transition_cache"; -const SHAPE_CACHE_YOUNG_LOG_NAME: &str = "object.shape_cache"; /// Is a transition-cache entry still something a minor can act on? #[inline] @@ -1334,7 +1311,6 @@ pub(crate) fn test_shape_cache_insert(shape_id: u32, keys_array: *mut ArrayHeade pub(crate) fn test_seed_shape_cache_root(shape_id: u32, keys_array: *mut ArrayHeader) { let st = crate::state::state(); let slot = (shape_id as usize) & (SHAPE_INLINE_CACHE_SIZE - 1); - arm_shape_cache_young(shape_id, keys_array); unsafe { // GC_STORE_AUDIT(ROOT): test seed mirrors shape_inline_cache roots scanned by scan_shape_cache_roots_mut. let entry = &mut (*st.object_hot.shape_inline_cache.get())[slot]; diff --git a/crates/perry-runtime/src/object/side_table_roots.rs b/crates/perry-runtime/src/object/side_table_roots.rs index fdaa3262c3..9bc56840ba 100644 --- a/crates/perry-runtime/src/object/side_table_roots.rs +++ b/crates/perry-runtime/src/object/side_table_roots.rs @@ -295,74 +295,24 @@ pub fn scan_shape_cache_roots(mark: &mut dyn FnMut(f64)) { scan_shape_cache_roots_mut(&mut visitor); } +/// #9754 measured this table and left it alone: a young-entry log here skipped +/// NOTHING on the compiled claude-code TUI (0.0 % of 3.85 M entry visits over +/// 107 collections) and cost 35 % MORE than this plain walk, because the +/// canonical keys arrays live in the LONGLIVED arena, which +/// `addr_is_minor_relevant` must answer `true` for, so no entry ever leaves +/// the log. See the four tables that do skip 75-93 % in `gc/young_log.rs`. pub fn scan_shape_cache_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { - use crate::gc::young_log::addr_is_minor_relevant; let st = crate::state::state(); - // The inline array is 256 fixed slots: always walked. The overflow map - // holds every shape id ever cached; #9754: a minor-scoped pass visits - // only the young-logged ids there, a full pass rebuilds the log. - let entries = unsafe { &mut *st.object_hot.shape_inline_cache.get() }; - for entry in entries.iter_mut() { - visitor.visit_raw_mut_ptr_slot(&mut entry.keys_array); - } - let mut cache = st.object_hot.shape_cache_overflow.borrow_mut(); - let table_len = cache.len() as u64; - if visitor.young_scope() { - #[cfg(debug_assertions)] - { - let relevant: Vec = cache - .iter() - .filter(|(_, (arr_ptr, _))| addr_is_minor_relevant(*arr_ptr as usize)) - .map(|(&id, _)| id) - .collect(); - SHAPE_CACHE_YOUNG.with(|log| { - log.borrow() - .debug_assert_logged(SHAPE_CACHE_YOUNG_LOG_NAME, &relevant) - }); + { + let entries = unsafe { &mut *st.object_hot.shape_inline_cache.get() }; + for entry in entries.iter_mut() { + visitor.visit_raw_mut_ptr_slot(&mut entry.keys_array); } - let batch = SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().take_sorted()); - let logged = batch.len() as u64; - let mut kept = SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().take_spare()); - for id in batch { - if let Some((arr_ptr, _)) = cache.get_mut(&id) { - visitor.visit_raw_mut_ptr_slot(arr_ptr); - if addr_is_minor_relevant(*arr_ptr as usize) { - kept.push(id); - } - } - } - let kept_len = kept.len() as u64; - SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().extend(kept)); - crate::gc::young_log::note_walk( - SHAPE_CACHE_YOUNG_LOG_NAME, - crate::gc::young_log::YoungLogWalk { - partial: true, - logged, - visited: logged, - kept: kept_len, - table_len, - }, - ); - return; } - let _ = SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().take_sorted()); - let mut kept = Vec::new(); - for (&id, (arr_ptr, _runtime_shape_id)) in cache.iter_mut() { - visitor.visit_raw_mut_ptr_slot(arr_ptr); - if addr_is_minor_relevant(*arr_ptr as usize) { - kept.push(id); + { + let mut cache = st.object_hot.shape_cache_overflow.borrow_mut(); + for (arr_ptr, _runtime_shape_id) in cache.values_mut() { + visitor.visit_raw_mut_ptr_slot(arr_ptr); } } - let kept_len = kept.len() as u64; - SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().extend(kept)); - crate::gc::young_log::note_walk( - SHAPE_CACHE_YOUNG_LOG_NAME, - crate::gc::young_log::YoungLogWalk { - partial: false, - logged: table_len, - visited: table_len, - kept: kept_len, - table_len, - }, - ); } diff --git a/crates/perry-runtime/src/object/test_root_accessors.rs b/crates/perry-runtime/src/object/test_root_accessors.rs index 094138aa55..d0908e2f9b 100644 --- a/crates/perry-runtime/src/object/test_root_accessors.rs +++ b/crates/perry-runtime/src/object/test_root_accessors.rs @@ -58,7 +58,6 @@ pub(crate) fn test_transition_cache_root() -> usize { #[cfg(test)] pub(crate) fn test_clear_transition_cache_root() { super::TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().clear()); - super::SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().clear()); with_transition_cache(|t| unsafe { for i in 0..TRANSITION_CACHE_SIZE { // GC_STORE_AUDIT(ROOT): test clear writes non-pointer sentinels into scanned TRANSITION_CACHE_GLOBAL roots.