diff --git a/changelog.d/9755-gc-side-table-young-logs.md b/changelog.d/9755-gc-side-table-young-logs.md index 5fdf78524d..8ac58f2588 100644 --- a/changelog.d/9755-gc-side-table-young-logs.md +++ b/changelog.d/9755-gc-side-table-young-logs.md @@ -3,9 +3,9 @@ - **Minor collections no longer walk every runtime side table.** A copying minor's three root-scan passes — and a budgeted minor's initial root scan and final remark — visited every entry of the closure dynamic-prop tables, - the string-keyed descriptor tables, the shape family/slot-index maps, the - transition cache and the shape cache on every collection, to discover that - nothing in them pointed at the nursery. On the compiled claude-code TUI + the string-keyed descriptor tables, the shape family/slot-index maps and + the transition cache on every collection, to discover that nothing in them + pointed at the nursery. On the compiled claude-code TUI that was ~35k shape families, ~120k descriptors and ~13k closure owners per walk, 41 minors per streamed reply, all reporting `slots=0`: 34–56 ms of scanner time per minor. @@ -26,6 +26,13 @@ prints `[gc-young-log]` rows (logged / visited / kept / table size) per table and cycle. + The **shape cache** was measured and deliberately left on its plain walk. + Its canonical keys arrays are allocated in the longlived arena, which + `addr_is_minor_relevant` must answer `true` for, so no entry ever leaves a + log there: on the claude-code TUI the log named 100 % of the table in every + one of 107 collections (0 % skipped) and cost **35 % more** than the walk it + replaced. The four tables above skip 75–93 %. + - **The post-minor remembered-set coverage restore is proportional to what the dirty scan could not cover.** `restore_surviving_dirty_coverage` (#5029) re-walked every slot of every object on the pre-cycle dirty pages diff --git a/crates/perry-runtime/src/gc/tests/young_log_tests.rs b/crates/perry-runtime/src/gc/tests/young_log_tests.rs index db66fadbf6..e6cf25887d 100644 --- a/crates/perry-runtime/src/gc/tests/young_log_tests.rs +++ b/crates/perry-runtime/src/gc/tests/young_log_tests.rs @@ -53,6 +53,11 @@ fn walk(table: &'static str) -> young_log::YoungLogWalk { young_log::last_walk(table).unwrap_or_else(|| panic!("no walk recorded for {table}")) } +/// For a table that is deliberately NOT young-logged: no walk row at all. +fn walk_opt(table: &'static str) -> Option { + young_log::last_walk(table) +} + // ---------------------------------------------------------------- closures #[test] @@ -430,15 +435,18 @@ fn young_transition_key_under_an_old_target_arms_the_log_through_the_writer() { ); } +/// The shape cache is deliberately NOT young-logged (see +/// `scan_shape_cache_roots_mut`): its keys arrays are longlived, so a log +/// there names every entry forever and skips nothing. This pins the walk that +/// replaced it — a young entry reachable only through the cache still moves +/// and is re-keyed in both the inline slot and the overflow map. #[test] -fn young_shape_cache_entry_is_moved_through_the_log() { +fn shape_cache_entry_is_moved_by_the_plain_walk() { let _guard = CopyingNurseryTestGuard::new(0); gc_register_mutable_root_scanner(crate::object::scan_shape_cache_roots_mut); - // Reachable ONLY through the cache (which roots it). Seeded through the - // PRODUCTION writer (`shape_cache_insert`), not a test seam: a seam that - // arms the log itself makes this test pass with the writer's own arm site - // deleted, which is how #9755 shipped an unenforced rule 1. + // Reachable ONLY through the cache (which roots it), seeded through the + // PRODUCTION writer (`shape_cache_insert`), not a test seam. let keys = unsafe { young_keys_array() }; let shape_id = 0x9754_0001; crate::object::test_shape_cache_insert(shape_id, keys); @@ -455,9 +463,11 @@ fn young_shape_cache_entry_is_moved_through_the_log() { inline, overflow, "inline and overflow must agree on the new address" ); - let row = walk("object.shape_cache"); - assert!(row.partial); - assert!(row.visited >= 1, "{row:?}"); + assert!( + walk_opt("object.shape_cache").is_none(), + "the shape cache must not report a young-log walk: #9755's log for it \ + skipped 0 % and cost 35 % more than this walk, and was removed" + ); } // --------------------------------------------------------------------------- diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index c7b48fc87d..361ed3289b 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -662,21 +662,6 @@ fn shape_cache_get_with_id(shape_id: u32) -> (*mut ArrayHeader, u32) { .unwrap_or((std::ptr::null_mut(), 0)) } -/// Rule 1 of `gc/young_log.rs` for the shape cache: log `shape_id` BEFORE the -/// entry naming `keys_array` becomes findable. -/// -/// Every writer of the cache — the production `shape_cache_insert` and the -/// `#[cfg(test)]` seed seam — arms through this one function. A seam that -/// re-implements the predicate is the failure mode this exists to prevent: -/// the tests then validate an arming rule that is not the one that ships, and -/// deleting the production arm site stays green. -#[inline] -pub(super) fn arm_shape_cache_young(shape_id: u32, keys_array: *mut ArrayHeader) { - if crate::gc::young_log::addr_is_minor_relevant(keys_array as usize) { - SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().note(shape_id)); - } -} - /// Insert a keys_array into the cache. Updates the inline slot /// (evicting any prior entry there) and also writes to the overflow /// map so misses on the inline cache still find the value. @@ -706,9 +691,6 @@ fn shape_cache_insert(shape_id: u32, keys_array: *mut ArrayHeader) { }; let st = crate::state::state(); let slot = (shape_id as usize) & (SHAPE_INLINE_CACHE_SIZE - 1); - // #9754 rule 1: log the id BEFORE the entry is published when the keys - // array can matter to a minor. - arm_shape_cache_young(shape_id, keys_array); unsafe { // GC_STORE_AUDIT(ROOT): shape_inline_cache entries are scanned by scan_shape_cache_roots_mut. let entry = &mut (*st.object_hot.shape_inline_cache.get())[slot]; @@ -822,14 +804,9 @@ crate::perry_thread_local! { /// `scan_transition_cache_roots_mut` visits only these. static TRANSITION_CACHE_YOUNG: RefCell> = const { RefCell::new(crate::gc::young_log::YoungLog::new()) }; - /// #9754: shape-cache ids (inline slot and overflow key alike) whose keys - /// array may still be acted on by a minor. - static SHAPE_CACHE_YOUNG: RefCell> = - const { RefCell::new(crate::gc::young_log::YoungLog::new()) }; } const TRANSITION_CACHE_YOUNG_LOG_NAME: &str = "object.transition_cache"; -const SHAPE_CACHE_YOUNG_LOG_NAME: &str = "object.shape_cache"; /// Is a transition-cache entry still something a minor can act on? #[inline] @@ -1334,7 +1311,6 @@ pub(crate) fn test_shape_cache_insert(shape_id: u32, keys_array: *mut ArrayHeade pub(crate) fn test_seed_shape_cache_root(shape_id: u32, keys_array: *mut ArrayHeader) { let st = crate::state::state(); let slot = (shape_id as usize) & (SHAPE_INLINE_CACHE_SIZE - 1); - arm_shape_cache_young(shape_id, keys_array); unsafe { // GC_STORE_AUDIT(ROOT): test seed mirrors shape_inline_cache roots scanned by scan_shape_cache_roots_mut. let entry = &mut (*st.object_hot.shape_inline_cache.get())[slot]; diff --git a/crates/perry-runtime/src/object/shapes.rs b/crates/perry-runtime/src/object/shapes.rs index f61cf84098..3e0ebf1a01 100644 --- a/crates/perry-runtime/src/object/shapes.rs +++ b/crates/perry-runtime/src/object/shapes.rs @@ -45,7 +45,7 @@ pub(crate) use shapes_slot_list::{ object_shape_hole_count, publish_object_shape_holes, rekey_stable_tombstone_shape_after_squeeze, retire_owned_shape_history, shape_index_migrate_after_delete, shape_index_shift_in_place, - try_update_stable_tombstone_shape, try_update_stable_tombstone_shape_cached, SlotList, + try_update_stable_tombstone_shape, try_update_stable_tombstone_shape_cached, SlotIndex, }; use shapes_store::{ IdList, ShapeRecord, ShapeSlab, RECORD_FLAG_CACHE_CARRIER, RECORD_FLAG_CARRIED_SEEN, @@ -68,7 +68,7 @@ pub(crate) struct ShapeIndex { /// `bench_populated_delete.ts` — perry's worst object-model gap against /// node — `hash_one::<&usize>` plus `sip::Hasher::write` were **14.7% of /// self time**, second only to the lookup that performs them. - slots: crate::fast_hash::PtrHashMap, + slots: SlotIndex, } /// Immutable facts named by one ShapeId, copied out of the table. @@ -1627,12 +1627,7 @@ unsafe fn index_range(shape: &mut ShapeIndex, keys: *const ArrayHeader, key_coun let v = crate::JSValue::from_bits((*slots.add(i as usize)).to_bits()); if let Some(b) = crate::string::js_string_key_bytes(v, &mut sso) { let h = super::key_bytes_hash(b.as_ptr(), b.len()); - match shape.slots.entry(h) { - std::collections::hash_map::Entry::Occupied(mut e) => e.get_mut().push(i), - std::collections::hash_map::Entry::Vacant(e) => { - e.insert(SlotList::One(i)); - } - } + shape.slots.push(h, i); } } shape.indexed_len = key_count; @@ -1699,7 +1694,7 @@ pub(crate) unsafe fn shape_slot_lookup_verdict( inner.note_young_keys(keys_id as u64); inner.indices.entry(keys_id).or_insert(ShapeIndex { indexed_len: 0, - slots: crate::fast_hash::new_ptr_hash_map(), + slots: SlotIndex::new(), }) } }; @@ -1712,12 +1707,9 @@ pub(crate) unsafe fn shape_slot_lookup_verdict( } else { KeysIndexVerdict::Unindexed }; - let Some(candidates) = shape.slots.get(&key_hash) else { - return absent; - }; let mut sso = [0u8; crate::value::SHORT_STRING_MAX_LEN]; let (slots, slot_len) = super::keys_array_dense_slots(keys); - for &i in candidates.iter() { + for i in shape.slots.candidates(key_hash) { if (i as usize) >= slot_len || i >= key_count { continue; } @@ -1746,12 +1738,7 @@ pub(crate) fn shape_note_append( if let Some(shape) = inner.indices.get_mut(&(keys as usize)) { if shape.indexed_len + 1 == new_count { shape.indexed_len = new_count; - match shape.slots.entry(key_hash) { - std::collections::hash_map::Entry::Occupied(mut e) => e.get_mut().push(slot), - std::collections::hash_map::Entry::Vacant(e) => { - e.insert(SlotList::One(slot)); - } - } + shape.slots.push(key_hash, slot); } } } @@ -1761,12 +1748,7 @@ pub(crate) fn shape_note_append( pub(crate) fn shape_note_hit(keys: *const ArrayHeader, key_hash: u64, slot: u32) { let mut inner = crate::state::state().shapes.inner.borrow_mut(); if let Some(shape) = inner.indices.get_mut(&(keys as usize)) { - match shape.slots.entry(key_hash) { - std::collections::hash_map::Entry::Occupied(mut e) => e.get_mut().push(slot), - std::collections::hash_map::Entry::Vacant(e) => { - e.insert(SlotList::One(slot)); - } - } + shape.slots.push(key_hash, slot); } } @@ -2314,17 +2296,13 @@ pub(crate) fn shrink_shape_tables() { /// `PERRY_GC_CENSUS`: the by-id slab, the per-shape key indices, the /// exact-facts accelerator and the keys-address family index. pub(crate) fn shape_table_census() -> Vec { - use crate::gc::census::{hash_table_bytes, map_bytes}; + use crate::gc::census::map_bytes; let table = &crate::state::state().shapes; let inner = table.inner.borrow(); let slab = table.slab(); let mut rows = Vec::new(); rows.push(("shapes.descriptors", slab.len(), slab.estimated_bytes())); - let index_inner: usize = inner - .indices - .values() - .map(|ix| hash_table_bytes(ix.slots.capacity(), std::mem::size_of::<(u64, SlotList)>())) - .sum(); + let index_inner: usize = inner.indices.values().map(|ix| ix.slots.heap_bytes()).sum(); rows.push(( "shapes.indices", inner.indices.len(), diff --git a/crates/perry-runtime/src/object/shapes_slot_list.rs b/crates/perry-runtime/src/object/shapes_slot_list.rs index e6f8b7b1e9..9c482d518d 100644 --- a/crates/perry-runtime/src/object/shapes_slot_list.rs +++ b/crates/perry-runtime/src/object/shapes_slot_list.rs @@ -1,81 +1,283 @@ -//! `SlotList` — the shape key index's per-hash slot list, in a sibling file. +//! `SlotIndex` — the shape key index's content-hash → slot table, in a +//! sibling file. //! //! Extracted from `shapes.rs` to keep it under the repo's 2000-line cap. -//! Also carries the two helpers that are mostly `SlotList` manipulation: +//! Also carries the two helpers that are mostly index manipulation: //! `record_shape_scan_outcome` (the shape scanner's per-descriptor //! bookkeeping) and `shape_index_migrate_after_delete`. -/// Slots sharing one content hash. +/// Content hash → candidate slots for one shape, as an open-addressing table +/// of packed `(hash tag, slot)` cells. /// -/// Almost always exactly one: the key is an FNV-1a hash of distinct property -/// names, so a bucket with two entries is a genuine hash collision. Storing -/// that common case inline removes a heap allocation PER KEY from every index -/// build — and the index is rebuilt on every populated delete, so a 500-key -/// object was making ~500 `Vec` allocations per `delete`. Allocator and page -/// churn is the dominant cost on that benchmark (`clear_page_erms` 5.6%, -/// `mi_free` 4.2%, `RawVecInner::finish_grow` 2.9%), well above the lookup -/// work itself. +/// #9754 memory. This used to be a `PtrHashMap` per shape — +/// a 33-byte hashbrown bucket (`(u64, enum { One(u32), Many(Vec) })` +/// plus its control byte) for every key, in a power-of-two table. The +/// compiled claude-code TUI holds ~34.5k of these indices, one per keys array +/// past `KEYS_INDEX_THRESHOLD`, at **2.6 KB each: 89 MB** of the process's +/// 170 MB of side tables (`PERRY_GC_CENSUS`, 2026-09-04), while the objects +/// they describe are ~40 keys wide. +/// +/// The table's only job is to answer "which slots might hold a key with this +/// hash" — every hit is then re-validated against the key BYTES +/// (`shape_slot_lookup_verdict`), so a wrong or colliding answer is a miss, +/// never a wrong property. That validation is what lets the stored hash be +/// narrow: a cell is a 16-bit tag (the top of a golden-ratio fold of the FNV-1a +/// hash) and a +/// 16-bit slot (`Narrow`, 4 bytes), widened to a 16-bit tag and a 32-bit slot +/// (`Wide`, 8 bytes) only for a shape with 65 535 or more keys. The probe +/// position is a function of the tag alone, so a cell can be re-placed from +/// its own bits when the table grows or is rebuilt after a delete. Same +/// hash, several slots (a genuine collision, or a note-hit under a stale +/// index) is just several cells with one tag on the probe chain. Load is +/// kept at or below 7/8. +/// +/// 40 keys: 64 cells × 4 B = 256 B against the 2.1 KB hashbrown table. +#[derive(Clone, Debug)] +pub(crate) struct SlotIndex { + cells: SlotCells, + len: u32, +} + #[derive(Clone, Debug)] -pub(crate) enum SlotList { - One(u32), - Many(Vec), +enum SlotCells { + /// `(tag16 << 16) | slot16`; slots up to `NARROW_MAX_SLOT`. + Narrow(Box<[u32]>), + /// `(tag16 << 32) | slot32`. + Wide(Box<[u64]>), +} + +const NARROW_EMPTY: u32 = u32::MAX; +const WIDE_EMPTY: u64 = u64::MAX; +/// Slot `0xFFFF` is never stored narrow, so `NARROW_EMPTY` is unambiguous. +const NARROW_MAX_SLOT: u32 = 0xFFFE; +const MIN_CELLS: usize = 8; + +impl Default for SlotIndex { + fn default() -> Self { + Self::new() + } } -impl SlotList { +impl SlotIndex { + pub(crate) fn new() -> Self { + Self { + cells: SlotCells::Narrow(Box::new([])), + len: 0, + } + } + #[inline] - pub(crate) fn push(&mut self, slot: u32) { - match self { - SlotList::One(existing) => { - *self = SlotList::Many(vec![*existing, slot]); - } - SlotList::Many(v) => v.push(slot), + fn capacity(&self) -> usize { + match &self.cells { + SlotCells::Narrow(cells) => cells.len(), + SlotCells::Wide(cells) => cells.len(), + } + } + + #[cfg(test)] + pub(crate) fn len(&self) -> usize { + self.len as usize + } + + /// Bytes of the cell array (`PERRY_GC_CENSUS`). + pub(crate) fn heap_bytes(&self) -> usize { + match &self.cells { + SlotCells::Narrow(cells) => cells.len() * std::mem::size_of::(), + SlotCells::Wide(cells) => cells.len() * std::mem::size_of::(), } } - /// Drop `removed` and shift every slot above it down by one. + /// The 16-bit tag of a key hash. FNV-1a's HIGH bits barely move for + /// short keys (`"a"` and `"b"` share their top 16), so fold the whole + /// word through a golden-ratio multiply first and take the top of that. #[inline] - pub(crate) fn retain_shift(&mut self, removed: u32) { - let shift = |s: u32| -> Option { - match s.cmp(&removed) { - std::cmp::Ordering::Equal => None, - std::cmp::Ordering::Less => Some(s), - std::cmp::Ordering::Greater => Some(s - 1), + fn tag_of(hash: u64) -> u32 { + (hash.wrapping_mul(0x9E37_79B9_7F4A_7C15) >> 48) as u32 + } + + /// Where a tag's probe chain starts. Below 65 536 cells the tag itself + /// indexes the table; above, its two copies cover the extra bits (two + /// tags then share a chain, which is a longer probe, never a wrong answer). + #[inline] + fn home(tag: u32, mask: usize) -> usize { + ((tag as usize) | ((tag as usize) << 16)) & mask + } + + /// Record that `slot` holds a key hashing to `hash`. A cell already + /// naming exactly this pair is left alone (a note-hit on an indexed key). + pub(crate) fn push(&mut self, hash: u64, slot: u32) { + self.insert(Self::tag_of(hash), slot); + } + + fn insert(&mut self, tag: u32, slot: u32) { + if slot > NARROW_MAX_SLOT { + self.widen(); + } + if (self.len as usize + 1) * 8 > self.capacity() * 7 { + self.grow(); + } + let mask = self.capacity() - 1; + let mut pos = Self::home(tag, mask); + match &mut self.cells { + SlotCells::Narrow(cells) => { + let cell = (tag << 16) | slot; + loop { + let existing = cells[pos]; + if existing == NARROW_EMPTY { + cells[pos] = cell; + self.len += 1; + return; + } + if existing == cell { + return; + } + pos = (pos + 1) & mask; + } } - }; - match self { - SlotList::One(slot) => match shift(*slot) { - Some(s) => *slot = s, - None => *self = SlotList::Many(Vec::new()), + SlotCells::Wide(cells) => { + let cell = (u64::from(tag) << 32) | u64::from(slot); + loop { + let existing = cells[pos]; + if existing == WIDE_EMPTY { + cells[pos] = cell; + self.len += 1; + return; + } + if existing == cell { + return; + } + pos = (pos + 1) & mask; + } + } + } + } + + /// Every slot recorded under `hash`, in probe order. Each must still be + /// validated against the key bytes by the caller. + pub(crate) fn candidates(&self, hash: u64) -> SlotCandidates<'_> { + let capacity = self.capacity(); + let tag = Self::tag_of(hash); + SlotCandidates { + index: self, + pos: if capacity == 0 { + 0 + } else { + Self::home(tag, capacity - 1) }, - SlotList::Many(v) => { - v.retain_mut(|s| match shift(*s) { - Some(n) => { - *s = n; - true + remaining: capacity, + tag, + } + } + + /// Drop the cell(s) for `removed` and shift every slot above it down by + /// one — the index of a keys array after an in-place or cloned delete. + pub(crate) fn retain_shift(&mut self, removed: u32) { + let pairs = self.drain_pairs(); + for (tag, slot) in pairs { + match slot.cmp(&removed) { + std::cmp::Ordering::Equal => {} + std::cmp::Ordering::Less => self.insert(tag, slot), + std::cmp::Ordering::Greater => self.insert(tag, slot - 1), + } + } + } + + /// Take every `(tag, slot)` pair out, leaving the cells empty at the same + /// capacity. + fn drain_pairs(&mut self) -> Vec<(u32, u32)> { + let mut pairs = Vec::with_capacity(self.len as usize); + match &mut self.cells { + SlotCells::Narrow(cells) => { + for cell in cells.iter_mut() { + if *cell != NARROW_EMPTY { + pairs.push((*cell >> 16, *cell & 0xFFFF)); + *cell = NARROW_EMPTY; + } + } + } + SlotCells::Wide(cells) => { + for cell in cells.iter_mut() { + if *cell != WIDE_EMPTY { + pairs.push(((*cell >> 32) as u32, (*cell & 0xFFFF_FFFF) as u32)); + *cell = WIDE_EMPTY; } - None => false, - }); - if v.len() == 1 { - *self = SlotList::One(v[0]); } } } + self.len = 0; + pairs } - #[inline] - pub(crate) fn is_empty(&self) -> bool { - match self { - SlotList::One(_) => false, - SlotList::Many(v) => v.is_empty(), + fn grow(&mut self) { + let new_capacity = (self.capacity() * 2).max(MIN_CELLS); + let pairs = self.drain_pairs(); + self.cells = match self.cells { + SlotCells::Narrow(_) => SlotCells::Narrow(vec![NARROW_EMPTY; new_capacity].into()), + SlotCells::Wide(_) => SlotCells::Wide(vec![WIDE_EMPTY; new_capacity].into()), + }; + for (tag, slot) in pairs { + self.insert(tag, slot); } } - #[inline] - pub(crate) fn iter(&self) -> impl Iterator { - match self { - SlotList::One(slot) => std::slice::from_ref(slot).iter(), - SlotList::Many(v) => v.iter(), + fn widen(&mut self) { + if matches!(self.cells, SlotCells::Wide(_)) { + return; + } + let capacity = self.capacity().max(MIN_CELLS); + let pairs = self.drain_pairs(); + self.cells = SlotCells::Wide(vec![WIDE_EMPTY; capacity].into()); + for (tag, slot) in pairs { + self.insert(tag, slot); + } + } +} + +/// The probe chain of [`SlotIndex::candidates`]. +pub(crate) struct SlotCandidates<'a> { + index: &'a SlotIndex, + pos: usize, + remaining: usize, + tag: u32, +} + +impl Iterator for SlotCandidates<'_> { + type Item = u32; + + fn next(&mut self) -> Option { + let capacity = self.index.capacity(); + if capacity == 0 { + return None; + } + let mask = capacity - 1; + while self.remaining != 0 { + self.remaining -= 1; + let pos = self.pos; + self.pos = (pos + 1) & mask; + match &self.index.cells { + SlotCells::Narrow(cells) => { + let cell = cells[pos]; + if cell == NARROW_EMPTY { + self.remaining = 0; + return None; + } + if cell >> 16 == self.tag { + return Some(cell & 0xFFFF); + } + } + SlotCells::Wide(cells) => { + let cell = cells[pos]; + if cell == WIDE_EMPTY { + self.remaining = 0; + return None; + } + if (cell >> 32) as u32 == self.tag { + return Some((cell & 0xFFFF_FFFF) as u32); + } + } + } } + None } } @@ -108,10 +310,7 @@ pub(crate) fn shape_index_shift_in_place( inner.indices.remove(&keys_id); return false; } - index.slots.retain(|_, list| { - list.retain_shift(removed_slot); - !list.is_empty() - }); + index.slots.retain_shift(removed_slot); index.indexed_len = old_key_count - 1; true } @@ -165,10 +364,7 @@ pub(crate) fn shape_index_migrate_after_delete( // misaligned. Dropping it preserves the previous behaviour exactly. return false; } - index.slots.retain(|_, list| { - list.retain_shift(removed_slot); - !list.is_empty() - }); + index.slots.retain_shift(removed_slot); index.indexed_len = old_key_count - 1; inner.note_young_keys(new_keys_id as u64); inner.indices.insert(new_keys_id, index); @@ -692,3 +888,79 @@ mod tests { } } } + +#[cfg(test)] +mod slot_index_tests { + use super::SlotIndex; + + fn fnv(bytes: &[u8]) -> u64 { + crate::object::key_bytes_hash(bytes.as_ptr(), bytes.len()) + } + + #[test] + fn every_pushed_pair_is_a_candidate_and_nothing_else_is() { + let mut index = SlotIndex::new(); + let names: Vec = (0..3000).map(|i| format!("key_{i}")).collect(); + for (slot, name) in names.iter().enumerate() { + index.push(fnv(name.as_bytes()), slot as u32); + } + assert_eq!(index.len(), 3000); + for (slot, name) in names.iter().enumerate() { + let found: Vec = index.candidates(fnv(name.as_bytes())).collect(); + assert!(found.contains(&(slot as u32)), "{name} missing: {found:?}"); + } + let absent: Vec = index.candidates(fnv(b"never_inserted")).collect(); + assert!( + absent.len() <= 2, + "a narrow tag should almost never alias: {absent:?}" + ); + assert!( + index.heap_bytes() <= 4096 * 4, + "3000 keys must fit 4096 narrow cells" + ); + } + + #[test] + fn a_repeated_note_hit_does_not_grow_the_table() { + let mut index = SlotIndex::new(); + let hash = fnv(b"hit"); + for _ in 0..100 { + index.push(hash, 7); + } + assert_eq!(index.len(), 1); + assert_eq!(index.candidates(hash).collect::>(), vec![7]); + } + + #[test] + fn retain_shift_drops_the_removed_slot_and_shifts_the_rest() { + let mut index = SlotIndex::new(); + let names: Vec = (0..50).map(|i| format!("k{i}")).collect(); + for (slot, name) in names.iter().enumerate() { + index.push(fnv(name.as_bytes()), slot as u32); + } + index.retain_shift(10); + assert_eq!(index.len(), 49); + assert!(index.candidates(fnv(b"k10")).next().is_none()); + for (slot, name) in names.iter().enumerate() { + if slot == 10 { + continue; + } + let expected = if slot > 10 { slot - 1 } else { slot } as u32; + let found: Vec = index.candidates(fnv(name.as_bytes())).collect(); + assert_eq!(found, vec![expected], "{name}"); + } + } + + #[test] + fn a_slot_past_the_narrow_range_widens_the_table() { + let mut index = SlotIndex::new(); + index.push(fnv(b"a"), 3); + index.push(fnv(b"b"), 70_000); + assert_eq!(index.candidates(fnv(b"a")).collect::>(), vec![3]); + assert_eq!( + index.candidates(fnv(b"b")).collect::>(), + vec![70_000] + ); + assert!(index.heap_bytes() >= 8 * 8, "wide cells are 8 bytes"); + } +} diff --git a/crates/perry-runtime/src/object/shapes_test_support.rs b/crates/perry-runtime/src/object/shapes_test_support.rs index 29977c46eb..6cb974c98c 100644 --- a/crates/perry-runtime/src/object/shapes_test_support.rs +++ b/crates/perry-runtime/src/object/shapes_test_support.rs @@ -138,18 +138,16 @@ pub(crate) fn test_shape_ids_for_keys(keys_id: usize) -> Vec { #[cfg(test)] pub(crate) fn test_seed_shape_entry(keys_id: usize) { - crate::state::state() - .shapes - .inner - .borrow_mut() - .indices - .insert( - keys_id, - ShapeIndex { - indexed_len: 0, - slots: crate::fast_hash::new_ptr_hash_map(), - }, - ); + let mut inner = crate::state::state().shapes.inner.borrow_mut(); + inner.note_young_keys(keys_id as u64); + inner.indices.insert( + keys_id, + ShapeIndex { + indexed_len: 0, + slots: SlotIndex::new(), + }, + ); + drop(inner); let _ = shape_descriptor_ensure(keys_id as *const ArrayHeader, 0, 0) .expect("test shape id range unexpectedly exhausted"); } diff --git a/crates/perry-runtime/src/object/side_table_roots.rs b/crates/perry-runtime/src/object/side_table_roots.rs index fdaa3262c3..9bc56840ba 100644 --- a/crates/perry-runtime/src/object/side_table_roots.rs +++ b/crates/perry-runtime/src/object/side_table_roots.rs @@ -295,74 +295,24 @@ pub fn scan_shape_cache_roots(mark: &mut dyn FnMut(f64)) { scan_shape_cache_roots_mut(&mut visitor); } +/// #9754 measured this table and left it alone: a young-entry log here skipped +/// NOTHING on the compiled claude-code TUI (0.0 % of 3.85 M entry visits over +/// 107 collections) and cost 35 % MORE than this plain walk, because the +/// canonical keys arrays live in the LONGLIVED arena, which +/// `addr_is_minor_relevant` must answer `true` for, so no entry ever leaves +/// the log. See the four tables that do skip 75-93 % in `gc/young_log.rs`. pub fn scan_shape_cache_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { - use crate::gc::young_log::addr_is_minor_relevant; let st = crate::state::state(); - // The inline array is 256 fixed slots: always walked. The overflow map - // holds every shape id ever cached; #9754: a minor-scoped pass visits - // only the young-logged ids there, a full pass rebuilds the log. - let entries = unsafe { &mut *st.object_hot.shape_inline_cache.get() }; - for entry in entries.iter_mut() { - visitor.visit_raw_mut_ptr_slot(&mut entry.keys_array); - } - let mut cache = st.object_hot.shape_cache_overflow.borrow_mut(); - let table_len = cache.len() as u64; - if visitor.young_scope() { - #[cfg(debug_assertions)] - { - let relevant: Vec = cache - .iter() - .filter(|(_, (arr_ptr, _))| addr_is_minor_relevant(*arr_ptr as usize)) - .map(|(&id, _)| id) - .collect(); - SHAPE_CACHE_YOUNG.with(|log| { - log.borrow() - .debug_assert_logged(SHAPE_CACHE_YOUNG_LOG_NAME, &relevant) - }); + { + let entries = unsafe { &mut *st.object_hot.shape_inline_cache.get() }; + for entry in entries.iter_mut() { + visitor.visit_raw_mut_ptr_slot(&mut entry.keys_array); } - let batch = SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().take_sorted()); - let logged = batch.len() as u64; - let mut kept = SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().take_spare()); - for id in batch { - if let Some((arr_ptr, _)) = cache.get_mut(&id) { - visitor.visit_raw_mut_ptr_slot(arr_ptr); - if addr_is_minor_relevant(*arr_ptr as usize) { - kept.push(id); - } - } - } - let kept_len = kept.len() as u64; - SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().extend(kept)); - crate::gc::young_log::note_walk( - SHAPE_CACHE_YOUNG_LOG_NAME, - crate::gc::young_log::YoungLogWalk { - partial: true, - logged, - visited: logged, - kept: kept_len, - table_len, - }, - ); - return; } - let _ = SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().take_sorted()); - let mut kept = Vec::new(); - for (&id, (arr_ptr, _runtime_shape_id)) in cache.iter_mut() { - visitor.visit_raw_mut_ptr_slot(arr_ptr); - if addr_is_minor_relevant(*arr_ptr as usize) { - kept.push(id); + { + let mut cache = st.object_hot.shape_cache_overflow.borrow_mut(); + for (arr_ptr, _runtime_shape_id) in cache.values_mut() { + visitor.visit_raw_mut_ptr_slot(arr_ptr); } } - let kept_len = kept.len() as u64; - SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().extend(kept)); - crate::gc::young_log::note_walk( - SHAPE_CACHE_YOUNG_LOG_NAME, - crate::gc::young_log::YoungLogWalk { - partial: false, - logged: table_len, - visited: table_len, - kept: kept_len, - table_len, - }, - ); } diff --git a/crates/perry-runtime/src/object/test_root_accessors.rs b/crates/perry-runtime/src/object/test_root_accessors.rs index 094138aa55..d0908e2f9b 100644 --- a/crates/perry-runtime/src/object/test_root_accessors.rs +++ b/crates/perry-runtime/src/object/test_root_accessors.rs @@ -58,7 +58,6 @@ pub(crate) fn test_transition_cache_root() -> usize { #[cfg(test)] pub(crate) fn test_clear_transition_cache_root() { super::TRANSITION_CACHE_YOUNG.with(|log| log.borrow_mut().clear()); - super::SHAPE_CACHE_YOUNG.with(|log| log.borrow_mut().clear()); with_transition_cache(|t| unsafe { for i in 0..TRANSITION_CACHE_SIZE { // GC_STORE_AUDIT(ROOT): test clear writes non-pointer sentinels into scanned TRANSITION_CACHE_GLOBAL roots.