diff --git a/changelog.d/11634-gc-minor-fixed-cost.md b/changelog.d/11634-gc-minor-fixed-cost.md new file mode 100644 index 0000000000..41c8cb8f4f --- /dev/null +++ b/changelog.d/11634-gc-minor-fixed-cost.md @@ -0,0 +1 @@ +perf(gc): a copying minor's fixed cost drops from ~753k to ~98k instructions on an allocation-bound loop (Part of #11549). The intern table now carries a young-entry log, so a minor visits only slots naming young strings instead of all 8192 twice. The two array-tail transition tables are skipped while no entry has ever been published on the thread. Built-in closure metadata gets a young-only dead-owner prune. A minor skips the small-int / ASCII-char string caches, which only hold longlived pinned strings. Each skip is backed by a debug/test completeness check and a sabotage test. With this, a 4 MB nursery costs 466.0 instructions/iteration on the alloc loop against main's 469.2 at 16 MB, where it used to cost +3.0%. The nursery default is unchanged: qs/parse_nested and qs/stringify_nested still regress +2.5% / +7.2% at 4 MB from survivor copying, which is not fixed cost. diff --git a/crates/perry-runtime/src/gc/dead_owner.rs b/crates/perry-runtime/src/gc/dead_owner.rs index 32e7d37c6e..16da65e108 100644 --- a/crates/perry-runtime/src/gc/dead_owner.rs +++ b/crates/perry-runtime/src/gc/dead_owner.rs @@ -475,7 +475,7 @@ pub(super) const DEAD_KEY_PRUNES: &[DeadKeyPrune] = &[ table: "BUILTIN_CLOSURE_LENGTH + BUILTIN_CLOSURE_NON_CONSTRUCTABLE", owner: DeadKeyOwner::Closure, prune: crate::object::prune_dead_builtin_closure_metadata_owners, - young_prune: None, + young_prune: Some(crate::object::prune_dead_builtin_closure_metadata_owners_young), }, // #8040: `FUNCTION_CLASS_IDS` is keyed by a synthetic-class function // value's closure address, and is REKEYED (not re-derived) when that diff --git a/crates/perry-runtime/src/gc/tests/minor_fixed_cost.rs b/crates/perry-runtime/src/gc/tests/minor_fixed_cost.rs new file mode 100644 index 0000000000..008f666f7b --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/minor_fixed_cost.rs @@ -0,0 +1,180 @@ +//! #11549 direction 2: the fixed per-minor cost of two tables that a copying +//! minor used to walk whole on every pass. +//! +//! * The intern table (8192 slots) now carries a young-entry log +//! (`gc/young_log.rs`): a minor visits only slots that may name a young +//! string. +//! * The two array-tail transition tables (2 x 8192 slots) are skipped +//! outright while no entry has ever been published on the thread. +//! +//! Each gets the young-log proof shape: the young entry still MOVES through +//! the narrowed walk, the walk really was narrowed (a skip needs a counter), +//! and a writer that forgets to arm is caught (sabotage). + +use super::super::*; +use super::support::*; + +const INTERN_LOG: &str = "string.intern_table"; + +/// Leaves both tables empty however the test exits, including by the +/// expected panic of a sabotage test. +struct ClearTablesOnDrop; + +impl Drop for ClearTablesOnDrop { + fn drop(&mut self) { + crate::string::test_clear_intern_table(); + crate::object::array_tail_transition::test_clear(); + } +} + +fn fnv(bytes: &[u8]) -> u64 { + let mut hash = 0xcbf2_9ce4_8422_2325u64; + for &b in bytes { + hash ^= b as u64; + hash = hash.wrapping_mul(0x0100_0000_01b3); + } + hash +} + +/// A young string interned through the production writer and reachable ONLY +/// through the intern table is evacuated by a copying minor, and the table +/// slot is rewritten to the new address — through the log, not a whole-table +/// walk. +#[test] +fn young_interned_string_is_rewritten_through_the_log() { + let _guard = CopyingNurseryTestGuard::new(0); + let _clear = ClearTablesOnDrop; + gc_register_mutable_root_scanner(crate::string::scan_intern_table_roots_mut); + crate::string::test_clear_intern_table(); + + let bytes = b"minor-fixed-cost-young-intern"; + let hash = fnv(bytes); + let young = crate::string::js_string_from_bytes(bytes.as_ptr(), bytes.len() as u32); + assert!(crate::arena::pointer_in_nursery(young as usize)); + assert_eq!(crate::string::js_string_intern(young, hash), young); + + let _ = gc_collect_minor(); + + let after = crate::string::test_intern_slot_ptr(hash); + assert_ne!(after, 0, "the interned string must still be tabled"); + assert_ne!( + after, young as usize, + "the slot must name the evacuated copy, not from-space" + ); + unsafe { + assert_string_bytes(after as *const crate::StringHeader, bytes); + } + let row = young_log::last_walk(INTERN_LOG).expect("intern walk recorded"); + assert!( + row.partial, + "a copying minor must take the logged walk: {row:?}" + ); + assert!(row.visited >= 1, "the young slot must be visited: {row:?}"); + assert!( + row.visited < row.table_len, + "an 8192-slot table must not be walked whole: {row:?}" + ); +} + +/// An OLD interned string notes nothing, so a minor visits no intern slot at +/// all even though the table is not empty — the skip fired. +#[test] +fn old_interned_string_is_not_visited_by_a_minor() { + let _guard = CopyingNurseryTestGuard::new(0); + let _clear = ClearTablesOnDrop; + gc_register_mutable_root_scanner(crate::string::scan_intern_table_roots_mut); + crate::string::test_clear_intern_table(); + + let bytes = b"minor-fixed-cost-old-intern"; + let hash = fnv(bytes); + let old = crate::arena::arena_alloc_gc_old(64, 8, GC_TYPE_STRING) as *mut crate::StringHeader; + unsafe { + crate::string::test_init_string_bytes(old, bytes); + } + assert!(!young_log::addr_is_minor_relevant(old as usize)); + assert_eq!(crate::string::js_string_intern(old, hash), old); + + let _ = gc_collect_minor(); + + assert_eq!(crate::string::test_intern_slot_ptr(hash), old as usize); + let row = young_log::last_walk(INTERN_LOG).expect("intern walk recorded"); + assert!(row.partial, "{row:?}"); + assert_eq!( + row.visited, 0, + "an old interned string must not be visited: {row:?}" + ); +} + +/// SABOTAGE (rule 2): a writer that publishes a young string into a slot +/// without noting it is caught by the log-completeness check the minor-scoped +/// walk runs first. This is the check that turns "someone added an intern +/// writer and forgot `arm_intern_young`" into a red test instead of a +/// from-space pointer left in the table. +#[test] +#[should_panic(expected = "young log for string.intern_table does not name")] +fn intern_writer_that_skips_the_log_is_caught() { + let _guard = CopyingNurseryTestGuard::new(0); + let _clear = ClearTablesOnDrop; + crate::string::test_clear_intern_table(); + let young = young_leaf(); + crate::string::test_write_intern_slot_without_logging(7, young); + crate::string::test_check_intern_young_logged(); +} + +/// SABOTAGE for the array-tail skip: an entry published without arming +/// `array_tail_occupied` is caught where the skip relies on the flag. +#[test] +#[should_panic(expected = "without arming the flag first")] +fn array_tail_publish_without_arming_is_caught() { + let _guard = CopyingNurseryTestGuard::new(0); + let _clear = ClearTablesOnDrop; + crate::object::array_tail_transition::test_clear(); + crate::object::array_tail_transition::test_publish_without_arming(3); + crate::object::array_tail_transition::test_prune(); +} + +/// The skip's positive half: with nothing published the scan and the prune +/// visit nothing, and once the production writer has armed the flag they walk +/// the tables again. +#[test] +fn array_tail_tables_are_skipped_only_while_unarmed() { + let _guard = CopyingNurseryTestGuard::new(0); + let _clear = ClearTablesOnDrop; + crate::object::array_tail_transition::test_clear(); + assert!(!crate::object::array_tail_transition::test_tables_may_hold_entries()); + let _ = gc_collect_minor(); + assert!(!crate::object::array_tail_transition::test_tables_may_hold_entries()); + crate::object::array_tail_transition::test_arm_and_publish(3); + assert!(crate::object::array_tail_transition::test_tables_may_hold_entries()); + crate::object::array_tail_transition::test_clear(); +} + +/// The small-int / ASCII-char caches are skipped by a minor outright. Their +/// real entries (longlived, pinned) satisfy the skip's precondition. +#[test] +fn small_string_caches_hold_only_pinned_non_young_strings() { + let _guard = CopyingNurseryTestGuard::new(0); + // Fill a few entries through the production writers. + let _ = crate::string::js_number_to_string(7.0); + let _ = crate::string::js_number_to_string(200.0); + let _ = gc_collect_minor(); + crate::string::debug_assert_small_string_caches_not_minor_relevant(); +} + +/// SABOTAGE: a writer that publishes a young string into the cache breaks +/// the precondition of the minor skip, and the check says so. +#[test] +#[should_panic(expected = "not a pinned non-young string")] +fn small_int_cache_writer_publishing_a_young_string_is_caught() { + let _guard = CopyingNurseryTestGuard::new(0); + struct Restore; + impl Drop for Restore { + fn drop(&mut self) { + crate::string::test_write_small_int_cache_slot(255, std::ptr::null_mut()); + } + } + let _restore = Restore; + let young = young_leaf() as *mut crate::StringHeader; + crate::string::test_write_small_int_cache_slot(255, young); + crate::string::debug_assert_small_string_caches_not_minor_relevant(); +} diff --git a/crates/perry-runtime/src/gc/tests/mod.rs b/crates/perry-runtime/src/gc/tests/mod.rs index d4de64fe1b..ca89beeee8 100644 --- a/crates/perry-runtime/src/gc/tests/mod.rs +++ b/crates/perry-runtime/src/gc/tests/mod.rs @@ -62,6 +62,7 @@ mod lazy_tape_side_alloc; mod leaf_marks; mod map_store; mod mark_slot_hoists; +mod minor_fixed_cost; mod noncollecting_root_lock; mod object_create; mod old_free_intrusive; diff --git a/crates/perry-runtime/src/object/array_tail_transition.rs b/crates/perry-runtime/src/object/array_tail_transition.rs index 626e562d66..af55d532d6 100644 --- a/crates/perry-runtime/src/object/array_tail_transition.rs +++ b/crates/perry-runtime/src/object/array_tail_transition.rs @@ -383,6 +383,8 @@ pub(crate) fn record_numeric_tail_transition( // chain: the generic fallback mints a different predecessor, after which // no historical edge can match. Preserve colliding entries with bounded // open addressing instead of overwriting one direct-mapped slot. + // Arm before publish: once set, the collector walks both tables. + object_hot_for_owner(owner).array_tail_occupied.set(true); let forward = with_forward_for_owner(owner, |table| unsafe { insert_forward(table, entry) }); let reverse = with_reverse_for_owner(owner, |table| unsafe { insert_reverse(table, entry) }); if forward.is_none() && reverse.is_none() { @@ -502,7 +504,55 @@ unsafe fn scan_table( } } +/// May either tail table hold a non-`EMPTY` slot on this thread? +/// +/// `false` is exact, not a heuristic: the flag is set by the only production +/// writer (`record_numeric_tail_transition`) before its first `publish_entry`, +/// and nothing but `test_clear` ever returns a slot to `EMPTY` (a pruned entry +/// becomes a tombstone). So while it is clear, every slot of both tables is +/// `EMPTY` and the GC scan and prune below would visit nothing — they were +/// 2 x 8192 slot reads per pass, ~265k instructions per copying minor, on +/// every program that never extends `Array`. +#[inline] +fn tables_may_hold_entries() -> bool { + let occupied = crate::state::state().object_hot.array_tail_occupied.get(); + #[cfg(any(debug_assertions, test))] + if !occupied { + debug_assert_tables_empty(); + } + occupied +} + +/// The proof obligation behind a skipped walk, checked where it is relied on: +/// a slot published without arming `array_tail_occupied` fails here instead of +/// leaving an unvisited keys-array root in the table. +#[cfg(any(debug_assertions, test))] +fn debug_assert_tables_empty() { + for (name, table) in [ + ( + "forward", + &crate::state::state().object_hot.array_tail_forward, + ), + ( + "reverse", + &crate::state::state().object_hot.array_tail_reverse, + ), + ] { + let table = unsafe { &*table.get() }; + if let Some(index) = table.iter().position(|entry| !entry.is_empty()) { + panic!( + "array-tail {name} table slot {index} is occupied but \ + `array_tail_occupied` is clear: a writer published an entry \ + without arming the flag first, so the GC would skip its roots" + ); + } + } +} + pub(crate) fn scan_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { + if !tables_may_hold_entries() { + return; + } with_forward(|table| unsafe { scan_table(table, visitor) }); with_reverse(|table| unsafe { scan_table(table, visitor) }); } @@ -547,12 +597,19 @@ unsafe fn note_live_entries( #[cold] pub(crate) fn prune_invalid_entries() { + if !tables_may_hold_entries() { + return; + } with_forward(|table| unsafe { prune_table(table) }); with_reverse(|table| unsafe { prune_table(table) }); } #[cfg(test)] pub(crate) fn test_clear() { + crate::state::state() + .object_hot + .array_tail_occupied + .set(false); with_forward(|table| unsafe { for entry in (*table).iter_mut() { *entry = ArrayTailTransitionEntry::EMPTY; @@ -565,6 +622,45 @@ pub(crate) fn test_clear() { }); } +#[cfg(test)] +pub(crate) fn test_tables_may_hold_entries() -> bool { + tables_may_hold_entries() +} + +#[cfg(test)] +pub(crate) fn test_prune() { + prune_invalid_entries(); +} + +#[cfg(test)] +fn test_entry(shape_id: u32) -> ArrayTailTransitionEntry { + ArrayTailTransitionEntry { + predecessor_keys: 0, + successor_keys: 0, + predecessor_shape_id: shape_id, + successor_shape_id: shape_id + 1, + slot: 0, + array_index: 0, + predecessor_live_inline_slots: 0, + successor_live_inline_slots: 0, + } +} + +/// A writer that forgets to arm: publish straight into the forward table. +#[cfg(test)] +pub(crate) fn test_publish_without_arming(shape_id: u32) { + with_forward(|table| unsafe { insert_forward(table, test_entry(shape_id)) }); +} + +/// Arm exactly as `record_numeric_tail_transition` does, then publish. +#[cfg(test)] +pub(crate) fn test_arm_and_publish(shape_id: u32) { + object_hot_for_owner(std::ptr::null()) + .array_tail_occupied + .set(true); + with_forward(|table| unsafe { insert_forward(table, test_entry(shape_id)) }); +} + #[cfg(test)] mod zeroed_table_tests { use super::*; diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index d761a5ed66..f2331cbd78 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -648,6 +648,10 @@ pub(crate) struct ObjectHotTables { /// entry and fall back to the complete open-addressed tables. pub(crate) array_tail_direct: std::cell::UnsafeCell>, + /// Set before the first entry is published into either tail table; while + /// false every slot of both is `EMPTY`, so the GC scan and the prune have + /// nothing to visit (see `array_tail_transition::tables_may_hold_entries`). + pub(crate) array_tail_occupied: Cell, } impl ObjectHotTables { @@ -681,6 +685,7 @@ impl ObjectHotTables { array_tail_direct: std::cell::UnsafeCell::new(crate::zeroed_cache::new_zeroed_cache( array_tail_transition::ARRAY_TAIL_TRANSITION_CACHE_SIZE, )), + array_tail_occupied: Cell::new(false), } } } diff --git a/crates/perry-runtime/src/object/native_module.rs b/crates/perry-runtime/src/object/native_module.rs index a544a6a808..61471deaa2 100644 --- a/crates/perry-runtime/src/object/native_module.rs +++ b/crates/perry-runtime/src/object/native_module.rs @@ -46,12 +46,12 @@ pub(crate) use callable_exports::{ module_cjs_cache_value, module_cjs_extensions_value, module_cjs_global_paths_value, module_cjs_path_cache_value, module_cjs_prototype_for_instance, module_constants_value, native_string_value, prune_dead_builtin_closure_metadata_owners, - scan_builtin_closure_metadata_roots_mut, scan_tls_derived_prototype_roots_mut, - set_bound_native_closure_name, set_builtin_closure_length, - set_builtin_closure_non_constructable, sqlite_session_constructor_value, - sqlite_statement_sync_constructor_value, timers_promises_parent_namespace, - tls_constructor_prototype_is_instance_of, util_inspect_default_options_value, - zlib_codes_object, + prune_dead_builtin_closure_metadata_owners_young, scan_builtin_closure_metadata_roots_mut, + scan_tls_derived_prototype_roots_mut, set_bound_native_closure_name, + set_builtin_closure_length, set_builtin_closure_non_constructable, + sqlite_session_constructor_value, sqlite_statement_sync_constructor_value, + timers_promises_parent_namespace, tls_constructor_prototype_is_instance_of, + util_inspect_default_options_value, zlib_codes_object, }; pub(crate) use constants::get_native_module_constant; pub(crate) use constructor_exports::{ 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 f3fb0be864..a53141cb94 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 @@ -193,6 +193,39 @@ pub(crate) fn prune_dead_builtin_closure_metadata_owners(is_dead_owner: &dyn Fn( }); } +/// [`prune_dead_builtin_closure_metadata_owners`] for a MINOR. A minor can +/// find only a minor-collectible owner dead, and every such owner is in +/// `BUILTIN_CLOSURE_YOUNG`: its writers note it (rule 1) and the minor's +/// young-scoped scan re-logs every owner that is still collectible, which a +/// dead owner (never moved, still in from-space) is. So the log is the +/// candidate set, and the full `retain` over both maps -- ~275k instructions +/// per copying minor on dotenv, where the maps hold every built-in closure +/// the program ever made -- is only needed on a full collection. +pub(crate) fn prune_dead_builtin_closure_metadata_owners_young( + is_dead_owner: &dyn Fn(usize) -> bool, +) { + #[cfg(any(debug_assertions, test))] + BUILTIN_CLOSURE_YOUNG.with(|log| { + log.borrow() + .debug_assert_logged(LOG_NAME, &relevant_owners()) + }); + let candidates = BUILTIN_CLOSURE_YOUNG.with(|log| log.borrow_mut().take_sorted()); + let mut kept = Vec::with_capacity(candidates.len()); + for owner in candidates { + if is_dead_owner(owner) { + BUILTIN_CLOSURE_LENGTH.with(|m| { + m.borrow_mut().remove(&owner); + }); + BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|m| { + m.borrow_mut().remove(&owner); + }); + } else if crate::gc::young_log::addr_is_minor_collectible(owner) { + kept.push(owner); + } + } + BUILTIN_CLOSURE_YOUNG.with(|log| log.borrow_mut().extend(kept)); +} + #[cfg(test)] mod tests { use super::*; @@ -217,4 +250,92 @@ mod tests { "sabotage: suppressing the setter's note must trip completeness" ); } + + fn old_closure() -> usize { + crate::arena::arena_alloc_gc_old( + std::mem::size_of::(), + std::mem::align_of::(), + crate::gc::GC_TYPE_CLOSURE, + ) as usize + } + + fn clear_all() { + BUILTIN_CLOSURE_LENGTH.with(|m| m.borrow_mut().clear()); + BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|m| m.borrow_mut().clear()); + BUILTIN_CLOSURE_YOUNG.with(|log| log.borrow_mut().clear()); + } + + /// The young prune's candidates are the log: a dead YOUNG owner's entries + /// go, and an OLD owner is never even asked about — even under a predicate + /// that calls everything dead, which only a full collection may apply. + #[test] + fn young_prune_drops_dead_young_owners_and_never_touches_old_ones() { + let _lock = crate::gc::global_side_table_test_lock(); + clear_all(); + let young = crate::closure::js_closure_alloc(std::ptr::null(), 0) as usize; + let old = old_closure(); + assert!(crate::gc::young_log::addr_is_minor_collectible(young)); + assert!(!crate::gc::young_log::addr_is_minor_collectible(old)); + set_builtin_closure_length(young, 2); + set_builtin_closure_non_constructable(young); + set_builtin_closure_length(old, 5); + set_builtin_closure_non_constructable(old); + + prune_dead_builtin_closure_metadata_owners_young(&|_| true); + + let (young_len, young_nc, old_len, old_nc) = ( + BUILTIN_CLOSURE_LENGTH.with(|m| m.borrow().get(&young).copied()), + BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|m| m.borrow().contains(&young)), + BUILTIN_CLOSURE_LENGTH.with(|m| m.borrow().get(&old).copied()), + BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|m| m.borrow().contains(&old)), + ); + clear_all(); + assert_eq!( + (young_len, young_nc), + (None, false), + "dead young owner kept" + ); + assert_eq!( + (old_len, old_nc), + (Some(5), true), + "old owner pruned by a minor" + ); + } + + /// A live young owner stays in the maps AND in the log, so the next minor + /// still considers it. + #[test] + fn young_prune_keeps_live_young_owners_logged() { + let _lock = crate::gc::global_side_table_test_lock(); + clear_all(); + let young = crate::closure::js_closure_alloc(std::ptr::null(), 0) as usize; + set_builtin_closure_length(young, 1); + prune_dead_builtin_closure_metadata_owners_young(&|_| false); + let still = BUILTIN_CLOSURE_LENGTH.with(|m| m.borrow().get(&young).copied()); + let logged = BUILTIN_CLOSURE_YOUNG.with(|log| log.borrow_mut().take_sorted()); + clear_all(); + assert_eq!(still, Some(1)); + assert_eq!(logged, vec![young]); + } + + /// SABOTAGE: a writer that publishes a young owner without noting it is + /// caught by the young prune's completeness check, before the prune + /// could silently keep that owner's entry after it dies. + #[test] + fn young_prune_rejects_a_suppressed_writer() { + let _lock = crate::gc::global_side_table_test_lock(); + clear_all(); + 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_non_constructable(closure); + TEST_SUPPRESS_BUILTIN_CLOSURE_YOUNG_NOTE.with(|flag| flag.set(false)); + let missed = std::panic::catch_unwind(|| { + prune_dead_builtin_closure_metadata_owners_young(&|_| false); + }); + clear_all(); + assert!( + missed.is_err(), + "the young prune must refuse an incomplete log" + ); + } } diff --git a/crates/perry-runtime/src/string/format.rs b/crates/perry-runtime/src/string/format.rs index b9f3116142..4b8762789e 100644 --- a/crates/perry-runtime/src/string/format.rs +++ b/crates/perry-runtime/src/string/format.rs @@ -336,6 +336,18 @@ pub fn scan_small_int_cache_roots(mark: &mut dyn FnMut(f64)) { } pub fn scan_small_int_cache_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { + // A minor-scoped pass can neither move nor free a longlived object, and + // it does not trace through a pinned one (`mark_classified` skips a + // `Longlived` header carrying `GC_FLAG_PINNED`). Both writers of these + // caches (`small_int_cache_fill`, `ascii_char_string`) publish only + // longlived, pinned leaf strings, so every visit a minor would make here + // is a no-op: 384 classifications per pass, two passes per copying minor. + // Full-scope passes (which can relocate longlived space) still walk both. + if visitor.young_scope() { + #[cfg(any(debug_assertions, test))] + debug_assert_small_string_caches_not_minor_relevant(); + return; + } SMALL_INT_CACHE.with(|c| unsafe { for slot in (*c.get()).iter_mut() { let mut addr = *slot as usize; @@ -359,6 +371,47 @@ pub fn scan_small_int_cache_roots_mut(visitor: &mut crate::gc::RuntimeRootVisito }); } +/// The proof obligation behind the minor skip above, checked where it is +/// relied on: a writer that ever publishes a young or unpinned string into +/// either cache fails here instead of leaving an unvisited young root. +#[cfg(any(debug_assertions, test))] +pub(crate) fn debug_assert_small_string_caches_not_minor_relevant() { + let check = |cache: &'static str, ptr: *mut StringHeader| { + if ptr.is_null() { + return; + } + let addr = ptr as usize; + let pinned = unsafe { + let header = (addr - crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader; + (*header).gc_flags & crate::gc::GC_FLAG_PINNED != 0 + }; + assert!( + pinned && !crate::gc::young_log::addr_is_minor_collectible(addr), + "{cache} holds {addr:#x}, which is not a pinned non-young string: \ + a minor-scoped scan skips this cache, so it must never hold one" + ); + }; + SMALL_INT_CACHE.with(|c| unsafe { + for &ptr in (*c.get()).iter() { + check("SMALL_INT_CACHE", ptr); + } + }); + ASCII_CHAR_CACHE.with(|c| unsafe { + for &ptr in (*c.get()).iter() { + check("ASCII_CHAR_CACHE", ptr); + } + }); +} + +/// A writer that breaks the residency contract: publish `ptr` as-is. +#[cfg(test)] +pub(crate) fn test_write_small_int_cache_slot(idx: usize, ptr: *mut StringHeader) { + SMALL_INT_CACHE.with(|c| unsafe { + // GC_STORE_AUDIT(ROOT): test-only sabotage writer; SMALL_INT_CACHE is scanned by scan_small_int_cache_roots_mut. + (*c.get())[idx % SMALL_INT_CACHE_SIZE] = ptr; + }); +} + fn is_undefined_arg(value: f64) -> bool { value.to_bits() == crate::value::TAG_UNDEFINED } diff --git a/crates/perry-runtime/src/string/intern.rs b/crates/perry-runtime/src/string/intern.rs index da9d2ef971..f67b50e49d 100644 --- a/crates/perry-runtime/src/string/intern.rs +++ b/crates/perry-runtime/src/string/intern.rs @@ -36,6 +36,28 @@ crate::perry_thread_local! { std::cell::UnsafeCell::new(crate::zeroed_cache::new_zeroed_cache(INTERN_TABLE_SIZE)); } +crate::perry_thread_local! { + /// Intern-table slots that may hold a string a minor can act on + /// (`gc/young_log.rs`). A minor-scoped `scan_intern_table_roots_mut` + /// visits only these instead of all [`INTERN_TABLE_SIZE`] slots — on an + /// allocation-bound loop the full walk was ~200k instructions per pass, + /// two passes per copying minor, over a table that was usually empty. + static INTERN_YOUNG: std::cell::RefCell> = + const { std::cell::RefCell::new(crate::gc::young_log::YoungLog::new()) }; +} + +const INTERN_YOUNG_LOG_NAME: &str = "string.intern_table"; + +/// Rule 1 of `gc/young_log.rs`: every writer of a slot calls this BEFORE the +/// slot names `string_ptr`. An old string notes nothing — a minor can neither +/// move nor free it, so visiting it would be a no-op. +#[inline] +fn arm_intern_young(slot: usize, string_ptr: usize) { + if crate::gc::young_log::addr_is_minor_relevant(string_ptr) { + INTERN_YOUNG.with(|log| log.borrow_mut().note(slot as u32)); + } +} + #[inline] pub(crate) fn with_intern_table( f: impl FnOnce(*mut [InternEntry; INTERN_TABLE_SIZE]) -> R, @@ -78,6 +100,7 @@ pub extern "C" fn js_string_intern(key: *const StringHeader, hash: u64) -> *cons } // Miss or collision — insert (evict on collision) + arm_intern_young(slot, key as usize); with_intern_table(|table| { (*table)[slot] = InternEntry { hash, @@ -161,6 +184,7 @@ pub(crate) fn intern_dispatch_bytes( (*gc_header).gc_flags |= crate::gc::GC_FLAG_INTERNED; (*key).refcount = 0; } + arm_intern_young(slot, key as usize); with_intern_table(|table| unsafe { (*table)[slot] = InternEntry { hash, @@ -261,12 +285,79 @@ pub fn scan_intern_table_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<' // `Object.prototype` `then` verdict) must not be flushed at loop-poll // cadence by the incremental collector. crate::object::prop_plan::prop_plan_gc_epoch_bump(); + // A minor-scoped pass can act only on a slot that names a young string, + // and every such slot is in `INTERN_YOUNG` (rule 1), so it visits the log + // instead of the table. A full pass walks the table and rebuilds the log. + if visitor.young_scope() { + #[cfg(any(debug_assertions, test))] + debug_assert_intern_young_logged(); + let mut kept = INTERN_YOUNG.with(|log| log.borrow_mut().take_spare()); + let batch = INTERN_YOUNG.with(|log| log.borrow_mut().take_sorted()); + let logged = batch.len() as u64; + with_intern_table(|table| unsafe { + for &slot in &batch { + let entry = &mut (*table)[slot as usize]; + visitor.visit_tagged_usize_slot(&mut entry.string_ptr, crate::value::STRING_TAG); + if crate::gc::young_log::addr_is_minor_relevant(entry.string_ptr) { + kept.push(slot); + } + } + }); + let kept_len = kept.len() as u64; + INTERN_YOUNG.with(|log| log.borrow_mut().extend(kept)); + crate::gc::young_log::note_walk( + INTERN_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: true, + logged, + visited: logged, + kept: kept_len, + table_len: INTERN_TABLE_SIZE as u64, + }, + ); + return; + } + let _ = INTERN_YOUNG.with(|log| log.borrow_mut().take_sorted()); + let mut kept = Vec::new(); with_intern_table(|table| unsafe { for i in 0..INTERN_TABLE_SIZE { let entry = &mut (*table)[i]; visitor.visit_tagged_usize_slot(&mut entry.string_ptr, crate::value::STRING_TAG); + if crate::gc::young_log::addr_is_minor_relevant(entry.string_ptr) { + kept.push(i as u32); + } } }); + let kept_len = kept.len() as u64; + INTERN_YOUNG.with(|log| log.borrow_mut().extend(kept)); + crate::gc::young_log::note_walk( + INTERN_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: false, + logged: INTERN_TABLE_SIZE as u64, + visited: INTERN_TABLE_SIZE as u64, + kept: kept_len, + table_len: INTERN_TABLE_SIZE as u64, + }, + ); +} + +/// Rule 2 of `gc/young_log.rs`: re-derive the minor-relevant slots from the +/// table itself and require the log to name every one. A writer that +/// publishes without [`arm_intern_young`] fails here on the next minor instead +/// of leaving a from-space pointer in the table. +#[cfg(any(debug_assertions, test))] +fn debug_assert_intern_young_logged() { + let relevant: Vec = with_intern_table(|table| unsafe { + (0..INTERN_TABLE_SIZE) + .filter(|&i| crate::gc::young_log::addr_is_minor_relevant((*table)[i].string_ptr)) + .map(|i| i as u32) + .collect() + }); + INTERN_YOUNG.with(|log| { + log.borrow() + .debug_assert_logged(INTERN_YOUNG_LOG_NAME, &relevant) + }); } /// #11507: the table is zero-allocated rather than filled, so a thread's first @@ -287,6 +378,7 @@ fn fresh_thread_intern_table_reads_empty_everywhere() { #[cfg(test)] pub(crate) fn test_seed_intern_table_root(string_ptr: usize) { + arm_intern_young(0, string_ptr); with_intern_table(|table| unsafe { (*table)[0] = InternEntry { hash: 0xC0DEC0DE, @@ -295,6 +387,62 @@ pub(crate) fn test_seed_intern_table_root(string_ptr: usize) { }); } +/// Empty every slot and the young log: a test that asserts on the log's walk +/// must not inherit another test's entries on a reused thread. +#[cfg(test)] +pub(crate) fn test_clear_intern_table() { + INTERN_YOUNG.with(|log| log.borrow_mut().clear()); + with_intern_table(|table| unsafe { + for entry in (*table).iter_mut() { + *entry = InternEntry { + hash: 0, + string_ptr: 0, + }; + } + }); +} + +/// The pointer the slot for `hash` currently names (0 when empty). +#[cfg(test)] +pub(crate) fn test_intern_slot_ptr(hash: u64) -> usize { + with_intern_table(|table| unsafe { (*table)[(hash as usize) & INTERN_TABLE_MASK].string_ptr }) +} + +/// A writer that forgets rule 1: publish without noting the slot. +#[cfg(test)] +pub(crate) fn test_write_intern_slot_without_logging(slot: usize, string_ptr: usize) { + with_intern_table(|table| unsafe { + (*table)[slot & INTERN_TABLE_MASK] = InternEntry { + hash: 0xC0DE, + string_ptr, + }; + }); +} + +/// Run the rule-2 check the minor-scoped walk runs first. +#[cfg(test)] +pub(crate) fn test_check_intern_young_logged() { + debug_assert_intern_young_logged(); +} + +/// Initialise a raw string allocation (e.g. an old-generation one a test +/// allocated directly) with ASCII `bytes`. +/// +/// # Safety +/// `ptr` must be a string allocation with room for `bytes`. +#[cfg(test)] +pub(crate) unsafe fn test_init_string_bytes(ptr: *mut StringHeader, bytes: &[u8]) { + init_string_header( + ptr, + bytes.len() as u32, + bytes.len() as u32, + bytes.len() as u32, + 0, + 0, + ); + std::ptr::copy_nonoverlapping(bytes.as_ptr(), string_data(ptr) as *mut u8, bytes.len()); +} + #[cfg(test)] pub(crate) fn test_intern_table_root() -> usize { with_intern_table(|table| unsafe { (*table)[0].string_ptr }) diff --git a/crates/perry-runtime/src/string/mod.rs b/crates/perry-runtime/src/string/mod.rs index 40de0c9758..5ff31dbf49 100644 --- a/crates/perry-runtime/src/string/mod.rs +++ b/crates/perry-runtime/src/string/mod.rs @@ -221,6 +221,10 @@ pub(crate) fn canonical_key(name: &[u8]) -> *mut StringHeader { } #[cfg(feature = "regex-engine")] pub use crate::regex::{js_string_split_js, js_string_split_n}; +#[cfg(test)] +pub(crate) use format::{ + debug_assert_small_string_caches_not_minor_relevant, test_write_small_int_cache_slot, +}; pub use format::{ js_number_to_exponential, js_number_to_fixed, js_number_to_precision, js_number_to_string, js_number_to_string_box, scan_small_int_cache_roots, scan_small_int_cache_roots_mut, @@ -232,6 +236,11 @@ pub use html::{ js_string_strike, js_string_sub, js_string_sup, }; pub use intern::{js_string_intern, scan_intern_table_roots, scan_intern_table_roots_mut}; +#[cfg(test)] +pub(crate) use intern::{ + test_check_intern_young_logged, test_clear_intern_table, test_init_string_bytes, + test_intern_slot_ptr, test_write_intern_slot_without_logging, +}; pub use io::{js_string_error, js_string_print, js_string_warn}; pub(crate) use iter_object::dispatch_string_iterator_method_builtin; pub use iter_object::{