From 64ef925e6bc142cdb6fef14fe841bad37554c620 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 07:16:33 +0200 Subject: [PATCH 01/11] perf(gc): scope shape and box scanners to young entries Keep minor remembered sets for boxed roots and the shape table's carrier mutations. Compact both sets after each minor while retaining authoritative full-table walks for major collection. Report whole copied-minor pause time and its scanner share together. --- changelog.d/minor-scanner-young-logs.md | 5 + crates/perry-runtime/src/box.rs | 145 +++++++++++ crates/perry-runtime/src/gc/copying.rs | 24 +- .../perry-runtime/src/gc/scanner_profile.rs | 7 +- .../src/gc/tests/young_log_tests.rs | 242 ++++++++++++++++++ crates/perry-runtime/src/gc/young_log.rs | 40 ++- crates/perry-runtime/src/object/shapes.rs | 142 ++++++++-- .../src/object/shapes_test_support.rs | 20 ++ 8 files changed, 586 insertions(+), 39 deletions(-) create mode 100644 changelog.d/minor-scanner-young-logs.md diff --git a/changelog.d/minor-scanner-young-logs.md b/changelog.d/minor-scanner-young-logs.md new file mode 100644 index 0000000000..cf4834a894 --- /dev/null +++ b/changelog.d/minor-scanner-young-logs.md @@ -0,0 +1,5 @@ +Copying-minor scans of shape descriptors and captured-variable boxes now walk +only entries that can still expose non-old GC pointers. This removes the two +largest table-size-dependent root-scan costs, while full collections retain +their authoritative whole-table walks. `PERRY_GC_DIAG=1` also reports the +whole copying-minor pause and its scanner share on each completed-minor line. diff --git a/crates/perry-runtime/src/box.rs b/crates/perry-runtime/src/box.rs index dc65e4f791..44079b511d 100644 --- a/crates/perry-runtime/src/box.rs +++ b/crates/perry-runtime/src/box.rs @@ -117,6 +117,28 @@ crate::perry_thread_local! { 16 * 1024, crate::fast_hash::PtrHasher, )); + /// Box addresses whose JSValue payload may matter to a minor collection. + /// The registry itself is the authoritative full/major root set; this is + /// only its minor remembered set. + static BOX_YOUNG_ROOTS: std::cell::RefCell> = + const { std::cell::RefCell::new(crate::gc::young_log::YoungLog::new()) }; + #[cfg(test)] + static BOX_YOUNG_LOG_SUPPRESSED: std::cell::Cell = const { std::cell::Cell::new(false) }; +} + +const BOX_YOUNG_LOG_NAME: &str = "box.roots"; + +/// Arm the box minor-root log before publishing a young payload. +#[inline] +fn note_box_young_root(addr: usize, bits: u64) { + if !crate::gc::young_log::bits_are_minor_relevant(bits) { + return; + } + #[cfg(test)] + if BOX_YOUNG_LOG_SUPPRESSED.with(std::cell::Cell::get) { + return; + } + BOX_YOUNG_ROOTS.with(|log| log.borrow_mut().note(addr)); } /// Number of slots in each registry's direct-mapped positive cache. Eight @@ -680,6 +702,7 @@ pub extern "C" fn js_box_alloc_bits(initial_bits: i64) -> *mut Box { unsafe { (*ptr).value = initial_bits as u64; } + note_box_young_root(addr, initial_bits as u64); BOX_REGISTRY.with(|r| { r.borrow_mut().insert(addr); }); @@ -699,6 +722,7 @@ pub extern "C" fn js_box_alloc_bits(initial_bits: i64) -> *mut Box { return std::ptr::null_mut(); } (*ptr).value = initial_bits as u64; + note_box_young_root(ptr as usize, initial_bits as u64); BOX_REGISTRY.with(|r| { r.borrow_mut().insert(ptr as usize); }); @@ -928,7 +952,14 @@ pub fn scan_box_roots(mark: &mut dyn FnMut(f64)) { } pub fn scan_box_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { + if visitor.young_scope() { + scan_box_young_roots_mut(visitor); + return; + } let full_trace = crate::gc::full_trace_active(); + let mut visited = 0u64; + let table_len = BOX_REGISTRY.with(|registry| registry.borrow().len()) as u64; + let mut kept = Vec::new(); ASYNC_PENDING_RELEASES.with(|pending| { let pending = pending.borrow(); BOX_REGISTRY.with(|r| { @@ -957,11 +988,103 @@ pub fn scan_box_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { if addr >= 0x1000 && (addr as u64) < 0x0001_0000_0000_0000 && addr % 8 == 0 { unsafe { visitor.visit_nanbox_u64_raw_slot(&raw mut (*ptr).value); + if crate::gc::young_log::bits_are_minor_relevant((*ptr).value) { + kept.push(addr); + } } + visited += 1; } } }); }); + let kept_len = kept.len() as u64; + BOX_YOUNG_ROOTS.with(|log| { + let mut log = log.borrow_mut(); + let _ = log.take_sorted(); + log.extend(kept); + }); + crate::gc::young_log::note_walk( + BOX_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: false, + logged: visited, + visited, + kept: kept_len, + table_len, + }, + ); +} + +/// Every live box whose current payload a minor can move, mark through, or +/// sweep. This is the authoritative debug re-derivation of the remembered set. +fn relevant_box_roots() -> Vec { + let mut relevant = BOX_REGISTRY.with(|registry| { + registry + .borrow() + .iter() + .copied() + .filter(|&addr| { + let ptr = addr as *mut Box; + is_plausible_box_ptr(ptr) + && unsafe { crate::gc::young_log::bits_are_minor_relevant((*ptr).value) } + }) + .collect::>() + }); + relevant.sort_unstable(); + relevant +} + +/// Minor root scan: price only the logged boxes, and compact the log from the +/// post-visit payloads. The visit counter lives here because this is the work +/// whose fixed cost the counter measures. +fn scan_box_young_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { + let table_len = BOX_REGISTRY.with(|registry| registry.borrow().len()) as u64; + #[cfg(any(debug_assertions, test))] + BOX_YOUNG_ROOTS.with(|log| { + let relevant = relevant_box_roots(); + log.borrow() + .debug_assert_logged(BOX_YOUNG_LOG_NAME, &relevant); + }); + + let mut logged = 0u64; + let mut visited = 0u64; + let mut kept = BOX_YOUNG_ROOTS.with(|log| log.borrow_mut().take_spare()); + loop { + let batch = BOX_YOUNG_ROOTS.with(|log| log.borrow_mut().take_sorted()); + if batch.is_empty() { + break; + } + logged += batch.len() as u64; + for addr in batch { + let registered = BOX_REGISTRY.with(|registry| registry.borrow().contains(&addr)); + if !registered { + continue; + } + let ptr = addr as *mut Box; + if !is_plausible_box_ptr(ptr) { + continue; + } + visited += 1; + unsafe { + visitor.visit_nanbox_u64_raw_slot(&raw mut (*ptr).value); + if crate::gc::young_log::bits_are_minor_relevant((*ptr).value) { + kept.push(addr); + } + } + } + } + let kept_len = kept.len() as u64; + BOX_YOUNG_ROOTS.with(|log| log.borrow_mut().extend(kept)); + crate::gc::young_log::note_walk( + BOX_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: true, + logged, + visited, + kept: kept_len, + table_len, + }, + ); } /// Get the raw JSValue bit pattern from a box. @@ -1213,6 +1336,7 @@ pub extern "C" fn js_box_set_bits(ptr: *mut Box, value_bits: i64) { return; } let bits = value_bits as u64; + note_box_young_root(ptr as usize, bits); (*ptr).value = bits; crate::gc::runtime_write_barrier_root_nanbox(bits); } @@ -1232,6 +1356,7 @@ pub extern "C" fn js_box_set_bits(ptr: *mut Box, value_bits: i64) { #[no_mangle] pub unsafe extern "C" fn js_box_set_bits_trusted_no_barrier(ptr: *mut Box, value_bits: i64) { unsafe { + note_box_young_root(ptr as usize, value_bits as u64); (*ptr).value = value_bits as u64; } } @@ -1479,6 +1604,7 @@ pub(crate) fn test_clear_box_registry() { BOX_REGISTRY.with(|r| r.borrow_mut().clear()); I32_BOX_REGISTRY.with(|r| r.borrow_mut().clear()); BOOL_BOX_REGISTRY.with(|r| r.borrow_mut().clear()); + BOX_YOUNG_ROOTS.with(|log| log.borrow_mut().clear()); BOX_FREE_HEAD.with(|h| h.set(0)); I32_BOX_FREE_HEAD.with(|h| h.set(0)); BOOL_BOX_FREE_HEAD.with(|h| h.set(0)); @@ -1501,6 +1627,25 @@ pub(crate) fn test_clear_box_registry() { } } +/// Test-only sabotage of the box write-side arming hook. The production +/// scanner's re-derivation must reject the missing log entry. +#[cfg(test)] +pub(crate) struct TestBoxYoungLogSuppression(bool); + +#[cfg(test)] +impl TestBoxYoungLogSuppression { + pub(crate) fn new() -> Self { + Self(BOX_YOUNG_LOG_SUPPRESSED.with(|cell| cell.replace(true))) + } +} + +#[cfg(test)] +impl Drop for TestBoxYoungLogSuppression { + fn drop(&mut self) { + BOX_YOUNG_LOG_SUPPRESSED.with(|cell| cell.set(self.0)); + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/perry-runtime/src/gc/copying.rs b/crates/perry-runtime/src/gc/copying.rs index 2eb134d64f..9afd510073 100644 --- a/crates/perry-runtime/src/gc/copying.rs +++ b/crates/perry-runtime/src/gc/copying.rs @@ -1872,9 +1872,23 @@ pub(super) fn run_copied_minor_attempt( collector.stats.copied_bytes, collector.stats.survivor_live_bytes, ); + if let Some(d) = collector.survival.as_ref() { + d.report(super::survival_diag::next_minor_seq()); + } + crate::arena::alloc_sample::report("minor"); + super::diag_sites::report_primitive_dispatch("minor"); + crate::object::shapes::id_list_report(); + report_forwarding_refusals("copying_minor"); + let scan_us = super::scanner_profile::report_and_reset("copying_minor"); if crate::gc::gc_diag_enabled() { + // This is intentionally the last diagnostic action before returning to + // the mutator: `pause_us` prices the whole copied-minor path, including + // finalization, pruning, policy feedback and the diagnostic work above. + let pause_us = start.elapsed().as_micros() as u64; eprintln!( - "[gc-copy-minor] ran in_place={} untraced={} untraced_cycles={} untraced_objects={} in_place_blocks={} in_place_dead_bytes={} sparse_blocks={} survival_permille={} copied_objects={} copied_bytes={} promoted_objects={} promoted_bytes={} freed_bytes={} tenuring_survivals={} eden_live_bytes={} trigger={:?} declared_safepoint={}", + "[gc-copy-minor] ran pause_us={} scan_us={} in_place={} untraced={} untraced_cycles={} untraced_objects={} in_place_blocks={} in_place_dead_bytes={} sparse_blocks={} survival_permille={} copied_objects={} copied_bytes={} promoted_objects={} promoted_bytes={} freed_bytes={} tenuring_survivals={} eden_live_bytes={} trigger={:?} declared_safepoint={}", + pause_us, + scan_us, collector.stats.in_place_promotion, untraced, super::untraced_promotion_cycles(), @@ -1894,14 +1908,6 @@ pub(super) fn run_copied_minor_attempt( super::policy::GC_AT_DECLARED_SAFEPOINT.with(std::cell::Cell::get) ); } - if let Some(d) = collector.survival.as_ref() { - d.report(super::survival_diag::next_minor_seq()); - } - crate::arena::alloc_sample::report("minor"); - super::diag_sites::report_primitive_dispatch("minor"); - crate::object::shapes::id_list_report(); - report_forwarding_refusals("copying_minor"); - super::scanner_profile::report_and_reset("copying_minor"); CopiedMinorAttempt::Done(Some(CopiedMinorFastPathOutcome { freed_bytes, malloc_swept: malloc_sweep_due, diff --git a/crates/perry-runtime/src/gc/scanner_profile.rs b/crates/perry-runtime/src/gc/scanner_profile.rs index 2e95310269..b86871070f 100644 --- a/crates/perry-runtime/src/gc/scanner_profile.rs +++ b/crates/perry-runtime/src/gc/scanner_profile.rs @@ -128,14 +128,14 @@ pub(super) fn note_scanner( /// Print the per-scanner breakdown accumulated since the last report, then /// clear it. Called once per copied minor from the `[gc-copy-minor]` diag site. -pub(super) fn report_and_reset(cycle_label: &str) { +pub(super) fn report_and_reset(cycle_label: &str) -> u64 { if !scanner_profile_enabled() { - return; + return 0; } 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; + return 0; } rows.sort_by(|a, b| b.1.nanos.cmp(&a.1.nanos)); let total_ns: u64 = rows.iter().map(|(_, row)| row.nanos).sum(); @@ -159,4 +159,5 @@ pub(super) fn report_and_reset(cycle_label: &str) { row.rewrites ); } + total_ns / 1000 } 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 e6cf25887d..53eaa899f3 100644 --- a/crates/perry-runtime/src/gc/tests/young_log_tests.rs +++ b/crates/perry-runtime/src/gc/tests/young_log_tests.rs @@ -38,6 +38,10 @@ fn old_closure() -> usize { ptr as usize } +fn old_leaf() -> usize { + crate::arena::arena_alloc_gc_old(32, 8, GC_TYPE_STRING) as usize +} + unsafe fn young_keys_array() -> *mut crate::array::ArrayHeader { let arr = crate::arena::arena_alloc_gc( std::mem::size_of::(), @@ -602,3 +606,241 @@ fn installing_an_external_shape_id_arms_the_family_log() { "the family must have followed the keys array" ); } + +// ------------------------------------------------------ fixed-cost scanners + +/// N old shape families plus k young ones must price exactly k entries in the +/// minor-scoped scanner. Sabotage: make `note_young_keys` a no-op; the +/// re-derivation fails before this count can be observed. +#[test] +fn shape_table_minor_walk_visits_exactly_k_young_entries() { + const N: usize = 96; + const K: usize = 3; + let _guard = CopyingNurseryTestGuard::new(K as u32); + gc_register_mutable_root_scanner(crate::object::shapes::scan_shape_table_rekey_mut); + crate::object::shapes::test_clear_shape_table(); + + for _ in 0..N { + 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; + } + crate::object::shapes::shape_descriptor_ensure(keys, 0, 0).expect("old shape"); + } + for slot in 0..K { + let keys = unsafe { young_keys_array() }; + js_shadow_slot_set(slot as u32, ptr_bits(keys as usize)); + crate::object::shapes::shape_descriptor_ensure(keys, 0, 0).expect("young shape"); + } + + let _ = gc_collect_minor(); + let row = walk("shapes.families+indices"); + assert!(row.partial, "{row:?}"); + assert_eq!( + row.visited, K as u64, + "minor work must be young-sized: {row:?}" + ); + assert!( + row.table_len >= (N + K) as u64, + "fixture did not build N+k: {row:?}" + ); +} + +/// The debug/test authoritative walk is the proof that every shape writer +/// arms the log. This deliberately suppresses the production family funnel; +/// deleting the assertion makes the sabotage go green. +#[test] +fn shape_table_rederivation_rejects_a_suppressed_logging_site() { + let _guard = CopyingNurseryTestGuard::new(0); + crate::object::shapes::test_clear_shape_table(); + let keys = unsafe { young_keys_array() }; + { + let _sabotage = crate::object::shapes::TestShapeYoungLogSuppression::new(); + crate::object::shapes::shape_descriptor_ensure(keys, 0, 0).expect("shape"); + } + let valid = build_valid_pointer_set(); + let mut visitor = RuntimeRootVisitor::for_mark_scoped(&valid, true); + let rejected = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + crate::object::shapes::scan_shape_table_rekey_mut(&mut visitor); + })); + assert!( + rejected.is_err(), + "a missing shape log note must be detected" + ); +} + +/// Promotion removes a shape address from the minor log, without removing the +/// descriptor from the authoritative table used by the next full/major walk. +/// Sabotage: change the post-visit keep predicate back to +/// `addr_is_minor_relevant(from_space)`; `kept` never reaches zero. +#[test] +fn promoted_shape_entry_leaves_young_log_and_remains_in_major_walk() { + 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::shape_descriptor_ensure(keys, 0, 0).expect("shape"); + + for _ in 0..4 { + let _ = gc_collect_minor(); + } + let promoted = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + assert!( + !crate::arena::pointer_in_nursery(promoted), + "fixture must promote" + ); + assert_eq!(walk("shapes.families+indices").kept, 0); + + let valid = build_valid_pointer_set(); + let mut visitor = RuntimeRootVisitor::for_rewrite(&valid); + crate::object::shapes::scan_shape_table_rekey_mut(&mut visitor); + let row = walk("shapes.families+indices"); + assert!( + !row.partial, + "major/full walk must remain authoritative: {row:?}" + ); + assert!( + row.visited >= 1, + "major/full walk must still see the descriptor" + ); + assert_eq!( + crate::object::shapes::shape_descriptor_by_id(id).map(|d| d.keys), + Some(promoted as u64) + ); +} + +/// An already-Longlived keys array can gain a new nursery key at the same +/// address. Re-stamping the old receiver is the structural publication +/// chokepoint that must re-arm it. +#[test] +fn shape_mutation_to_new_young_key_rearms_minor_log() { + let _guard = CopyingNurseryTestGuard::new(0); + gc_register_mutable_root_scanner(crate::object::shapes::scan_shape_table_rekey_mut); + crate::object::shapes::test_clear_shape_table(); + unsafe { + let bytes = std::mem::size_of::() + 8; + let keys = crate::arena::arena_alloc_gc_longlived(bytes, 8, GC_TYPE_ARRAY) + as *mut crate::array::ArrayHeader; + (*keys).length = 1; + (*keys).capacity = 1; + let slot = + (keys as *mut u8).add(std::mem::size_of::()) as *mut f64; + *slot = f64::from_bits(string_bits(old_leaf())); + let id = crate::object::shapes::shape_descriptor_ensure(keys, 1, 0).expect("shape"); + let (owner, _) = alloc_old_test_object(0); + crate::object::shapes::stamp_object_shape_id_with_carrier_note(owner, id); + let _ = gc_collect_minor(); + assert_eq!(walk("shapes.families+indices").kept, 0); + + let young = young_leaf(); + *slot = f64::from_bits(string_bits(young)); + crate::object::shapes::stamp_object_shape_id_with_carrier_note(owner, id); + let _ = gc_collect_minor(); + let moved = ((*slot).to_bits() & POINTER_MASK) as usize; + assert_ne!( + moved, young, + "the mutation hook must make the new key visible" + ); + assert!(walk("shapes.families+indices").visited >= 1); + } +} + +/// N old box payloads plus k young payloads must price exactly k registry +/// entries. The counter is recorded inside `scan_box_young_roots_mut`. +#[test] +fn box_roots_minor_walk_visits_exactly_k_young_entries() { + const N: usize = 128; + const K: usize = 4; + let _guard = CopyingNurseryTestGuard::new(0); + gc_register_mutable_root_scanner(crate::r#box::scan_box_roots_mut); + for _ in 0..N { + crate::r#box::js_box_alloc_bits(string_bits(old_leaf()) as i64); + } + for _ in 0..K { + crate::r#box::js_box_alloc_bits(string_bits(young_leaf()) as i64); + } + + let _ = gc_collect_minor(); + let row = walk("box.roots"); + assert!(row.partial, "{row:?}"); + assert_eq!( + row.visited, K as u64, + "minor work must be young-sized: {row:?}" + ); + assert_eq!( + row.table_len, + (N + K) as u64, + "fixture registry mismatch: {row:?}" + ); +} + +/// Suppress the real `js_box_set_bits` arming site and prove the full-registry +/// re-derivation catches the omission. +#[test] +fn box_root_rederivation_rejects_a_suppressed_mutation_hook() { + let _guard = CopyingNurseryTestGuard::new(0); + let cell = crate::r#box::js_box_alloc_bits(string_bits(old_leaf()) as i64); + let young = young_leaf(); + { + let _sabotage = crate::r#box::TestBoxYoungLogSuppression::new(); + crate::r#box::js_box_set_bits(cell, string_bits(young) as i64); + } + let valid = build_valid_pointer_set(); + let mut visitor = RuntimeRootVisitor::for_mark_scoped(&valid, true); + let rejected = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + crate::r#box::scan_box_roots_mut(&mut visitor); + })); + assert!( + rejected.is_err(), + "a missing box mutation note must be detected" + ); +} + +#[test] +fn box_mutation_to_new_young_object_is_visited() { + let _guard = CopyingNurseryTestGuard::new(0); + gc_register_mutable_root_scanner(crate::r#box::scan_box_roots_mut); + let cell = crate::r#box::js_box_alloc_bits(string_bits(old_leaf()) as i64); + let young = young_leaf(); + crate::r#box::js_box_set_bits(cell, string_bits(young) as i64); + + let _ = gc_collect_minor(); + let moved = (crate::r#box::js_box_get_bits(cell) as u64 & POINTER_MASK) as usize; + assert_ne!(moved, young, "setter must re-arm a previously old box"); + assert_eq!(walk("box.roots").visited, 1); +} + +#[test] +fn promoted_box_root_leaves_log_and_is_found_by_full_walk() { + let _guard = CopyingNurseryTestGuard::new(0); + gc_register_mutable_root_scanner(crate::r#box::scan_box_roots_mut); + let cell = crate::r#box::js_box_alloc_bits(string_bits(young_leaf()) as i64); + for _ in 0..4 { + let _ = gc_collect_minor(); + } + let promoted_bits = crate::r#box::js_box_get_bits(cell) as u64; + let promoted = (promoted_bits & POINTER_MASK) as usize; + assert!( + !crate::arena::pointer_in_nursery(promoted), + "fixture must promote" + ); + assert_eq!(walk("box.roots").kept, 0); + + let mut seen = false; + crate::r#box::scan_box_roots(&mut |value| { + if value.to_bits() == promoted_bits { + seen = true; + } + }); + assert!( + seen, + "the unchanged full walk must still enumerate promoted roots" + ); + assert!(!walk("box.roots").partial); +} diff --git a/crates/perry-runtime/src/gc/young_log.rs b/crates/perry-runtime/src/gc/young_log.rs index 9f6d43b22f..d83d9bb36c 100644 --- a/crates/perry-runtime/src/gc/young_log.rs +++ b/crates/perry-runtime/src/gc/young_log.rs @@ -155,7 +155,7 @@ impl YoungLog { /// 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)] + #[cfg(any(debug_assertions, test))] pub(crate) fn debug_assert_logged(&self, table: &'static str, relevant: &[K]) where K: std::fmt::Debug, @@ -210,6 +210,44 @@ pub(crate) fn addr_is_minor_relevant(addr: usize) -> bool { } } +/// Can a minor move or reclaim the object at `addr`? +/// +/// This is narrower than [`addr_is_minor_relevant`]: `Longlived` objects must +/// sometimes be traced *through*, but they are never themselves moved or +/// swept. Side tables whose entries name known GC leaves (shape property +/// keys are strings/symbol headers) use this predicate so an immortal leaf +/// does not pin its entry in a young log forever. +#[inline] +pub(crate) fn addr_is_minor_collectible(addr: usize) -> bool { + if addr == 0 { + return false; + } + match crate::arena::classify_heap_space(addr) { + HeapSpace::NurseryEden + | HeapSpace::Survivor0 + | HeapSpace::Survivor1 + | HeapSpace::PromotedYoung => true, + HeapSpace::Old | HeapSpace::Longlived => false, + HeapSpace::Unknown => { + addr > GC_HEADER_SIZE + && super::malloc::gc_malloc_header_is_tracked( + (addr - GC_HEADER_SIZE) as *const super::GcHeader, + ) + } + } +} + +/// [`addr_is_minor_collectible`] for a NaN-boxed value. +#[inline] +pub(crate) fn bits_are_minor_collectible(bits: u64) -> bool { + let tag = bits & TAG_MASK; + if tag == POINTER_TAG || tag == STRING_TAG || tag == BIGINT_TAG { + addr_is_minor_collectible((bits & POINTER_MASK) as usize) + } else { + false + } +} + /// [`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. diff --git a/crates/perry-runtime/src/object/shapes.rs b/crates/perry-runtime/src/object/shapes.rs index b0d47aac53..51724f362b 100644 --- a/crates/perry-runtime/src/object/shapes.rs +++ b/crates/perry-runtime/src/object/shapes.rs @@ -264,6 +264,28 @@ struct ShapeTableInner { const SHAPE_YOUNG_LOG_NAME: &str = "shapes.families+indices"; +crate::perry_thread_local! { + /// Carrier notes can be produced while a GC walk already borrows the shape + /// table. Keep that write-side stream separate and merge it at the next + /// scanner entry rather than re-borrowing `ShapeTableInner` recursively. + static SHAPE_CARRIER_YOUNG_KEYS: RefCell> = + const { RefCell::new(crate::gc::young_log::YoungLog::new()) }; + #[cfg(test)] + static SHAPE_YOUNG_LOG_SUPPRESSED: std::cell::Cell = const { std::cell::Cell::new(false) }; +} + +#[inline] +fn note_shape_carrier_candidate(keys: u64) { + if !crate::gc::young_log::addr_is_minor_relevant(keys as usize) { + return; + } + #[cfg(test)] + if SHAPE_YOUNG_LOG_SUPPRESSED.with(std::cell::Cell::get) { + return; + } + SHAPE_CARRIER_YOUNG_KEYS.with(|log| log.borrow_mut().note(keys)); +} + /// Re-export of the id-list operation counters' report, so the collector does /// not have to name a private sibling module. One `[gc-idlist]` line per /// copying minor under `PERRY_GC_DIAG=1`; `elems_moved` is the falsifier for @@ -281,7 +303,11 @@ impl ShapeTableInner { /// call this themselves. #[inline] fn note_young_keys(&mut self, keys: u64) { - if crate::gc::young_log::addr_is_minor_relevant(keys as usize) { + #[cfg(test)] + if SHAPE_YOUNG_LOG_SUPPRESSED.with(std::cell::Cell::get) { + return; + } + if crate::gc::young_log::addr_is_minor_collectible(keys as usize) { self.young_keys.note(keys); } } @@ -692,8 +718,12 @@ pub(crate) unsafe fn note_old_generation_carrier(descriptor: Option) { return; } let record = descriptor.record as *mut ShapeRecord; + let newly_armed = !(*record).has(RECORD_FLAG_CACHE_CARRIER); // GC_STORE_AUDIT(POINTER_FREE): liveness bookkeeping bit, never a heap reference. (*record).set(RECORD_FLAG_CACHE_CARRIER, true); + if newly_armed { + note_shape_carrier_candidate(descriptor.keys); + } } /// The post-birth publication point for a ShapeId into a receiver's header @@ -770,7 +804,15 @@ pub(crate) unsafe fn stamp_object_shape_id_with_carrier_note( ) { (*obj).parent_class_id = id; if !crate::arena::pointer_in_nursery(obj as usize) { - note_old_generation_carrier(shape_descriptor_by_id(id)); + let descriptor = shape_descriptor_by_id(id); + note_old_generation_carrier(descriptor); + // This stamp is the structural-mutation publication funnel. Re-arm + // even when the descriptor was already an old carrier: an owned + // Longlived keys array may have just gained a nursery key at the same + // address, and its carrier flag alone cannot express that transition. + if let Some(descriptor) = descriptor { + note_shape_carrier_candidate(descriptor.keys); + } } } @@ -1930,6 +1972,8 @@ pub(crate) fn prune_dead_shape_keys_young(is_dead_owner: &dyn Fn(usize) -> bool) pub(crate) fn scan_shape_table_rekey_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { let table = &crate::state::state().shapes; let mut inner = table.inner.borrow_mut(); + let carrier_notes = SHAPE_CARRIER_YOUNG_KEYS.with(|log| log.borrow_mut().take_sorted()); + inner.young_keys.extend(carrier_notes); 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. @@ -2045,7 +2089,7 @@ pub(crate) fn scan_shape_table_rekey_mut(visitor: &mut crate::gc::RuntimeRootVis } // A full walk is authoritative: rebuild the young log from the tables. - let kept = relevant_shape_keys(&inner); + let kept = relevant_shape_keys(table, &inner); let kept_len = kept.len() as u64; let _ = inner.young_keys.take_sorted(); inner.young_keys.extend(kept); @@ -2080,33 +2124,82 @@ fn move_shape_family(table: &ShapeTable, inner: &mut ShapeTableInner, old: u64, 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); + // Scanner-internal rekey: the caller keeps `new` from its post-visit + // relevance result (or the full walk rebuilds the log). Re-entering + // the writer funnel here would enqueue the same family mid-walk and + // price it twice in one minor. + inner.families.entry(new).or_default().push_back(id); } } /// 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() - .copied() - .filter(|&keys| keys != 0 && addr_is_minor_relevant(keys as usize)) - .collect(); - relevant.extend( - inner - .indices - .keys() - .copied() - .filter(|&keys| addr_is_minor_relevant(keys)) - .map(|keys| keys as u64), - ); +fn relevant_shape_keys(table: &ShapeTable, inner: &ShapeTableInner) -> Vec { + let mut relevant: Vec = inner.families.keys().copied().collect(); + relevant.extend(inner.indices.keys().copied().map(|keys| keys as u64)); relevant.sort_unstable(); relevant.dedup(); + relevant.retain(|&keys| shape_keys_entry_is_minor_relevant(table, inner, keys)); relevant } +/// Exact minor-work predicate for one shape-table key. +/// +/// Nursery addresses must be rekeyed even for weak metadata entries. Malloc +/// arrays must be rooted when a carrier owns the family. A Longlived keys +/// array never moves or dies, so it matters only while a rooted family exposes +/// a collectible property-key leaf from its payload. Property keys are +/// strings/symbol headers and both are GC leaves; tracing through an immortal +/// key cannot discover a younger grandchild. +fn shape_keys_entry_is_minor_relevant( + table: &ShapeTable, + inner: &ShapeTableInner, + keys: u64, +) -> bool { + if keys == 0 { + return false; + } + let addr = keys as usize; + match crate::arena::classify_heap_space(addr) { + crate::arena::HeapSpace::NurseryEden + | crate::arena::HeapSpace::Survivor0 + | crate::arena::HeapSpace::Survivor1 + | crate::arena::HeapSpace::PromotedYoung => return true, + crate::arena::HeapSpace::Old => return false, + crate::arena::HeapSpace::Unknown => { + return family_has_root_carrier(table, inner, keys) + && crate::gc::young_log::addr_is_minor_collectible(addr); + } + crate::arena::HeapSpace::Longlived => {} + } + if !family_has_root_carrier(table, inner, keys) { + return false; + } + unsafe { + let Some(header) = crate::value::addr_class::try_read_tracked_gc_header(addr) else { + return false; + }; + if (*header.as_ptr()).obj_type != crate::gc::GC_TYPE_ARRAY { + return false; + } + let (slots, len) = super::keys_array_dense_slots(addr as *const ArrayHeader); + (0..len).any(|index| { + crate::gc::young_log::bits_are_minor_collectible((*slots.add(index)).to_bits()) + }) + } +} + +fn family_has_root_carrier(table: &ShapeTable, inner: &ShapeTableInner, keys: u64) -> bool { + inner.families.get(&keys).is_some_and(|ids| { + ids.as_slice().iter().any(|&id| { + table + .slab() + .get(id) + .is_some_and(|record| record.has(RECORD_FLAG_OLD_CARRIER) || record.cache_carrier()) + }) + }) +} + /// 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 @@ -2118,9 +2211,9 @@ fn scan_shape_table_young( rewrite_phase: bool, ) { let table_len = (inner.families.len() + inner.indices.len()) as u64; - #[cfg(debug_assertions)] + #[cfg(any(debug_assertions, test))] { - let relevant = relevant_shape_keys(inner); + let relevant = relevant_shape_keys(table, inner); inner .young_keys .debug_assert_logged(SHAPE_YOUNG_LOG_NAME, &relevant); @@ -2242,10 +2335,7 @@ fn scan_shape_keys_address( inner.indices.remove(&addr); } } - ( - post, - crate::gc::young_log::addr_is_minor_relevant(post as usize), - ) + (post, shape_keys_entry_is_minor_relevant(table, inner, post)) } // #8112 sabotage switch. Suppressing the descriptor edge proves the fixture's diff --git a/crates/perry-runtime/src/object/shapes_test_support.rs b/crates/perry-runtime/src/object/shapes_test_support.rs index 6cb974c98c..21bc8f7b4a 100644 --- a/crates/perry-runtime/src/object/shapes_test_support.rs +++ b/crates/perry-runtime/src/object/shapes_test_support.rs @@ -44,6 +44,25 @@ pub(crate) struct TestRecycledKeysCheckSuppression { previous: bool, } +/// Suppress both shape-table young-log writer funnels. A test using this guard +/// must be rejected by the scanner's authoritative re-derivation. +#[cfg(test)] +pub(crate) struct TestShapeYoungLogSuppression(bool); + +#[cfg(test)] +impl TestShapeYoungLogSuppression { + pub(crate) fn new() -> Self { + Self(SHAPE_YOUNG_LOG_SUPPRESSED.with(|cell| cell.replace(true))) + } +} + +#[cfg(test)] +impl Drop for TestShapeYoungLogSuppression { + fn drop(&mut self) { + SHAPE_YOUNG_LOG_SUPPRESSED.with(|cell| cell.set(self.0)); + } +} + #[cfg(test)] impl TestRecycledKeysCheckSuppression { pub(crate) fn new() -> Self { @@ -84,6 +103,7 @@ pub(crate) fn test_clear_shape_table() { inner.by_facts.clear(); inner.families.clear(); inner.young_keys.clear(); + SHAPE_CARRIER_YOUNG_KEYS.with(|log| log.borrow_mut().clear()); // SAFETY: test-only reset with no slab reference held. unsafe { table.slab_mut().clear() }; drop(inner); From 194fcb67675fe298457e75b3f6b9808f01e9ac64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 07:18:42 +0200 Subject: [PATCH 02/11] docs(perf): report minor scanner young logs Record the scanner map, sabotage-able test coverage, disk-gated validation, predictions, and the exact perrymaster follow-up request. --- .../codex/REPORT_minor_scanner_young_logs.md | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 cc-perf-campaign/codex/REPORT_minor_scanner_young_logs.md diff --git a/cc-perf-campaign/codex/REPORT_minor_scanner_young_logs.md b/cc-perf-campaign/codex/REPORT_minor_scanner_young_logs.md new file mode 100644 index 0000000000..fe8570d4a8 --- /dev/null +++ b/cc-perf-campaign/codex/REPORT_minor_scanner_young_logs.md @@ -0,0 +1,107 @@ +# Minor scanner young logs + +Implementation SHA: `d399c39ddb638a92b2735a6bacc2aef13def944a` + +## Map and mechanism + +- `object/shapes.rs:1972` scans two address-keyed structures. `families` maps a + keys-array address to every descriptor id whose slab record carries that + address; the descriptor record is the authoritative rewritable `keys` edge. + A family is a strong minor root only when an old receiver or an optimization + cache carries one of its descriptors. `indices` is a weak key-to-slot + accelerator keyed by the same keys-array address and needs only relocation + repair. Shape property-key payloads are strings/symbol headers, both GC + leaves. Nursery keys arrays can move; old arrays cannot; Longlived arrays do + not move or die but can temporarily contain a collectible key leaf. +- Shapes already had #9755's `young_keys` address log and the four + `shapes.indices` arm sites. Its keep predicate was + `addr_is_minor_relevant`, so every Longlived keys array stayed in the log + forever. `object/shapes.rs:2154` now re-derives actual minor work: nursery + addresses remain for relocation, malloc roots remain while carrier-owned, + and a Longlived carrier remains only while its property-key payload contains + a collectible leaf. `object/shapes.rs:271` receives old/cache carrier notes + without recursively borrowing the shape table; `object/shapes.rs:801` is the + enforced structural-publication funnel that re-arms a same-address mutation. + Scanner-internal rekeys do not enqueue a duplicate visit. +- `box.rs:954` previously walked every address in `BOX_REGISTRY`. These are + malloc-allocated mutable-capture/async state cells; the registry address is + not a GC pointer. Only the `Box::value` NaN-box can point into the nursery. + `I32Box` and `BoolBox` registries contain no GC edge and were never part of + this scanner. There was no partial box log. +- `box.rs:123` adds the box remembered set. Both allocation arms and both + mutation ABIs arm it before publishing a minor-relevant payload + (`box.rs:133`, `box.rs:705`, `box.rs:725`, `box.rs:1318`, `box.rs:1357`). + The trusted setter is included because generated boxed-local stores use it; + omitting that silent path would violate the enforced-funnel rule. Release + paths only clear/de-register cells, and scanner rewrites compact their own + entries. `box.rs:1040` owns the priced `visited` counter. +- Both minor walks sort/deduplicate their logged addresses, drop stale keys, + and keep only post-visit non-old entries. Full/major scans still enumerate + the authoritative whole tables and rebuild the logs. Under + `debug_assertions` and in lib tests, each minor scan re-derives the relevant + set from the whole table and asserts that the log is complete. +- `gc/copying.rs:1889` now emits `pause_us=` and `scan_us=` together on every + completed `[gc-copy-minor] ran` line. `pause_us` is sampled as the final + action before the copied-minor returns to the mutator; `scan_us` is the + already-profiled scanner total returned by `gc/scanner_profile.rs:131`. + Timing remains behind the existing cached `PERRY_GC_DIAG` gate. + +## Tests and sabotages + +- `shape_table_minor_walk_visits_exactly_k_young_entries`: N old families and + k young families produce `visited == k`. Sabotage: remove + `note_young_keys`; the completeness re-derivation panics. +- `shape_table_rederivation_rejects_a_suppressed_logging_site`: a test-only + suppression skips the production family arm and the scan must panic. +- `shape_mutation_to_new_young_key_rearms_minor_log`: a Longlived carrier that + gains a new nursery key at the same address must move that key. Sabotage: + remove the re-arm in `stamp_object_shape_id_with_carrier_note`. +- `box_roots_minor_walk_visits_exactly_k_young_entries`: N old payloads and k + young payloads produce `visited == k`. Sabotage: remove either allocator arm. +- `box_root_rederivation_rejects_a_suppressed_mutation_hook`: a test-only + suppression skips `js_box_set_bits` logging and the authoritative registry + walk must panic. +- `box_mutation_to_new_young_object_is_visited`: an old box changed to a new + nursery object is visited. Sabotage: remove the setter hook. +- `promoted_shape_entry_leaves_young_log_and_remains_in_major_walk` and + `promoted_box_root_leaves_log_and_is_found_by_full_walk`: promotion makes + `kept == 0`, while the next authoritative full walk still visits the entry. + Sabotage: retain the pre-visit/from-space classification or scope the full + walk to the log. +- Existing scanner-completeness and moving-witness suites are unchanged and + remain part of the requested runtime-lib gate. + +## Validation + +- `scripts/check_file_size.sh`: PASS. +- `git diff --check`: PASS. +- Cargo gates: NOT RUN. `df -g /` immediately before the first possible Cargo + invocation reported `0` GB available, below the binding 12 GB floor. Per the + task rule, no Cargo command was started and no wait for disk was attempted. +- Not run for the same reason: + `cargo test -p perry-runtime --release --lib -- --test-threads=1`; + `cargo build --release -p perry-runtime --features wasm-host`; + `cargo build --release -p perry`. + +## Predictions and exact perrymaster request + +Predictions: on a zero-live steady minor, +`object::shapes::scan_shape_table_rekey_mut` and +`r#box::scan_box_roots_mut` each fall from about 2 ms to at most 0.2 ms; +steady-minor scanner total falls from 7–8 ms to at most 3 ms; every completed +minor reports `pause_us` and `scan_us`. CPU bound is about -3% at 3300 chars +and larger at 400 chars, where minors are a larger share. RSS should be +unchanged (small retained log capacities only, within the allowed 1–10%). + +Perrymaster request, from pushed SHA: relink on the I7-view tree +(runtime-only), then run the three gates through +`/Users/amlug/projects/perry/secret-tests/cc-perf-campaign/measure_lock.sh --build` +at `-j4` using detached `nohup`: (1) +`cargo test -p perry-runtime --release --lib -- --test-threads=1`, (2) +`cargo build --release -p perry-runtime --features wasm-host`, and (3) +`cargo build --release -p perry`. Because this is GC-adjacent, the coordinator +must apply `run-extended-tests`. After green gates, do one graceful four-turn +3300-char run and one 400-char run with `PERRY_GC_DIAG=1`, preserving complete +`[gc-copy-minor] ran pause_us=... scan_us=...` and +`[gc-scanner-profile] copying_minor` lines. Then run paired 5x3300 + 3x400 +against I7-view for CPU and RSS. From 09846784cf9fdb277fcd83b8b2c27c068dd0f003 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 11:20:56 +0200 Subject: [PATCH 03/11] perf(gc): split copying minor diagnostics by phase Account for the successful copying-minor path with diagnostic-only wall-time buckets that partition the same interval as pause_us. Include per-table from-space finalization and dead-owner prune detail so the remaining fixed minor cost can be localized from one complete ran line. --- crates/perry-runtime/src/gc/copying.rs | 101 +++++++-- crates/perry-runtime/src/gc/copying_phase.rs | 204 ++++++++++++++++++ crates/perry-runtime/src/gc/dead_owner.rs | 20 +- crates/perry-runtime/src/gc/mod.rs | 1 + .../src/node_submodules/diagnostics_gc.rs | 4 +- 5 files changed, 309 insertions(+), 21 deletions(-) create mode 100644 crates/perry-runtime/src/gc/copying_phase.rs diff --git a/crates/perry-runtime/src/gc/copying.rs b/crates/perry-runtime/src/gc/copying.rs index 9afd510073..0e6d48a23a 100644 --- a/crates/perry-runtime/src/gc/copying.rs +++ b/crates/perry-runtime/src/gc/copying.rs @@ -1,3 +1,7 @@ +use super::copying_phase::{ + finalize_dead_copied_minor_from_space_side_allocations, CopyingMinorPhase as Phase, + CopyingMinorPhaseDiag as PhaseDiag, +}; use super::*; /// Largest object `move_young` will relocate. See its use site for the @@ -1213,6 +1217,7 @@ pub(super) fn run_copied_minor_attempt( let ptrs = eligibility .ptrs .expect("eligible copied-minor decision must carry pointer classifier"); + let mut phase_diag = PhaseDiag::enabled(); let phase_start = trace_phase_start(trace); let from_space_bytes = crate::arena::copying_from_space_in_use_bytes(); @@ -1236,11 +1241,15 @@ pub(super) fn run_copied_minor_attempt( && ptrs.malloc_registry_empty_at_start && untraced_promotion_instrument_veto().is_none() && super::should_attempt_first_cycle_promotion(); + let promotion_phase_start = PhaseDiag::start(&phase_diag); let promotion = if super::should_promote_young_in_place() || speculate_first_cycle { crate::arena::retag_young_for_in_place_promotion(speculate_first_cycle) } else { crate::arena::InPlacePromotion::default() }; + if let Some(diag) = phase_diag.as_mut() { + diag.record(Phase::Promotion, promotion_phase_start); + } // An empty plan (nothing in use to promote) falls back to the ordinary // path, so the from-space reset still runs. let promoting_in_place = !promotion.is_empty(); @@ -1319,8 +1328,13 @@ pub(super) fn run_copied_minor_attempt( "policy (should_promote_young_untraced)" }) }); + let reset_phase_start = PhaseDiag::start(&phase_diag); collector.stats.reset_blocks += crate::arena::copying_prepare_to_space(); + if let Some(diag) = phase_diag.as_mut() { + diag.record(Phase::BlockResetFlip, reset_phase_start); + } + let root_scan_phase_start = PhaseDiag::start(&phase_diag); let native_stack_walk = if untraced { Default::default() } else { @@ -1399,6 +1413,9 @@ pub(super) fn run_copied_minor_attempt( } visit_ffi_mutable_registered_roots_with_sources(&mut visitor, root_sources); } + if let Some(diag) = phase_diag.as_mut() { + diag.record(Phase::RootScan, root_scan_phase_start); + } // On an untraced promotion the dirty SCAN is where the whole per-object // mark pass lived: `retain`'s array store has a young child in every page @@ -1411,6 +1428,7 @@ pub(super) fn run_copied_minor_attempt( // read path for the remembered set, which is where #7187's lazy barrier // arming happens. Skipping it would leave the barrier unarmed for the next // cycle — a missing-edge bug one collection later. + let remembered_phase_start = PhaseDiag::start(&phase_diag); 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`). @@ -1428,6 +1446,8 @@ pub(super) fn run_copied_minor_attempt( // reserved bytes, and under-estimating falls back to ordinary growth. let mut dirty_scan_covered = crate::fast_hash::new_ptr_hash_set_with_capacity(previous_dirty_covered_estimate()); + let mut remembered_entries = 0usize; + let mut remembered_slots = 0usize; if !untraced { let _phase = super::pin::CopyingWalkPhaseGuard::enter("remembered_set"); let remembered_stats = scan_remembered_dirty_slots_copying( @@ -1447,11 +1467,21 @@ pub(super) fn run_copied_minor_attempt( if let Some(trace) = trace.as_mut() { trace.remembered_set = remembered_stats; } + remembered_entries = remembered_stats.entries_scanned; + remembered_slots = remembered_stats.dirty_slots_scanned; } + if let Some(diag) = phase_diag.as_mut() { + diag.record(Phase::RememberedSetYoungLogs, remembered_phase_start); + } + let copy_phase_start = PhaseDiag::start(&phase_diag); unsafe { let _phase = super::pin::CopyingWalkPhaseGuard::enter("worklist_drain"); collector.drain(); } + if let Some(diag) = phase_diag.as_mut() { + diag.record(Phase::CopyEvacuation, copy_phase_start); + } + let rewrite_root_scan_phase_start = PhaseDiag::start(&phase_diag); { let scanners: Vec = if untraced { Vec::new() @@ -1483,6 +1513,9 @@ pub(super) fn run_copied_minor_attempt( } visit_ffi_mutable_registered_roots_with_sources(&mut visitor, root_sources); } + if let Some(diag) = phase_diag.as_mut() { + diag.record(Phase::RootScan, rewrite_root_scan_phase_start); + } // #7803 THE FIX: rebuild the promoted-object remembered set AFTER the last // phase that can move an object, not before the drain. // @@ -1507,6 +1540,7 @@ pub(super) fn run_copied_minor_attempt( // the rebuild performs is exact rather than a from-space // over-approximation. Headers still carry GC_FLAG_MARKED (clear_marks // runs later), which the per-object gate requires. + let forwarding_phase_start = PhaseDiag::start(&phase_diag); if !collector.skip_remembering { let promoted_sticky = rebuild_evacuated_old_to_young_remembered_set(&collector.moved_headers); @@ -1528,6 +1562,9 @@ pub(super) fn run_copied_minor_attempt( super::roots::stack_maps_native_slot_verify(untraced, &|addr| { format!("{:?}", collector.ptrs.classify(addr).map(|ptr| ptr.kind)) }); + if let Some(diag) = phase_diag.as_mut() { + diag.record(Phase::ForwardingFixups, forwarding_phase_start); + } trace_phase_record(trace, "copying_nursery", phase_start); // #7937: the attempt's own trace has finished, so the ratio it was missing @@ -1630,8 +1667,16 @@ pub(super) fn run_copied_minor_attempt( // run here, same window as fromspace_scan (after rewrite, before reset). super::native_stack_scan::run_native_stack_scan(); - crate::promise::cleanup_copied_minor_promise_contexts_for_gc(); - finalize_dead_copied_minor_from_space_side_allocations(); + let finalization = finalize_dead_copied_minor_from_space_side_allocations(); + if let Some(diag) = phase_diag.as_mut() { + diag.add_nanos(Phase::DeadOwnerSideTablePruning, finalization.dead_owner_ns); + diag.add_nanos( + Phase::FromSpaceFinalization, + finalization + .total_ns + .saturating_sub(finalization.dead_owner_ns), + ); + } // #7742: on a promoting cycle the young blocks are handed to old-gen // instead of being reset. This MUST stay before `clear_marks` — the finish // walk reads `GC_FLAG_MARKED` to decide which objects to index — and it @@ -1639,6 +1684,7 @@ pub(super) fn run_copied_minor_attempt( // blocks the reset would recycle are the blocks this keeps. let (reset, promotion_stats) = if promoting_in_place { let phase_start = trace_phase_start(trace); + let promotion_phase_start = PhaseDiag::start(&phase_diag); super::note_promoted_young_capacity(promotion.reserved_bytes()); let promotion_stats = crate::arena::finish_in_place_promotion( promotion, @@ -1648,6 +1694,9 @@ pub(super) fn run_copied_minor_attempt( crate::arena::PromotionLiveness::Marked }, ); + if let Some(diag) = phase_diag.as_mut() { + diag.record(Phase::Promotion, promotion_phase_start); + } trace_phase_record(trace, "in_place_promotion", phase_start); ( crate::arena::ArenaResetStats { @@ -1658,10 +1707,12 @@ pub(super) fn run_copied_minor_attempt( promotion_stats, ) } else { - ( - crate::arena::copying_reset_from_spaces_and_flip(), - crate::arena::InPlacePromotionStats::default(), - ) + let reset_phase_start = PhaseDiag::start(&phase_diag); + let reset = crate::arena::copying_reset_from_spaces_and_flip(); + if let Some(diag) = phase_diag.as_mut() { + diag.record(Phase::BlockResetFlip, reset_phase_start); + } + (reset, crate::arena::InPlacePromotionStats::default()) }; collector.stats.reset_blocks += reset.reset_blocks; if untraced { @@ -1686,6 +1737,7 @@ pub(super) fn run_copied_minor_attempt( if let Some(trace) = trace.as_mut() { trace.old_pages = crate::arena::old_page_summary(); } + let remembered_restore_phase_start = PhaseDiag::start(&phase_diag); remembered_set_clear(); collector.sticky.restore(); if !collector.skip_remembering { @@ -1694,6 +1746,12 @@ pub(super) fn run_copied_minor_attempt( // the last line before the kill is the answer. crate::arena::page_class_table_report(); } + if let Some(diag) = phase_diag.as_mut() { + diag.record( + Phase::RememberedSetYoungLogs, + remembered_restore_phase_start, + ); + } // The mechanism, counted rather than assumed: with the pre-size working, // `capacity` is already >= `len` on entry and hashbrown never grows the // table, so `reserve_rehash` disappears from this path. A capacity that @@ -1884,11 +1942,27 @@ pub(super) fn run_copied_minor_attempt( // This is intentionally the last diagnostic action before returning to // the mutator: `pause_us` prices the whole copied-minor path, including // finalization, pruning, policy feedback and the diagnostic work above. - let pause_us = start.elapsed().as_micros() as u64; + let pause_ns = start.elapsed().as_nanos() as u64; + let pause_us = pause_ns / 1000; + let phases = phase_diag + .as_ref() + .expect("PERRY_GC_DIAG phase accounting must be enabled") + .render( + pause_ns, + scan_us, + collector.stats.copied_objects, + collector.stats.copied_bytes, + collector.stats.promoted_objects, + collector.stats.promoted_bytes, + remembered_entries, + remembered_slots, + &finalization, + ); eprintln!( - "[gc-copy-minor] ran pause_us={} scan_us={} in_place={} untraced={} untraced_cycles={} untraced_objects={} in_place_blocks={} in_place_dead_bytes={} sparse_blocks={} survival_permille={} copied_objects={} copied_bytes={} promoted_objects={} promoted_bytes={} freed_bytes={} tenuring_survivals={} eden_live_bytes={} trigger={:?} declared_safepoint={}", + "[gc-copy-minor] ran pause_us={} scan_us={} phases: {} in_place={} untraced={} untraced_cycles={} untraced_objects={} in_place_blocks={} in_place_dead_bytes={} sparse_blocks={} survival_permille={} copied_objects={} copied_bytes={} promoted_objects={} promoted_bytes={} freed_bytes={} tenuring_survivals={} eden_live_bytes={} trigger={:?} declared_safepoint={}", pause_us, scan_us, + phases, collector.stats.in_place_promotion, untraced, super::untraced_promotion_cycles(), @@ -1913,14 +1987,3 @@ pub(super) fn run_copied_minor_attempt( malloc_swept: malloc_sweep_due, })) } - -fn finalize_dead_copied_minor_from_space_side_allocations() { - crate::map::finalize_dead_copied_minor_from_space_maps(); - crate::set::finalize_dead_copied_minor_from_space_sets(); - crate::node_submodules::diagnostics_gc::finalize_dead_copied_minor_from_space_errors(); - crate::regex::finalize_dead_copied_minor_from_space_regexps(); - // 2026-07-09 GC audit wave 2: the from-space flip runs no per-object - // finalize hooks, so entries keyed by dead from-space owners in the - // object-address-keyed side tables are pruned here (headers still intact). - super::dead_owner::prune_dead_owner_side_tables_copied_minor(); -} diff --git a/crates/perry-runtime/src/gc/copying_phase.rs b/crates/perry-runtime/src/gc/copying_phase.rs new file mode 100644 index 0000000000..5c693f7005 --- /dev/null +++ b/crates/perry-runtime/src/gc/copying_phase.rs @@ -0,0 +1,204 @@ +//! Diagnostic-only phase accounting for the copying minor. +//! +//! The collector's phases are not all contiguous: registered roots are walked +//! once to evacuate and once to repair forwarding addresses, and in-place +//! promotion has an early retag plus a late finish. The accumulator therefore +//! records non-overlapping spans into semantic buckets. Anything outside a +//! priced span is reported as `other`; top-level buckets plus `other` are an +//! exact partition of the same `Instant` interval used for `pause_us`. + +use std::fmt::Write; +use std::time::Instant; + +#[derive(Clone, Copy)] +pub(super) enum CopyingMinorPhase { + RootScan, + CopyEvacuation, + RememberedSetYoungLogs, + Promotion, + DeadOwnerSideTablePruning, + FromSpaceFinalization, + ForwardingFixups, + BlockResetFlip, +} + +#[derive(Default)] +pub(super) struct CopyingMinorPhaseDiag { + root_scan_ns: u64, + copy_evacuation_ns: u64, + remembered_set_young_logs_ns: u64, + promotion_ns: u64, + dead_owner_side_table_pruning_ns: u64, + from_space_finalization_ns: u64, + forwarding_fixups_ns: u64, + block_reset_flip_ns: u64, +} + +impl CopyingMinorPhaseDiag { + #[inline] + pub(super) fn enabled() -> Option { + super::gc_diag_enabled().then(Self::default) + } + + #[inline] + pub(super) fn start(diag: &Option) -> Option { + diag.as_ref().map(|_| Instant::now()) + } + + #[inline] + pub(super) fn record(&mut self, phase: CopyingMinorPhase, start: Option) { + let Some(start) = start else { + return; + }; + self.add_nanos(phase, start.elapsed().as_nanos() as u64); + } + + #[inline] + pub(super) fn add_nanos(&mut self, phase: CopyingMinorPhase, nanos: u64) { + let slot = match phase { + CopyingMinorPhase::RootScan => &mut self.root_scan_ns, + CopyingMinorPhase::CopyEvacuation => &mut self.copy_evacuation_ns, + CopyingMinorPhase::RememberedSetYoungLogs => &mut self.remembered_set_young_logs_ns, + CopyingMinorPhase::Promotion => &mut self.promotion_ns, + CopyingMinorPhase::DeadOwnerSideTablePruning => { + &mut self.dead_owner_side_table_pruning_ns + } + CopyingMinorPhase::FromSpaceFinalization => &mut self.from_space_finalization_ns, + CopyingMinorPhase::ForwardingFixups => &mut self.forwarding_fixups_ns, + CopyingMinorPhase::BlockResetFlip => &mut self.block_reset_flip_ns, + }; + *slot = slot.saturating_add(nanos); + } + + fn named_nanos(&self) -> u64 { + self.root_scan_ns + .saturating_add(self.copy_evacuation_ns) + .saturating_add(self.remembered_set_young_logs_ns) + .saturating_add(self.promotion_ns) + .saturating_add(self.dead_owner_side_table_pruning_ns) + .saturating_add(self.from_space_finalization_ns) + .saturating_add(self.forwarding_fixups_ns) + .saturating_add(self.block_reset_flip_ns) + } + + pub(super) fn render( + &self, + pause_ns: u64, + scan_us: u64, + copied_objects: usize, + copied_bytes: usize, + promoted_objects: usize, + promoted_bytes: usize, + remembered_entries: usize, + remembered_slots: usize, + finalization: &CopiedMinorFinalizationDiag, + ) -> String { + let named_ns = self.named_nanos(); + let other_ns = pause_ns.saturating_sub(named_ns); + let phase_sum_ns = named_ns.saturating_add(other_ns); + let mut out = String::new(); + write!( + out, + "root_scan={}/{} copy_evacuation={}/{}/{} remembered_set_young_logs={}/{}/{} promotion={}/{}/{} dead_owner_side_table_pruning={}{} from_space_finalization={}/map:{}/{}+set:{}/{}+errors:{}/{}+regex:{}/{} forwarding_fixups={} block_reset_flip={} other={} phase_sum_us={}", + self.root_scan_ns / 1000, + scan_us, + self.copy_evacuation_ns / 1000, + copied_objects, + copied_bytes, + self.remembered_set_young_logs_ns / 1000, + remembered_entries, + remembered_slots, + self.promotion_ns / 1000, + promoted_objects, + promoted_bytes, + self.dead_owner_side_table_pruning_ns / 1000, + finalization.dead_owner_detail, + self.from_space_finalization_ns / 1000, + finalization.map_ns / 1000, + finalization.maps, + finalization.set_ns / 1000, + finalization.sets, + finalization.errors_ns / 1000, + finalization.errors, + finalization.regex_ns / 1000, + finalization.regexps, + self.forwarding_fixups_ns / 1000, + self.block_reset_flip_ns / 1000, + other_ns / 1000, + phase_sum_ns / 1000, + ) + .expect("writing phase diagnostics to a String cannot fail"); + out + } +} + +#[derive(Default)] +pub(super) struct CopiedMinorFinalizationDiag { + pub(super) total_ns: u64, + pub(super) map_ns: u64, + pub(super) maps: usize, + pub(super) set_ns: u64, + pub(super) sets: usize, + pub(super) errors_ns: u64, + pub(super) errors: usize, + pub(super) regex_ns: u64, + pub(super) regexps: usize, + pub(super) dead_owner_ns: u64, + pub(super) dead_owner_detail: String, +} + +/// Finalize the side allocations whose from-space owners just died. The +/// clocks live here, beside the calls they price; without `PERRY_GC_DIAG` this +/// performs the original calls without reading the clock or building strings. +pub(super) fn finalize_dead_copied_minor_from_space_side_allocations() -> CopiedMinorFinalizationDiag +{ + let diag = super::gc_diag_enabled(); + let total_start = diag.then(Instant::now); + let mut out = CopiedMinorFinalizationDiag::default(); + + crate::promise::cleanup_copied_minor_promise_contexts_for_gc(); + + let start = diag.then(Instant::now); + out.maps = crate::map::finalize_dead_copied_minor_from_space_maps(); + out.map_ns = start.map_or(0, |start| start.elapsed().as_nanos() as u64); + + let start = diag.then(Instant::now); + out.sets = crate::set::finalize_dead_copied_minor_from_space_sets(); + out.set_ns = start.map_or(0, |start| start.elapsed().as_nanos() as u64); + + let start = diag.then(Instant::now); + out.errors = + crate::node_submodules::diagnostics_gc::finalize_dead_copied_minor_from_space_errors(); + out.errors_ns = start.map_or(0, |start| start.elapsed().as_nanos() as u64); + + let start = diag.then(Instant::now); + out.regexps = crate::regex::finalize_dead_copied_minor_from_space_regexps(); + out.regex_ns = start.map_or(0, |start| start.elapsed().as_nanos() as u64); + + let start = diag.then(Instant::now); + out.dead_owner_detail = super::dead_owner::prune_dead_owner_side_tables_copied_minor(); + out.dead_owner_ns = start.map_or(0, |start| start.elapsed().as_nanos() as u64); + out.total_ns = total_start.map_or(0, |start| start.elapsed().as_nanos() as u64); + out +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn copied_minor_phase_residual_makes_the_partition_exact() { + let mut diag = CopyingMinorPhaseDiag::default(); + diag.add_nanos(CopyingMinorPhase::RootScan, 11_000); + diag.add_nanos(CopyingMinorPhase::CopyEvacuation, 7_000); + diag.add_nanos(CopyingMinorPhase::BlockResetFlip, 3_000); + + let pause_ns: u64 = 29_000; + let other_ns = pause_ns.saturating_sub(diag.named_nanos()); + assert_eq!(diag.named_nanos() + other_ns, pause_ns); + assert_eq!(other_ns, 8_000); + // Sabotage: remove one named bucket from `named_nanos`; this exact + // residual assertion changes and the test fails rather than merely + // checking that phase reporting did not panic. + } +} diff --git a/crates/perry-runtime/src/gc/dead_owner.rs b/crates/perry-runtime/src/gc/dead_owner.rs index 33e2884256..2eeff26bbe 100644 --- a/crates/perry-runtime/src/gc/dead_owner.rs +++ b/crates/perry-runtime/src/gc/dead_owner.rs @@ -241,6 +241,7 @@ pub(super) fn prune_dead_owner_side_tables_post_trace( &|addr| probe.owner_is_dead(addr, Some(GC_TYPE_CLOSURE)), &|addr| probe.owner_is_dead(addr, Some(GC_TYPE_STRING)), /* young_only = */ !full_trace, + None, ); // #6182: drop dead weak-target HOLDERS (WeakRef / FinalizationRegistry / // WeakMap-WeakSet entry — all GC_TYPE_OBJECT) from the registry so the @@ -257,13 +258,23 @@ pub(super) fn prune_dead_owner_side_tables_post_trace( /// Copied-minor fan-out: prune entries owned by dead from-space objects /// before the flip destroys their headers. Nursery-only by construction, so /// the tenured/malloc caveat cannot mis-fire here. -pub(super) fn prune_dead_owner_side_tables_copied_minor() { +pub(super) fn prune_dead_owner_side_tables_copied_minor() -> String { + let mut detail = String::new(); + let diag = super::gc_diag_enabled(); + if diag { + detail.push('['); + } fan_out( &|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, + diag.then_some(&mut detail), ); + if diag { + detail.push(']'); + } + detail } /// Which of the pass's three deadness predicates a registered prune is handed. @@ -502,6 +513,7 @@ fn fan_out( is_dead_closure: &dyn Fn(usize) -> bool, is_dead_symbol: &dyn Fn(usize) -> bool, young_only: bool, + mut diag: Option<&mut String>, ) { // Interned key pointers cached in the store-plan cache may die in this // collection — flush every cached verdict. Pointer identity only: the @@ -515,9 +527,15 @@ fn fan_out( DeadKeyOwner::Closure => is_dead_closure, DeadKeyOwner::Symbol => is_dead_symbol, }; + let start = diag.as_ref().map(|_| std::time::Instant::now()); match entry.young_prune { Some(young_prune) if young_only => young_prune(is_dead), _ => (entry.prune)(is_dead), } + if let (Some(detail), Some(start)) = (diag.as_deref_mut(), start) { + use std::fmt::Write; + write!(detail, " {:?}:{}", entry.table, start.elapsed().as_micros()) + .expect("writing dead-owner diagnostics to a String cannot fail"); + } } } diff --git a/crates/perry-runtime/src/gc/mod.rs b/crates/perry-runtime/src/gc/mod.rs index 64409430cb..47f60769ee 100644 --- a/crates/perry-runtime/src/gc/mod.rs +++ b/crates/perry-runtime/src/gc/mod.rs @@ -157,6 +157,7 @@ mod prefetch; mod copying; mod copying_first_cycle; +mod copying_phase; mod copying_pointer_set; mod diag_sites; pub(crate) use diag_sites::primitive_dispatch as diag_primitive_dispatch; diff --git a/crates/perry-runtime/src/node_submodules/diagnostics_gc.rs b/crates/perry-runtime/src/node_submodules/diagnostics_gc.rs index 8b68e16e92..511a1999ca 100644 --- a/crates/perry-runtime/src/node_submodules/diagnostics_gc.rs +++ b/crates/perry-runtime/src/node_submodules/diagnostics_gc.rs @@ -46,7 +46,7 @@ pub(crate) fn error_side_tables_clear_dead(user_ptr: usize) { /// key is a dead from-space error — unmarked, unforwarded, nursery-space, /// still typed `GC_TYPE_ERROR`. Mirrors /// `finalize_dead_copied_minor_from_space_maps`. -pub(crate) fn finalize_dead_copied_minor_from_space_errors() { +pub(crate) fn finalize_dead_copied_minor_from_space_errors() -> usize { fn is_dead_from_space_error(addr: usize) -> bool { let space = crate::arena::classify_heap_space(addr); if !matches!(space, crate::arena::HeapSpace::NurseryEden) @@ -73,7 +73,9 @@ pub(crate) fn finalize_dead_copied_minor_from_space_errors() { .filter(|addr| is_dead_from_space_error(*addr)) .collect() }); + let count = dead.len(); for addr in dead { error_side_tables_clear_dead(addr); } + count } From dae5192966b6f41ffbdc3ad655a3e1ae931c7b65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 11:57:41 +0200 Subject: [PATCH 04/11] perf(gc): log young roots for remaining minor scanners Make descriptor, closure metadata, template, array named-property, and symbol side-table minor scans proportional to entries that can still move, die, or expose a young strong edge. Keep full scans authoritative and use test-only re-derivation sabotage checks to enforce every write funnel. Narrow descriptor and closure owner retention to collectible metadata keys; long-lived values remain logged only where their transitive edges require it. --- crates/perry-runtime/src/array/header.rs | 64 +-- .../src/array/header/young_roots.rs | 302 +++++++++++++ .../src/closure/dynamic_props.rs | 46 +- .../src/object/descriptor_state.rs | 48 +- .../src/object/descriptor_state/gc_scan.rs | 4 +- .../src/object/descriptor_state/young.rs | 8 +- .../object/native_module/callable_exports.rs | 103 +---- .../builtin_closure_metadata.rs | 201 +++++++++ crates/perry-runtime/src/symbol.rs | 3 + crates/perry-runtime/src/symbol/accessors.rs | 47 +- crates/perry-runtime/src/symbol/gc_roots.rs | 411 ++++++++++++++++-- crates/perry-runtime/src/symbol/properties.rs | 1 + 12 files changed, 1027 insertions(+), 211 deletions(-) create mode 100644 crates/perry-runtime/src/array/header/young_roots.rs create mode 100644 crates/perry-runtime/src/object/native_module/callable_exports/builtin_closure_metadata.rs diff --git a/crates/perry-runtime/src/array/header.rs b/crates/perry-runtime/src/array/header.rs index ec23d3aa85..63e1e059b5 100644 --- a/crates/perry-runtime/src/array/header.rs +++ b/crates/perry-runtime/src/array/header.rs @@ -4,6 +4,10 @@ pub(crate) use super::header_gc_slots::*; +mod young_roots; +pub use young_roots::scan_template_raw_roots_mut; +use young_roots::{note_array_named, note_template_cache, note_template_raw}; + use std::cell::RefCell; use std::collections::HashMap; @@ -184,6 +188,7 @@ unsafe fn register_template_raw_pair(cooked: *mut ArrayHeader, raw: *mut ArrayHe if cooked.is_null() || raw.is_null() { return; } + note_template_raw(cooked as usize, raw); TEMPLATE_RAW_MAP.with(|m| { m.borrow_mut().insert(cooked as usize, raw); }); @@ -254,6 +259,7 @@ pub extern "C" fn js_tagged_template_get_or_init( mark_template_array_frozen(raw); mark_template_array_frozen(cooked); register_template_raw_pair(cooked, raw); + note_template_cache(site_id, cooked, raw); TEMPLATE_OBJECT_CACHE.with(|m| { m.borrow_mut().insert(site_id, (cooked, raw)); }); @@ -294,33 +300,6 @@ pub fn scan_template_raw_roots(mark: &mut dyn FnMut(f64)) { scan_template_raw_roots_mut(&mut visitor); } -pub fn scan_template_raw_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { - TEMPLATE_OBJECT_CACHE.with(|m| { - let mut map = m.borrow_mut(); - for (_, (cooked_ptr, raw_ptr)) in map.iter_mut() { - visitor.visit_raw_mut_ptr_slot(cooked_ptr); - visitor.visit_raw_mut_ptr_slot(raw_ptr); - } - }); - TEMPLATE_RAW_MAP.with(|m| { - let mut map = m.borrow_mut(); - let mut moved = Vec::new(); - for (&cooked_addr, raw_ptr) in map.iter_mut() { - let mut new_cooked_addr = cooked_addr; - if visitor.visit_usize_slot(&mut new_cooked_addr) { - moved.push((cooked_addr, new_cooked_addr)); - } - visitor.visit_raw_mut_ptr_slot(raw_ptr); - } - for (old_addr, new_addr) in moved { - if let Some(raw_ptr) = map.remove(&old_addr) { - map.insert(new_addr, raw_ptr); - } - } - }); - scan_array_named_property_roots_mut(visitor); -} - fn barrier_array_named_props(owner: usize, props: &mut [ArrayNamedProperty]) { for prop in props.iter_mut() { crate::gc::runtime_write_barrier_external_slot( @@ -363,28 +342,10 @@ pub(crate) fn transfer_array_named_property_owner(old_owner: usize, new_owner: u ARRAY_NAMED_PROPS.with(|m| { let mut props = m.borrow_mut(); if let Some(old_props) = props.remove(&old_owner) { - merge_array_named_props(&mut props, new_owner, old_props); - } - }); -} - -pub(crate) fn scan_array_named_property_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { - ARRAY_NAMED_PROPS.with(|m| { - let mut props = m.borrow_mut(); - let mut moved = Vec::new(); - for (&owner, owner_props) in props.iter_mut() { - let mut new_owner = owner; - if visitor.visit_metadata_usize_slot(&mut new_owner) { - moved.push((owner, new_owner)); - } - for prop in owner_props.iter_mut() { - visitor.visit_nanbox_f64_slot(&mut prop.value); - } - } - for (old_owner, new_owner) in moved { - if let Some(old_props) = props.remove(&old_owner) { - merge_array_named_props(&mut props, new_owner, old_props); + for prop in &old_props { + note_array_named(new_owner, prop.value.to_bits()); } + merge_array_named_props(&mut props, new_owner, old_props); } }); } @@ -405,6 +366,7 @@ pub(crate) fn test_array_named_property_owner_exists(owner: usize) -> bool { #[cfg(test)] pub(crate) fn test_clear_array_named_property_roots() { ARRAY_NAMED_PROPS.with(|m| m.borrow_mut().clear()); + young_roots::clear_named_log(); } unsafe fn string_header_as_str<'a>(key: *const crate::StringHeader) -> Option<&'a str> { @@ -445,6 +407,7 @@ pub(crate) unsafe fn array_named_property_set( }; let owner = arr as usize; note_array_named_props_ever(); + note_array_named(owner, value.to_bits()); ARRAY_NAMED_PROPS.with(|m| { let mut map = m.borrow_mut(); let props = map.entry(owner).or_default(); @@ -478,6 +441,10 @@ pub(crate) unsafe fn array_named_props_install_fresh( return; } let owner = arr as usize; + note_array_named_props_ever(); + for (_, value) in entries { + note_array_named(owner, value.to_bits()); + } ARRAY_NAMED_PROPS.with(|m| { let mut map = m.borrow_mut(); let props = map.entry(owner).or_default(); @@ -634,6 +601,7 @@ pub(crate) unsafe fn array_named_property_delete_by_name( #[cfg(test)] pub(crate) fn test_seed_template_raw_roots(cooked: *mut ArrayHeader, raw: *mut ArrayHeader) { + note_template_raw(cooked as usize, raw); TEMPLATE_RAW_MAP.with(|m| { let mut m = m.borrow_mut(); m.clear(); diff --git a/crates/perry-runtime/src/array/header/young_roots.rs b/crates/perry-runtime/src/array/header/young_roots.rs new file mode 100644 index 0000000000..946e428c0f --- /dev/null +++ b/crates/perry-runtime/src/array/header/young_roots.rs @@ -0,0 +1,302 @@ +//! Young-entry logs for tagged-template and array named-property roots. + +use super::*; + +crate::perry_thread_local! { + static TEMPLATE_CACHE_YOUNG: RefCell> = + const { RefCell::new(crate::gc::young_log::YoungLog::new()) }; + static TEMPLATE_RAW_YOUNG: RefCell> = + const { RefCell::new(crate::gc::young_log::YoungLog::new()) }; + static ARRAY_NAMED_YOUNG: RefCell> = + const { RefCell::new(crate::gc::young_log::YoungLog::new()) }; + #[cfg(test)] + static TEST_SUPPRESS_ARRAY_NAMED_YOUNG_NOTE: std::cell::Cell = + const { std::cell::Cell::new(false) }; + #[cfg(test)] + static TEST_SUPPRESS_TEMPLATE_RAW_YOUNG_NOTE: std::cell::Cell = + const { std::cell::Cell::new(false) }; +} + +const CACHE_LOG: &str = "array.template_object_cache"; +const RAW_LOG: &str = "array.template_raw_map"; +const NAMED_LOG: &str = "array.named_properties"; + +#[inline] +fn ptr_relevant(ptr: *mut ArrayHeader) -> bool { + crate::gc::young_log::addr_is_minor_relevant(ptr as usize) +} + +pub(super) fn note_template_cache(site: u64, cooked: *mut ArrayHeader, raw: *mut ArrayHeader) { + if ptr_relevant(cooked) || ptr_relevant(raw) { + TEMPLATE_CACHE_YOUNG.with(|log| log.borrow_mut().note(site)); + } +} + +pub(super) fn note_template_raw(cooked: usize, raw: *mut ArrayHeader) { + if crate::gc::young_log::addr_is_minor_relevant(cooked) || ptr_relevant(raw) { + #[cfg(test)] + if TEST_SUPPRESS_TEMPLATE_RAW_YOUNG_NOTE.with(std::cell::Cell::get) { + return; + } + TEMPLATE_RAW_YOUNG.with(|log| log.borrow_mut().note(cooked)); + } +} + +pub(super) fn note_array_named(owner: usize, value_bits: u64) { + if !crate::gc::young_log::addr_is_minor_collectible(owner) + && !crate::gc::young_log::bits_are_minor_relevant(value_bits) + { + return; + } + #[cfg(test)] + if TEST_SUPPRESS_ARRAY_NAMED_YOUNG_NOTE.with(std::cell::Cell::get) { + return; + } + ARRAY_NAMED_YOUNG.with(|log| log.borrow_mut().note(owner)); +} + +fn visit_cache_site(visitor: &mut crate::gc::RuntimeRootVisitor<'_>, site: u64) -> bool { + TEMPLATE_OBJECT_CACHE.with(|m| { + let mut map = m.borrow_mut(); + let Some((cooked, raw)) = map.get_mut(&site) else { + return false; + }; + visitor.visit_raw_mut_ptr_slot(cooked); + visitor.visit_raw_mut_ptr_slot(raw); + ptr_relevant(*cooked) || ptr_relevant(*raw) + }) +} + +fn visit_raw_owner(visitor: &mut crate::gc::RuntimeRootVisitor<'_>, owner: usize) -> Option { + TEMPLATE_RAW_MAP.with(|m| { + let mut map = m.borrow_mut(); + let mut raw = map.remove(&owner)?; + let mut new_owner = owner; + visitor.visit_usize_slot(&mut new_owner); + visitor.visit_raw_mut_ptr_slot(&mut raw); + map.insert(new_owner, raw); + (crate::gc::young_log::addr_is_minor_relevant(new_owner) || ptr_relevant(raw)) + .then_some(new_owner) + }) +} + +fn visit_named_owner( + visitor: &mut crate::gc::RuntimeRootVisitor<'_>, + owner: usize, +) -> Option { + ARRAY_NAMED_PROPS.with(|m| { + let mut map = m.borrow_mut(); + let mut props = map.remove(&owner)?; + let mut new_owner = owner; + visitor.visit_metadata_usize_slot(&mut new_owner); + let mut relevant = crate::gc::young_log::addr_is_minor_collectible(new_owner); + for prop in &mut props { + visitor.visit_nanbox_f64_slot(&mut prop.value); + relevant |= crate::gc::young_log::bits_are_minor_relevant(prop.value.to_bits()); + } + merge_array_named_props(&mut map, new_owner, props); + relevant.then_some(new_owner) + }) +} + +#[cfg(any(debug_assertions, test))] +fn relevant_cache_sites() -> Vec { + TEMPLATE_OBJECT_CACHE.with(|m| { + m.borrow() + .iter() + .filter_map(|(&site, &(cooked, raw))| { + (ptr_relevant(cooked) || ptr_relevant(raw)).then_some(site) + }) + .collect() + }) +} + +#[cfg(any(debug_assertions, test))] +fn relevant_raw_owners() -> Vec { + TEMPLATE_RAW_MAP.with(|m| { + m.borrow() + .iter() + .filter_map(|(&owner, &raw)| { + (crate::gc::young_log::addr_is_minor_relevant(owner) || ptr_relevant(raw)) + .then_some(owner) + }) + .collect() + }) +} + +#[cfg(any(debug_assertions, test))] +fn relevant_named_owners() -> Vec { + ARRAY_NAMED_PROPS.with(|m| { + m.borrow() + .iter() + .filter_map(|(&owner, props)| { + (crate::gc::young_log::addr_is_minor_collectible(owner) + || props.iter().any(|prop| { + crate::gc::young_log::bits_are_minor_relevant(prop.value.to_bits()) + })) + .then_some(owner) + }) + .collect() + }) +} + +fn drain_log( + log: &'static crate::tls_hot::HotKey>>, + mut visit: impl FnMut(K) -> Option, +) -> (u64, u64, u64) { + let mut logged = 0; + let mut visited = 0; + let mut kept = log.with(|log| log.borrow_mut().take_spare()); + loop { + let batch = log.with(|log| log.borrow_mut().take_sorted()); + if batch.is_empty() { + break; + } + logged += batch.len() as u64; + for key in batch { + visited += 1; + if let Some(key) = visit(key) { + kept.push(key); + } + } + } + let kept_len = kept.len() as u64; + log.with(|log| log.borrow_mut().extend(kept)); + (logged, visited, kept_len) +} + +fn report(name: &'static str, partial: bool, row: (u64, u64, u64), table_len: usize) { + crate::gc::young_log::note_walk( + name, + crate::gc::young_log::YoungLogWalk { + partial, + logged: row.0, + visited: row.1, + kept: row.2, + table_len: table_len as u64, + }, + ); +} + +pub fn scan_template_raw_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { + let cache_len = TEMPLATE_OBJECT_CACHE.with(|m| m.borrow().len()); + let raw_len = TEMPLATE_RAW_MAP.with(|m| m.borrow().len()); + let named_len = ARRAY_NAMED_PROPS.with(|m| m.borrow().len()); + if visitor.young_scope() { + #[cfg(any(debug_assertions, test))] + { + TEMPLATE_CACHE_YOUNG.with(|log| { + log.borrow() + .debug_assert_logged(CACHE_LOG, &relevant_cache_sites()) + }); + TEMPLATE_RAW_YOUNG.with(|log| { + log.borrow() + .debug_assert_logged(RAW_LOG, &relevant_raw_owners()) + }); + ARRAY_NAMED_YOUNG.with(|log| { + log.borrow() + .debug_assert_logged(NAMED_LOG, &relevant_named_owners()) + }); + } + let cache = drain_log(&TEMPLATE_CACHE_YOUNG, |site| { + visit_cache_site(visitor, site).then_some(site) + }); + let raw = drain_log(&TEMPLATE_RAW_YOUNG, |owner| visit_raw_owner(visitor, owner)); + let named = drain_log(&ARRAY_NAMED_YOUNG, |owner| { + visit_named_owner(visitor, owner) + }); + report(CACHE_LOG, true, cache, cache_len); + report(RAW_LOG, true, raw, raw_len); + report(NAMED_LOG, true, named, named_len); + return; + } + + let cache_sites: Vec = + TEMPLATE_OBJECT_CACHE.with(|m| m.borrow().keys().copied().collect()); + let raw_owners: Vec = TEMPLATE_RAW_MAP.with(|m| m.borrow().keys().copied().collect()); + let named_owners: Vec = ARRAY_NAMED_PROPS.with(|m| m.borrow().keys().copied().collect()); + let _ = TEMPLATE_CACHE_YOUNG.with(|log| log.borrow_mut().take_sorted()); + let _ = TEMPLATE_RAW_YOUNG.with(|log| log.borrow_mut().take_sorted()); + let _ = ARRAY_NAMED_YOUNG.with(|log| log.borrow_mut().take_sorted()); + // `drain_log` consumes the log, so seed it with the authoritative keys. + TEMPLATE_CACHE_YOUNG.with(|log| log.borrow_mut().extend(cache_sites)); + let cache = drain_log(&TEMPLATE_CACHE_YOUNG, |site| { + visit_cache_site(visitor, site).then_some(site) + }); + TEMPLATE_RAW_YOUNG.with(|log| log.borrow_mut().extend(raw_owners)); + let raw = drain_log(&TEMPLATE_RAW_YOUNG, |owner| visit_raw_owner(visitor, owner)); + ARRAY_NAMED_YOUNG.with(|log| log.borrow_mut().extend(named_owners)); + let named = drain_log(&ARRAY_NAMED_YOUNG, |owner| { + visit_named_owner(visitor, owner) + }); + report(CACHE_LOG, false, cache, cache_len); + report(RAW_LOG, false, raw, raw_len); + report(NAMED_LOG, false, named, named_len); +} + +#[cfg(test)] +pub(super) fn clear_named_log() { + ARRAY_NAMED_YOUNG.with(|log| log.borrow_mut().clear()); +} + +#[cfg(test)] +mod tests { + use super::*; + + fn alloc_empty_array() -> *mut ArrayHeader { + let arr = crate::arena::arena_alloc_gc( + std::mem::size_of::(), + std::mem::align_of::(), + crate::gc::GC_TYPE_ARRAY, + ) as *mut ArrayHeader; + unsafe { + (*arr).length = 0; + (*arr).capacity = 0; + } + arr + } + + #[test] + fn template_raw_log_rederivation_rejects_a_suppressed_writer() { + let _lock = crate::gc::global_side_table_test_lock(); + let cooked = alloc_empty_array(); + let raw = alloc_empty_array(); + TEST_SUPPRESS_TEMPLATE_RAW_YOUNG_NOTE.with(|flag| flag.set(true)); + test_seed_template_raw_roots(cooked, raw); + TEST_SUPPRESS_TEMPLATE_RAW_YOUNG_NOTE.with(|flag| flag.set(false)); + let missed = std::panic::catch_unwind(|| { + TEMPLATE_RAW_YOUNG.with(|log| { + log.borrow() + .debug_assert_logged(RAW_LOG, &relevant_raw_owners()) + }); + }); + TEMPLATE_RAW_MAP.with(|m| m.borrow_mut().clear()); + TEMPLATE_RAW_YOUNG.with(|log| log.borrow_mut().clear()); + assert!( + missed.is_err(), + "sabotage: suppressing the template-raw writer's note must trip completeness" + ); + } + + #[test] + fn array_named_log_rederivation_rejects_a_suppressed_setter() { + let _lock = crate::gc::global_side_table_test_lock(); + let arr = alloc_empty_array(); + let key = crate::string::js_string_from_bytes(b"sabotage".as_ptr(), 8); + TEST_SUPPRESS_ARRAY_NAMED_YOUNG_NOTE.with(|flag| flag.set(true)); + unsafe { array_named_property_set(arr, key, 7.0) }; + TEST_SUPPRESS_ARRAY_NAMED_YOUNG_NOTE.with(|flag| flag.set(false)); + let missed = std::panic::catch_unwind(|| { + ARRAY_NAMED_YOUNG.with(|log| { + log.borrow() + .debug_assert_logged(NAMED_LOG, &relevant_named_owners()) + }); + }); + ARRAY_NAMED_PROPS.with(|m| m.borrow_mut().remove(&(arr as usize))); + clear_named_log(); + assert!( + missed.is_err(), + "sabotage: suppressing array_named_property_set's note must trip completeness" + ); + } +} diff --git a/crates/perry-runtime/src/closure/dynamic_props.rs b/crates/perry-runtime/src/closure/dynamic_props.rs index d3ea542188..06e760bd7c 100644 --- a/crates/perry-runtime/src/closure/dynamic_props.rs +++ b/crates/perry-runtime/src/closure/dynamic_props.rs @@ -104,6 +104,9 @@ crate::perry_thread_local! { /// 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()) }; + #[cfg(test)] + static TEST_SUPPRESS_CLOSURE_YOUNG_NOTE: std::cell::Cell = + const { std::cell::Cell::new(false) }; } const CLOSURE_YOUNG_LOG_NAME: &str = "closure.dynamic_props"; @@ -112,13 +115,38 @@ const CLOSURE_YOUNG_LOG_NAME: &str = "closure.dynamic_props"; /// 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) + if crate::gc::young_log::addr_is_minor_collectible(owner) || crate::gc::young_log::bits_are_minor_relevant(value_bits) { + #[cfg(test)] + if TEST_SUPPRESS_CLOSURE_YOUNG_NOTE.with(std::cell::Cell::get) { + return; + } CLOSURE_YOUNG_OWNERS.with(|log| log.borrow_mut().note(owner)); } } +#[cfg(test)] +mod young_log_sabotage_tests { + use super::*; + + #[test] + fn closure_log_rederivation_rejects_a_suppressed_setter() { + let _lock = crate::gc::global_side_table_test_lock(); + test_clear_closure_side_tables(); + let owner = crate::closure::js_closure_alloc(std::ptr::null(), 0) as usize; + TEST_SUPPRESS_CLOSURE_YOUNG_NOTE.with(|flag| flag.set(true)); + closure_set_dynamic_prop(owner, "sabotage", 7.0); + TEST_SUPPRESS_CLOSURE_YOUNG_NOTE.with(|flag| flag.set(false)); + let missed = std::panic::catch_unwind(debug_assert_closure_young_log_complete); + test_clear_closure_side_tables(); + assert!( + missed.is_err(), + "sabotage: suppressing closure_set_dynamic_prop's note must trip completeness" + ); + } +} + /// 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. @@ -515,7 +543,7 @@ fn scan_closure_side_tables_young(visitor: &mut crate::gc::RuntimeRootVisitor<'_ .unwrap_or(0); (props + prototypes + deleted) as u64 }; - #[cfg(debug_assertions)] + #[cfg(any(debug_assertions, test))] debug_assert_closure_young_log_complete(); let mut logged = 0u64; let mut visited = 0u64; @@ -550,13 +578,13 @@ fn scan_closure_side_tables_young(visitor: &mut crate::gc::RuntimeRootVisitor<'_ /// 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)] +#[cfg(any(debug_assertions, test))] fn debug_assert_closure_young_log_complete() { - use crate::gc::young_log::{addr_is_minor_relevant, bits_are_minor_relevant}; + use crate::gc::young_log::{addr_is_minor_collectible, 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) + if addr_is_minor_collectible(owner) || entry .values .values() @@ -568,14 +596,14 @@ fn debug_assert_closure_young_log_complete() { } 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) { + if addr_is_minor_collectible(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) { + if addr_is_minor_collectible(owner) { relevant.push(owner); } } @@ -593,7 +621,7 @@ 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}; + use crate::gc::young_log::{addr_is_minor_collectible, bits_are_minor_relevant}; let mut relevant = false; let mut current_owner = owner; @@ -657,7 +685,7 @@ fn scan_closure_owner( } } - relevant |= addr_is_minor_relevant(current_owner); + relevant |= addr_is_minor_collectible(current_owner); (current_owner, relevant) } diff --git a/crates/perry-runtime/src/object/descriptor_state.rs b/crates/perry-runtime/src/object/descriptor_state.rs index 31d0a54ce4..21966ee59e 100644 --- a/crates/perry-runtime/src/object/descriptor_state.rs +++ b/crates/perry-runtime/src/object/descriptor_state.rs @@ -130,6 +130,11 @@ impl DescriptorTables { const DESCRIPTOR_YOUNG_LOG_NAME: &str = "object.descriptors"; +#[cfg(test)] +thread_local! { + static TEST_SUPPRESS_DESCRIPTOR_YOUNG_NOTE: Cell = const { Cell::new(false) }; +} + mod gc_scan; mod young; pub(crate) use gc_scan::{scan_descriptor_owner, scan_descriptor_roots_mut}; @@ -145,15 +150,54 @@ fn note_young_descriptor_owner( owner: usize, acc: Option<&AccessorDescriptor>, ) { - use crate::gc::young_log::{addr_is_minor_relevant, bits_are_minor_relevant}; - if addr_is_minor_relevant(owner) + use crate::gc::young_log::{addr_is_minor_collectible, bits_are_minor_relevant}; + if addr_is_minor_collectible(owner) || acc .is_some_and(|acc| bits_are_minor_relevant(acc.get) || bits_are_minor_relevant(acc.set)) { + #[cfg(test)] + if TEST_SUPPRESS_DESCRIPTOR_YOUNG_NOTE.with(Cell::get) { + return; + } st.descriptors.young_owners.borrow_mut().note(owner); } } +#[cfg(test)] +mod young_log_sabotage_tests { + use super::*; + + #[test] + fn descriptor_log_rederivation_rejects_a_suppressed_setter() { + let _lock = crate::gc::global_side_table_test_lock(); + let owner = crate::object::js_object_alloc(0, 0) as usize; + state().descriptors.young_owners.borrow_mut().clear(); + TEST_SUPPRESS_DESCRIPTOR_YOUNG_NOTE.with(|flag| flag.set(true)); + set_property_attrs( + owner, + "sabotage".to_string(), + PropertyAttrs::new(true, true, true), + ); + TEST_SUPPRESS_DESCRIPTOR_YOUNG_NOTE.with(|flag| flag.set(false)); + let missed = std::panic::catch_unwind(|| { + state() + .descriptors + .young_owners + .borrow() + .debug_assert_logged( + DESCRIPTOR_YOUNG_LOG_NAME, + &relevant_descriptor_owners(state()), + ); + }); + clear_property_attrs(owner, "sabotage"); + state().descriptors.young_owners.borrow_mut().clear(); + assert!( + missed.is_err(), + "sabotage: suppressing set_property_attrs' note must trip completeness" + ); + } +} + /// 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`. diff --git a/crates/perry-runtime/src/object/descriptor_state/gc_scan.rs b/crates/perry-runtime/src/object/descriptor_state/gc_scan.rs index fe0f22ca36..48471ca672 100644 --- a/crates/perry-runtime/src/object/descriptor_state/gc_scan.rs +++ b/crates/perry-runtime/src/object/descriptor_state/gc_scan.rs @@ -137,7 +137,7 @@ pub(crate) fn scan_descriptor_owner( st: &crate::state::RuntimeState, owner: usize, ) -> (usize, bool) { - use crate::gc::young_log::{addr_is_minor_relevant, bits_are_minor_relevant}; + use crate::gc::young_log::{addr_is_minor_collectible, bits_are_minor_relevant}; let new_owner = rewrite_descriptor_owner(visitor, owner); let mut relevant = false; let accessor_keys = st @@ -187,7 +187,7 @@ pub(crate) fn scan_descriptor_owner( 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); + relevant |= addr_is_minor_collectible(new_owner); (new_owner, relevant) } diff --git a/crates/perry-runtime/src/object/descriptor_state/young.rs b/crates/perry-runtime/src/object/descriptor_state/young.rs index 0133fc7549..631abbc548 100644 --- a/crates/perry-runtime/src/object/descriptor_state/young.rs +++ b/crates/perry-runtime/src/object/descriptor_state/young.rs @@ -13,15 +13,15 @@ use super::*; /// 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}; + use crate::gc::young_log::{addr_is_minor_collectible, 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) { + if addr_is_minor_collectible(owner) { relevant.push(owner); } } for &owner in st.descriptors.accessor_keys_by_owner.borrow().keys() { - if addr_is_minor_relevant(owner) { + if addr_is_minor_collectible(owner) { relevant.push(owner); } } @@ -45,7 +45,7 @@ pub(super) fn scan_descriptor_roots_young( ) { 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)] + #[cfg(any(debug_assertions, test))] { let relevant = relevant_descriptor_owners(st); st.descriptors diff --git a/crates/perry-runtime/src/object/native_module/callable_exports.rs b/crates/perry-runtime/src/object/native_module/callable_exports.rs index ea47d3ea81..431382981a 100644 --- a/crates/perry-runtime/src/object/native_module/callable_exports.rs +++ b/crates/perry-runtime/src/object/native_module/callable_exports.rs @@ -1,6 +1,8 @@ use super::callable_export_arity_table::native_callable_export_arity; use super::*; +mod builtin_closure_metadata; mod module_cjs; +pub(crate) use builtin_closure_metadata::*; use module_cjs::attach_module_cjs_constructor_statics; pub(crate) use module_cjs::{ module_builtin_modules_value, module_cjs_cache_value, module_cjs_extensions_value, @@ -1483,107 +1485,6 @@ pub(crate) fn set_bound_native_closure_name( ); } -thread_local! { - /// Per-closure spec `.length` for built-in *prototype methods*. Those - /// methods all share one no-op closure thunk - /// (`global_this_builtin_noop_thunk`), so the func-ptr-keyed - /// the closure body registry can't give `Array.prototype.map.length === 1` - /// while `Array.prototype.slice.length === 2` — the last install would - /// win for every method. Recording the length per *closure instance* here - /// (keyed by the closure pointer, like the user-facing dynamic-prop table - /// but isolated from it so a user `fn.length = x` write can't perturb it) - /// lets the `.length` value-read and `getOwnPropertyDescriptor` agree with - /// the spec count. #3143. - static BUILTIN_CLOSURE_LENGTH: std::cell::RefCell> = - std::cell::RefCell::new(crate::fast_hash::new_ptr_hash_map()); - - /// Built-in method closures are callable but lack ECMAScript - /// `[[Construct]]`. Track the installed closure values so the dynamic - /// `new` / `Reflect.construct` paths can reject them without changing - /// ordinary user closures or global constructor closures. - static BUILTIN_CLOSURE_NON_CONSTRUCTABLE: std::cell::RefCell> = - std::cell::RefCell::new(crate::fast_hash::new_ptr_hash_set()); -} - -/// Record the spec `.length` for a built-in prototype-method closure. See -/// [`BUILTIN_CLOSURE_LENGTH`]. -pub(crate) fn set_builtin_closure_length(closure: usize, length: u32) { - BUILTIN_CLOSURE_LENGTH.with(|m| { - m.borrow_mut().insert(closure, length); - }); -} - -/// Look up the recorded spec `.length` for a built-in prototype-method -/// closure, or `None` if this closure isn't one. See [`BUILTIN_CLOSURE_LENGTH`]. -pub(crate) fn builtin_closure_length(closure: usize) -> Option { - BUILTIN_CLOSURE_LENGTH.with(|m| m.borrow().get(&closure).copied()) -} - -pub(crate) fn set_builtin_closure_non_constructable(closure: usize) { - BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|m| { - m.borrow_mut().insert(closure); - }); -} - -pub(crate) fn builtin_closure_is_non_constructable(closure: usize) -> bool { - BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|m| m.borrow().contains(&closure)) -} - -/// Rekey per-instance built-in closure metadata after a moving collection. -/// -/// The keys are identities, not roots: prototype/global objects keep live -/// built-in closures reachable, while dead closures must remain collectable. -/// `visit_metadata_usize_slot` therefore only follows forwarding records. -pub(crate) fn scan_builtin_closure_metadata_roots_mut( - visitor: &mut crate::gc::RuntimeRootVisitor<'_>, -) { - BUILTIN_CLOSURE_LENGTH.with(|lengths| { - let mut lengths = lengths.borrow_mut(); - let mut moved = Vec::new(); - for old_owner in lengths.keys().copied() { - let mut new_owner = old_owner; - if visitor.visit_metadata_usize_slot(&mut new_owner) && new_owner != old_owner { - moved.push((old_owner, new_owner)); - } - } - for (old_owner, new_owner) in moved { - if let Some(length) = lengths.remove(&old_owner) { - lengths.insert(new_owner, length); - } - } - }); - - BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|non_constructable| { - let mut non_constructable = non_constructable.borrow_mut(); - let mut moved = Vec::new(); - for old_owner in non_constructable.iter().copied() { - let mut new_owner = old_owner; - if visitor.visit_metadata_usize_slot(&mut new_owner) && new_owner != old_owner { - moved.push((old_owner, new_owner)); - } - } - for (old_owner, new_owner) in moved { - non_constructable.remove(&old_owner); - non_constructable.insert(new_owner); - } - }); -} - -/// Drop metadata for closures proved dead by the collector before their arena -/// addresses can be recycled for unrelated objects. -pub(crate) fn prune_dead_builtin_closure_metadata_owners(is_dead_owner: &dyn Fn(usize) -> bool) { - BUILTIN_CLOSURE_LENGTH.with(|lengths| { - lengths - .borrow_mut() - .retain(|owner, _| !is_dead_owner(*owner)); - }); - BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|non_constructable| { - non_constructable - .borrow_mut() - .retain(|owner| !is_dead_owner(*owner)); - }); -} - pub(crate) fn builtin_closure_is_non_constructable_value(value: f64) -> bool { let jv = JSValue::from_bits(value.to_bits()); if !jv.is_pointer() { diff --git a/crates/perry-runtime/src/object/native_module/callable_exports/builtin_closure_metadata.rs b/crates/perry-runtime/src/object/native_module/callable_exports/builtin_closure_metadata.rs new file mode 100644 index 0000000000..fe1a2f75c6 --- /dev/null +++ b/crates/perry-runtime/src/object/native_module/callable_exports/builtin_closure_metadata.rs @@ -0,0 +1,201 @@ +//! Young-scoped GC maintenance for per-instance built-in closure metadata. + +thread_local! { + static BUILTIN_CLOSURE_LENGTH: std::cell::RefCell> = + std::cell::RefCell::new(crate::fast_hash::new_ptr_hash_map()); + static BUILTIN_CLOSURE_NON_CONSTRUCTABLE: std::cell::RefCell> = + std::cell::RefCell::new(crate::fast_hash::new_ptr_hash_set()); + static BUILTIN_CLOSURE_YOUNG: std::cell::RefCell> = + const { std::cell::RefCell::new(crate::gc::young_log::YoungLog::new()) }; + #[cfg(test)] + static TEST_SUPPRESS_BUILTIN_CLOSURE_YOUNG_NOTE: std::cell::Cell = + const { std::cell::Cell::new(false) }; +} + +const LOG_NAME: &str = "object.builtin_closure_metadata"; + +#[inline] +fn note(closure: usize) { + if !crate::gc::young_log::addr_is_minor_collectible(closure) { + return; + } + #[cfg(test)] + if TEST_SUPPRESS_BUILTIN_CLOSURE_YOUNG_NOTE.with(std::cell::Cell::get) { + return; + } + BUILTIN_CLOSURE_YOUNG.with(|log| log.borrow_mut().note(closure)); +} + +pub(crate) fn set_builtin_closure_length(closure: usize, length: u32) { + note(closure); + BUILTIN_CLOSURE_LENGTH.with(|m| { + m.borrow_mut().insert(closure, length); + }); +} + +pub(crate) fn builtin_closure_length(closure: usize) -> Option { + BUILTIN_CLOSURE_LENGTH.with(|m| m.borrow().get(&closure).copied()) +} + +pub(crate) fn set_builtin_closure_non_constructable(closure: usize) { + note(closure); + BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|m| { + m.borrow_mut().insert(closure); + }); +} + +pub(crate) fn builtin_closure_is_non_constructable(closure: usize) -> bool { + BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|m| m.borrow().contains(&closure)) +} + +#[cfg(any(debug_assertions, test))] +fn relevant_owners() -> Vec { + let mut owners = Vec::new(); + BUILTIN_CLOSURE_LENGTH.with(|m| { + owners.extend( + m.borrow() + .keys() + .copied() + .filter(|owner| crate::gc::young_log::addr_is_minor_collectible(*owner)), + ); + }); + BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|m| { + owners.extend( + m.borrow() + .iter() + .copied() + .filter(|owner| crate::gc::young_log::addr_is_minor_collectible(*owner)), + ); + }); + owners.sort_unstable(); + owners.dedup(); + owners +} + +fn visit_owner(visitor: &mut crate::gc::RuntimeRootVisitor<'_>, owner: usize) -> Option { + let mut new_owner = owner; + visitor.visit_metadata_usize_slot(&mut new_owner); + BUILTIN_CLOSURE_LENGTH.with(|lengths| { + let mut lengths = lengths.borrow_mut(); + if new_owner != owner { + if let Some(length) = lengths.remove(&owner) { + lengths.insert(new_owner, length); + } + } + }); + BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|set| { + let mut set = set.borrow_mut(); + if new_owner != owner && set.remove(&owner) { + set.insert(new_owner); + } + }); + crate::gc::young_log::addr_is_minor_collectible(new_owner).then_some(new_owner) +} + +pub(crate) fn scan_builtin_closure_metadata_roots_mut( + visitor: &mut crate::gc::RuntimeRootVisitor<'_>, +) { + let table_len = BUILTIN_CLOSURE_LENGTH.with(|m| m.borrow().len()) + + BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|m| m.borrow().len()); + if visitor.young_scope() { + #[cfg(any(debug_assertions, test))] + BUILTIN_CLOSURE_YOUNG.with(|log| { + log.borrow() + .debug_assert_logged(LOG_NAME, &relevant_owners()) + }); + let mut logged = 0u64; + let mut visited = 0u64; + let mut kept = BUILTIN_CLOSURE_YOUNG.with(|log| log.borrow_mut().take_spare()); + loop { + let batch = BUILTIN_CLOSURE_YOUNG.with(|log| log.borrow_mut().take_sorted()); + if batch.is_empty() { + break; + } + logged += batch.len() as u64; + for owner in batch { + visited += 1; + if let Some(owner) = visit_owner(visitor, owner) { + kept.push(owner); + } + } + } + let kept_len = kept.len() as u64; + BUILTIN_CLOSURE_YOUNG.with(|log| log.borrow_mut().extend(kept)); + crate::gc::young_log::note_walk( + LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: true, + logged, + visited, + kept: kept_len, + table_len: table_len as u64, + }, + ); + return; + } + + let mut owners = Vec::new(); + BUILTIN_CLOSURE_LENGTH.with(|m| owners.extend(m.borrow().keys().copied())); + BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|m| owners.extend(m.borrow().iter().copied())); + owners.sort_unstable(); + owners.dedup(); + let visited = owners.len() as u64; + let _ = BUILTIN_CLOSURE_YOUNG.with(|log| log.borrow_mut().take_sorted()); + let mut kept = BUILTIN_CLOSURE_YOUNG.with(|log| log.borrow_mut().take_spare()); + for owner in owners { + if let Some(owner) = visit_owner(visitor, owner) { + kept.push(owner); + } + } + let kept_len = kept.len() as u64; + BUILTIN_CLOSURE_YOUNG.with(|log| log.borrow_mut().extend(kept)); + crate::gc::young_log::note_walk( + LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: false, + logged: visited, + visited, + kept: kept_len, + table_len: table_len as u64, + }, + ); +} + +pub(crate) fn prune_dead_builtin_closure_metadata_owners(is_dead_owner: &dyn Fn(usize) -> bool) { + BUILTIN_CLOSURE_LENGTH.with(|lengths| { + lengths + .borrow_mut() + .retain(|owner, _| !is_dead_owner(*owner)); + }); + BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|non_constructable| { + non_constructable + .borrow_mut() + .retain(|owner| !is_dead_owner(*owner)); + }); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn builtin_closure_log_rederivation_rejects_a_suppressed_writer() { + let _lock = crate::gc::global_side_table_test_lock(); + let closure = crate::closure::js_closure_alloc(std::ptr::null(), 0) as usize; + TEST_SUPPRESS_BUILTIN_CLOSURE_YOUNG_NOTE.with(|flag| flag.set(true)); + set_builtin_closure_length(closure, 3); + TEST_SUPPRESS_BUILTIN_CLOSURE_YOUNG_NOTE.with(|flag| flag.set(false)); + let missed = std::panic::catch_unwind(|| { + BUILTIN_CLOSURE_YOUNG.with(|log| { + log.borrow() + .debug_assert_logged(LOG_NAME, &relevant_owners()) + }); + }); + BUILTIN_CLOSURE_LENGTH.with(|m| m.borrow_mut().remove(&closure)); + BUILTIN_CLOSURE_YOUNG.with(|log| log.borrow_mut().clear()); + assert!( + missed.is_err(), + "sabotage: suppressing the setter's note must trip completeness" + ); + } +} diff --git a/crates/perry-runtime/src/symbol.rs b/crates/perry-runtime/src/symbol.rs index 3abb55867f..57e17c12ee 100644 --- a/crates/perry-runtime/src/symbol.rs +++ b/crates/perry-runtime/src/symbol.rs @@ -579,6 +579,7 @@ pub(crate) fn register_symbol_pointer(ptr: usize) { SYMBOL_EVER_REGISTERED.arm(); // Admit before the insert, for the same reason. admit_symbol_pointer(ptr); + gc_roots::note_symbol_pointer(ptr); let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_POINTERS); if guard.is_none() { *guard = Some(new_ptr_hash_set()); @@ -1026,6 +1027,7 @@ pub(crate) fn store_object_symbol_property_root( value_bits: u64, ) -> bool { note_symbol_key_installed(sym_key); + gc_roots::note_symbol_property_root(obj_key, sym_key, value_bits); { let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTIES); if guard.is_none() { @@ -1060,6 +1062,7 @@ pub(crate) static CLASS_STATIC_SYMBOLS_LATCH: crate::registry_latch::RegistryLat pub(crate) fn store_class_static_symbol_root(class_id: u32, sym_key: usize, value_bits: u64) { note_symbol_key_installed(sym_key); + gc_roots::note_class_static_symbol(class_id, sym_key, value_bits); CLASS_STATIC_SYMBOLS_LATCH.arm(); let symbol_id = unsafe { (*(sym_key as *const SymbolHeader)).id }; let created; diff --git a/crates/perry-runtime/src/symbol/accessors.rs b/crates/perry-runtime/src/symbol/accessors.rs index 9ceeddbe3c..9fb977e553 100644 --- a/crates/perry-runtime/src/symbol/accessors.rs +++ b/crates/perry-runtime/src/symbol/accessors.rs @@ -68,6 +68,7 @@ pub(crate) fn test_symbol_accessor_property_count() -> usize { #[cfg(test)] pub(crate) fn test_seed_symbol_accessor_property(obj_key: usize, sym_key: usize, get_bits: u64) { + super::gc_roots::note_symbol_accessor(obj_key, sym_key, get_bits, TAG_UNDEFINED); let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_ACCESSOR_PROPERTIES); guard.get_or_insert_with(HashMap::new).insert( (obj_key, sym_key), @@ -90,6 +91,8 @@ pub(crate) unsafe fn set_symbol_accessor_property( return; } crate::symbol::note_symbol_key_installed(sym_key); + super::gc_roots::note_symbol_property_root(obj_key, sym_key, crate::value::TAG_UNDEFINED); + super::gc_roots::note_symbol_accessor(obj_key, sym_key, get_bits, set_bits); { // `SYMBOL_PROPERTIES` is the only insertion-ordered record of symbol // property CREATION order, which `[[OwnPropertyKeys]]` must report @@ -219,6 +222,29 @@ pub(super) fn accessor_property_keys() -> Vec<(usize, usize)> { .unwrap_or_default() } +pub(super) fn accessor_property_count() -> usize { + let guard = crate::gc::lock_gc_root_registry(&SYMBOL_ACCESSOR_PROPERTIES); + guard.as_ref().map_or(0, HashMap::len) +} + +pub(super) fn relevant_accessor_property_keys() -> Vec<(usize, usize)> { + let guard = crate::gc::lock_gc_root_registry(&SYMBOL_ACCESSOR_PROPERTIES); + guard + .as_ref() + .map(|map| { + map.iter() + .filter_map(|(&(owner, sym_key), acc)| { + (crate::gc::young_log::addr_is_minor_collectible(owner) + || crate::gc::young_log::addr_is_minor_relevant(sym_key) + || crate::gc::young_log::bits_are_minor_relevant(acc.get) + || crate::gc::young_log::bits_are_minor_relevant(acc.set)) + .then_some((owner, sym_key)) + }) + .collect() + }) + .unwrap_or_default() +} + /// Step twin of `scan_symbol_accessor_roots_mut` for one snapshot key: /// strong-visits the get/set closures and rekeys owner/sym on a move. /// Cycle-based collections run ONLY the step scanner, so before this @@ -228,13 +254,13 @@ pub(super) fn scan_symbol_accessor_root_slot( visitor: &mut crate::gc::RuntimeRootVisitor<'_>, owner: usize, sym_key: usize, -) { +) -> Option<(usize, usize)> { let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_ACCESSOR_PROPERTIES); let Some(map) = guard.as_mut() else { - return; + return None; }; let Some(acc) = map.get_mut(&(owner, sym_key)) else { - return; + return None; }; let mut new_owner = owner; let mut new_sym_key = sym_key; @@ -251,6 +277,21 @@ pub(super) fn scan_symbol_accessor_root_slot( map.insert((new_owner, new_sym_key), acc); } } + symbol_accessor_root_relevant_in(map, new_owner, new_sym_key) + .then_some((new_owner, new_sym_key)) +} + +fn symbol_accessor_root_relevant_in( + map: &HashMap<(usize, usize), SymbolAccessorDescriptor>, + owner: usize, + sym_key: usize, +) -> bool { + map.get(&(owner, sym_key)).is_some_and(|acc| { + crate::gc::young_log::addr_is_minor_collectible(owner) + || crate::gc::young_log::addr_is_minor_relevant(sym_key) + || crate::gc::young_log::bits_are_minor_relevant(acc.get) + || crate::gc::young_log::bits_are_minor_relevant(acc.set) + }) } pub(super) fn has_own_symbol_accessor(obj_key: usize, sym_key: usize) -> bool { diff --git a/crates/perry-runtime/src/symbol/gc_roots.rs b/crates/perry-runtime/src/symbol/gc_roots.rs index 8c05be3014..7fec611384 100644 --- a/crates/perry-runtime/src/symbol/gc_roots.rs +++ b/crates/perry-runtime/src/symbol/gc_roots.rs @@ -21,11 +21,16 @@ pub fn scan_symbol_side_table_roots(mark: &mut dyn FnMut(f64)) { } pub fn scan_symbol_side_table_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { + if visitor.young_scope() { + scan_young_symbol_side_table_roots_mut(visitor); + return; + } scan_symbol_property_roots_mut(visitor); scan_symbol_property_attrs_mut(visitor); accessors::scan_symbol_accessor_roots_mut(visitor); scan_class_static_symbol_roots_mut(visitor); scan_symbol_pointer_metadata_roots_mut(visitor); + rebuild_symbol_young_log(); } fn scan_symbol_property_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { @@ -131,7 +136,7 @@ fn scan_symbol_pointer_metadata_roots_mut(visitor: &mut crate::gc::RuntimeRootVi } } -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd)] enum SymbolSideTableRootSlot { SymbolPropertyOwner { owner: usize }, SymbolPropertyEntry { owner: usize, sym_key: usize }, @@ -141,15 +146,83 @@ enum SymbolSideTableRootSlot { SymbolPointer { ptr: usize }, } +crate::perry_thread_local! { + static SYMBOL_SIDE_TABLE_YOUNG: RefCell> = + const { RefCell::new(crate::gc::young_log::YoungLog::new()) }; + #[cfg(test)] + static TEST_SUPPRESS_SYMBOL_YOUNG_NOTE: std::cell::Cell = + const { std::cell::Cell::new(false) }; +} + +const SYMBOL_YOUNG_LOG_NAME: &str = "symbol.side_tables"; + +#[inline] +fn note_symbol_slot(slot: SymbolSideTableRootSlot) { + #[cfg(test)] + if TEST_SUPPRESS_SYMBOL_YOUNG_NOTE.with(std::cell::Cell::get) { + return; + } + SYMBOL_SIDE_TABLE_YOUNG.with(|log| log.borrow_mut().note(slot)); +} + +pub(super) fn note_symbol_property_root(owner: usize, sym_key: usize, value_bits: u64) { + if crate::gc::young_log::addr_is_minor_collectible(owner) { + note_symbol_slot(SymbolSideTableRootSlot::SymbolPropertyOwner { owner }); + } + if crate::gc::young_log::addr_is_minor_relevant(sym_key) + || crate::gc::young_log::bits_are_minor_relevant(value_bits) + { + note_symbol_slot(SymbolSideTableRootSlot::SymbolPropertyEntry { owner, sym_key }); + } +} + +pub(super) fn note_symbol_property_attrs(owner: usize, sym_key: usize) { + if crate::gc::young_log::addr_is_minor_collectible(owner) + || crate::gc::young_log::addr_is_minor_relevant(sym_key) + { + note_symbol_slot(SymbolSideTableRootSlot::SymbolPropertyAttrs { owner, sym_key }); + } +} + +pub(super) fn note_symbol_accessor(owner: usize, sym_key: usize, get_bits: u64, set_bits: u64) { + if crate::gc::young_log::addr_is_minor_collectible(owner) + || crate::gc::young_log::addr_is_minor_relevant(sym_key) + || crate::gc::young_log::bits_are_minor_relevant(get_bits) + || crate::gc::young_log::bits_are_minor_relevant(set_bits) + { + note_symbol_slot(SymbolSideTableRootSlot::SymbolAccessorProperty { owner, sym_key }); + } +} + +pub(super) fn note_class_static_symbol(class_id: u32, sym_key: usize, value_bits: u64) { + if crate::gc::young_log::addr_is_minor_relevant(sym_key) + || crate::gc::young_log::bits_are_minor_relevant(value_bits) + { + note_symbol_slot(SymbolSideTableRootSlot::ClassStaticSymbol { class_id, sym_key }); + } +} + +pub(super) fn note_symbol_pointer(ptr: usize) { + if crate::gc::young_log::addr_is_minor_collectible(ptr) { + note_symbol_slot(SymbolSideTableRootSlot::SymbolPointer { ptr }); + } +} + pub(crate) struct SymbolSideTableRootScanState { - slots: Vec, + slots: Option>, + kept: Vec, cursor: usize, + young: bool, + table_len: usize, } pub(crate) fn new_symbol_side_table_root_scan_state() -> Box { Box::new(SymbolSideTableRootScanState { - slots: symbol_side_table_root_snapshot(), + slots: None, + kept: Vec::new(), cursor: 0, + young: false, + table_len: 0, }) } @@ -161,12 +234,49 @@ pub(crate) fn scan_symbol_side_table_roots_mut_step( let state = state .downcast_mut::() .expect("symbol side-table root scanner state type"); - while *remaining > 0 && state.cursor < state.slots.len() { - scan_symbol_side_table_root_slot(visitor, state.slots[state.cursor]); + if state.slots.is_none() { + state.young = visitor.young_scope(); + if state.young { + state.table_len = symbol_side_table_root_len(); + #[cfg(any(debug_assertions, test))] + SYMBOL_SIDE_TABLE_YOUNG.with(|log| { + log.borrow() + .debug_assert_logged(SYMBOL_YOUNG_LOG_NAME, &relevant_symbol_slots()) + }); + state.slots = Some(SYMBOL_SIDE_TABLE_YOUNG.with(|log| log.borrow_mut().take_sorted())); + } else { + let authoritative = symbol_side_table_root_snapshot(); + state.table_len = authoritative.len(); + let _ = SYMBOL_SIDE_TABLE_YOUNG.with(|log| log.borrow_mut().take_sorted()); + state.slots = Some(authoritative); + } + } + let slots = state.slots.as_ref().expect("symbol slots initialized"); + while *remaining > 0 && state.cursor < slots.len() { + if let Some(slot) = scan_symbol_side_table_root_slot(visitor, slots[state.cursor]) { + state.kept.push(slot); + } state.cursor += 1; *remaining -= 1; } - state.cursor >= state.slots.len() + let done = state.cursor >= slots.len(); + if done { + let logged = slots.len() as u64; + let kept = std::mem::take(&mut state.kept); + let kept_len = kept.len() as u64; + SYMBOL_SIDE_TABLE_YOUNG.with(|log| log.borrow_mut().extend(kept)); + crate::gc::young_log::note_walk( + SYMBOL_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: state.young, + logged, + visited: state.cursor as u64, + kept: kept_len, + table_len: state.table_len as u64, + }, + ); + } + done } fn symbol_side_table_root_snapshot() -> Vec { @@ -218,13 +328,163 @@ fn symbol_side_table_root_snapshot() -> Vec { slots } +fn symbol_side_table_root_len() -> usize { + // Exact only for diagnostics: counting the property vectors is itself a + // whole-table walk, which the release minor must not pay merely to report + // how much work it skipped. + if !crate::gc::gc_diag_enabled() { + return 0; + } + let properties = { + let guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTIES); + guard.as_ref().map_or(0, |map| { + map.len() + map.values().map(Vec::len).sum::() + }) + }; + let attrs = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTY_ATTRS) + .as_ref() + .map_or(0, |map| map.len()); + let class_statics = crate::gc::lock_gc_root_registry(&CLASS_STATIC_SYMBOLS) + .as_ref() + .map_or(0, |map| map.len()); + let pointers = crate::gc::lock_gc_root_registry(&SYMBOL_POINTERS) + .as_ref() + .map_or(0, |set| set.len()); + properties + attrs + accessors::accessor_property_count() + class_statics + pointers +} + +fn collect_relevant_symbol_slots() -> Vec { + let mut slots = Vec::new(); + { + let guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTIES); + if let Some(map) = guard.as_ref() { + for (&owner, entries) in map { + if crate::gc::young_log::addr_is_minor_collectible(owner) { + slots.push(SymbolSideTableRootSlot::SymbolPropertyOwner { owner }); + } + for &(sym_key, value_bits) in entries { + if crate::gc::young_log::addr_is_minor_relevant(sym_key) + || crate::gc::young_log::bits_are_minor_relevant(value_bits) + { + slots.push(SymbolSideTableRootSlot::SymbolPropertyEntry { owner, sym_key }); + } + } + } + } + } + { + let guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTY_ATTRS); + if let Some(map) = guard.as_ref() { + slots.extend(map.keys().filter_map(|&(owner, sym_key)| { + (crate::gc::young_log::addr_is_minor_collectible(owner) + || crate::gc::young_log::addr_is_minor_relevant(sym_key)) + .then_some(SymbolSideTableRootSlot::SymbolPropertyAttrs { owner, sym_key }) + })); + } + } + slots.extend( + accessors::relevant_accessor_property_keys() + .into_iter() + .map( + |(owner, sym_key)| SymbolSideTableRootSlot::SymbolAccessorProperty { + owner, + sym_key, + }, + ), + ); + { + let guard = crate::gc::lock_gc_root_registry(&CLASS_STATIC_SYMBOLS); + if let Some(map) = guard.as_ref() { + slots.extend( + map.iter() + .filter_map(|(&(class_id, sym_key), &value_bits)| { + (crate::gc::young_log::addr_is_minor_relevant(sym_key) + || crate::gc::young_log::bits_are_minor_relevant(value_bits)) + .then_some(SymbolSideTableRootSlot::ClassStaticSymbol { class_id, sym_key }) + }), + ); + } + } + { + let guard = crate::gc::lock_gc_root_registry(&SYMBOL_POINTERS); + if let Some(set) = guard.as_ref() { + slots.extend(set.iter().filter_map(|&ptr| { + crate::gc::young_log::addr_is_minor_collectible(ptr) + .then_some(SymbolSideTableRootSlot::SymbolPointer { ptr }) + })); + } + } + slots +} + +#[cfg(any(debug_assertions, test))] +fn relevant_symbol_slots() -> Vec { + collect_relevant_symbol_slots() +} + +fn rebuild_symbol_young_log() { + let table_len = symbol_side_table_root_len(); + let relevant = collect_relevant_symbol_slots(); + let _ = SYMBOL_SIDE_TABLE_YOUNG.with(|log| log.borrow_mut().take_sorted()); + let kept = relevant.len() as u64; + SYMBOL_SIDE_TABLE_YOUNG.with(|log| log.borrow_mut().extend(relevant)); + crate::gc::young_log::note_walk( + SYMBOL_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: false, + logged: table_len as u64, + visited: table_len as u64, + kept, + table_len: table_len as u64, + }, + ); +} + +fn scan_young_symbol_side_table_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { + let table_len = symbol_side_table_root_len(); + #[cfg(any(debug_assertions, test))] + SYMBOL_SIDE_TABLE_YOUNG.with(|log| { + log.borrow() + .debug_assert_logged(SYMBOL_YOUNG_LOG_NAME, &relevant_symbol_slots()) + }); + let mut kept = SYMBOL_SIDE_TABLE_YOUNG.with(|log| log.borrow_mut().take_spare()); + let mut logged = 0_u64; + loop { + let batch = SYMBOL_SIDE_TABLE_YOUNG.with(|log| log.borrow_mut().take_sorted()); + if batch.is_empty() { + break; + } + logged += batch.len() as u64; + for slot in batch { + if let Some(slot) = scan_symbol_side_table_root_slot(visitor, slot) { + kept.push(slot); + } + } + } + let kept_len = kept.len() as u64; + SYMBOL_SIDE_TABLE_YOUNG.with(|log| log.borrow_mut().extend(kept)); + crate::gc::young_log::note_walk( + SYMBOL_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: true, + logged, + visited: logged, + kept: kept_len, + table_len: table_len as u64, + }, + ); +} + fn scan_symbol_side_table_root_slot( visitor: &mut crate::gc::RuntimeRootVisitor<'_>, slot: SymbolSideTableRootSlot, -) { +) -> Option { match slot { SymbolSideTableRootSlot::SymbolPropertyOwner { owner } => { - rewrite_symbol_property_owner_if_forwarded(visitor, owner); + rewrite_symbol_property_owner_if_forwarded(visitor, owner).and_then(|owner| { + crate::gc::young_log::addr_is_minor_collectible(owner) + .then_some(SymbolSideTableRootSlot::SymbolPropertyOwner { owner }) + }) } SymbolSideTableRootSlot::SymbolPropertyEntry { owner, sym_key } => { // The preceding budget slice may already have rekeyed this @@ -235,7 +495,7 @@ fn scan_symbol_side_table_root_slot( visitor.visit_metadata_usize_slot(&mut healed_owner); let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTIES); let Some(map) = guard.as_mut() else { - return; + return None; }; let lookup_owner = if map.contains_key(&healed_owner) { healed_owner @@ -246,22 +506,43 @@ fn scan_symbol_side_table_root_slot( .get_mut(&lookup_owner) .and_then(|entries| entries.iter_mut().find(|entry| entry.0 == sym_key)) else { - return; + return None; }; visitor.visit_usize_slot(entry_sym); visitor.visit_nanbox_u64_slot(value_bits); + (crate::gc::young_log::addr_is_minor_relevant(*entry_sym) + || crate::gc::young_log::bits_are_minor_relevant(*value_bits)) + .then_some(SymbolSideTableRootSlot::SymbolPropertyEntry { + owner: lookup_owner, + sym_key: *entry_sym, + }) } SymbolSideTableRootSlot::SymbolAccessorProperty { owner, sym_key } => { - accessors::scan_symbol_accessor_root_slot(visitor, owner, sym_key); + accessors::scan_symbol_accessor_root_slot(visitor, owner, sym_key).map( + |(owner, sym_key)| SymbolSideTableRootSlot::SymbolAccessorProperty { + owner, + sym_key, + }, + ) } SymbolSideTableRootSlot::SymbolPropertyAttrs { owner, sym_key } => { - rewrite_symbol_property_attrs_if_forwarded(visitor, owner, sym_key); + rewrite_symbol_property_attrs_if_forwarded(visitor, owner, sym_key).and_then( + |(owner, sym_key)| { + (crate::gc::young_log::addr_is_minor_collectible(owner) + || crate::gc::young_log::addr_is_minor_relevant(sym_key)) + .then_some(SymbolSideTableRootSlot::SymbolPropertyAttrs { owner, sym_key }) + }, + ) } SymbolSideTableRootSlot::ClassStaticSymbol { class_id, sym_key } => { - rewrite_class_static_symbol_entry_if_forwarded(visitor, class_id, sym_key); + rewrite_class_static_symbol_entry_if_forwarded(visitor, class_id, sym_key) + .map(|sym_key| SymbolSideTableRootSlot::ClassStaticSymbol { class_id, sym_key }) } SymbolSideTableRootSlot::SymbolPointer { ptr } => { - rewrite_symbol_pointer_metadata_if_forwarded(visitor, ptr); + rewrite_symbol_pointer_metadata_if_forwarded(visitor, ptr).and_then(|ptr| { + crate::gc::young_log::addr_is_minor_collectible(ptr) + .then_some(SymbolSideTableRootSlot::SymbolPointer { ptr }) + }) } } } @@ -269,37 +550,41 @@ fn scan_symbol_side_table_root_slot( fn rewrite_symbol_property_owner_if_forwarded( visitor: &mut crate::gc::RuntimeRootVisitor<'_>, owner: usize, -) { +) -> Option { let mut new_owner = owner; - if !visitor.visit_metadata_usize_slot(&mut new_owner) || new_owner == owner { - return; - } - let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTIES); - if let Some(map) = guard.as_mut() { - if let Some(entries) = map.remove(&owner) { - match map.entry(new_owner) { - std::collections::hash_map::Entry::Occupied(mut entry) => { - merge_symbol_property_entries(entry.get_mut(), entries); - } - std::collections::hash_map::Entry::Vacant(entry) => { - entry.insert(entries); + if visitor.visit_metadata_usize_slot(&mut new_owner) && new_owner != owner { + let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTIES); + if let Some(map) = guard.as_mut() { + if let Some(entries) = map.remove(&owner) { + match map.entry(new_owner) { + std::collections::hash_map::Entry::Occupied(mut entry) => { + merge_symbol_property_entries(entry.get_mut(), entries); + } + std::collections::hash_map::Entry::Vacant(entry) => { + entry.insert(entries); + } } } } } + let guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTIES); + guard + .as_ref() + .is_some_and(|map| map.contains_key(&new_owner)) + .then_some(new_owner) } fn rewrite_symbol_property_attrs_if_forwarded( visitor: &mut crate::gc::RuntimeRootVisitor<'_>, owner: usize, sym_key: usize, -) { +) -> Option<(usize, usize)> { let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTY_ATTRS); let Some(map) = guard.as_mut() else { - return; + return None; }; if !map.contains_key(&(owner, sym_key)) { - return; + return None; } let mut new_owner = owner; let mut new_sym_key = sym_key; @@ -310,19 +595,21 @@ fn rewrite_symbol_property_attrs_if_forwarded( map.insert((new_owner, new_sym_key), attrs); } } + map.contains_key(&(new_owner, new_sym_key)) + .then_some((new_owner, new_sym_key)) } fn rewrite_class_static_symbol_entry_if_forwarded( visitor: &mut crate::gc::RuntimeRootVisitor<'_>, class_id: u32, sym_key: usize, -) { +) -> Option { let mut guard = crate::gc::lock_gc_root_registry(&CLASS_STATIC_SYMBOLS); let Some(map) = guard.as_mut() else { - return; + return None; }; let Some(value_bits) = map.get_mut(&(class_id, sym_key)) else { - return; + return None; }; let mut new_sym_key = sym_key; let moved = visitor.visit_usize_slot(&mut new_sym_key); @@ -332,27 +619,37 @@ fn rewrite_class_static_symbol_entry_if_forwarded( map.insert((class_id, new_sym_key), value_bits); } } + map.get(&(class_id, new_sym_key)).and_then(|value_bits| { + (crate::gc::young_log::addr_is_minor_relevant(new_sym_key) + || crate::gc::young_log::bits_are_minor_relevant(*value_bits)) + .then_some(new_sym_key) + }) } fn rewrite_symbol_pointer_metadata_if_forwarded( visitor: &mut crate::gc::RuntimeRootVisitor<'_>, ptr: usize, -) { +) -> Option { let mut new_ptr = ptr; - if !visitor.visit_metadata_usize_slot(&mut new_ptr) || new_ptr == ptr { - return; - } - let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_POINTERS); - if let Some(set) = guard.as_mut() { - set.remove(&ptr); - if new_ptr != 0 { - insert_symbol_pointer_in_set(set, new_ptr); + if visitor.visit_metadata_usize_slot(&mut new_ptr) && new_ptr != ptr { + let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_POINTERS); + if let Some(set) = guard.as_mut() { + set.remove(&ptr); + if new_ptr != 0 { + insert_symbol_pointer_in_set(set, new_ptr); + } } } + let guard = crate::gc::lock_gc_root_registry(&SYMBOL_POINTERS); + guard + .as_ref() + .is_some_and(|set| set.contains(&new_ptr)) + .then_some(new_ptr) } #[cfg(test)] pub(crate) fn test_clear_symbol_side_table_roots() { + SYMBOL_SIDE_TABLE_YOUNG.with(|log| log.borrow_mut().clear()); *crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTIES) = None; *crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTY_ATTRS) = None; *crate::gc::lock_gc_root_registry(&CLASS_STATIC_SYMBOLS) = None; @@ -379,6 +676,7 @@ pub(crate) fn test_clear_symbol_side_table_roots() { } else { let mut set = new_ptr_hash_set(); for ptr in persistent { + note_symbol_pointer(ptr); insert_symbol_pointer_in_set(&mut set, ptr); } *guard = Some(set); @@ -425,6 +723,7 @@ pub(crate) fn test_seed_class_static_symbol_root(class_id: u32, sym_key: usize, // an unaligned sentinel. Seed only the root table they exercise; // production registration additionally reads SymbolHeader::id for // [[OwnPropertyKeys]] ordering and therefore requires a real Symbol. + note_class_static_symbol(class_id, sym_key, value_bits); CLASS_STATIC_SYMBOLS_LATCH.arm(); let mut guard = crate::gc::lock_gc_root_registry(&CLASS_STATIC_SYMBOLS); if guard.is_none() { @@ -474,3 +773,31 @@ pub(crate) fn test_symbol_pointer_root_contains(ptr: usize) -> bool { let guard = crate::gc::lock_gc_root_registry(&SYMBOL_POINTERS); guard.as_ref().is_some_and(|set| set.contains(&ptr)) } + +#[cfg(test)] +mod young_log_sabotage_tests { + use super::*; + + #[test] + fn symbol_log_rederivation_rejects_a_suppressed_property_writer() { + let _lock = crate::gc::global_side_table_test_lock(); + test_clear_symbol_side_table_roots(); + let owner = crate::object::js_object_alloc(0, 0) as usize; + let sym_bits = unsafe { crate::symbol::js_symbol_new_empty() }.to_bits(); + let sym_key = (sym_bits & POINTER_MASK) as usize; + TEST_SUPPRESS_SYMBOL_YOUNG_NOTE.with(|flag| flag.set(true)); + store_object_symbol_property_root(owner, sym_key, 7.0_f64.to_bits()); + TEST_SUPPRESS_SYMBOL_YOUNG_NOTE.with(|flag| flag.set(false)); + let missed = std::panic::catch_unwind(|| { + SYMBOL_SIDE_TABLE_YOUNG.with(|log| { + log.borrow() + .debug_assert_logged(SYMBOL_YOUNG_LOG_NAME, &relevant_symbol_slots()) + }); + }); + test_clear_symbol_side_table_roots(); + assert!( + missed.is_err(), + "sabotage: suppressing the property-store note must trip completeness" + ); + } +} diff --git a/crates/perry-runtime/src/symbol/properties.rs b/crates/perry-runtime/src/symbol/properties.rs index ddcbcf4f32..49adaada16 100644 --- a/crates/perry-runtime/src/symbol/properties.rs +++ b/crates/perry-runtime/src/symbol/properties.rs @@ -90,6 +90,7 @@ pub(crate) fn set_symbol_property_attrs( return; } super::note_symbol_key_installed(sym_key); + super::gc_roots::note_symbol_property_attrs(owner, sym_key); let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTY_ATTRS); if guard.is_none() { *guard = Some(crate::fast_hash::new_fast_key_hash_map()); From de6bfd5c5d88140dcac198e6bddcae109501a881 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 12:00:51 +0200 Subject: [PATCH 05/11] docs(perf): report minor phases and scanner logs Record the phase instrument, young-log mappings and sabotage coverage, shape residual analysis, validation results, performance predictions, and the exact perrymaster relink and measurement request. --- .../codex/REPORT_minor_phases_and_logs.md | 206 ++++++++++++++++++ 1 file changed, 206 insertions(+) create mode 100644 cc-perf-campaign/codex/REPORT_minor_phases_and_logs.md diff --git a/cc-perf-campaign/codex/REPORT_minor_phases_and_logs.md b/cc-perf-campaign/codex/REPORT_minor_phases_and_logs.md new file mode 100644 index 0000000000..6fac981962 --- /dev/null +++ b/cc-perf-campaign/codex/REPORT_minor_phases_and_logs.md @@ -0,0 +1,206 @@ +# Copying-minor phases and remaining scanner young logs + +Phase-instrument commit: `b444f0251221d8c40dd645c741367dcf5276dff9` + +Scanner-log implementation commit: `364ed3f07c54e365e65bbb5e23bd703a9fc54a0e` + +Branch: `perf/minor-phases-and-logs`, based on +`e2eee113d486b5208c56ae1e3f0f4d0dffcbf2b2`. + +## Copying-minor phase instrument + +- `crates/perry-runtime/src/gc/copying_phase.rs:26` is the diagnostic-only + accumulator. It uses the same `Instant` clock as `pause_us` and records + non-overlapping spans for `root_scan`, `copy_evacuation`, + `remembered_set_young_logs`, `promotion`, + `dead_owner_side_table_pruning`, `from_space_finalization`, + `forwarding_fixups`, and `block_reset_flip`. `other` is the exact residual + between those named spans and the whole pause, and `phase_sum_us` is formed + from the nanosecond partition before conversion, so it equals `pause_us` + apart from the shared sub-microsecond truncation (well inside 2%). +- `crates/perry-runtime/src/gc/copying.rs:1220-1749` starts and records the + counters in the functions whose work they price. The two registered-root + passes accumulate in `root_scan`; the transitive worklist drain is + `copy_evacuation`; remembered snapshot/dirty scan and post-cycle restore are + accumulated together; promotion covers retag plus finish; forwarding covers + promoted-edge rebuild and verification/fixup work; reset covers to-space + preparation plus the final reset/flip. +- `crates/perry-runtime/src/gc/copying_phase.rs:84` renders counts where the + collector already owns them: copied/promoted objects and bytes, remembered + entries and dirty slots, and finalized map/set/error/regexp owners. The + dead-owner fan-out at `crates/perry-runtime/src/gc/dead_owner.rs:261` clocks + every registry table separately and appends those table names and + microseconds to the same field. Those prune callbacks expose no removed-row + count, so no invented count is printed. +- `crates/perry-runtime/src/gc/copying.rs:1962` appends `phases:` to every + completed `[gc-copy-minor] ran` line. `PERRY_GC_DIAG` off creates no phase + accumulator, takes no phase clocks, and builds no detail strings. +- The sabotage unit + `copied_minor_phase_residual_makes_the_partition_exact` removes a named + bucket from the expected arithmetic if the partition is widened or omitted. + +## Scanner map and young-entry logs + +### `scan_descriptor_roots_mut` + +This walks string-keyed property-attribute and accessor tables plus their two +owner indexes. Owner addresses are metadata-only and need a minor visit only +while movable/reclaimable; accessor get/set NaN-boxes are strong roots and may +require tracing through Longlived values. A #9754 owner log already existed. +The write funnel at `object/descriptor_state.rs:148` is present at all five +publication/transfer sites (`:985`, `:1208`, `:1277`, `:1394`, `:1428`). This +change narrows the metadata-key half from `addr_is_minor_relevant` to +`addr_is_minor_collectible`; the re-derivation and post-visit keep predicate +use the same rule at `object/descriptor_state/young.rs:42` and +`object/descriptor_state/gc_scan.rs:17`. The full walk is unchanged and +rebuilds the log. + +### `scan_closure_dynamic_props_roots_mut` + +This walks `CLOSURE_PROPS` values, `CLOSURE_STATIC_PROTOTYPES` values, and the +metadata-only owners of those tables and `CLOSURE_DELETED_KEYS`. A #9754 owner +log already existed. The enforced write funnels are +`closure/dynamic_props.rs:117`, `:186`, `:241`, `:388`, and `:1068`. Owner +retention is now collectible-only; property/prototype values keep the broader +transitive predicate. The minor path is `:533`; the full path remains whole +table and rebuilds the log. + +### `scan_builtin_closure_metadata_roots_mut` + +This walks two owner-keyed, pointer-free metadata tables: closure arity and the +non-constructable set. Only the closure address can move or die. There was no +partial log. The tables and their complete setters were extracted to +`object/native_module/callable_exports/builtin_closure_metadata.rs`; `:18` +arms the owner log before either setter publishes, `:95` drains only logged +collectible owners on a minor, and the unchanged full walk visits all owners +and rebuilds the log. + +### `scan_template_raw_roots_mut` + +This scanner actually owns three tables: call-site to cooked/raw template +arrays, cooked to raw template arrays, and array named properties. Template +array keys/values are strong roots, so they deliberately keep +`addr_is_minor_relevant`: a Longlived template can contain transitive GC +edges. Array named-property owners are metadata-only and collectible-only; +their NaN-box values remain broad strong roots. No partial log existed. +`array/header/young_roots.rs:29`, `:35`, and `:45` define the three logs; +every publication and array-growth transfer arms before publish at +`array/header.rs:191`, `:262`, `:346`, `:410`, `:446`, and test seeding at +`:604`. The minor scanner is `array/header/young_roots.rs:181`; the full walk +still visits every entry and rebuilds all three logs. + +### `scan_symbol_side_table_roots_mut` + +This walks six slot shapes: `SYMBOL_PROPERTIES` owner metadata and strong +symbol/value pairs, `SYMBOL_PROPERTY_ATTRS` owner metadata and strong symbol +keys, symbol accessors plus get/set roots, class-static symbol/value pairs, +and metadata-only `SYMBOL_POINTERS`. There was no partial log. A typed slot log +at `symbol/gc_roots.rs:146-209` records exactly the slot shape that can matter +to a minor. Production funnels arm before publication in `symbol.rs:582`, +`:1030`, `:1065`, `symbol/properties.rs:93`, and +`symbol/accessors.rs:94-95`; direct test seeders follow the same contract. The +direct minor path at `symbol/gc_roots.rs:443` and the budgeted step path at +`:229` take only logged slots. Property-owner slots sort before their entries, +so owner rekeying precedes entry lookup; entry scans heal a snapshot owner +through forwarding. Full direct and step walks still take authoritative +whole-table snapshots and rebuild the log. + +All five scanners emit their existing `[gc-young-log]` accounting with +logged/visited/kept/table size. Release minors do not enumerate a whole table +to obtain the symbol table size: that exact diagnostic count is itself gated +on `PERRY_GC_DIAG`. + +## Sabotage tests + +- `descriptor_log_rederivation_rejects_a_suppressed_setter`: suppresses the + real property-attrs funnel; re-derivation must report the missing owner. +- `closure_log_rederivation_rejects_a_suppressed_setter`: suppresses the real + closure dynamic-property funnel; re-derivation must report the missing + owner. +- `builtin_closure_log_rederivation_rejects_a_suppressed_writer`: suppresses + the arity setter; re-derivation must report the missing closure. +- `template_raw_log_rederivation_rejects_a_suppressed_writer`: suppresses the + cooked/raw publication funnel; re-derivation must report the missing pair. + `array_named_log_rederivation_rejects_a_suppressed_setter` independently + covers the third table owned by that scanner. +- `symbol_log_rederivation_rejects_a_suppressed_property_writer`: suppresses + the production symbol-property store; re-derivation must report its missing + typed slots. + +Each completeness check is compiled under `debug_assertions` and `test`. In +the release lib run below, every named sabotage test passed. + +## Shape residual + +The residual is real young work, not another whole-table leak. The exact keep +predicate is `object/shapes.rs:2154`: + +- Nursery Eden, either survivor half, and `PromotedYoung` keys arrays stay + logged because their table keys must be rewritten if they move. +- Malloc-GC keys arrays stay only when an old/cache carrier makes the family a + root and the allocation remains minor-collectible. +- Longlived keys arrays stay only when an old/cache carrier roots the family + **and** at least one property-key leaf in the array is collectible. Longlived + non-carriers and carriers whose leaves are all old/Longlived drop out. +- Old keys arrays always drop out. + +There is one intentional transient duplicate at `object/shapes.rs:2244`: the +mark pass may move a family before the metadata-only slot index is repaired in +the rewrite pass, so both the post-copy address and stale index address must +survive between the passes. Tightening any of these remaining cases would +skip relocation, collection of malloc keys, a strong carrier edge, or the +between-pass index repair. This explains why shape time appears only on the +steady minors that create/grow a burst of genuinely young shape-key arrays; +there is no sound additional predicate tightening in this change. + +## Validation + +- `git diff --check`: PASS. +- `scripts/check_file_size.sh`: PASS (all Rust files at most 2,000 lines). +- `scripts/gc_runtime_root_holders.py`: PASS. +- `scripts/gc_rekeyed_key_tables.py`: PASS. +- `cargo test -p perry-runtime --release --lib -j4 -- --test-threads=1` via + `measure_lock.sh --build`: NOT GREEN solely because it was run detached with + a PTY. Compilation completed and 3,273 tests passed (including every new + sabotage), 4 were ignored, and the sole failure was + `tty::tests::columns_undefined_when_not_tty`, whose assertion correctly saw + the allocated PTY. Two earlier attempts stopped at compile diagnostics in + the newly extracted module; those visibility/TLS/null-pointer/type issues + were fixed before this complete run. +- The required non-PTY rerun was NOT RUN: immediately afterward `df -g /` + reported 7 GB free, below the binding 12 GB floor. Per the task rule, no + further Cargo command was started and no disk wait was attempted. +- `cargo build --release -p perry-runtime --features wasm-host -j4`: NOT RUN, + same 7 GB disk stop. +- `cargo build --release -p perry -j4`: NOT RUN, same 7 GB disk stop. + +## Predictions and exact perrymaster request + +Predictions: on a zero-live steady minor, each of +`scan_descriptor_roots_mut`, `scan_closure_dynamic_props_roots_mut`, +`scan_builtin_closure_metadata_roots_mut`, `scan_template_raw_roots_mut`, and +`scan_symbol_side_table_roots_mut` is at most **0.3 ms**. Steady scanner total +is at most **5 ms**. This removes roughly 10 ms from a representative steady +minor when the five logs are empty; the phase table, not that estimate, must +name the next non-scanner lever. RSS changes should be small retained log +buffers and remain inside Ralph's allowed +1-10% band. + +Exact perrymaster request: fetch pushed branch `perf/minor-phases-and-logs` and +relink this runtime-only change on main's cache. Run the three required gates +through +`/Users/amlug/projects/perry/secret-tests/cc-perf-campaign/measure_lock.sh --build` +detached, using exactly: + +1. `cargo test -p perry-runtime --release --lib -j4 -- --test-threads=1` +2. `cargo build --release -p perry-runtime --features wasm-host -j4` +3. `cargo build --release -p perry -j4` + +Then run one +graceful four-turn 3300-character cc workload and one 400-character workload +with `PERRY_GC_DIAG=1`, printing and preserving **every complete** +`[gc-copy-minor] ran` line. The phase table for a steady minor is the +deliverable that names the next lever. Confirm all five named scanners are at +most 0.3 ms on zero-live steady minors and steady scanner total is at most 5 +ms. Finally run paired **5x3300 + 3x400** against both main and #9950's runtime, +reporting cc turn CPU and peak RSS; target node/bun CPU parity, allowing only ++1-10% RSS. From 151680fa0f6ffbf6ebb7d8166e2424fc0d031b49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 15:05:09 +0200 Subject: [PATCH 06/11] perf(gc): drop slower template and symbol young logs Restore the dense full-table walkers for template roots and symbol side tables after MP measurements showed that their keyed young paths cost more. Remove the associated publication upkeep and rederivation tests while leaving the three measured wins and descriptor narrowing intact. Move the built-in closure young log onto hot TLS and update the thread-local and rekey policy inventories for the callable-exports module split. Claude-Session: https://claude.ai/code/session_011dhBmdn4vGgNibjo3oZqTo --- .../codex/REPORT_minor_phases_and_logs.md | 150 ++++--- crates/perry-runtime/src/array/header.rs | 64 ++- .../src/array/header/young_roots.rs | 302 ------------- .../builtin_closure_metadata.rs | 8 +- crates/perry-runtime/src/symbol.rs | 3 - crates/perry-runtime/src/symbol/accessors.rs | 47 +- crates/perry-runtime/src/symbol/gc_roots.rs | 411 ++---------------- crates/perry-runtime/src/symbol/properties.rs | 1 - scripts/gc_rekeyed_key_tables.json | 4 +- scripts/thread_local_cold_allowlist.json | 5 +- 10 files changed, 193 insertions(+), 802 deletions(-) delete mode 100644 crates/perry-runtime/src/array/header/young_roots.rs diff --git a/cc-perf-campaign/codex/REPORT_minor_phases_and_logs.md b/cc-perf-campaign/codex/REPORT_minor_phases_and_logs.md index 6fac981962..80af5e11b4 100644 --- a/cc-perf-campaign/codex/REPORT_minor_phases_and_logs.md +++ b/cc-perf-campaign/codex/REPORT_minor_phases_and_logs.md @@ -1,11 +1,11 @@ # Copying-minor phases and remaining scanner young logs -Phase-instrument commit: `b444f0251221d8c40dd645c741367dcf5276dff9` +Phase-instrument commit: `09846784c` -Scanner-log implementation commit: `364ed3f07c54e365e65bbb5e23bd703a9fc54a0e` +Scanner-log implementation commit: `dae519296` Branch: `perf/minor-phases-and-logs`, based on -`e2eee113d486b5208c56ae1e3f0f4d0dffcbf2b2`. +`8b7dc3342`. ## Copying-minor phase instrument @@ -77,38 +77,70 @@ and rebuilds the log. ### `scan_template_raw_roots_mut` -This scanner actually owns three tables: call-site to cooked/raw template -arrays, cooked to raw template arrays, and array named properties. Template -array keys/values are strong roots, so they deliberately keep -`addr_is_minor_relevant`: a Longlived template can contain transitive GC -edges. Array named-property owners are metadata-only and collectible-only; -their NaN-box values remain broad strong roots. No partial log existed. -`array/header/young_roots.rs:29`, `:35`, and `:45` define the three logs; -every publication and array-growth transfer arms before publish at -`array/header.rs:191`, `:262`, `:346`, `:410`, `:446`, and test seeding at -`:604`. The minor scanner is `array/header/young_roots.rs:181`; the full walk -still visits every entry and rebuilds all three logs. +This scanner owns three small tables: call-site to cooked/raw template arrays, +cooked to raw template arrays, and array named properties. The attempted young +logs were reverted after MP measurement showed 2.76 ms for the keyed path +against 1.83 ms for the original full walk. The scanner again walks the three +authoritative tables directly, with no insert-side log upkeep. ### `scan_symbol_side_table_roots_mut` This walks six slot shapes: `SYMBOL_PROPERTIES` owner metadata and strong symbol/value pairs, `SYMBOL_PROPERTY_ATTRS` owner metadata and strong symbol keys, symbol accessors plus get/set roots, class-static symbol/value pairs, -and metadata-only `SYMBOL_POINTERS`. There was no partial log. A typed slot log -at `symbol/gc_roots.rs:146-209` records exactly the slot shape that can matter -to a minor. Production funnels arm before publication in `symbol.rs:582`, -`:1030`, `:1065`, `symbol/properties.rs:93`, and -`symbol/accessors.rs:94-95`; direct test seeders follow the same contract. The -direct minor path at `symbol/gc_roots.rs:443` and the budgeted step path at -`:229` take only logged slots. Property-owner slots sort before their entries, -so owner rekeying precedes entry lookup; entry scans heal a snapshot owner -through forwarding. Full direct and step walks still take authoritative -whole-table snapshots and rebuild the log. - -All five scanners emit their existing `[gc-young-log]` accounting with -logged/visited/kept/table size. Release minors do not enumerate a whole table -to obtain the symbol table size: that exact diagnostic count is itself gated -on `PERRY_GC_DIAG`. +and metadata-only `SYMBOL_POINTERS`. The attempted typed-slot young log was +reverted after MP measurement showed 2.47 ms for the keyed path against +1.74 ms for the original full walk. Direct scans again iterate the +authoritative tables, and budgeted scans again use the pre-existing full slot +snapshot; none of the symbol writers pays young-log upkeep. + +The retained descriptor, closure-dynamic-property, built-in-closure-metadata, +and shape-cache young logs continue to emit `[gc-young-log]` accounting with +logged/visited/kept/table size. + +## MP measurement and the two reverted/fixed logs + +Perrymaster measured MP-stage medians over 16 steady 3300-character minor +collections, comparing app-m6mp (the five scanner changes) with app-m6ms +(without them): + +| scanner | m6ms full walk | m6mp young log | delta | +|---|---:|---:|---:| +| descriptor_roots | 4.12 ms | 4.02 ms | -0.1 ms | +| closure_dynamic_props | 3.75 ms | 3.06 ms | **-0.7 ms** | +| builtin_closure_metadata | 1.42 ms | 0.93 ms | **-0.5 ms** | +| shape_cache | 0.68 ms | 0.27 ms | **-0.4 ms** | +| **template_raw_roots** | 1.83 ms | **2.76 ms** | **+0.9 ms** | +| **symbol_side_table** | 1.74 ms | **2.47 ms** | **+0.7 ms** | +| transition_cache / intern / class_side / singleton_closure / box | flat | flat | flat | +| **total** | **15.5 ms** | **15.3 ms** | **-0.2 ms** | + +Both regressions are case (b): the logged path made each visited entry more +expensive than the dense full walk. They are not duplicate-log failures: +`YoungLog::take_sorted` sorts and globally deduplicates every batch, and each +writer tests the young/relevant predicate before noting a key. + +- `template_raw_roots`: the full scanner streams each map directly and only + removes/reinserts keys that actually move. The young path sorted its keys, + performed a hash lookup for every cache entry, and unconditionally removed + and reinserted every logged raw-map and named-property owner even when the + owner did not move. A pointer or index cannot safely retain the full walk's + per-entry cost because insertion and rekeying can relocate these `HashMap` + entries. The three logs, their publication hooks, and their two rederivation + tests were therefore removed. +- `symbol_side_table`: the full scanner streams the maps and their property + vectors. The typed-slot path sorted its keys, then recovered every property + entry through an owner hash lookup plus a linear search of that owner's + vector; the other slot shapes also paid keyed table lookups. Hash-map + rekeying and vector growth make raw entry pointers or indices unstable, so a + safe O(young) path with the full walk's per-entry cost would require a + structural table redesign. The typed log, all writer hooks, and its + rederivation test were therefore removed. + +Re-measurement falsifier: on perrymaster, `template_raw_roots` must be at most +**1.83 ms** and `symbol_side_table` at most **1.74 ms** at the median, the +three improved scanners must remain unchanged, and total scanner time must be +at most **14 ms**. ## Sabotage tests @@ -119,13 +151,11 @@ on `PERRY_GC_DIAG`. owner. - `builtin_closure_log_rederivation_rejects_a_suppressed_writer`: suppresses the arity setter; re-derivation must report the missing closure. -- `template_raw_log_rederivation_rejects_a_suppressed_writer`: suppresses the - cooked/raw publication funnel; re-derivation must report the missing pair. - `array_named_log_rederivation_rejects_a_suppressed_setter` independently - covers the third table owned by that scanner. -- `symbol_log_rederivation_rejects_a_suppressed_property_writer`: suppresses - the production symbol-property store; re-derivation must report its missing - typed slots. +- `template_raw_log_rederivation_rejects_a_suppressed_writer`, + `array_named_log_rederivation_rejects_a_suppressed_setter`, and + `symbol_log_rederivation_rejects_a_suppressed_property_writer` were removed + with the two reverted logs; their enforced-writer invariant no longer + exists. Each completeness check is compiled under `debug_assertions` and `test`. In the release lib run below, every named sabotage test passed. @@ -157,33 +187,28 @@ there is no sound additional predicate tightening in this change. - `git diff --check`: PASS. - `scripts/check_file_size.sh`: PASS (all Rust files at most 2,000 lines). -- `scripts/gc_runtime_root_holders.py`: PASS. -- `scripts/gc_rekeyed_key_tables.py`: PASS. +- `cargo fmt --all -- --check`: PASS. +- `scripts/check_thread_locals.py --self-test`: PASS in all seven directions. +- `scripts/check_thread_locals.py`: PASS, 411 hot declarations and 273 cold + declarations in 84 recorded files, below the 768-slot hot capacity. +- `scripts/gc_rekeyed_key_tables.py`: PASS, 42 sites and 25 registered prunes + classified with zero gaps. The split child now owns the `visit_owner` + inventory entry. - `cargo test -p perry-runtime --release --lib -j4 -- --test-threads=1` via - `measure_lock.sh --build`: NOT GREEN solely because it was run detached with - a PTY. Compilation completed and 3,273 tests passed (including every new - sabotage), 4 were ignored, and the sole failure was - `tty::tests::columns_undefined_when_not_tty`, whose assertion correctly saw - the allocated PTY. Two earlier attempts stopped at compile diagnostics in - the newly extracted module; those visibility/TLS/null-pointer/type issues - were fixed before this complete run. -- The required non-PTY rerun was NOT RUN: immediately afterward `df -g /` - reported 7 GB free, below the binding 12 GB floor. Per the task rule, no - further Cargo command was started and no disk wait was attempted. -- `cargo build --release -p perry-runtime --features wasm-host -j4`: NOT RUN, - same 7 GB disk stop. -- `cargo build --release -p perry -j4`: NOT RUN, same 7 GB disk stop. + `measure_lock.sh --build`: PASS, 3,271 passed, 0 failed, 4 ignored. The three + rederivation tests tied to the reverted logs were explicitly removed; the + retained sabotage tests passed. +- `cargo build --release -p perry-runtime --features wasm-host -j4` via + `measure_lock.sh --build`: PASS. ## Predictions and exact perrymaster request -Predictions: on a zero-live steady minor, each of -`scan_descriptor_roots_mut`, `scan_closure_dynamic_props_roots_mut`, -`scan_builtin_closure_metadata_roots_mut`, `scan_template_raw_roots_mut`, and -`scan_symbol_side_table_roots_mut` is at most **0.3 ms**. Steady scanner total -is at most **5 ms**. This removes roughly 10 ms from a representative steady -minor when the five logs are empty; the phase table, not that estimate, must -name the next non-scanner lever. RSS changes should be small retained log -buffers and remain inside Ralph's allowed +1-10% band. +Prediction after the MP follow-up: the two reverted scanners return to their +measured full-walk medians or better, the retained closure, built-in closure, +and shape-cache improvements remain, and steady scanner total is at most +**14 ms**. The phase table, not an estimate, must name the next non-scanner +lever. RSS should fall slightly because the two reverted logs and their +retained buffers are gone. Exact perrymaster request: fetch pushed branch `perf/minor-phases-and-logs` and relink this runtime-only change on main's cache. Run the three required gates @@ -199,8 +224,9 @@ Then run one graceful four-turn 3300-character cc workload and one 400-character workload with `PERRY_GC_DIAG=1`, printing and preserving **every complete** `[gc-copy-minor] ran` line. The phase table for a steady minor is the -deliverable that names the next lever. Confirm all five named scanners are at -most 0.3 ms on zero-live steady minors and steady scanner total is at most 5 -ms. Finally run paired **5x3300 + 3x400** against both main and #9950's runtime, +deliverable that names the next lever. Confirm `template_raw_roots` is at most +1.83 ms, `symbol_side_table` is at most 1.74 ms, the three improved scanner +medians are unchanged, and total scanner time is at most 14 ms. Finally run +paired **5x3300 + 3x400** against both main and #9950's runtime, reporting cc turn CPU and peak RSS; target node/bun CPU parity, allowing only +1-10% RSS. diff --git a/crates/perry-runtime/src/array/header.rs b/crates/perry-runtime/src/array/header.rs index 63e1e059b5..ec23d3aa85 100644 --- a/crates/perry-runtime/src/array/header.rs +++ b/crates/perry-runtime/src/array/header.rs @@ -4,10 +4,6 @@ pub(crate) use super::header_gc_slots::*; -mod young_roots; -pub use young_roots::scan_template_raw_roots_mut; -use young_roots::{note_array_named, note_template_cache, note_template_raw}; - use std::cell::RefCell; use std::collections::HashMap; @@ -188,7 +184,6 @@ unsafe fn register_template_raw_pair(cooked: *mut ArrayHeader, raw: *mut ArrayHe if cooked.is_null() || raw.is_null() { return; } - note_template_raw(cooked as usize, raw); TEMPLATE_RAW_MAP.with(|m| { m.borrow_mut().insert(cooked as usize, raw); }); @@ -259,7 +254,6 @@ pub extern "C" fn js_tagged_template_get_or_init( mark_template_array_frozen(raw); mark_template_array_frozen(cooked); register_template_raw_pair(cooked, raw); - note_template_cache(site_id, cooked, raw); TEMPLATE_OBJECT_CACHE.with(|m| { m.borrow_mut().insert(site_id, (cooked, raw)); }); @@ -300,6 +294,33 @@ pub fn scan_template_raw_roots(mark: &mut dyn FnMut(f64)) { scan_template_raw_roots_mut(&mut visitor); } +pub fn scan_template_raw_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { + TEMPLATE_OBJECT_CACHE.with(|m| { + let mut map = m.borrow_mut(); + for (_, (cooked_ptr, raw_ptr)) in map.iter_mut() { + visitor.visit_raw_mut_ptr_slot(cooked_ptr); + visitor.visit_raw_mut_ptr_slot(raw_ptr); + } + }); + TEMPLATE_RAW_MAP.with(|m| { + let mut map = m.borrow_mut(); + let mut moved = Vec::new(); + for (&cooked_addr, raw_ptr) in map.iter_mut() { + let mut new_cooked_addr = cooked_addr; + if visitor.visit_usize_slot(&mut new_cooked_addr) { + moved.push((cooked_addr, new_cooked_addr)); + } + visitor.visit_raw_mut_ptr_slot(raw_ptr); + } + for (old_addr, new_addr) in moved { + if let Some(raw_ptr) = map.remove(&old_addr) { + map.insert(new_addr, raw_ptr); + } + } + }); + scan_array_named_property_roots_mut(visitor); +} + fn barrier_array_named_props(owner: usize, props: &mut [ArrayNamedProperty]) { for prop in props.iter_mut() { crate::gc::runtime_write_barrier_external_slot( @@ -342,14 +363,32 @@ pub(crate) fn transfer_array_named_property_owner(old_owner: usize, new_owner: u ARRAY_NAMED_PROPS.with(|m| { let mut props = m.borrow_mut(); if let Some(old_props) = props.remove(&old_owner) { - for prop in &old_props { - note_array_named(new_owner, prop.value.to_bits()); - } merge_array_named_props(&mut props, new_owner, old_props); } }); } +pub(crate) fn scan_array_named_property_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { + ARRAY_NAMED_PROPS.with(|m| { + let mut props = m.borrow_mut(); + let mut moved = Vec::new(); + for (&owner, owner_props) in props.iter_mut() { + let mut new_owner = owner; + if visitor.visit_metadata_usize_slot(&mut new_owner) { + moved.push((owner, new_owner)); + } + for prop in owner_props.iter_mut() { + visitor.visit_nanbox_f64_slot(&mut prop.value); + } + } + for (old_owner, new_owner) in moved { + if let Some(old_props) = props.remove(&old_owner) { + merge_array_named_props(&mut props, new_owner, old_props); + } + } + }); +} + /// Remove named-property entries whose array owners are provably dead under /// the centralized collection-specific liveness policy. pub(crate) fn prune_dead_array_named_property_owners(is_dead_owner: &dyn Fn(usize) -> bool) { @@ -366,7 +405,6 @@ pub(crate) fn test_array_named_property_owner_exists(owner: usize) -> bool { #[cfg(test)] pub(crate) fn test_clear_array_named_property_roots() { ARRAY_NAMED_PROPS.with(|m| m.borrow_mut().clear()); - young_roots::clear_named_log(); } unsafe fn string_header_as_str<'a>(key: *const crate::StringHeader) -> Option<&'a str> { @@ -407,7 +445,6 @@ pub(crate) unsafe fn array_named_property_set( }; let owner = arr as usize; note_array_named_props_ever(); - note_array_named(owner, value.to_bits()); ARRAY_NAMED_PROPS.with(|m| { let mut map = m.borrow_mut(); let props = map.entry(owner).or_default(); @@ -441,10 +478,6 @@ pub(crate) unsafe fn array_named_props_install_fresh( return; } let owner = arr as usize; - note_array_named_props_ever(); - for (_, value) in entries { - note_array_named(owner, value.to_bits()); - } ARRAY_NAMED_PROPS.with(|m| { let mut map = m.borrow_mut(); let props = map.entry(owner).or_default(); @@ -601,7 +634,6 @@ pub(crate) unsafe fn array_named_property_delete_by_name( #[cfg(test)] pub(crate) fn test_seed_template_raw_roots(cooked: *mut ArrayHeader, raw: *mut ArrayHeader) { - note_template_raw(cooked as usize, raw); TEMPLATE_RAW_MAP.with(|m| { let mut m = m.borrow_mut(); m.clear(); diff --git a/crates/perry-runtime/src/array/header/young_roots.rs b/crates/perry-runtime/src/array/header/young_roots.rs deleted file mode 100644 index 946e428c0f..0000000000 --- a/crates/perry-runtime/src/array/header/young_roots.rs +++ /dev/null @@ -1,302 +0,0 @@ -//! Young-entry logs for tagged-template and array named-property roots. - -use super::*; - -crate::perry_thread_local! { - static TEMPLATE_CACHE_YOUNG: RefCell> = - const { RefCell::new(crate::gc::young_log::YoungLog::new()) }; - static TEMPLATE_RAW_YOUNG: RefCell> = - const { RefCell::new(crate::gc::young_log::YoungLog::new()) }; - static ARRAY_NAMED_YOUNG: RefCell> = - const { RefCell::new(crate::gc::young_log::YoungLog::new()) }; - #[cfg(test)] - static TEST_SUPPRESS_ARRAY_NAMED_YOUNG_NOTE: std::cell::Cell = - const { std::cell::Cell::new(false) }; - #[cfg(test)] - static TEST_SUPPRESS_TEMPLATE_RAW_YOUNG_NOTE: std::cell::Cell = - const { std::cell::Cell::new(false) }; -} - -const CACHE_LOG: &str = "array.template_object_cache"; -const RAW_LOG: &str = "array.template_raw_map"; -const NAMED_LOG: &str = "array.named_properties"; - -#[inline] -fn ptr_relevant(ptr: *mut ArrayHeader) -> bool { - crate::gc::young_log::addr_is_minor_relevant(ptr as usize) -} - -pub(super) fn note_template_cache(site: u64, cooked: *mut ArrayHeader, raw: *mut ArrayHeader) { - if ptr_relevant(cooked) || ptr_relevant(raw) { - TEMPLATE_CACHE_YOUNG.with(|log| log.borrow_mut().note(site)); - } -} - -pub(super) fn note_template_raw(cooked: usize, raw: *mut ArrayHeader) { - if crate::gc::young_log::addr_is_minor_relevant(cooked) || ptr_relevant(raw) { - #[cfg(test)] - if TEST_SUPPRESS_TEMPLATE_RAW_YOUNG_NOTE.with(std::cell::Cell::get) { - return; - } - TEMPLATE_RAW_YOUNG.with(|log| log.borrow_mut().note(cooked)); - } -} - -pub(super) fn note_array_named(owner: usize, value_bits: u64) { - if !crate::gc::young_log::addr_is_minor_collectible(owner) - && !crate::gc::young_log::bits_are_minor_relevant(value_bits) - { - return; - } - #[cfg(test)] - if TEST_SUPPRESS_ARRAY_NAMED_YOUNG_NOTE.with(std::cell::Cell::get) { - return; - } - ARRAY_NAMED_YOUNG.with(|log| log.borrow_mut().note(owner)); -} - -fn visit_cache_site(visitor: &mut crate::gc::RuntimeRootVisitor<'_>, site: u64) -> bool { - TEMPLATE_OBJECT_CACHE.with(|m| { - let mut map = m.borrow_mut(); - let Some((cooked, raw)) = map.get_mut(&site) else { - return false; - }; - visitor.visit_raw_mut_ptr_slot(cooked); - visitor.visit_raw_mut_ptr_slot(raw); - ptr_relevant(*cooked) || ptr_relevant(*raw) - }) -} - -fn visit_raw_owner(visitor: &mut crate::gc::RuntimeRootVisitor<'_>, owner: usize) -> Option { - TEMPLATE_RAW_MAP.with(|m| { - let mut map = m.borrow_mut(); - let mut raw = map.remove(&owner)?; - let mut new_owner = owner; - visitor.visit_usize_slot(&mut new_owner); - visitor.visit_raw_mut_ptr_slot(&mut raw); - map.insert(new_owner, raw); - (crate::gc::young_log::addr_is_minor_relevant(new_owner) || ptr_relevant(raw)) - .then_some(new_owner) - }) -} - -fn visit_named_owner( - visitor: &mut crate::gc::RuntimeRootVisitor<'_>, - owner: usize, -) -> Option { - ARRAY_NAMED_PROPS.with(|m| { - let mut map = m.borrow_mut(); - let mut props = map.remove(&owner)?; - let mut new_owner = owner; - visitor.visit_metadata_usize_slot(&mut new_owner); - let mut relevant = crate::gc::young_log::addr_is_minor_collectible(new_owner); - for prop in &mut props { - visitor.visit_nanbox_f64_slot(&mut prop.value); - relevant |= crate::gc::young_log::bits_are_minor_relevant(prop.value.to_bits()); - } - merge_array_named_props(&mut map, new_owner, props); - relevant.then_some(new_owner) - }) -} - -#[cfg(any(debug_assertions, test))] -fn relevant_cache_sites() -> Vec { - TEMPLATE_OBJECT_CACHE.with(|m| { - m.borrow() - .iter() - .filter_map(|(&site, &(cooked, raw))| { - (ptr_relevant(cooked) || ptr_relevant(raw)).then_some(site) - }) - .collect() - }) -} - -#[cfg(any(debug_assertions, test))] -fn relevant_raw_owners() -> Vec { - TEMPLATE_RAW_MAP.with(|m| { - m.borrow() - .iter() - .filter_map(|(&owner, &raw)| { - (crate::gc::young_log::addr_is_minor_relevant(owner) || ptr_relevant(raw)) - .then_some(owner) - }) - .collect() - }) -} - -#[cfg(any(debug_assertions, test))] -fn relevant_named_owners() -> Vec { - ARRAY_NAMED_PROPS.with(|m| { - m.borrow() - .iter() - .filter_map(|(&owner, props)| { - (crate::gc::young_log::addr_is_minor_collectible(owner) - || props.iter().any(|prop| { - crate::gc::young_log::bits_are_minor_relevant(prop.value.to_bits()) - })) - .then_some(owner) - }) - .collect() - }) -} - -fn drain_log( - log: &'static crate::tls_hot::HotKey>>, - mut visit: impl FnMut(K) -> Option, -) -> (u64, u64, u64) { - let mut logged = 0; - let mut visited = 0; - let mut kept = log.with(|log| log.borrow_mut().take_spare()); - loop { - let batch = log.with(|log| log.borrow_mut().take_sorted()); - if batch.is_empty() { - break; - } - logged += batch.len() as u64; - for key in batch { - visited += 1; - if let Some(key) = visit(key) { - kept.push(key); - } - } - } - let kept_len = kept.len() as u64; - log.with(|log| log.borrow_mut().extend(kept)); - (logged, visited, kept_len) -} - -fn report(name: &'static str, partial: bool, row: (u64, u64, u64), table_len: usize) { - crate::gc::young_log::note_walk( - name, - crate::gc::young_log::YoungLogWalk { - partial, - logged: row.0, - visited: row.1, - kept: row.2, - table_len: table_len as u64, - }, - ); -} - -pub fn scan_template_raw_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { - let cache_len = TEMPLATE_OBJECT_CACHE.with(|m| m.borrow().len()); - let raw_len = TEMPLATE_RAW_MAP.with(|m| m.borrow().len()); - let named_len = ARRAY_NAMED_PROPS.with(|m| m.borrow().len()); - if visitor.young_scope() { - #[cfg(any(debug_assertions, test))] - { - TEMPLATE_CACHE_YOUNG.with(|log| { - log.borrow() - .debug_assert_logged(CACHE_LOG, &relevant_cache_sites()) - }); - TEMPLATE_RAW_YOUNG.with(|log| { - log.borrow() - .debug_assert_logged(RAW_LOG, &relevant_raw_owners()) - }); - ARRAY_NAMED_YOUNG.with(|log| { - log.borrow() - .debug_assert_logged(NAMED_LOG, &relevant_named_owners()) - }); - } - let cache = drain_log(&TEMPLATE_CACHE_YOUNG, |site| { - visit_cache_site(visitor, site).then_some(site) - }); - let raw = drain_log(&TEMPLATE_RAW_YOUNG, |owner| visit_raw_owner(visitor, owner)); - let named = drain_log(&ARRAY_NAMED_YOUNG, |owner| { - visit_named_owner(visitor, owner) - }); - report(CACHE_LOG, true, cache, cache_len); - report(RAW_LOG, true, raw, raw_len); - report(NAMED_LOG, true, named, named_len); - return; - } - - let cache_sites: Vec = - TEMPLATE_OBJECT_CACHE.with(|m| m.borrow().keys().copied().collect()); - let raw_owners: Vec = TEMPLATE_RAW_MAP.with(|m| m.borrow().keys().copied().collect()); - let named_owners: Vec = ARRAY_NAMED_PROPS.with(|m| m.borrow().keys().copied().collect()); - let _ = TEMPLATE_CACHE_YOUNG.with(|log| log.borrow_mut().take_sorted()); - let _ = TEMPLATE_RAW_YOUNG.with(|log| log.borrow_mut().take_sorted()); - let _ = ARRAY_NAMED_YOUNG.with(|log| log.borrow_mut().take_sorted()); - // `drain_log` consumes the log, so seed it with the authoritative keys. - TEMPLATE_CACHE_YOUNG.with(|log| log.borrow_mut().extend(cache_sites)); - let cache = drain_log(&TEMPLATE_CACHE_YOUNG, |site| { - visit_cache_site(visitor, site).then_some(site) - }); - TEMPLATE_RAW_YOUNG.with(|log| log.borrow_mut().extend(raw_owners)); - let raw = drain_log(&TEMPLATE_RAW_YOUNG, |owner| visit_raw_owner(visitor, owner)); - ARRAY_NAMED_YOUNG.with(|log| log.borrow_mut().extend(named_owners)); - let named = drain_log(&ARRAY_NAMED_YOUNG, |owner| { - visit_named_owner(visitor, owner) - }); - report(CACHE_LOG, false, cache, cache_len); - report(RAW_LOG, false, raw, raw_len); - report(NAMED_LOG, false, named, named_len); -} - -#[cfg(test)] -pub(super) fn clear_named_log() { - ARRAY_NAMED_YOUNG.with(|log| log.borrow_mut().clear()); -} - -#[cfg(test)] -mod tests { - use super::*; - - fn alloc_empty_array() -> *mut ArrayHeader { - let arr = crate::arena::arena_alloc_gc( - std::mem::size_of::(), - std::mem::align_of::(), - crate::gc::GC_TYPE_ARRAY, - ) as *mut ArrayHeader; - unsafe { - (*arr).length = 0; - (*arr).capacity = 0; - } - arr - } - - #[test] - fn template_raw_log_rederivation_rejects_a_suppressed_writer() { - let _lock = crate::gc::global_side_table_test_lock(); - let cooked = alloc_empty_array(); - let raw = alloc_empty_array(); - TEST_SUPPRESS_TEMPLATE_RAW_YOUNG_NOTE.with(|flag| flag.set(true)); - test_seed_template_raw_roots(cooked, raw); - TEST_SUPPRESS_TEMPLATE_RAW_YOUNG_NOTE.with(|flag| flag.set(false)); - let missed = std::panic::catch_unwind(|| { - TEMPLATE_RAW_YOUNG.with(|log| { - log.borrow() - .debug_assert_logged(RAW_LOG, &relevant_raw_owners()) - }); - }); - TEMPLATE_RAW_MAP.with(|m| m.borrow_mut().clear()); - TEMPLATE_RAW_YOUNG.with(|log| log.borrow_mut().clear()); - assert!( - missed.is_err(), - "sabotage: suppressing the template-raw writer's note must trip completeness" - ); - } - - #[test] - fn array_named_log_rederivation_rejects_a_suppressed_setter() { - let _lock = crate::gc::global_side_table_test_lock(); - let arr = alloc_empty_array(); - let key = crate::string::js_string_from_bytes(b"sabotage".as_ptr(), 8); - TEST_SUPPRESS_ARRAY_NAMED_YOUNG_NOTE.with(|flag| flag.set(true)); - unsafe { array_named_property_set(arr, key, 7.0) }; - TEST_SUPPRESS_ARRAY_NAMED_YOUNG_NOTE.with(|flag| flag.set(false)); - let missed = std::panic::catch_unwind(|| { - ARRAY_NAMED_YOUNG.with(|log| { - log.borrow() - .debug_assert_logged(NAMED_LOG, &relevant_named_owners()) - }); - }); - ARRAY_NAMED_PROPS.with(|m| m.borrow_mut().remove(&(arr as usize))); - clear_named_log(); - assert!( - missed.is_err(), - "sabotage: suppressing array_named_property_set's note must trip completeness" - ); - } -} diff --git a/crates/perry-runtime/src/object/native_module/callable_exports/builtin_closure_metadata.rs b/crates/perry-runtime/src/object/native_module/callable_exports/builtin_closure_metadata.rs index fe1a2f75c6..a3d1ec5ee3 100644 --- a/crates/perry-runtime/src/object/native_module/callable_exports/builtin_closure_metadata.rs +++ b/crates/perry-runtime/src/object/native_module/callable_exports/builtin_closure_metadata.rs @@ -5,9 +5,15 @@ thread_local! { std::cell::RefCell::new(crate::fast_hash::new_ptr_hash_map()); static BUILTIN_CLOSURE_NON_CONSTRUCTABLE: std::cell::RefCell> = std::cell::RefCell::new(crate::fast_hash::new_ptr_hash_set()); +} + +crate::perry_thread_local! { static BUILTIN_CLOSURE_YOUNG: std::cell::RefCell> = const { std::cell::RefCell::new(crate::gc::young_log::YoungLog::new()) }; - #[cfg(test)] +} + +#[cfg(test)] +thread_local! { static TEST_SUPPRESS_BUILTIN_CLOSURE_YOUNG_NOTE: std::cell::Cell = const { std::cell::Cell::new(false) }; } diff --git a/crates/perry-runtime/src/symbol.rs b/crates/perry-runtime/src/symbol.rs index 57e17c12ee..3abb55867f 100644 --- a/crates/perry-runtime/src/symbol.rs +++ b/crates/perry-runtime/src/symbol.rs @@ -579,7 +579,6 @@ pub(crate) fn register_symbol_pointer(ptr: usize) { SYMBOL_EVER_REGISTERED.arm(); // Admit before the insert, for the same reason. admit_symbol_pointer(ptr); - gc_roots::note_symbol_pointer(ptr); let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_POINTERS); if guard.is_none() { *guard = Some(new_ptr_hash_set()); @@ -1027,7 +1026,6 @@ pub(crate) fn store_object_symbol_property_root( value_bits: u64, ) -> bool { note_symbol_key_installed(sym_key); - gc_roots::note_symbol_property_root(obj_key, sym_key, value_bits); { let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTIES); if guard.is_none() { @@ -1062,7 +1060,6 @@ pub(crate) static CLASS_STATIC_SYMBOLS_LATCH: crate::registry_latch::RegistryLat pub(crate) fn store_class_static_symbol_root(class_id: u32, sym_key: usize, value_bits: u64) { note_symbol_key_installed(sym_key); - gc_roots::note_class_static_symbol(class_id, sym_key, value_bits); CLASS_STATIC_SYMBOLS_LATCH.arm(); let symbol_id = unsafe { (*(sym_key as *const SymbolHeader)).id }; let created; diff --git a/crates/perry-runtime/src/symbol/accessors.rs b/crates/perry-runtime/src/symbol/accessors.rs index 9fb977e553..9ceeddbe3c 100644 --- a/crates/perry-runtime/src/symbol/accessors.rs +++ b/crates/perry-runtime/src/symbol/accessors.rs @@ -68,7 +68,6 @@ pub(crate) fn test_symbol_accessor_property_count() -> usize { #[cfg(test)] pub(crate) fn test_seed_symbol_accessor_property(obj_key: usize, sym_key: usize, get_bits: u64) { - super::gc_roots::note_symbol_accessor(obj_key, sym_key, get_bits, TAG_UNDEFINED); let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_ACCESSOR_PROPERTIES); guard.get_or_insert_with(HashMap::new).insert( (obj_key, sym_key), @@ -91,8 +90,6 @@ pub(crate) unsafe fn set_symbol_accessor_property( return; } crate::symbol::note_symbol_key_installed(sym_key); - super::gc_roots::note_symbol_property_root(obj_key, sym_key, crate::value::TAG_UNDEFINED); - super::gc_roots::note_symbol_accessor(obj_key, sym_key, get_bits, set_bits); { // `SYMBOL_PROPERTIES` is the only insertion-ordered record of symbol // property CREATION order, which `[[OwnPropertyKeys]]` must report @@ -222,29 +219,6 @@ pub(super) fn accessor_property_keys() -> Vec<(usize, usize)> { .unwrap_or_default() } -pub(super) fn accessor_property_count() -> usize { - let guard = crate::gc::lock_gc_root_registry(&SYMBOL_ACCESSOR_PROPERTIES); - guard.as_ref().map_or(0, HashMap::len) -} - -pub(super) fn relevant_accessor_property_keys() -> Vec<(usize, usize)> { - let guard = crate::gc::lock_gc_root_registry(&SYMBOL_ACCESSOR_PROPERTIES); - guard - .as_ref() - .map(|map| { - map.iter() - .filter_map(|(&(owner, sym_key), acc)| { - (crate::gc::young_log::addr_is_minor_collectible(owner) - || crate::gc::young_log::addr_is_minor_relevant(sym_key) - || crate::gc::young_log::bits_are_minor_relevant(acc.get) - || crate::gc::young_log::bits_are_minor_relevant(acc.set)) - .then_some((owner, sym_key)) - }) - .collect() - }) - .unwrap_or_default() -} - /// Step twin of `scan_symbol_accessor_roots_mut` for one snapshot key: /// strong-visits the get/set closures and rekeys owner/sym on a move. /// Cycle-based collections run ONLY the step scanner, so before this @@ -254,13 +228,13 @@ pub(super) fn scan_symbol_accessor_root_slot( visitor: &mut crate::gc::RuntimeRootVisitor<'_>, owner: usize, sym_key: usize, -) -> Option<(usize, usize)> { +) { let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_ACCESSOR_PROPERTIES); let Some(map) = guard.as_mut() else { - return None; + return; }; let Some(acc) = map.get_mut(&(owner, sym_key)) else { - return None; + return; }; let mut new_owner = owner; let mut new_sym_key = sym_key; @@ -277,21 +251,6 @@ pub(super) fn scan_symbol_accessor_root_slot( map.insert((new_owner, new_sym_key), acc); } } - symbol_accessor_root_relevant_in(map, new_owner, new_sym_key) - .then_some((new_owner, new_sym_key)) -} - -fn symbol_accessor_root_relevant_in( - map: &HashMap<(usize, usize), SymbolAccessorDescriptor>, - owner: usize, - sym_key: usize, -) -> bool { - map.get(&(owner, sym_key)).is_some_and(|acc| { - crate::gc::young_log::addr_is_minor_collectible(owner) - || crate::gc::young_log::addr_is_minor_relevant(sym_key) - || crate::gc::young_log::bits_are_minor_relevant(acc.get) - || crate::gc::young_log::bits_are_minor_relevant(acc.set) - }) } pub(super) fn has_own_symbol_accessor(obj_key: usize, sym_key: usize) -> bool { diff --git a/crates/perry-runtime/src/symbol/gc_roots.rs b/crates/perry-runtime/src/symbol/gc_roots.rs index 7fec611384..8c05be3014 100644 --- a/crates/perry-runtime/src/symbol/gc_roots.rs +++ b/crates/perry-runtime/src/symbol/gc_roots.rs @@ -21,16 +21,11 @@ pub fn scan_symbol_side_table_roots(mark: &mut dyn FnMut(f64)) { } pub fn scan_symbol_side_table_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { - if visitor.young_scope() { - scan_young_symbol_side_table_roots_mut(visitor); - return; - } scan_symbol_property_roots_mut(visitor); scan_symbol_property_attrs_mut(visitor); accessors::scan_symbol_accessor_roots_mut(visitor); scan_class_static_symbol_roots_mut(visitor); scan_symbol_pointer_metadata_roots_mut(visitor); - rebuild_symbol_young_log(); } fn scan_symbol_property_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { @@ -136,7 +131,7 @@ fn scan_symbol_pointer_metadata_roots_mut(visitor: &mut crate::gc::RuntimeRootVi } } -#[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd)] +#[derive(Clone, Copy)] enum SymbolSideTableRootSlot { SymbolPropertyOwner { owner: usize }, SymbolPropertyEntry { owner: usize, sym_key: usize }, @@ -146,83 +141,15 @@ enum SymbolSideTableRootSlot { SymbolPointer { ptr: usize }, } -crate::perry_thread_local! { - static SYMBOL_SIDE_TABLE_YOUNG: RefCell> = - const { RefCell::new(crate::gc::young_log::YoungLog::new()) }; - #[cfg(test)] - static TEST_SUPPRESS_SYMBOL_YOUNG_NOTE: std::cell::Cell = - const { std::cell::Cell::new(false) }; -} - -const SYMBOL_YOUNG_LOG_NAME: &str = "symbol.side_tables"; - -#[inline] -fn note_symbol_slot(slot: SymbolSideTableRootSlot) { - #[cfg(test)] - if TEST_SUPPRESS_SYMBOL_YOUNG_NOTE.with(std::cell::Cell::get) { - return; - } - SYMBOL_SIDE_TABLE_YOUNG.with(|log| log.borrow_mut().note(slot)); -} - -pub(super) fn note_symbol_property_root(owner: usize, sym_key: usize, value_bits: u64) { - if crate::gc::young_log::addr_is_minor_collectible(owner) { - note_symbol_slot(SymbolSideTableRootSlot::SymbolPropertyOwner { owner }); - } - if crate::gc::young_log::addr_is_minor_relevant(sym_key) - || crate::gc::young_log::bits_are_minor_relevant(value_bits) - { - note_symbol_slot(SymbolSideTableRootSlot::SymbolPropertyEntry { owner, sym_key }); - } -} - -pub(super) fn note_symbol_property_attrs(owner: usize, sym_key: usize) { - if crate::gc::young_log::addr_is_minor_collectible(owner) - || crate::gc::young_log::addr_is_minor_relevant(sym_key) - { - note_symbol_slot(SymbolSideTableRootSlot::SymbolPropertyAttrs { owner, sym_key }); - } -} - -pub(super) fn note_symbol_accessor(owner: usize, sym_key: usize, get_bits: u64, set_bits: u64) { - if crate::gc::young_log::addr_is_minor_collectible(owner) - || crate::gc::young_log::addr_is_minor_relevant(sym_key) - || crate::gc::young_log::bits_are_minor_relevant(get_bits) - || crate::gc::young_log::bits_are_minor_relevant(set_bits) - { - note_symbol_slot(SymbolSideTableRootSlot::SymbolAccessorProperty { owner, sym_key }); - } -} - -pub(super) fn note_class_static_symbol(class_id: u32, sym_key: usize, value_bits: u64) { - if crate::gc::young_log::addr_is_minor_relevant(sym_key) - || crate::gc::young_log::bits_are_minor_relevant(value_bits) - { - note_symbol_slot(SymbolSideTableRootSlot::ClassStaticSymbol { class_id, sym_key }); - } -} - -pub(super) fn note_symbol_pointer(ptr: usize) { - if crate::gc::young_log::addr_is_minor_collectible(ptr) { - note_symbol_slot(SymbolSideTableRootSlot::SymbolPointer { ptr }); - } -} - pub(crate) struct SymbolSideTableRootScanState { - slots: Option>, - kept: Vec, + slots: Vec, cursor: usize, - young: bool, - table_len: usize, } pub(crate) fn new_symbol_side_table_root_scan_state() -> Box { Box::new(SymbolSideTableRootScanState { - slots: None, - kept: Vec::new(), + slots: symbol_side_table_root_snapshot(), cursor: 0, - young: false, - table_len: 0, }) } @@ -234,49 +161,12 @@ pub(crate) fn scan_symbol_side_table_roots_mut_step( let state = state .downcast_mut::() .expect("symbol side-table root scanner state type"); - if state.slots.is_none() { - state.young = visitor.young_scope(); - if state.young { - state.table_len = symbol_side_table_root_len(); - #[cfg(any(debug_assertions, test))] - SYMBOL_SIDE_TABLE_YOUNG.with(|log| { - log.borrow() - .debug_assert_logged(SYMBOL_YOUNG_LOG_NAME, &relevant_symbol_slots()) - }); - state.slots = Some(SYMBOL_SIDE_TABLE_YOUNG.with(|log| log.borrow_mut().take_sorted())); - } else { - let authoritative = symbol_side_table_root_snapshot(); - state.table_len = authoritative.len(); - let _ = SYMBOL_SIDE_TABLE_YOUNG.with(|log| log.borrow_mut().take_sorted()); - state.slots = Some(authoritative); - } - } - let slots = state.slots.as_ref().expect("symbol slots initialized"); - while *remaining > 0 && state.cursor < slots.len() { - if let Some(slot) = scan_symbol_side_table_root_slot(visitor, slots[state.cursor]) { - state.kept.push(slot); - } + while *remaining > 0 && state.cursor < state.slots.len() { + scan_symbol_side_table_root_slot(visitor, state.slots[state.cursor]); state.cursor += 1; *remaining -= 1; } - let done = state.cursor >= slots.len(); - if done { - let logged = slots.len() as u64; - let kept = std::mem::take(&mut state.kept); - let kept_len = kept.len() as u64; - SYMBOL_SIDE_TABLE_YOUNG.with(|log| log.borrow_mut().extend(kept)); - crate::gc::young_log::note_walk( - SYMBOL_YOUNG_LOG_NAME, - crate::gc::young_log::YoungLogWalk { - partial: state.young, - logged, - visited: state.cursor as u64, - kept: kept_len, - table_len: state.table_len as u64, - }, - ); - } - done + state.cursor >= state.slots.len() } fn symbol_side_table_root_snapshot() -> Vec { @@ -328,163 +218,13 @@ fn symbol_side_table_root_snapshot() -> Vec { slots } -fn symbol_side_table_root_len() -> usize { - // Exact only for diagnostics: counting the property vectors is itself a - // whole-table walk, which the release minor must not pay merely to report - // how much work it skipped. - if !crate::gc::gc_diag_enabled() { - return 0; - } - let properties = { - let guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTIES); - guard.as_ref().map_or(0, |map| { - map.len() + map.values().map(Vec::len).sum::() - }) - }; - let attrs = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTY_ATTRS) - .as_ref() - .map_or(0, |map| map.len()); - let class_statics = crate::gc::lock_gc_root_registry(&CLASS_STATIC_SYMBOLS) - .as_ref() - .map_or(0, |map| map.len()); - let pointers = crate::gc::lock_gc_root_registry(&SYMBOL_POINTERS) - .as_ref() - .map_or(0, |set| set.len()); - properties + attrs + accessors::accessor_property_count() + class_statics + pointers -} - -fn collect_relevant_symbol_slots() -> Vec { - let mut slots = Vec::new(); - { - let guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTIES); - if let Some(map) = guard.as_ref() { - for (&owner, entries) in map { - if crate::gc::young_log::addr_is_minor_collectible(owner) { - slots.push(SymbolSideTableRootSlot::SymbolPropertyOwner { owner }); - } - for &(sym_key, value_bits) in entries { - if crate::gc::young_log::addr_is_minor_relevant(sym_key) - || crate::gc::young_log::bits_are_minor_relevant(value_bits) - { - slots.push(SymbolSideTableRootSlot::SymbolPropertyEntry { owner, sym_key }); - } - } - } - } - } - { - let guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTY_ATTRS); - if let Some(map) = guard.as_ref() { - slots.extend(map.keys().filter_map(|&(owner, sym_key)| { - (crate::gc::young_log::addr_is_minor_collectible(owner) - || crate::gc::young_log::addr_is_minor_relevant(sym_key)) - .then_some(SymbolSideTableRootSlot::SymbolPropertyAttrs { owner, sym_key }) - })); - } - } - slots.extend( - accessors::relevant_accessor_property_keys() - .into_iter() - .map( - |(owner, sym_key)| SymbolSideTableRootSlot::SymbolAccessorProperty { - owner, - sym_key, - }, - ), - ); - { - let guard = crate::gc::lock_gc_root_registry(&CLASS_STATIC_SYMBOLS); - if let Some(map) = guard.as_ref() { - slots.extend( - map.iter() - .filter_map(|(&(class_id, sym_key), &value_bits)| { - (crate::gc::young_log::addr_is_minor_relevant(sym_key) - || crate::gc::young_log::bits_are_minor_relevant(value_bits)) - .then_some(SymbolSideTableRootSlot::ClassStaticSymbol { class_id, sym_key }) - }), - ); - } - } - { - let guard = crate::gc::lock_gc_root_registry(&SYMBOL_POINTERS); - if let Some(set) = guard.as_ref() { - slots.extend(set.iter().filter_map(|&ptr| { - crate::gc::young_log::addr_is_minor_collectible(ptr) - .then_some(SymbolSideTableRootSlot::SymbolPointer { ptr }) - })); - } - } - slots -} - -#[cfg(any(debug_assertions, test))] -fn relevant_symbol_slots() -> Vec { - collect_relevant_symbol_slots() -} - -fn rebuild_symbol_young_log() { - let table_len = symbol_side_table_root_len(); - let relevant = collect_relevant_symbol_slots(); - let _ = SYMBOL_SIDE_TABLE_YOUNG.with(|log| log.borrow_mut().take_sorted()); - let kept = relevant.len() as u64; - SYMBOL_SIDE_TABLE_YOUNG.with(|log| log.borrow_mut().extend(relevant)); - crate::gc::young_log::note_walk( - SYMBOL_YOUNG_LOG_NAME, - crate::gc::young_log::YoungLogWalk { - partial: false, - logged: table_len as u64, - visited: table_len as u64, - kept, - table_len: table_len as u64, - }, - ); -} - -fn scan_young_symbol_side_table_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { - let table_len = symbol_side_table_root_len(); - #[cfg(any(debug_assertions, test))] - SYMBOL_SIDE_TABLE_YOUNG.with(|log| { - log.borrow() - .debug_assert_logged(SYMBOL_YOUNG_LOG_NAME, &relevant_symbol_slots()) - }); - let mut kept = SYMBOL_SIDE_TABLE_YOUNG.with(|log| log.borrow_mut().take_spare()); - let mut logged = 0_u64; - loop { - let batch = SYMBOL_SIDE_TABLE_YOUNG.with(|log| log.borrow_mut().take_sorted()); - if batch.is_empty() { - break; - } - logged += batch.len() as u64; - for slot in batch { - if let Some(slot) = scan_symbol_side_table_root_slot(visitor, slot) { - kept.push(slot); - } - } - } - let kept_len = kept.len() as u64; - SYMBOL_SIDE_TABLE_YOUNG.with(|log| log.borrow_mut().extend(kept)); - crate::gc::young_log::note_walk( - SYMBOL_YOUNG_LOG_NAME, - crate::gc::young_log::YoungLogWalk { - partial: true, - logged, - visited: logged, - kept: kept_len, - table_len: table_len as u64, - }, - ); -} - fn scan_symbol_side_table_root_slot( visitor: &mut crate::gc::RuntimeRootVisitor<'_>, slot: SymbolSideTableRootSlot, -) -> Option { +) { match slot { SymbolSideTableRootSlot::SymbolPropertyOwner { owner } => { - rewrite_symbol_property_owner_if_forwarded(visitor, owner).and_then(|owner| { - crate::gc::young_log::addr_is_minor_collectible(owner) - .then_some(SymbolSideTableRootSlot::SymbolPropertyOwner { owner }) - }) + rewrite_symbol_property_owner_if_forwarded(visitor, owner); } SymbolSideTableRootSlot::SymbolPropertyEntry { owner, sym_key } => { // The preceding budget slice may already have rekeyed this @@ -495,7 +235,7 @@ fn scan_symbol_side_table_root_slot( visitor.visit_metadata_usize_slot(&mut healed_owner); let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTIES); let Some(map) = guard.as_mut() else { - return None; + return; }; let lookup_owner = if map.contains_key(&healed_owner) { healed_owner @@ -506,43 +246,22 @@ fn scan_symbol_side_table_root_slot( .get_mut(&lookup_owner) .and_then(|entries| entries.iter_mut().find(|entry| entry.0 == sym_key)) else { - return None; + return; }; visitor.visit_usize_slot(entry_sym); visitor.visit_nanbox_u64_slot(value_bits); - (crate::gc::young_log::addr_is_minor_relevant(*entry_sym) - || crate::gc::young_log::bits_are_minor_relevant(*value_bits)) - .then_some(SymbolSideTableRootSlot::SymbolPropertyEntry { - owner: lookup_owner, - sym_key: *entry_sym, - }) } SymbolSideTableRootSlot::SymbolAccessorProperty { owner, sym_key } => { - accessors::scan_symbol_accessor_root_slot(visitor, owner, sym_key).map( - |(owner, sym_key)| SymbolSideTableRootSlot::SymbolAccessorProperty { - owner, - sym_key, - }, - ) + accessors::scan_symbol_accessor_root_slot(visitor, owner, sym_key); } SymbolSideTableRootSlot::SymbolPropertyAttrs { owner, sym_key } => { - rewrite_symbol_property_attrs_if_forwarded(visitor, owner, sym_key).and_then( - |(owner, sym_key)| { - (crate::gc::young_log::addr_is_minor_collectible(owner) - || crate::gc::young_log::addr_is_minor_relevant(sym_key)) - .then_some(SymbolSideTableRootSlot::SymbolPropertyAttrs { owner, sym_key }) - }, - ) + rewrite_symbol_property_attrs_if_forwarded(visitor, owner, sym_key); } SymbolSideTableRootSlot::ClassStaticSymbol { class_id, sym_key } => { - rewrite_class_static_symbol_entry_if_forwarded(visitor, class_id, sym_key) - .map(|sym_key| SymbolSideTableRootSlot::ClassStaticSymbol { class_id, sym_key }) + rewrite_class_static_symbol_entry_if_forwarded(visitor, class_id, sym_key); } SymbolSideTableRootSlot::SymbolPointer { ptr } => { - rewrite_symbol_pointer_metadata_if_forwarded(visitor, ptr).and_then(|ptr| { - crate::gc::young_log::addr_is_minor_collectible(ptr) - .then_some(SymbolSideTableRootSlot::SymbolPointer { ptr }) - }) + rewrite_symbol_pointer_metadata_if_forwarded(visitor, ptr); } } } @@ -550,41 +269,37 @@ fn scan_symbol_side_table_root_slot( fn rewrite_symbol_property_owner_if_forwarded( visitor: &mut crate::gc::RuntimeRootVisitor<'_>, owner: usize, -) -> Option { +) { let mut new_owner = owner; - if visitor.visit_metadata_usize_slot(&mut new_owner) && new_owner != owner { - let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTIES); - if let Some(map) = guard.as_mut() { - if let Some(entries) = map.remove(&owner) { - match map.entry(new_owner) { - std::collections::hash_map::Entry::Occupied(mut entry) => { - merge_symbol_property_entries(entry.get_mut(), entries); - } - std::collections::hash_map::Entry::Vacant(entry) => { - entry.insert(entries); - } + if !visitor.visit_metadata_usize_slot(&mut new_owner) || new_owner == owner { + return; + } + let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTIES); + if let Some(map) = guard.as_mut() { + if let Some(entries) = map.remove(&owner) { + match map.entry(new_owner) { + std::collections::hash_map::Entry::Occupied(mut entry) => { + merge_symbol_property_entries(entry.get_mut(), entries); + } + std::collections::hash_map::Entry::Vacant(entry) => { + entry.insert(entries); } } } } - let guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTIES); - guard - .as_ref() - .is_some_and(|map| map.contains_key(&new_owner)) - .then_some(new_owner) } fn rewrite_symbol_property_attrs_if_forwarded( visitor: &mut crate::gc::RuntimeRootVisitor<'_>, owner: usize, sym_key: usize, -) -> Option<(usize, usize)> { +) { let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTY_ATTRS); let Some(map) = guard.as_mut() else { - return None; + return; }; if !map.contains_key(&(owner, sym_key)) { - return None; + return; } let mut new_owner = owner; let mut new_sym_key = sym_key; @@ -595,21 +310,19 @@ fn rewrite_symbol_property_attrs_if_forwarded( map.insert((new_owner, new_sym_key), attrs); } } - map.contains_key(&(new_owner, new_sym_key)) - .then_some((new_owner, new_sym_key)) } fn rewrite_class_static_symbol_entry_if_forwarded( visitor: &mut crate::gc::RuntimeRootVisitor<'_>, class_id: u32, sym_key: usize, -) -> Option { +) { let mut guard = crate::gc::lock_gc_root_registry(&CLASS_STATIC_SYMBOLS); let Some(map) = guard.as_mut() else { - return None; + return; }; let Some(value_bits) = map.get_mut(&(class_id, sym_key)) else { - return None; + return; }; let mut new_sym_key = sym_key; let moved = visitor.visit_usize_slot(&mut new_sym_key); @@ -619,37 +332,27 @@ fn rewrite_class_static_symbol_entry_if_forwarded( map.insert((class_id, new_sym_key), value_bits); } } - map.get(&(class_id, new_sym_key)).and_then(|value_bits| { - (crate::gc::young_log::addr_is_minor_relevant(new_sym_key) - || crate::gc::young_log::bits_are_minor_relevant(*value_bits)) - .then_some(new_sym_key) - }) } fn rewrite_symbol_pointer_metadata_if_forwarded( visitor: &mut crate::gc::RuntimeRootVisitor<'_>, ptr: usize, -) -> Option { +) { let mut new_ptr = ptr; - if visitor.visit_metadata_usize_slot(&mut new_ptr) && new_ptr != ptr { - let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_POINTERS); - if let Some(set) = guard.as_mut() { - set.remove(&ptr); - if new_ptr != 0 { - insert_symbol_pointer_in_set(set, new_ptr); - } + if !visitor.visit_metadata_usize_slot(&mut new_ptr) || new_ptr == ptr { + return; + } + let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_POINTERS); + if let Some(set) = guard.as_mut() { + set.remove(&ptr); + if new_ptr != 0 { + insert_symbol_pointer_in_set(set, new_ptr); } } - let guard = crate::gc::lock_gc_root_registry(&SYMBOL_POINTERS); - guard - .as_ref() - .is_some_and(|set| set.contains(&new_ptr)) - .then_some(new_ptr) } #[cfg(test)] pub(crate) fn test_clear_symbol_side_table_roots() { - SYMBOL_SIDE_TABLE_YOUNG.with(|log| log.borrow_mut().clear()); *crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTIES) = None; *crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTY_ATTRS) = None; *crate::gc::lock_gc_root_registry(&CLASS_STATIC_SYMBOLS) = None; @@ -676,7 +379,6 @@ pub(crate) fn test_clear_symbol_side_table_roots() { } else { let mut set = new_ptr_hash_set(); for ptr in persistent { - note_symbol_pointer(ptr); insert_symbol_pointer_in_set(&mut set, ptr); } *guard = Some(set); @@ -723,7 +425,6 @@ pub(crate) fn test_seed_class_static_symbol_root(class_id: u32, sym_key: usize, // an unaligned sentinel. Seed only the root table they exercise; // production registration additionally reads SymbolHeader::id for // [[OwnPropertyKeys]] ordering and therefore requires a real Symbol. - note_class_static_symbol(class_id, sym_key, value_bits); CLASS_STATIC_SYMBOLS_LATCH.arm(); let mut guard = crate::gc::lock_gc_root_registry(&CLASS_STATIC_SYMBOLS); if guard.is_none() { @@ -773,31 +474,3 @@ pub(crate) fn test_symbol_pointer_root_contains(ptr: usize) -> bool { let guard = crate::gc::lock_gc_root_registry(&SYMBOL_POINTERS); guard.as_ref().is_some_and(|set| set.contains(&ptr)) } - -#[cfg(test)] -mod young_log_sabotage_tests { - use super::*; - - #[test] - fn symbol_log_rederivation_rejects_a_suppressed_property_writer() { - let _lock = crate::gc::global_side_table_test_lock(); - test_clear_symbol_side_table_roots(); - let owner = crate::object::js_object_alloc(0, 0) as usize; - let sym_bits = unsafe { crate::symbol::js_symbol_new_empty() }.to_bits(); - let sym_key = (sym_bits & POINTER_MASK) as usize; - TEST_SUPPRESS_SYMBOL_YOUNG_NOTE.with(|flag| flag.set(true)); - store_object_symbol_property_root(owner, sym_key, 7.0_f64.to_bits()); - TEST_SUPPRESS_SYMBOL_YOUNG_NOTE.with(|flag| flag.set(false)); - let missed = std::panic::catch_unwind(|| { - SYMBOL_SIDE_TABLE_YOUNG.with(|log| { - log.borrow() - .debug_assert_logged(SYMBOL_YOUNG_LOG_NAME, &relevant_symbol_slots()) - }); - }); - test_clear_symbol_side_table_roots(); - assert!( - missed.is_err(), - "sabotage: suppressing the property-store note must trip completeness" - ); - } -} diff --git a/crates/perry-runtime/src/symbol/properties.rs b/crates/perry-runtime/src/symbol/properties.rs index 49adaada16..ddcbcf4f32 100644 --- a/crates/perry-runtime/src/symbol/properties.rs +++ b/crates/perry-runtime/src/symbol/properties.rs @@ -90,7 +90,6 @@ pub(crate) fn set_symbol_property_attrs( return; } super::note_symbol_key_installed(sym_key); - super::gc_roots::note_symbol_property_attrs(owner, sym_key); let mut guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTY_ATTRS); if guard.is_none() { *guard = Some(crate::fast_hash::new_fast_key_hash_map()); diff --git a/scripts/gc_rekeyed_key_tables.json b/scripts/gc_rekeyed_key_tables.json index b31756956e..09687911b2 100644 --- a/scripts/gc_rekeyed_key_tables.json +++ b/scripts/gc_rekeyed_key_tables.json @@ -104,10 +104,10 @@ "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", + "site": "crates/perry-runtime/src/object/native_module/callable_exports/builtin_closure_metadata.rs::visit_owner", "table": "BUILTIN_CLOSURE_LENGTH / BUILTIN_CLOSURE_NON_CONSTRUCTABLE", "death": "dead_owner:prune_dead_builtin_closure_metadata_owners", - "why": "#8393: both tables are retained on the GC_TYPE_CLOSURE-narrowed predicate over their closure-address keys." + "why": "#8393: both tables are retained on the GC_TYPE_CLOSURE-narrowed predicate over their closure-address keys. The extracted visit_owner helper is the shared rekey path for the full and young-log walks." }, { "site": "crates/perry-runtime/src/object/shapes.rs::scan_shape_table_rekey_mut", diff --git a/scripts/thread_local_cold_allowlist.json b/scripts/thread_local_cold_allowlist.json index b27978e5db..e566b4aa29 100644 --- a/scripts/thread_local_cold_allowlist.json +++ b/scripts/thread_local_cold_allowlist.json @@ -1,6 +1,6 @@ { "_comment": "Files still declaring raw `thread_local!`. The count is the number of DECLARATIONS that survive into a shipping build \u2014 each one pays `_tlv_get_addr` per read on Darwin \u2014 and it is a ratchet, so adding a `static` to an already-listed file fails whether or not it opens a new block. New code should use `crate::perry_thread_local!` \u2014 see crates/perry-runtime/src/tls_hot.rs. Regenerate with scripts/check_thread_locals.py --update.", - "_hot_declarations": 405, + "_hot_declarations": 411, "files": { "crates/perry-runtime/src/agent.rs": 1, "crates/perry-runtime/src/arena/block.rs": 5, @@ -64,7 +64,8 @@ "crates/perry-runtime/src/node_submodules/test_once_unit_tests.rs": 2, "crates/perry-runtime/src/node_submodules/test_property.rs": 1, "crates/perry-runtime/src/node_submodules/trace_events.rs": 8, - "crates/perry-runtime/src/object/native_module/callable_exports.rs": 3, + "crates/perry-runtime/src/object/native_module/callable_exports.rs": 1, + "crates/perry-runtime/src/object/native_module/callable_exports/builtin_closure_metadata.rs": 2, "crates/perry-runtime/src/object/spill.rs": 1, "crates/perry-runtime/src/os/os_process_emitter.rs": 1, "crates/perry-runtime/src/os_process_streams.rs": 3, From dd279a8ba2de6b86a044d1074955e87fccf76d53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 15:24:16 +0200 Subject: [PATCH 07/11] perf(gc): prune per-object layout tables from a young-entry log on a minor A minor can only remove nursery owners, so walk the per-object layout tables' young-entry log rather than every standing key. Full collections retain the whole-table prune and rebuild the log from survivors. Replay 19a6cd201 on the #9957 phase-instrument tree. Its runtime hunks are unchanged; the young-log test file keeps both the newer fixed-cost scanner tests and the replayed layout-prune tests. Claude-Session: https://claude.ai/code/session_011dhBmdn4vGgNibjo3oZqTo --- changelog.d/9841-layout-prune-young-log.md | 43 ++++ crates/perry-runtime/src/gc/dead_owner.rs | 2 +- crates/perry-runtime/src/gc/layout.rs | 16 +- crates/perry-runtime/src/gc/layout_tables.rs | 230 +++++++++++++++++- .../src/gc/tests/young_log_tests.rs | 131 ++++++++++ crates/perry-runtime/src/gc/young_log.rs | 7 +- 6 files changed, 417 insertions(+), 12 deletions(-) create mode 100644 changelog.d/9841-layout-prune-young-log.md diff --git a/changelog.d/9841-layout-prune-young-log.md b/changelog.d/9841-layout-prune-young-log.md new file mode 100644 index 0000000000..5804e50beb --- /dev/null +++ b/changelog.d/9841-layout-prune-young-log.md @@ -0,0 +1,43 @@ +**A minor's per-object layout death-prune now walks a young-entry log instead +of both tables** — on a compiled claude-code streamed reply it visited +**6,792,375 entries where at most 125,367 (1.85 %) could possibly have died, +and on 37 % of minors nothing could have died at all.** + +`prune_dead_per_object_layout_owners` asks "which owners died?" of every key +in `LAYOUT_SLOT_MASKS` + `TYPED_LAYOUTS`, tables sized by everything the +program ever created (~66k live keys on cc, from a history far larger). But +both of a minor's deadness predicates require the owner to be in the nursery: +`owner_is_dead_copied_minor_from_space` demands eden or the active survivor +half, and `PostTraceProbe::owner_is_dead` on a minor demands an in-arena, +untenured `HeapGeneration::Nursery` address. An owner that was old at the last +prune is still old, so the walk over it cannot remove anything. + +So the two maps get the young-entry log of #9754 (`gc/young_log.rs`): every +writer notes a key whose owner `layout_key_may_be_nursery` admits before the +entry becomes findable, a minor prunes from the log, and a survivor is +re-logged only while it is still young — a promoted owner leaves the log and +no later minor visits it again. A full prune keeps its whole-table walk (old +owners do die in a full trace) and rebuilds the log from the survivors it is +already classifying, at no extra pass. + +**Why this table pays where the scanners of #9754 did not.** Read back per +table on an unmodified binary, that PR's four converted tables are a net 0.78x +on cc and `closure.dynamic_props` is a 2.56x regression, because a scanner +keeps `addr_is_minor_relevant` — true for `Longlived` **by design**, since a +longlived object can point at a young one — and cc allocates its shape-key +arrays longlived, so those logs never drain (`kept/logged` median 1.000). A +prune's predicate is `layout_key_may_be_nursery`, which excludes `Longlived` +**and** `Old`; cc's tenuring promotes every survivor after one survival, so a +key leaves this log after one minor. Same mechanism, opposite sign, decided +entirely by which predicate the walk keeps on. The measured over-visit is 54x +at a 3300-character reply and 25x at 400, with `dead <= young_before` on +152/152 minor prunes — the empirical proof that the log's predicate is a sound +superset of what a minor can kill. + +Rule 2 of the design travels with it: under `debug_assertions` the young prune +re-derives the candidate set from the authoritative maps and panics on any +young key the log does not name, so deleting an arming site is a red test +rather than a dead owner's record surviving in silence. The in-borrow mask +mint in `layout_note_slot` — the dominant insert path on cc, and the one site +that published a young record without counting it — is armed for the first +time here. diff --git a/crates/perry-runtime/src/gc/dead_owner.rs b/crates/perry-runtime/src/gc/dead_owner.rs index 2eeff26bbe..126ce3e938 100644 --- a/crates/perry-runtime/src/gc/dead_owner.rs +++ b/crates/perry-runtime/src/gc/dead_owner.rs @@ -354,7 +354,7 @@ pub(super) const DEAD_KEY_PRUNES: &[DeadKeyPrune] = &[ table: "LAYOUT_SLOT_MASKS + TYPED_LAYOUTS", owner: DeadKeyOwner::Any, prune: crate::gc::layout_tables::prune_dead_per_object_layout_owners, - young_prune: None, + young_prune: Some(crate::gc::layout_tables::prune_dead_per_object_layout_owners_young), }, // Re-keyed by the per-object move hook, not by a metadata visitor. DeadKeyPrune { diff --git a/crates/perry-runtime/src/gc/layout.rs b/crates/perry-runtime/src/gc/layout.rs index 5ee4f87cb7..74ae34248f 100644 --- a/crates/perry-runtime/src/gc/layout.rs +++ b/crates/perry-runtime/src/gc/layout.rs @@ -954,11 +954,23 @@ pub(crate) fn layout_note_slot(parent_user: usize, slot_index: usize, value_bits } else { let mut mask = LayoutSlotMask::Inline(0); mask.set_slot(slot_index); + // The one insert site that holds its own `borrow_mut`, + // so it maintains the address filter, the young log + // and the young-record count inline too. The log lives + // in the hint, not in this map, so arming it here + // takes no second borrow — and it goes BEFORE the + // insert (`gc/young_log.rs` rule 1). Before #9841 this + // site published a young record without counting it; + // on cc it is the DOMINANT insert path (`TYPED_LAYOUTS` + // is empty there), so it is where a missing arm would + // do the most damage. + let young = super::layout_tables::arm_young_layout_key(parent_user); masks.insert(parent_user, mask); mark_per_object_layouts_nonempty(); - // The one insert site that holds its own `borrow_mut`, - // so it maintains the address filter inline too. super::layout_tables::layout_addr_filter_note(parent_user); + if young { + super::layout_tables::count_new_young_layout_record(); + } set_layout_state(header, GC_LAYOUT_SIDE_MASK); } } else { diff --git a/crates/perry-runtime/src/gc/layout_tables.rs b/crates/perry-runtime/src/gc/layout_tables.rs index 420c2b1df0..97da20152f 100644 --- a/crates/perry-runtime/src/gc/layout_tables.rs +++ b/crates/perry-runtime/src/gc/layout_tables.rs @@ -76,6 +76,16 @@ pub(in crate::gc) struct PerObjectLayoutHint { /// on every new nursery-keyed insert; made exact again by /// [`recount_young_layout_records`] after each collection's death prune. pub(in crate::gc) young_records: Cell, + /// #9754-style young-entry log for BOTH per-object maps + /// (`gc/young_log.rs`): the keys whose owner may still sit on a page a + /// minor can act on. A minor's death prune walks this instead of the + /// maps — an owner that was old at the last prune is still old, so only + /// a logged key can be found dead by a minor. + /// + /// It lives here, in the same hot slot as the flag and the filter, so a + /// writer arms it with the thread-local resolution it has already paid + /// for, and so nothing new is declared for `tls_hot::fill` to resolve. + pub(in crate::gc) young_keys: RefCell>, } impl PerObjectLayoutHint { @@ -85,10 +95,14 @@ impl PerObjectLayoutHint { sets: Cell::new(0), filter: std::cell::UnsafeCell::new([0u64; LAYOUT_ADDR_FILTER_WORDS]), young_records: Cell::new(0), + young_keys: RefCell::new(crate::gc::young_log::YoungLog::new()), } } } +/// The `[gc-young-log]` / `young_log::last_walk` row name for the two maps. +pub(in crate::gc) const LAYOUT_YOUNG_LOG_NAME: &str = "gc.layout_tables"; + impl Drop for PerObjectLayoutHint { fn drop(&mut self) { // The ownership bit and its teardown live in this ONE TLS value. The @@ -155,12 +169,28 @@ fn layout_key_may_be_nursery(addr: usize) -> bool { ) } -/// A NEW per-object record was keyed by `user_ptr`. +/// A per-object record is ABOUT to be keyed by `user_ptr`: if the owner sits +/// where a minor could kill it, log the key. Rule 1 of `gc/young_log.rs` — +/// note BEFORE the entry is findable. Returns that youngness so the caller can +/// bump the young-record count once it knows the insert was fresh, without a +/// second classification. #[inline] -fn note_new_layout_record(user_ptr: usize) { +pub(in crate::gc) fn arm_young_layout_key(user_ptr: usize) -> bool { if !layout_key_may_be_nursery(user_ptr) { - return; + return false; } + hot_per_object_layout_hint() + .young_keys + .borrow_mut() + .note(user_ptr); + true +} + +/// A NEW nursery-keyed record was published: keep the inline allocator's gate +/// ([`PERRY_YOUNG_LAYOUT_RECORDS`]) conservative until the next prune makes it +/// exact. +#[inline] +pub(in crate::gc) fn count_new_young_layout_record() { let hint = hot_per_object_layout_hint(); if let Some(next) = hint.young_records.get().checked_add(1) { hint.young_records.set(next); @@ -168,6 +198,33 @@ fn note_new_layout_record(user_ptr: usize) { } } +/// The flag proved BOTH maps empty, so every key the log still names is +/// stale. Dropping them here is what keeps the log bounded: a prune that +/// early-returns on the emptiness proof never drains it, so a workload that +/// repeatedly fills and empties the maps between collections would otherwise +/// accumulate one dead key per insert for ever. +#[cold] +fn drop_stale_young_layout_log() { + hot_per_object_layout_hint().young_keys.borrow_mut().clear(); +} + +/// A record is being re-keyed to `new_user` by the per-object move hook +/// (`transfer_per_object_*`), which runs during evacuation — i.e. BEFORE the +/// copied minor's prune, so the key this notes is one the prune will classify +/// in this very collection. +/// +/// Logged unconditionally: the destination is a to-space survivor (young), a +/// promoted address (old), or mid-evacuation not yet classifiable. Noting it +/// without asking is correct (the prune classifies once and an old key simply +/// drops) and keeps a page-map probe out of the evacuation loop. +#[inline] +fn arm_moved_layout_key(new_user: usize) { + hot_per_object_layout_hint() + .young_keys + .borrow_mut() + .note(new_user); +} + /// Publish this thread's young-record count and the delta to the process /// total. The count itself is derived by the death prune's single pass over /// the live keys (all cycle kinds), so promotion (a key moving to an old page) @@ -192,6 +249,7 @@ fn publish_young_layout_records(live: u32) { /// inline allocator's gate reads that instead of probing. pub(in crate::gc) fn prune_dead_per_object_layout_owners(is_dead_owner: &dyn Fn(usize) -> bool) { if !per_object_layouts_maybe_nonempty() { + drop_stale_young_layout_log(); return; } // ONE pass over each table, not three. The old shape visited every live @@ -217,6 +275,12 @@ pub(in crate::gc) fn prune_dead_per_object_layout_owners(is_dead_owner: &dyn Fn( layout_addr_filter_saturate(); } let mut young: u32 = 0; + // A full walk is authoritative, so it also REBUILDS the young log — from + // the survivors it is classifying anyway, at the cost of one `push` per + // young key and no extra pass (`young_log.rs`: "a full-scope scanner + // walks the whole table as before and REBUILDS the log from what it + // found"). + let mut kept = hint.young_keys.borrow_mut().take_spare(); let mut keep = |key: usize| { if is_dead_owner(key) { return false; @@ -233,6 +297,7 @@ pub(in crate::gc) fn prune_dead_per_object_layout_owners(is_dead_owner: &dyn Fn( } if layout_key_may_be_nursery(key) { young = young.saturating_add(1); + kept.push(key); } true }; @@ -248,6 +313,23 @@ pub(in crate::gc) fn prune_dead_per_object_layout_owners(is_dead_owner: &dyn Fn( typed.retain(|key, _| keep(*key)); had && typed.is_empty() }; + // A full walk is authoritative: rebuild the young log from the tables + // (same shape as the shape/descriptor full scanners). + { + let mut log = hint.young_keys.borrow_mut(); + let _ = log.take_sorted(); + log.extend(kept); + } + crate::gc::young_log::note_walk( + LAYOUT_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: false, + logged: occupancy as u64, + visited: occupancy as u64, + kept: u64::from(young), + table_len: occupancy as u64, + }, + ); publish_young_layout_records(young); // Runs last: when it finds both tables empty it disarms the flag, zeroes // the young count published above and clears the filter, which is the @@ -258,6 +340,134 @@ pub(in crate::gc) fn prune_dead_per_object_layout_owners(is_dead_owner: &dyn Fn( } } +/// [`prune_dead_per_object_layout_owners`] for a MINOR (`DEAD_KEY_PRUNES` +/// `young_prune`). +/// +/// # Why this is sound +/// +/// A minor's two deadness predicates both require the owner to be in the +/// nursery: `owner_is_dead_copied_minor_from_space` demands eden or the active +/// survivor half, and `PostTraceProbe::owner_is_dead` on a minor demands an +/// in-arena, untenured `HeapGeneration::Nursery` address. So the only keys a +/// minor can remove are the ones [`layout_key_may_be_nursery`] admits — which +/// is a strict SUPERSET of both (it also admits an unclassifiable address, and +/// classifies from the same page map). Every writer notes such a key before +/// the entry becomes findable, and every walk re-logs a survivor that is still +/// young, so the log names every candidate and the walk loses nothing. +/// +/// That predicate is the whole difference between this conversion and the +/// scanner conversions of #9754: a scanner keeps `addr_is_minor_relevant`, +/// which admits `Longlived` **by design** (a longlived object can point at a +/// young one), whereas a prune asks who DIED and therefore excludes +/// `Longlived` and `Old` both. +/// +/// A logged key that is in neither map is stale (moved away, removed) and +/// drops; a present key whose owner is dead is removed from both maps; a live +/// key is re-logged iff its owner is still young, so a promoted owner leaves +/// the log and no later minor visits it again. +/// +/// The address filter is NOT rebuilt here — the whole-table walk that rebuilt +/// it is exactly what this replaces. Its `false` is the only load-bearing +/// answer and a stale set bit is a false positive, so leaving bits behind is +/// safe; the amortised rebuild in [`layout_addr_filter_add`] and the full +/// prune keep it selective. +pub(in crate::gc) fn prune_dead_per_object_layout_owners_young( + is_dead_owner: &dyn Fn(usize) -> bool, +) { + if !per_object_layouts_maybe_nonempty() { + drop_stale_young_layout_log(); + return; + } + let hint = hot_per_object_layout_hint(); + let table_len = + (hot_layout_slot_masks().borrow().len() + hot_typed_layouts().borrow().len()) as u64; + // Rule 2 (`gc/young_log.rs`): re-derive the candidate set from the + // authoritative maps and refuse to run a partial walk that would miss one. + // A miss is a writer that published a young-keyed record without arming + // the log, which in release would silently keep a dead owner's record. + #[cfg(debug_assertions)] + { + let relevant: Vec = { + let masks = hot_layout_slot_masks().borrow(); + let typed = hot_typed_layouts().borrow(); + masks + .keys() + .chain(typed.keys()) + .copied() + .filter(|key| layout_key_may_be_nursery(*key)) + .collect() + }; + hint.young_keys + .borrow() + .debug_assert_logged(LAYOUT_YOUNG_LOG_NAME, &relevant); + } + let mut logged = 0u64; + let mut visited = 0u64; + // The record count is per MAP ENTRY, as the full prune counts it: a key + // present in both maps is two records and one log entry. + let mut young: u32 = 0; + let mut kept = hint.young_keys.borrow_mut().take_spare(); + let (masks_emptied, typed_emptied) = { + let mut masks = hot_layout_slot_masks().borrow_mut(); + let mut typed = hot_typed_layouts().borrow_mut(); + let had_masks = !masks.is_empty(); + let had_typed = !typed.is_empty(); + loop { + // Re-drained in a loop so a note made while this walk runs (the + // move hooks fire from inside a collection) is not lost. + let batch = hint.young_keys.borrow_mut().take_sorted(); + if batch.is_empty() { + break; + } + logged += batch.len() as u64; + for key in batch { + let in_masks = masks.contains_key(&key); + let in_typed = typed.contains_key(&key); + if !in_masks && !in_typed { + continue; + } + visited += 1; + if is_dead_owner(key) { + if in_masks { + masks.remove(&key); + } + if in_typed { + typed.remove(&key); + } + continue; + } + if layout_key_may_be_nursery(key) { + young = young + .saturating_add(u32::from(in_masks)) + .saturating_add(u32::from(in_typed)); + kept.push(key); + } + } + } + (had_masks && masks.is_empty(), had_typed && typed.is_empty()) + }; + let kept_len = kept.len() as u64; + hint.young_keys.borrow_mut().extend(kept); + crate::gc::young_log::note_walk( + LAYOUT_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: true, + logged, + visited, + kept: kept_len, + table_len, + }, + ); + publish_young_layout_records(young); + // Runs last, as in the full prune: with both maps empty it disarms the + // flag, zeroes the count published above and clears the filter. + refresh_per_object_layouts_flag(masks_emptied || typed_emptied); + if crate::hot_diag::layout_on() { + // `rebuilt_filter = false`: a young prune never rebuilds it. + layout_diag_note_prune(false); + } +} + /// `PERRY_LAYOUT_DIAG`'s per-prune sample. Out of line and behind /// [`crate::hot_diag::layout_on`] so an unarmed build pays one relaxed load. #[cold] @@ -831,12 +1041,14 @@ pub(in crate::gc) fn refresh_per_object_layouts_flag(touched_map_emptied: bool) pub(in crate::gc) fn typed_layouts_insert(user_ptr: usize, descriptor: TypedLayoutDescriptor) { mark_per_object_layouts_nonempty(); layout_addr_filter_add(user_ptr); + // Armed BEFORE the insert makes the entry findable (young-log rule 1). + let young = arm_young_layout_key(user_ptr); let fresh = hot_typed_layouts() .borrow_mut() .insert(user_ptr, descriptor) .is_none(); - if fresh { - note_new_layout_record(user_ptr); + if fresh && young { + count_new_young_layout_record(); } } @@ -845,12 +1057,14 @@ pub(in crate::gc) fn typed_layouts_insert(user_ptr: usize, descriptor: TypedLayo pub(in crate::gc) fn slot_masks_insert(user_ptr: usize, mask: LayoutSlotMask) { mark_per_object_layouts_nonempty(); layout_addr_filter_add(user_ptr); + // Armed BEFORE the insert makes the entry findable (young-log rule 1). + let young = arm_young_layout_key(user_ptr); let fresh = hot_layout_slot_masks() .borrow_mut() .insert(user_ptr, mask) .is_none(); - if fresh { - note_new_layout_record(user_ptr); + if fresh && young { + count_new_young_layout_record(); } } @@ -936,6 +1150,7 @@ pub(in crate::gc) fn transfer_per_object_descriptor(old_user: usize, new_user: u typed.remove(&new_user); match typed.remove(&old_user) { Some(layout) => { + arm_moved_layout_key(new_user); typed.insert(new_user, layout); drop(typed); layout_addr_filter_add(new_user); @@ -956,6 +1171,7 @@ pub(in crate::gc) fn transfer_per_object_slot_mask(old_user: usize, new_user: us let mut masks = hot_layout_slot_masks().borrow_mut(); masks.remove(&new_user); if let Some(mask) = masks.remove(&old_user) { + arm_moved_layout_key(new_user); masks.insert(new_user, mask); drop(masks); layout_addr_filter_add(new_user); diff --git a/crates/perry-runtime/src/gc/tests/young_log_tests.rs b/crates/perry-runtime/src/gc/tests/young_log_tests.rs index 53eaa899f3..27ed7c6afe 100644 --- a/crates/perry-runtime/src/gc/tests/young_log_tests.rs +++ b/crates/perry-runtime/src/gc/tests/young_log_tests.rs @@ -844,3 +844,134 @@ fn promoted_box_root_leaves_log_and_is_found_by_full_walk() { ); assert!(!walk("box.roots").partial); } + +// -------------------------------------------------- per-object layout tables +// +// #9841: the DEATH PRUNE of `LAYOUT_SLOT_MASKS + TYPED_LAYOUTS`, not a root +// scanner. Its predicate is `layout_key_may_be_nursery`, which excludes +// `Longlived` AND `Old` — strictly stronger than the scanners' +// `addr_is_minor_relevant` — so an old-keyed record is not merely cheap to +// visit, it is provably impossible for a minor to remove. + +use crate::gc::layout_tables::{test_per_object_layout_present, LAYOUT_YOUNG_LOG_NAME}; + +/// A nursery object whose header says POINTER_FREE and which then takes a +/// pointer store — the mutator path that mints a mask from inside +/// `layout_note_slot`'s own `borrow_mut` (WRITER 3). On cc that is the +/// dominant insert path: `TYPED_LAYOUTS` is empty there and every one of the +/// ~66k live keys is a `LAYOUT_SLOT_MASKS` entry. +fn young_masked_object() -> usize { + let obj = crate::object::js_object_alloc(0, 8); + crate::object::js_object_set_field(obj, 0, crate::value::JSValue::number(1.0)); + crate::object::js_object_set_field(obj, 1, crate::value::JSValue::number(2.0)); + crate::gc::layout_clear_for_ptr(obj as usize); + unsafe { crate::gc::layout_init_pointer_free(obj as *mut u8) }; + let child = crate::string::js_string_from_bytes(b"late-pointer".as_ptr(), 12); + crate::object::js_object_set_field(obj, 1, crate::value::JSValue::string_ptr(child)); + assert!( + test_per_object_layout_present(obj as usize), + "premise: the in-place mask mint published a per-object record" + ); + obj as usize +} + +/// WRITER 3's arming site. Delete `arm_young_layout_key` from +/// `gc/layout.rs`'s in-borrow mint and this goes red: under +/// `debug_assertions` on the log-completeness re-derivation, and in release +/// on the record the young prune can no longer see. +#[test] +fn dead_young_masked_owner_is_pruned_through_the_layout_log() { + let _guard = CopyingNurseryTestGuard::new(1); + // One rooted young object so the minor has real work; the owner is not it. + js_shadow_slot_set(0, string_bits(young_leaf())); + + let dead = young_masked_object(); + + let _ = gc_collect_minor(); + + assert!( + !test_per_object_layout_present(dead), + "the dead young owner's per-object layout record must be pruned from the log" + ); + let row = walk(LAYOUT_YOUNG_LOG_NAME); + assert!( + row.partial, + "a copying minor must take the young-scoped prune: {row:?}" + ); + assert!( + row.visited >= 1, + "the logged key must have been visited: {row:?}" + ); +} + +/// WRITER 4's arming site (`transfer_per_object_slot_mask`, which runs during +/// evacuation and therefore BEFORE this collection's prune). Delete its +/// `arm_moved_layout_key` and the re-derivation panics here on the to-space +/// key. +#[test] +fn surviving_young_masked_owner_is_rekeyed_and_stays_logged() { + let _guard = CopyingNurseryTestGuard::new(1); + + let obj = young_masked_object(); + js_shadow_slot_set(0, ptr_bits(obj)); + + let _ = gc_collect_minor(); + + let after = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + assert_ne!(after, obj, "the rooted owner must have been evacuated"); + assert!( + test_per_object_layout_present(after), + "the mask must follow its owner to the new address" + ); + assert!( + !test_per_object_layout_present(obj), + "the stale from-space key must be gone" + ); + let row = walk(LAYOUT_YOUNG_LOG_NAME); + assert!(row.partial, "{row:?}"); + assert!( + row.visited >= 1, + "the move hook's key must have been logged and visited: {row:?}" + ); + if crate::arena::pointer_in_nursery(after) { + assert!( + row.kept >= 1, + "a survivor still in the nursery must stay logged: {row:?}" + ); + } +} + +/// Rule 3: the skip has to be observable, or a latch that never fires looks +/// landed. An OLD-keyed record cannot be found dead by any minor, so the +/// young prune must not visit it at all. +#[test] +fn old_layout_records_are_skipped_by_a_minor() { + let _guard = CopyingNurseryTestGuard::new(0); + + // Drain whatever this thread's earlier tests left young, so `visited` + // below is about the record installed after it. + let _ = gc_collect_minor(); + + let (owner, _) = unsafe { alloc_old_test_object(2) }; + crate::gc::layout_tables::slot_masks_insert( + owner as usize, + crate::gc::layout::LayoutSlotMask::from_words(&[1]), + ); + + let _ = gc_collect_minor(); + + assert!( + test_per_object_layout_present(owner as usize), + "an old owner's record must survive a minor" + ); + let row = walk(LAYOUT_YOUNG_LOG_NAME); + assert!(row.partial, "{row:?}"); + assert!(row.table_len >= 1, "{row:?}"); + assert_eq!( + row.visited, 0, + "an old-keyed record is not a candidate for any minor and must not be \ + visited: {row:?}" + ); + + crate::gc::layout_clear_for_ptr(owner as usize); +} diff --git a/crates/perry-runtime/src/gc/young_log.rs b/crates/perry-runtime/src/gc/young_log.rs index d83d9bb36c..bfc482c07c 100644 --- a/crates/perry-runtime/src/gc/young_log.rs +++ b/crates/perry-runtime/src/gc/young_log.rs @@ -145,8 +145,11 @@ impl YoungLog { } } - /// Test-only: the table resets (`test_clear_*`) clear their log with them. - #[cfg(test)] + /// Drop every logged key, keeping both buffers' capacity. For a caller + /// that has just PROVED its table empty: every key in the log is then + /// stale, and a walk that early-returns on that proof would otherwise + /// carry them forward for ever (the table resets `test_clear_*` use it + /// for the same reason). pub(crate) fn clear(&mut self) { self.keys.clear(); self.spare.clear(); From 97b550085869f108c0789ec07ac620ab9de7f295 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 15:59:48 +0200 Subject: [PATCH 08/11] diag(gc): histogram the surviving per-object layout-mask residue Under PERRY_LAYOUT_DIAG, time each existing death-prune walk and emit marginal histograms for surviving mask owners, slot counts, pointer share, and heap space. Price the standing residue as per-key prune nanoseconds and the maximum tag checks masks can save in one full trace. Keep insert provenance in diagnostic-only counters so LayoutSlotMask and the unarmed trace/store representation do not change. Cover kind/bucket routing and prove the unarmed histogram loop stays dark with sabotage-capable tests. Claude-Session: https://claude.ai/code/session_011dhBmdn4vGgNibjo3oZqTo --- crates/perry-runtime/src/gc/layout.rs | 47 ++--- crates/perry-runtime/src/gc/layout_tables.rs | 163 +++++++++++++++++- .../src/gc/tests/layout_residue_histogram.rs | 99 +++++++++++ crates/perry-runtime/src/gc/tests/mod.rs | 1 + crates/perry-runtime/src/hot_diag.rs | 136 ++++++++++++++- 5 files changed, 409 insertions(+), 37 deletions(-) create mode 100644 crates/perry-runtime/src/gc/tests/layout_residue_histogram.rs diff --git a/crates/perry-runtime/src/gc/layout.rs b/crates/perry-runtime/src/gc/layout.rs index 74ae34248f..2882f4f75e 100644 --- a/crates/perry-runtime/src/gc/layout.rs +++ b/crates/perry-runtime/src/gc/layout.rs @@ -1,28 +1,23 @@ -//! Per-object pointer-slot layout: the `GcHeader._reserved` layout states, -//! store-time descriptor maintenance (`layout_note_slot`), rebuild/transfer -//! across copying GC, and the child-slot enumeration the collector walks. -//! The slot-mask representation lives in `layout/slot_mask.rs`; the -//! typed-shape descriptor *installation* protocol (`js_gc_init_typed_shape_layout` -//! / `js_gc_declare_typed_shape_layout`) lives in `layout/typed_shape.rs`. +//! Per-object pointer-slot states, store maintenance, copying-GC transfer and +//! child-slot enumeration. Mask storage is in `layout/slot_mask.rs`; typed +//! descriptor installation is in `layout/typed_shape.rs`. use super::hot_tls::{hot_layout_slot_masks, hot_shape_layouts}; use super::layout_tables::{ - layout_forget_object, mark_per_object_layouts_nonempty, per_object_slot_mask, - refresh_per_object_layouts_flag, slot_masks_insert, slot_masks_remove, + layout_forget_object, layout_note_store_mask_insert, mark_per_object_layouts_nonempty, + per_object_slot_mask, refresh_per_object_layouts_flag, slot_masks_insert, + slot_masks_insert_birth, slot_masks_insert_rebuild, slot_masks_remove, transfer_per_object_descriptor, transfer_per_object_slot_mask, typed_layouts_insert, typed_layouts_remove, with_per_object_descriptor, }; use super::*; - -// Copied-nursery survival age stored in otherwise-unused low -// GcHeader._reserved bits. Bits 0..2 remain object freeze/seal flags -// and bits 14..15 remain layout state. +// Copied-nursery survival age in otherwise-unused low `_reserved` bits; +// bits 0..2 remain object flags and bits 14..15 remain layout state. pub(super) const GC_COPY_SURVIVAL_AGE_SHIFT: usize = 3; pub(super) const GC_COPY_SURVIVAL_AGE_MASK: u16 = 0x0038; pub(super) const GC_COPY_PROMOTION_SURVIVALS: u8 = 4; -// Pointer-slot layout state stored in the high bits of GcHeader._reserved. -// Low bits remain object freeze/seal/preventExtensions flags. +// Pointer-slot layout state in high `_reserved` bits; low bits remain object flags. pub const GC_LAYOUT_STATE_MASK: u16 = 0xC000; pub(super) const GC_LAYOUT_UNKNOWN: u16 = 0x0000; /// No payload slot holds a pointer, so `heap_payload_slot_selection` skips the @@ -42,18 +37,11 @@ pub(super) const GC_LAYOUT_UNKNOWN: u16 = 0x0000; /// probe read its records only after the last GC. Under `PERRY_JSON_TAPE=0` the /// same sabotage SIGSEGVs. So: /// -/// - "clean at rate 1 + from-space protect" is evidence only once you have -/// shown the misdeclared object EXISTED during a collection; -/// - `PERRY_GC_FROMSPACE_SCAN=1` is the instrument to prefer — its -/// whole-payload word scan consults no layout state, and it reported the -/// stranded children at exactly `dangling=8000 owners=4000`; -/// - `PERRY_GC_VERIFY_EVACUATION` is blind here by construction: it walks the -/// same enumeration the rewrite pass walks, which is to say it asks this -/// state which slots exist. -/// -/// The workload-free detectors are the child-slot enumerator and relocation -/// across a copying minor; worked example, sabotage-verified in both -/// directions: `gc/tests/copying/deferred_finalize_7635.rs`. +/// Therefore first prove the object existed during collection; prefer +/// `PERRY_GC_FROMSPACE_SCAN=1`, whose whole-payload scan ignores layout state. +/// `PERRY_GC_VERIFY_EVACUATION` is blind because it trusts this enumeration. +/// Workload-free coverage lives in the child-slot and copying-relocation tests +/// in `gc/tests/copying/deferred_finalize_7635.rs`. pub const GC_LAYOUT_POINTER_FREE: u16 = 0x4000; pub(crate) const GC_LAYOUT_SIDE_MASK: u16 = 0x8000; // A side-layout payload whose entire live prefix contains pointers. Bit 13 is @@ -966,6 +954,7 @@ pub(crate) fn layout_note_slot(parent_user: usize, slot_index: usize, value_bits // do the most damage. let young = super::layout_tables::arm_young_layout_key(parent_user); masks.insert(parent_user, mask); + layout_note_store_mask_insert(); mark_per_object_layouts_nonempty(); super::layout_tables::layout_addr_filter_note(parent_user); if young { @@ -1173,7 +1162,7 @@ pub(super) unsafe fn layout_rebuild_from_slots_with_policy( slot_masks_remove(user_ptr as usize); } else { set_layout_state(header, GC_LAYOUT_SIDE_MASK); - slot_masks_insert(user_ptr as usize, mask); + slot_masks_insert_rebuild(user_ptr as usize, mask); } } @@ -1228,7 +1217,7 @@ pub(crate) unsafe fn layout_init_from_slots( set_layout_state(header, GC_LAYOUT_UNKNOWN); } else { set_layout_state(header, GC_LAYOUT_SIDE_MASK); - slot_masks_insert(user_ptr as usize, LayoutSlotMask::Inline(bits)); + slot_masks_insert_birth(user_ptr as usize, LayoutSlotMask::Inline(bits)); } return any_pointer; } @@ -1247,7 +1236,7 @@ pub(crate) unsafe fn layout_init_from_slots( set_layout_state(header, GC_LAYOUT_UNKNOWN); } else { set_layout_state(header, GC_LAYOUT_SIDE_MASK); - slot_masks_insert(user_ptr as usize, mask); + slot_masks_insert_birth(user_ptr as usize, mask); } any_pointer } diff --git a/crates/perry-runtime/src/gc/layout_tables.rs b/crates/perry-runtime/src/gc/layout_tables.rs index 97da20152f..76a9f298b0 100644 --- a/crates/perry-runtime/src/gc/layout_tables.rs +++ b/crates/perry-runtime/src/gc/layout_tables.rs @@ -31,7 +31,9 @@ use super::hot_tls::{hot_layout_slot_masks, hot_per_object_layout_hint, hot_typed_layouts}; use super::layout::{LayoutSlotMask, TypedLayoutDescriptor}; -use super::types::{GcHeader, GC_FLAG_ARENA, GC_HEADER_SIZE, GC_TYPE_ARRAY, GC_TYPE_OBJECT}; +use super::types::{ + GcHeader, GC_FLAG_ARENA, GC_HEADER_SIZE, GC_TYPE_ARRAY, GC_TYPE_CLOSURE, GC_TYPE_OBJECT, +}; use std::cell::{Cell, RefCell}; thread_local! { @@ -252,6 +254,9 @@ pub(in crate::gc) fn prune_dead_per_object_layout_owners(is_dead_owner: &dyn Fn( drop_stale_young_layout_log(); return; } + // Exactly one arm test per non-empty prune. Everything below it — the + // timer and the residue-wide histogram — is absent when the sink is off. + let layout_diag = crate::hot_diag::layout_on(); // ONE pass over each table, not three. The old shape visited every live // key three times per collection — `retain`, then // `layout_addr_filter_rebuild` (which first collected them all into a @@ -301,6 +306,7 @@ pub(in crate::gc) fn prune_dead_per_object_layout_owners(is_dead_owner: &dyn Fn( } true }; + let prune_walk_started = layout_diag.then(std::time::Instant::now); let masks_emptied = { let mut masks = hot_layout_slot_masks().borrow_mut(); let had = !masks.is_empty(); @@ -313,6 +319,9 @@ pub(in crate::gc) fn prune_dead_per_object_layout_owners(is_dead_owner: &dyn Fn( typed.retain(|key, _| keep(*key)); had && typed.is_empty() }; + let prune_walk_us = prune_walk_started.map_or(0, |started| { + started.elapsed().as_micros().min(u128::from(u64::MAX)) as u64 + }); // A full walk is authoritative: rebuild the young log from the tables // (same shape as the shape/descriptor full scanners). { @@ -335,8 +344,8 @@ pub(in crate::gc) fn prune_dead_per_object_layout_owners(is_dead_owner: &dyn Fn( // the young count published above and clears the filter, which is the // correct end state whichever branch the pass took. refresh_per_object_layouts_flag(masks_emptied || typed_emptied); - if crate::hot_diag::layout_on() { - layout_diag_note_prune(rebuild_filter); + if layout_diag { + layout_diag_note_prune(rebuild_filter, prune_walk_us); } } @@ -378,6 +387,8 @@ pub(in crate::gc) fn prune_dead_per_object_layout_owners_young( drop_stale_young_layout_log(); return; } + // Exactly one arm test per non-empty prune; see the full-prune twin. + let layout_diag = crate::hot_diag::layout_on(); let hint = hot_per_object_layout_hint(); let table_len = (hot_layout_slot_masks().borrow().len() + hot_typed_layouts().borrow().len()) as u64; @@ -407,6 +418,7 @@ pub(in crate::gc) fn prune_dead_per_object_layout_owners_young( // present in both maps is two records and one log entry. let mut young: u32 = 0; let mut kept = hint.young_keys.borrow_mut().take_spare(); + let prune_walk_started = layout_diag.then(std::time::Instant::now); let (masks_emptied, typed_emptied) = { let mut masks = hot_layout_slot_masks().borrow_mut(); let mut typed = hot_typed_layouts().borrow_mut(); @@ -446,6 +458,9 @@ pub(in crate::gc) fn prune_dead_per_object_layout_owners_young( } (had_masks && masks.is_empty(), had_typed && typed.is_empty()) }; + let prune_walk_us = prune_walk_started.map_or(0, |started| { + started.elapsed().as_micros().min(u128::from(u64::MAX)) as u64 + }); let kept_len = kept.len() as u64; hint.young_keys.borrow_mut().extend(kept); crate::gc::young_log::note_walk( @@ -462,20 +477,21 @@ pub(in crate::gc) fn prune_dead_per_object_layout_owners_young( // Runs last, as in the full prune: with both maps empty it disarms the // flag, zeroes the count published above and clears the filter. refresh_per_object_layouts_flag(masks_emptied || typed_emptied); - if crate::hot_diag::layout_on() { + if layout_diag { // `rebuilt_filter = false`: a young prune never rebuilds it. - layout_diag_note_prune(false); + layout_diag_note_prune(false, prune_walk_us); } } /// `PERRY_LAYOUT_DIAG`'s per-prune sample. Out of line and behind /// [`crate::hot_diag::layout_on`] so an unarmed build pays one relaxed load. #[cold] -fn layout_diag_note_prune(rebuilt_filter: bool) { +fn layout_diag_note_prune(rebuilt_filter: bool, prune_walk_us: u64) { let (typed_len, masks_len) = ( hot_typed_layouts().borrow().len(), hot_layout_slot_masks().borrow().len(), ); + let residue = layout_residue_histogram(prune_walk_us); let hint = hot_per_object_layout_hint(); // SAFETY: as in the pass above — this thread's own filter, no other // reference live. @@ -492,9 +508,117 @@ fn layout_diag_note_prune(rebuilt_filter: bool) { LAYOUT_ADDR_FILTER_BITS, rebuilt_filter, layout_addr_filter_saturating_occupancy(), + residue, ); } +/// Walk the surviving mask table once for `PERRY_LAYOUT_DIAG` only. +/// +/// The logical slot bound comes from the same owner metadata the tracer uses: +/// array length, shape-derived object live slots, or real closure captures. +/// A mask cannot legitimately belong to any other GC kind, but `other` keeps +/// the diagnostic total honest if a stale/corrupt entry is ever observed. +#[cold] +fn layout_residue_histogram(prune_walk_us: u64) -> crate::hot_diag::LayoutResidueHistogram { + let mut out = crate::hot_diag::LayoutResidueHistogram { + prune_walk_us, + ..Default::default() + }; + let masks = hot_layout_slot_masks().borrow(); + out.keys = masks.len() as u64; + for (&owner, mask) in masks.iter() { + #[cfg(test)] + LAYOUT_RESIDUE_HISTOGRAM_ENTRIES.with(|n| n.set(n.get().saturating_add(1))); + + let Some(header) = (unsafe { crate::value::addr_class::try_read_tracked_gc_header(owner) }) + else { + out.other += 1; + out.slots[0] += 1; + out.pointer_share[0] += 1; + out.space[2] += 1; + continue; + }; + // SAFETY: `try_read_tracked_gc_header` proved this exact owner belongs + // to either an arena allocation or the tracked malloc registry. + let header = unsafe { header.as_ref() }; + let slot_count = unsafe { + match header.obj_type { + GC_TYPE_CLOSURE => { + out.closure += 1; + let closure = owner as *const crate::closure::ClosureHeader; + crate::closure::real_capture_count((*closure).capture_count) as usize + } + GC_TYPE_OBJECT => { + out.object += 1; + crate::object::object_live_slot_count( + owner as *const crate::object::ObjectHeader, + ) as usize + } + GC_TYPE_ARRAY => { + out.array += 1; + let array = owner as *const crate::array::ArrayHeader; + ((*array).length as usize).min((*array).capacity as usize) + } + _ => { + out.other += 1; + (header.size as usize).saturating_sub(GC_HEADER_SIZE) / 8 + } + } + }; + + let slot_bucket = match slot_count { + 0..=7 => 0, + 8..=15 => 1, + 16..=31 => 2, + 32..=63 => 3, + 64..=255 => 4, + _ => 5, + }; + out.slots[slot_bucket] += 1; + + let pointer_slots = mask.count_slots(slot_count); + let share_bucket = if pointer_slots.saturating_mul(4) <= slot_count { + 0 + } else if pointer_slots.saturating_mul(2) <= slot_count { + 1 + } else if pointer_slots.saturating_mul(4) <= slot_count.saturating_mul(3) { + 2 + } else { + 3 + }; + out.pointer_share[share_bucket] += 1; + out.est_tag_checks_saved_per_trace = out + .est_tag_checks_saved_per_trace + .saturating_add(slot_count.saturating_sub(pointer_slots) as u64); + + if header.gc_flags & GC_FLAG_ARENA == 0 { + out.space[2] += 1; + } else if crate::arena::classify_heap_space(owner).is_nursery() { + out.space[0] += 1; + } else { + // Old, Longlived and the transient PromotedYoung classification + // are all old-page residents for this three-way price split. + out.space[1] += 1; + } + } + out +} + +#[cfg(test)] +crate::perry_thread_local! { + static LAYOUT_RESIDUE_HISTOGRAM_ENTRIES: Cell = const { Cell::new(0) }; +} + +#[cfg(test)] +pub(in crate::gc) fn test_reset_layout_residue_histogram_entries() { + LAYOUT_RESIDUE_HISTOGRAM_ENTRIES.with(|n| n.set(0)); +} + +#[cfg(test)] +pub(in crate::gc) fn test_layout_residue_histogram_entries() -> usize { + LAYOUT_RESIDUE_HISTOGRAM_ENTRIES.with(Cell::get) +} + #[cfg(test)] pub(in crate::gc) fn test_per_object_layout_present(user_ptr: usize) -> bool { hot_layout_slot_masks().borrow().contains_key(&user_ptr) @@ -1054,7 +1178,7 @@ pub(in crate::gc) fn typed_layouts_insert(user_ptr: usize, descriptor: TypedLayo /// The one way to add a per-object pointer mask. #[inline] -pub(in crate::gc) fn slot_masks_insert(user_ptr: usize, mask: LayoutSlotMask) { +pub(in crate::gc) fn slot_masks_insert(user_ptr: usize, mask: LayoutSlotMask) -> bool { mark_per_object_layouts_nonempty(); layout_addr_filter_add(user_ptr); // Armed BEFORE the insert makes the entry findable (young-log rule 1). @@ -1066,6 +1190,31 @@ pub(in crate::gc) fn slot_masks_insert(user_ptr: usize, mask: LayoutSlotMask) { if fresh && young { count_new_young_layout_record(); } + fresh +} + +/// Insert-site wrappers for the diagnostic counter. Keeping them here avoids +/// carrying provenance in `LayoutSlotMask`, whose size and hot-path shape must +/// not change for an optional instrument. +#[inline] +pub(in crate::gc) fn slot_masks_insert_birth(user_ptr: usize, mask: LayoutSlotMask) { + if slot_masks_insert(user_ptr, mask) && crate::hot_diag::layout_on() { + crate::hot_diag::layout_note_mask_insert(crate::hot_diag::LayoutMaskInsertSite::Birth); + } +} + +#[inline] +pub(in crate::gc) fn slot_masks_insert_rebuild(user_ptr: usize, mask: LayoutSlotMask) { + if slot_masks_insert(user_ptr, mask) && crate::hot_diag::layout_on() { + crate::hot_diag::layout_note_mask_insert(crate::hot_diag::LayoutMaskInsertSite::Rebuild); + } +} + +#[inline] +pub(in crate::gc) fn layout_note_store_mask_insert() { + if crate::hot_diag::layout_on() { + crate::hot_diag::layout_note_mask_insert(crate::hot_diag::LayoutMaskInsertSite::Store); + } } /// Drop `user_ptr`'s per-object typed descriptor (only). diff --git a/crates/perry-runtime/src/gc/tests/layout_residue_histogram.rs b/crates/perry-runtime/src/gc/tests/layout_residue_histogram.rs new file mode 100644 index 0000000000..d7c4c66ae3 --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/layout_residue_histogram.rs @@ -0,0 +1,99 @@ +use super::support::*; + +fn pointer_bits() -> u64 { + let child = crate::string::js_string_from_bytes(b"residue-child".as_ptr(), 13); + string_bits(child as usize) +} + +fn closure_with_captures(slot_count: usize, pointer_slots: usize) -> usize { + let pointer = pointer_bits(); + let mut captures = vec![1.0f64.to_bits(); slot_count]; + captures[..pointer_slots].fill(pointer); + let closure = crate::closure::js_closure_alloc(std::ptr::null(), slot_count as u32); + unsafe { + let slots = crate::closure::closure_capture_slots_mut(closure); + std::ptr::copy_nonoverlapping(captures.as_ptr(), slots, slot_count); + crate::gc::layout_init_from_slots(closure as *mut u8, slots, slot_count); + } + closure as usize +} + +/// The requested marginal-histogram fixture. Swapping the `8..=15` and +/// `16..=31` bounds makes the 20-capture assertion fail. +#[test] +fn layout_residue_histogram_counts_by_kind_and_bucket() { + let _gc = CopyingNurseryTestGuard::new(0); + let _diag = crate::hot_diag::LayoutDiagTestGuard::force(true); + + let closure5 = closure_with_captures(5, 1); + let closure20 = closure_with_captures(20, 10); + + let object70 = crate::object::js_object_alloc(0, 70); + let pointer = pointer_bits(); + unsafe { + let fields = (object70 as *mut u8).add(std::mem::size_of::()) + as *mut u64; + for slot in 0..70 { + fields.add(slot).write(if slot < 60 { + pointer + } else { + (slot as f64).to_bits() + }); + } + crate::object::rebuild_object_field_layout(object70, 70); + } + + crate::gc::layout_tables::test_reset_layout_residue_histogram_entries(); + crate::gc::layout_tables::prune_dead_per_object_layout_owners(&|_| false); + + let residue = crate::hot_diag::LayoutDiagTestGuard::residue(); + assert_eq!(residue.keys, 3, "{residue:?}"); + assert_eq!(residue.closure, 2, "{residue:?}"); + assert_eq!(residue.object, 1, "{residue:?}"); + assert_eq!(residue.array, 0, "{residue:?}"); + assert_eq!(residue.other, 0, "{residue:?}"); + assert_eq!(residue.slots, [1, 0, 1, 0, 1, 0], "{residue:?}"); + assert_eq!(residue.pointer_share, [1, 1, 0, 1], "{residue:?}"); + assert_eq!(residue.space, [3, 0, 0], "{residue:?}"); + assert_eq!(residue.est_tag_checks_saved_per_trace, 24, "{residue:?}"); + assert_eq!( + crate::gc::layout_tables::test_layout_residue_histogram_entries(), + 3 + ); + + let output = crate::hot_diag::LayoutDiagTestGuard::output(); + assert!(output.contains("[layout-diag] residue keys=3 closure=2 object=1 array=0 other=0")); + assert!(output.contains("slots{4-7=1 8-15=0 16-31=1 32-63=0 64-255=1 256+=0}")); + assert!(output.contains("ptr_share{q1=1 q2=1 q3=0 q4=1}")); + assert!(output.contains("space{nursery=3 old=0 malloc=0}")); + assert!(output.contains("inserts_since{birth=2 rebuild=1 store=0}")); + assert!(output.contains("[layout-diag] price per_key_prune_ns=")); + assert!(output.contains("est_tag_checks_saved_per_trace=24")); + + for owner in [closure5, closure20, object70 as usize] { + crate::gc::layout_clear_for_ptr(owner); + } +} + +/// Dropping the single `layout_on()` gate in either prune makes the test-only +/// entry counter non-zero, even though the output sink remains unarmed. +#[test] +fn layout_residue_histogram_is_silent_when_unarmed() { + let _gc = CopyingNurseryTestGuard::new(0); + let _diag = crate::hot_diag::LayoutDiagTestGuard::force(false); + let closure = closure_with_captures(5, 1); + + crate::gc::layout_tables::test_reset_layout_residue_histogram_entries(); + crate::gc::layout_tables::prune_dead_per_object_layout_owners(&|_| false); + + assert!( + crate::hot_diag::LayoutDiagTestGuard::output().is_empty(), + "an unarmed prune must emit no residue line" + ); + assert_eq!( + crate::gc::layout_tables::test_layout_residue_histogram_entries(), + 0, + "the histogram entry loop must not run while the sink is off" + ); + crate::gc::layout_clear_for_ptr(closure); +} diff --git a/crates/perry-runtime/src/gc/tests/mod.rs b/crates/perry-runtime/src/gc/tests/mod.rs index a698870282..67832ef5ba 100644 --- a/crates/perry-runtime/src/gc/tests/mod.rs +++ b/crates/perry-runtime/src/gc/tests/mod.rs @@ -37,6 +37,7 @@ mod incremental_sweep_reclaim; mod inline_generation_gate_contract; mod inline_pointer_bearing_contract; mod layout_pointer_free_hazard; +mod layout_residue_histogram; mod layout_trace; mod lazy_intrinsic_towers; mod lazy_tape_side_alloc; diff --git a/crates/perry-runtime/src/hot_diag.rs b/crates/perry-runtime/src/hot_diag.rs index 51615783ca..a5ed04f1c6 100644 --- a/crates/perry-runtime/src/hot_diag.rs +++ b/crates/perry-runtime/src/hot_diag.rs @@ -428,6 +428,14 @@ fn ic_sink() -> &'static Option { static LAYOUT_SINK: OnceLock> = OnceLock::new(); static LAYOUT_ON: AtomicBool = AtomicBool::new(false); +#[cfg(test)] +thread_local! { + /// Per-test override/capture: mutating the process environment cannot + /// safely arm one libtest thread without affecting its neighbours. + static LAYOUT_TEST_ARMED: std::cell::Cell> = const { std::cell::Cell::new(None) }; + static LAYOUT_TEST_OUTPUT: RefCell = const { RefCell::new(String::new()) }; +} + fn layout_sink() -> &'static Option { LAYOUT_SINK.get_or_init(|| { let sink = sink_from_env("PERRY_LAYOUT_DIAG"); @@ -439,12 +447,43 @@ fn layout_sink() -> &'static Option { /// Is the per-object-layout occupancy instrument armed? #[inline] pub fn layout_on() -> bool { + #[cfg(test)] + if let Some(armed) = LAYOUT_TEST_ARMED.with(std::cell::Cell::get) { + return armed; + } if LAYOUT_SINK.get().is_none() { layout_sink(); } LAYOUT_ON.load(Ordering::Relaxed) } +/// Which of the three dynamically learned mask paths inserted a new key. +/// +/// Kept as a diagnostic counter rather than a field on `LayoutSlotMask`: the +/// latter is on the trace/store path even when diagnostics are off, and +/// changing its representation would violate this instrument's no-op contract. +#[derive(Clone, Copy)] +pub(crate) enum LayoutMaskInsertSite { + Birth = 0, + Rebuild = 1, + Store = 2, +} + +/// Marginal histograms of the surviving `LAYOUT_SLOT_MASKS` residue. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(crate) struct LayoutResidueHistogram { + pub(crate) keys: u64, + pub(crate) closure: u64, + pub(crate) object: u64, + pub(crate) array: u64, + pub(crate) other: u64, + pub(crate) slots: [u64; 6], + pub(crate) pointer_share: [u64; 4], + pub(crate) space: [u64; 3], + pub(crate) est_tag_checks_saved_per_trace: u64, + pub(crate) prune_walk_us: u64, +} + /// One collection's view of the per-object layout tables and the 4096-bit /// address filter that is supposed to keep evacuation off them. /// @@ -478,22 +517,36 @@ pub struct LayoutDiag { outgrown: u64, /// Keys visited by prunes that DID rebuild — the walk that is still paid. rebuilt_keys: u64, + residue: LayoutResidueHistogram, + inserts_since: [u64; 3], } crate::perry_thread_local! { static LAYOUT_DIAG: RefCell = RefCell::new(LayoutDiag::default()); } +/// Record one newly inserted per-object pointer mask. The caller has already +/// tested [`layout_on`], so an unarmed run never resolves this counter's TLS. +#[inline] +pub(crate) fn layout_note_mask_insert(site: LayoutMaskInsertSite) { + LAYOUT_DIAG.with(|d| { + let mut d = d.borrow_mut(); + let counter = &mut d.inserts_since[site as usize]; + *counter = counter.saturating_add(1); + }); +} + /// Record one death-prune's occupancy. `rebuilt_filter` says whether this /// prune rebuilt the address filter from its survivors, or found the tables /// too full for a 4,096-bit sketch to discriminate and saturated it instead. -pub fn layout_note_prune( +pub(crate) fn layout_note_prune( typed_len: usize, masks_len: usize, filter_bits_set: usize, filter_bits_total: usize, rebuilt_filter: bool, useful_keys: usize, + residue: LayoutResidueHistogram, ) { LAYOUT_DIAG.with(|d| { let mut d = d.borrow_mut(); @@ -506,6 +559,7 @@ pub fn layout_note_prune( d.filter_bits_total = filter_bits_total; d.filter_bits_set_max = d.filter_bits_set_max.max(filter_bits_set); d.useful_keys = useful_keys; + d.residue = residue; if rebuilt_filter { d.rebuilt += 1; d.rebuilt_keys += (typed_len + masks_len) as u64; @@ -513,6 +567,12 @@ pub fn layout_note_prune( d.outgrown += 1; } let text = d.render(); + d.inserts_since = [0; 3]; + #[cfg(test)] + if LAYOUT_TEST_ARMED.with(std::cell::Cell::get) == Some(true) { + LAYOUT_TEST_OUTPUT.with(|out| out.borrow_mut().push_str(&text)); + return; + } if let Some(sink) = layout_sink() { write_sink(sink, &text); } @@ -565,10 +625,84 @@ impl LayoutDiag { " filter rebuilds={} over {} keys walked; outgrown-and-skipped={}", self.rebuilt, self.rebuilt_keys, self.outgrown ); + let r = self.residue; + let _ = writeln!( + out, + "[layout-diag] residue keys={} closure={} object={} array={} other={} \ + slots{{4-7={} 8-15={} 16-31={} 32-63={} 64-255={} 256+={}}} \ + ptr_share{{q1={} q2={} q3={} q4={}}} \ + space{{nursery={} old={} malloc={}}} \ + inserts_since{{birth={} rebuild={} store={}}} prune_walk_us={}", + r.keys, + r.closure, + r.object, + r.array, + r.other, + r.slots[0], + r.slots[1], + r.slots[2], + r.slots[3], + r.slots[4], + r.slots[5], + r.pointer_share[0], + r.pointer_share[1], + r.pointer_share[2], + r.pointer_share[3], + r.space[0], + r.space[1], + r.space[2], + self.inserts_since[0], + self.inserts_since[1], + self.inserts_since[2], + r.prune_walk_us, + ); + let per_key_prune_ns = if r.keys == 0 { + 0 + } else { + r.prune_walk_us.saturating_mul(1_000) / r.keys + }; + let _ = writeln!( + out, + "[layout-diag] price per_key_prune_ns={} est_tag_checks_saved_per_trace={}", + per_key_prune_ns, r.est_tag_checks_saved_per_trace + ); out } } +/// Test-only per-thread sink override, matching `GcDiagTestGuard`'s shape. +#[cfg(test)] +pub(crate) struct LayoutDiagTestGuard { + previous: Option, +} + +#[cfg(test)] +impl LayoutDiagTestGuard { + pub(crate) fn force(armed: bool) -> Self { + let previous = LAYOUT_TEST_ARMED.with(|value| value.replace(Some(armed))); + LAYOUT_TEST_OUTPUT.with(|out| out.borrow_mut().clear()); + LAYOUT_DIAG.with(|diag| *diag.borrow_mut() = LayoutDiag::default()); + Self { previous } + } + + pub(crate) fn output() -> String { + LAYOUT_TEST_OUTPUT.with(|out| out.borrow().clone()) + } + + pub(crate) fn residue() -> LayoutResidueHistogram { + LAYOUT_DIAG.with(|diag| diag.borrow().residue) + } +} + +#[cfg(test)] +impl Drop for LayoutDiagTestGuard { + fn drop(&mut self) { + LAYOUT_TEST_ARMED.with(|value| value.set(self.previous)); + LAYOUT_TEST_OUTPUT.with(|out| out.borrow_mut().clear()); + LAYOUT_DIAG.with(|diag| *diag.borrow_mut() = LayoutDiag::default()); + } +} + /// Is the IC-miss instrument armed? One relaxed load once initialised. #[inline] pub fn ic_on() -> bool { From 227811137de9ec662c600016542d6a091e5c77cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 16:07:13 +0200 Subject: [PATCH 09/11] docs(perf): report layout-mask residue histogram Record the replay evidence, field derivations, sabotage-capable tests, follow-up rule candidates, and exact Stage LP measurement request. Preserve the binding disk-floor result for the release gates that could not start. Claude-Session: https://claude.ai/code/session_011dhBmdn4vGgNibjo3oZqTo --- .../codex/REPORT_layout_residue.md | 207 ++++++++++++++++++ 1 file changed, 207 insertions(+) create mode 100644 cc-perf-campaign/codex/REPORT_layout_residue.md diff --git a/cc-perf-campaign/codex/REPORT_layout_residue.md b/cc-perf-campaign/codex/REPORT_layout_residue.md new file mode 100644 index 0000000000..6a2d42f90b --- /dev/null +++ b/cc-perf-campaign/codex/REPORT_layout_residue.md @@ -0,0 +1,207 @@ +# Per-object layout-mask residue histogram + +Runtime implementation SHA: `97b550085869f108c0789ec07ac620ab9de7f295` + +Young-log replay SHA: `dd279a8ba2de6b86a044d1074955e87fccf76d53` + +Branch: `perf/layout-residue-histogram`, based on +`fork/perf/minor-phases-and-logs` at +`151680fa0f6ffbf6ebb7d8166e2424fc0d031b49`. + +## Part 0: #9895 replay + +`git cherry 151680fa0 0a427a39f` prints `- 390796a11`: +`390796a11`'s one-pass layout prune and saturating filter are already in the +base. It prints `+ 19a6cd201`, so that commit alone was replayed. The only +conflict was `gc/tests/young_log_tests.rs`; the #9957 fixed-cost scanner tests +and #9895's layout-prune tests were kept as adjacent blocks. The runtime hunks +were not edited. + +`git range-diff 19a6cd201^! dd279a8ba^!` shows only commit metadata/message and +the expected `young_log_tests.rs` insertion context. No runtime/code hunk +differs. The replayed tests are present: + +- `dead_young_masked_owner_is_pruned_through_the_layout_log` +- `surviving_young_masked_owner_is_rekeyed_and_stays_logged` +- `old_layout_records_are_skipped_by_a_minor` + +The superseded `41a8af7da`, `bdd1fc003`, and `0a427a39f` were not replayed. +Consequently their full-walk test names/guard are intentionally absent: +`young_closure_prop_value_is_traced_and_moved_by_a_minor`, +`young_value_under_an_old_closure_owner_is_traced_by_a_minor`, +`old_closure_entries_survive_a_minor_full_walk`, and +`dropping_a_logged_closure_owner_trips_the_prune_rule2_check`. The original +young-walk tests remain because #9957 measured `closure.dynamic_props` winning +3.75 -> 3.06 ms and the base retains that log. + +## Diagnostic fields and derivation + +The instrument calls `layout_on()` exactly once in each non-empty full or +young death prune. When it is false, neither an `Instant` nor the surviving +mask pass is created. When true, `prune_walk_us` times only the existing prune +entry loop: the two `retain` walks for a full prune, or the drained young-key +loop for a minor. The diagnostic histogram pass runs afterwards and is not +included in that price. + +The first new line is: + +```text +[layout-diag] residue keys= closure= object= array= other= slots{4-7= 8-15= 16-31= 32-63= 64-255= 256+=} ptr_share{q1= q2= q3= q4=} space{nursery= old= malloc=} inserts_since{birth= rebuild= store=} prune_walk_us= +``` + +- `keys` is the surviving `LAYOUT_SLOT_MASKS.len()` after the death prune. +- `closure`, `object`, `array`, and `other` come from the tracked owner's + `GcHeader.obj_type`. `other` is defensive: legitimate mask owners are the + three layout-bearing kinds. +- Logical slots use the same owner facts as tracing: masked closure + `real_capture_count`, shape-derived `object_live_slot_count`, and array + `min(length, capacity)`. Other types fall back to GC payload words. The six + indices are `<=7`, 8-15, 16-31, 32-63, 64-255, and 256+. Under the default + floor a legitimate first-bucket mask is 4-7; the `<=7` implementation keeps + totals exhaustive if an override or corrupt residue produces a smaller one. +- Pointer slots are `LayoutSlotMask::count_slots(logical_slots)`. Quartiles + are non-overlapping: q1 `<=25%`, q2 `>25% and <=50%`, q3 `>50% and <=75%`, + q4 `>75%`. +- `nursery` is an arena owner in Eden or either survivor half. `malloc` is an + owner without `GC_FLAG_ARENA`. Remaining arena spaces (`Old`, `Longlived`, + and transient `PromotedYoung`) are `old` for this three-way price split. +- `birth`, `rebuild`, and `store` count fresh HashMap keys inserted by + `layout_init_from_slots`, `layout_rebuild_from_slots` (including the exact + rebuild wrapper), and `layout_note_slot`. Updates to an existing mask and + GC rekeys do not count. The three counters reset after every emitted prune. + Provenance is deliberately not stored on each `LayoutSlotMask`: doing so + would change a representation and trace/store path that exists when the + diagnostic is off. The separate counters answer insertion traffic, not the + historical origin of each survivor. +- `prune_walk_us` is `Instant::elapsed().as_micros()` around the existing loop, + saturated to `u64`. + +The second new line is: + +```text +[layout-diag] price per_key_prune_ns= est_tag_checks_saved_per_trace= +``` + +`per_key_prune_ns` is integer `prune_walk_us * 1000 / keys` (zero for no mask +keys). `est_tag_checks_saved_per_trace` is the saturated sum of +`logical_slots - pointer_slots` over every surviving mask: the maximum number +of exact tag checks the whole residue can avoid in one full trace. It is a +benefit ceiling, not a claim that every object is traced every cycle. + +This histogram can decide which owner kind and logical-slot bucket dominates, +how sparse the masks are, where their owners live, and whether the 64+ buckets +where a mask can plausibly earn program-global upkeep are a rounding error. +It does not change a threshold or selection rule. + +## Tests and sabotage evidence + +- `layout_residue_histogram_counts_by_kind_and_bucket` arms a per-thread test + sink, creates 5- and 20-capture closures with pointer captures, rebuilds one + 70-slot object, prunes with all owners surviving, and asserts kind, slot, + quartile, space, insert-site, saved-check, and output fields. It passed in + the focused release `layout` run (137 passed, 0 failed). Sabotage swapped + the 8-15 and 16-31 destinations; the + test failed with slots `[1, 1, 0, 0, 1, 0]` against expected + `[1, 0, 1, 0, 1, 0]`. +- `layout_residue_histogram_is_silent_when_unarmed` forces the per-thread sink + off, performs a prune, asserts no residue output, and reads a test-only + histogram-entry counter. It passed in the same focused release run. + Sabotage removed the full-prune `if layout_diag` guard; it failed with 1 + entry visited against expected 0. + +## Follow-up rule candidates + +### (a) Young-entry log for the prune + +This is orthogonal to deciding which masks deserve to exist, and Part 0 has +already stacked it here. `PerObjectLayoutHint.young_keys` is armed before every +new/moved young record becomes findable. A minor drains only those candidates, +drops stale/dead/promoted keys, and a full prune rebuilds the log from its +authoritative table walk. It changes repeated minor pruning from O(standing +keys) toward O(young churn), while the histogram describes the residue and is +armed-only. It cannot remove the full-trace walk or the mask's insert/store/ +death costs, so a high floor or shared representation can still win on top. + +### (b) Measured break-even floor + +A follow-up can replace the corpus default of four with a floor derived from +`per_key_prune_ns`, expected prunes during an owner's lifetime, and a separately +measured tag-check nanosecond cost. The mask's maximum per-trace return is +already printed as `slots - pointer_slots`; its standing prune cost is printed +per key. The relevant funnels are centralized: bulk birth and rebuild compare +against `layout_mask_min_slots`, while store-time creation goes through +`layout_prefers_scan_over_mask` (with the existing object-specific default of +eight). The missing input is tag-check cost and trace frequency by lifetime; +without those, converting the current price line directly into a slot number +would mix one prune with one full trace and repeat the campaign's wrong-ratio +failure mode. + +### (c) Closure masks keyed by function + +This is structurally possible when every instance agrees. `ClosureHeader` +provides a stable native `func_ptr` and `real_capture_count`, and +`layout_init_from_slots` observes the complete birth mask. A function-keyed +descriptor can store `(slot_count, mask)`, reuse it for agreeing instances, +and poison the function on the first differing birth or later capture store, +matching `SHAPE_LAYOUTS`' `Some/None` ambiguity pattern. Poison must make every +instance fall back to conservative tag scanning (safe even for earlier +instances that omitted per-object masks), while a diverging stored instance +can retain its exact per-object mask. The present header has only +`GC_LAYOUT_SIDE_MASK`, not a function-shared state, so mask resolution and +`layout_note_slot` would need an explicit shared lookup/fallback protocol. +Function entries then need no death prune because code pointers are stable and +the table is O(functions), but agreement/poison tests must cover post-birth +stores and differing capture counts before this becomes a rule. + +## Validation + +- `cargo test -p perry-runtime --release --lib -j4 -- --test-threads=1 layout`: + PASS before the two sabotage checks: 137 passed, 0 failed, 3,143 filtered. + Both sabotages were then reverted. A final rerun against the restored source + could not start because the mandatory free-space check fell below 12 GB. +- Full `cargo test -p perry-runtime --release --lib -j4 -- --test-threads=1`: + NOT RUN: blocked by the same free-space floor. +- `cargo build --release -p perry-runtime --features wasm-host -j4`: NOT RUN: + blocked by the same free-space floor. +- `rustfmt --check`: PASS on every changed Rust file. +- `git diff --check`: PASS. +- `scripts/check_file_size.sh`: PASS; no Rust file exceeds 2,000 lines + (`gc/layout.rs` is 1,999). +- No local cc run, as requested. + +Every Cargo attempt checked `df -g /` immediately beforehand. The final gate +rerun is currently blocked because the latest check reports 10 GB available, +below the binding 12 GB floor; no below-floor Cargo command was started. + +## Stage LP: exact perrymaster request and falsifiers + +Relink the #9957 tree plus `97b550085869f108c0789ec07ac620ab9de7f295` +(young-log prune plus residue histogram) on m6mp's object cache. Run two +graceful four-turn 3300-character candidate repetitions against `app-m6mp` +with `PERRY_GC_DIAG=1`; do not enable layout diagnostics for the CPU A/B, +because its deliberate whole-residue histogram is measurement overhead. Then +run one additional candidate capture with +`PERRY_GC_DIAG=1 PERRY_LAYOUT_DIAG=stderr`. + +The falsifiers are: + +- `dead_owner_side_table_pruning` for `LAYOUT_SLOT_MASKS + TYPED_LAYOUTS` + falls from 8.7-9.3 ms to at most 1 ms per steady minor, following the + +3...+10 young-key churn rather than 155-161k standing keys. +- Total steady copied-minor pause falls from about 46 ms to about 38 ms. +- Four-turn CPU improves by 1-2%; report both repetitions, not only a minimum. +- Peak and settled RSS remain within +3%. + +For the +29 MB settled result #9895 observed on main, one 160k-key log retains +two `Vec` buffers. At a 262,144-entry capacity that is about 4 MiB, so a +single table-owning thread cannot explain +29 MB; that delta would require +about seven similarly grown thread-local logs or allocator-retained secondary +effects. Stage LP must either report enough per-thread log capacity to account +for it or show the settled delta gone. Do not label +29 MB “the log” without +that reconciliation. + +From the layout-diagnostic run, preserve the residue lines for minors 5, 10, +13 (turn-2 maximum), 17, and 25. Report medians of both price fields over the +steady minors, alongside the phase values. Those rows decide whether closure/ +object/array and the 4-7/8-15/16-31/32-63/64+ populations justify a threshold +or function-keyed follow-up. From a4a6978a8606922bd41b41839bcae5e7537ed242 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 16:09:37 +0200 Subject: [PATCH 10/11] docs(perf): record parked-lane artifact cleanup Document the required target cleanup after the final Cargo gates were blocked by the campaign's 12 GB free-space floor. Claude-Session: https://claude.ai/code/session_011dhBmdn4vGgNibjo3oZqTo --- cc-perf-campaign/codex/REPORT_layout_residue.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cc-perf-campaign/codex/REPORT_layout_residue.md b/cc-perf-campaign/codex/REPORT_layout_residue.md index 6a2d42f90b..c670db000e 100644 --- a/cc-perf-campaign/codex/REPORT_layout_residue.md +++ b/cc-perf-campaign/codex/REPORT_layout_residue.md @@ -170,8 +170,13 @@ stores and differing capture counts before this becomes a rule. - No local cc run, as requested. Every Cargo attempt checked `df -g /` immediately beforehand. The final gate -rerun is currently blocked because the latest check reports 10 GB available, +rerun is currently blocked because the latest check reports 11 GB available, below the binding 12 GB floor; no below-floor Cargo command was started. +Per the campaign's parked-lane rule, this worktree's disposable `target/` was +then removed. The focused run did compile and execute its release test binary, +but that cleanup means there is no retained artifact mtime to present as final +build proof; the focused result is evidence for the named tests, not a +substitute for the blocked final gates. ## Stage LP: exact perrymaster request and falsifiers From 324166e4f6f2d65c6989d51e70fcffffe821bb17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 16:10:50 +0200 Subject: [PATCH 11/11] docs(perf): qualify replay test evidence Distinguish the two replay tests covered by the focused layout filter from the remaining named test whose full-suite gate was blocked by disk capacity. Claude-Session: https://claude.ai/code/session_011dhBmdn4vGgNibjo3oZqTo --- cc-perf-campaign/codex/REPORT_layout_residue.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cc-perf-campaign/codex/REPORT_layout_residue.md b/cc-perf-campaign/codex/REPORT_layout_residue.md index c670db000e..69611e80ac 100644 --- a/cc-perf-campaign/codex/REPORT_layout_residue.md +++ b/cc-perf-campaign/codex/REPORT_layout_residue.md @@ -25,6 +25,11 @@ differs. The replayed tests are present: - `surviving_young_masked_owner_is_rekeyed_and_stays_logged` - `old_layout_records_are_skipped_by_a_minor` +The focused `layout` filter ran the first and third green. The middle name +does not contain `layout`; its required full-suite execution is one of the +gates the 12 GB disk floor prevented, so it is present but not claimed green +on this machine. + The superseded `41a8af7da`, `bdd1fc003`, and `0a427a39f` were not replayed. Consequently their full-walk test names/guard are intentionally absent: `young_closure_prop_value_is_traced_and_moved_by_a_minor`,