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 1/6] 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 2/6] 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 3/6] 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 4/6] 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 5/6] 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 6/6] 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,