From dd69b1b74c243d46c19ce6e83db1a81819f5fd3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 17 Sep 2026 10:39:00 +0000 Subject: [PATCH 1/2] perf(gc): decode a visited word once in the copying minor (#10362) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Base: e6dcb6274d0ce91a38c7b43850cc79d8ce870857 (main). A raw (untagged) word was classified TWICE on the copying minor's slot path: `CopyingPointerSet::decode_bits` classified it only to validate it, and `mark_addr` classified it again. Every traced shaped object visits its shape record's `keys` word, a raw address, so that was a second page-table probe and `plausible_gc_header` read per traced object. The remembering arm then re-decoded the very slot the visit had just decoded. `visit_value_bits_child` now decodes, classifies and marks once, and returns the child's address as the word reads after the visit. The validating classification is the one the mark uses (the memo is still consulted after it, in the same order as before), and the remembering arm reuses that child; only a raw word that MOVED is validated again, which is all the re-decode could still reject. Two codegen facts are load-bearing, measured, and pinned by comment: * the decode is `#[inline(always)]` — out of line, its frame and the by-memory return of its result cost as much as the classification it saves (that first cut measured flat to +1.05% on the six fixtures); * `barrier_parent_needs_remembering` is asked BEFORE the visit. It reads only the parent and the slot's own address, never the child, so the order cannot change its answer — but asked after, the optimizer duplicated the call into both decode arms and stopped inlining it, which cost a third of the win on gc3 and more on w20000. instructions:u, min of 5, same host, base vs this: gc3 11,756,388,818 -> 11,541,805,498 -1.83% w5000 1,886,467,237 -> 1,853,133,603 -1.77% w20000 4,727,775,800 -> 4,655,712,038 -1.52% oldyoung 1,454,636,978 -> 1,433,367,558 -1.46% w1000 1,045,691,197 -> 1,036,890,921 -0.84% alloc 320,204,861 -> 320,203,910 -0.00% Exact counts under callgrind agree (gc3 -1.79%) and attribute it: `classify_arena` calls fall from 6.09M to 4.20M on gc3, and on the pointer-slot control the per-slot term falls from 379.2 to 349.6 instructions at K=16. Peak RSS and max pause are flat within their own run-to-run spread on all six fixtures. Witness: `gc::tests::copy_slot_decode`, two behavioural tests each with a sabotaged twin — a raw word's child must be evacuated and the word rewritten (sabotage: drop the validated raw word, and the word goes stale), and an old parent's edge must be re-remembered from the decoded child (sabotage: forget it, and `restore_surviving_dirty_coverage`'s cross-check refuses the cycle). --- .../10362-copying-minor-single-decode.md | 35 ++++ crates/perry-runtime/src/gc/copying.rs | 29 ++-- .../src/gc/copying_parent_facts.rs | 150 ++++++++++++++++-- .../src/gc/tests/copy_slot_decode.rs | 141 ++++++++++++++++ crates/perry-runtime/src/gc/tests/mod.rs | 1 + 5 files changed, 326 insertions(+), 30 deletions(-) create mode 100644 changelog.d/10362-copying-minor-single-decode.md create mode 100644 crates/perry-runtime/src/gc/tests/copy_slot_decode.rs diff --git a/changelog.d/10362-copying-minor-single-decode.md b/changelog.d/10362-copying-minor-single-decode.md new file mode 100644 index 0000000000..ee5939e024 --- /dev/null +++ b/changelog.d/10362-copying-minor-single-decode.md @@ -0,0 +1,35 @@ +Decode each word the copying minor visits once. A raw (untagged) word was +classified twice: `CopyingPointerSet::decode_bits` classified it only to +validate it, and `mark_addr` classified it again. Every traced shaped object +visits its shape record's `keys` word, a raw address, so that was a second +page-table probe and header read per traced object. The slot visit's +remembering arm then re-decoded the slot it had just decoded. The validating +classification is now the one the mark uses (the memo is still consulted after +it, as before), and the remembering arm reuses the child the visit decoded; +only a raw word that moved is validated again, which is all the re-decode could +still reject. + +Two codegen facts are load-bearing and pinned by comment. The decode is +`#[inline(always)]`: out of line, its call frame and the by-memory return of +its result cost as much as the classification it saves (the first cut measured +flat to +1.05%). And `barrier_parent_needs_remembering` is asked before the +visit rather than after. It reads only the parent and the slot's address, so +the order does not change the answer, but asked after, the optimizer +duplicated the call into both decode arms and stopped inlining it, which gave +back a third of the win on gc3 (-1.20% instead of -1.80%) and more than a third +on w20000 (-0.84% instead of -1.44%). + +Measured on six GC fixtures, instructions:u min-of-5: gc3 -1.83%, w5000 -1.77%, +w20000 -1.52%, oldyoung -1.46%, w1000 -0.84%, alloc flat (-951 instructions). +Exact instruction counts under callgrind agree: gc3 -1.79%, with +`classify_arena` calls down from 6.09M to 4.20M. On the pointer-slot control +(60k records whose K fields all point at one shared object, against the same +records holding doubles) the per-slot term falls from 379.2 to 349.6 +instructions at K=16, counted exactly under callgrind: a memo hit no longer pays a call to +`mark_addr`, and the re-decode's classification is gone. + +The page-generation cache was read before any of this was attempted. It runs +the direct-mapped table arm with a 93.4-97.3% hit rate, and at most 0.02% of +lookups are capacity misses. Nearly every miss is an address in no registered +block: the shape record's `keys` slot, which lives outside the heap. So the +cache's size was not the problem, and nothing here changes it. diff --git a/crates/perry-runtime/src/gc/copying.rs b/crates/perry-runtime/src/gc/copying.rs index 52442f4f6f..7a510d1d64 100644 --- a/crates/perry-runtime/src/gc/copying.rs +++ b/crates/perry-runtime/src/gc/copying.rs @@ -313,19 +313,6 @@ impl CopyingNurseryCollector { } } - pub(super) fn visit_value_bits(&mut self, bits: u64) -> Option { - let (addr, is_nanbox, tag) = self.ptrs.decode_bits(bits)?; - let new_addr = self.mark_addr(addr)?; - if new_addr == addr { - return None; - } - Some(if is_nanbox { - tag | (new_addr as u64 & POINTER_MASK) - } else { - new_addr as u64 - }) - } - pub(super) fn visit_raw_addr(&mut self, addr: usize) -> Option { let new_addr = self.mark_addr(addr)?; (new_addr != addr).then_some(new_addr) @@ -430,6 +417,20 @@ impl CopyingNurseryCollector { return Some(self.memo_result); } let ptr = self.ptrs.classify(addr)?; + Some(self.mark_classified(addr, ptr)) + } + + /// [`mark_addr`](Self::mark_addr) for an address the caller has already + /// classified: the memo, then the mark, without classifying again. + #[inline] + pub(super) fn mark_classified_addr(&mut self, addr: usize, ptr: CopyingPointer) -> usize { + if addr == self.memo_addr { + return self.memo_result; + } + self.mark_classified(addr, ptr) + } + + fn mark_classified(&mut self, addr: usize, ptr: CopyingPointer) -> usize { let result = match ptr.kind { CopyingPointerKind::Eden | CopyingPointerKind::FromSurvivor => unsafe { self.move_young(ptr) @@ -457,7 +458,7 @@ impl CopyingNurseryCollector { }; self.memo_addr = addr; self.memo_result = result; - Some(result) + result } /// #7742: the object's block is being promoted whole, in place. It does not diff --git a/crates/perry-runtime/src/gc/copying_parent_facts.rs b/crates/perry-runtime/src/gc/copying_parent_facts.rs index 2cecd82327..19b6d06833 100644 --- a/crates/perry-runtime/src/gc/copying_parent_facts.rs +++ b/crates/perry-runtime/src/gc/copying_parent_facts.rs @@ -1,5 +1,6 @@ -//! The per-parent weak-holder fact the copying minor's slot visit reads, and -//! the slot visit itself. Split out of `gc/copying.rs` for the 2000-line lint. +//! The per-parent weak-holder fact the copying minor's slot visit reads, the +//! slot visit itself, and its single decode of the visited word. Split out of +//! `gc/copying.rs` for the 2000-line lint. use super::*; @@ -58,7 +59,107 @@ pub(crate) mod copy_hoist_sabotage { } } +/// Test-only sabotage for the single decode per slot visit +/// (`visit_value_bits_child`). Witness: `gc::tests::copy_slot_decode`. +#[cfg(test)] +pub(crate) mod copy_decode_sabotage { + use std::cell::Cell; + + /// The validated raw word is dropped instead of marked. + pub(crate) const RAW_MARK: u8 = 1; + /// The remembering arm loses the child the visit decoded. + pub(crate) const CHILD: u8 = 2; + + thread_local! { + static FORGET: Cell = const { Cell::new(0) }; + } + + pub(crate) fn forgetting(what: u8) -> bool { + FORGET.with(|f| f.get() & what != 0) + } + + pub(crate) struct Guard(u8); + + impl Guard { + pub(crate) fn arm(what: u8) -> Self { + Self(FORGET.with(|f| f.replace(f.get() | what))) + } + } + + impl Drop for Guard { + fn drop(&mut self) { + FORGET.with(|f| f.set(self.0)); + } + } +} + impl CopyingNurseryCollector { + /// The root visitors' form of [`Self::visit_value_bits_child`]. Always + /// inlined too: out of line it added a call frame per root word. + #[inline(always)] + pub(super) fn visit_value_bits(&mut self, bits: u64) -> Option { + self.visit_value_bits_child(bits)?.1 + } + + /// Decode, classify and mark one value word ONCE: the child's address as + /// the word reads after this visit, the word's new bits if the child + /// moved, and whether the word was raw. `None` is exactly + /// `CopyingPointerSet::decode_bits`'s `None`: not a heap reference. + /// + /// A raw word used to be classified twice — by `decode_bits`, only to + /// validate it, and again by `mark_addr`. Every traced shaped object visits + /// its shape record's `keys` word, a raw address, so that was a second + /// page-table probe and header read per traced object. The validating + /// classification is now the one the mark uses; the memo is still + /// consulted after it, as before. + /// + /// Always inlined: out of line, the extra frame and the by-memory return + /// of the triple cost as much as the classification it saves. + #[inline(always)] + pub(super) fn visit_value_bits_child( + &mut self, + bits: u64, + ) -> Option<(usize, Option, bool)> { + let tag = bits & TAG_MASK; + if tag == POINTER_TAG || tag == STRING_TAG || tag == BIGINT_TAG { + let addr = (bits & POINTER_MASK) as usize; + if addr == 0 { + return None; + } + // An unclassifiable NaN-boxed word is still a reference to the + // remembering arm, which never classified NaN-boxed words. + return Some(match self.mark_addr(addr) { + Some(new_addr) if new_addr != addr => { + let new_bits = tag | (new_addr as u64 & POINTER_MASK); + ((new_bits & POINTER_MASK) as usize, Some(new_bits), false) + } + _ => (addr, None, false), + }); + } + if tag >= 0x7FF8_0000_0000_0000 || !CopyingPointerSet::raw_pointer_candidate(bits) { + return None; + } + let addr = bits as usize; + let ptr = self.ptrs.classify(addr)?; + #[cfg(test)] + if copy_decode_sabotage::forgetting(copy_decode_sabotage::RAW_MARK) { + return None; + } + let new_addr = self.mark_classified_addr(addr, ptr); + Some(( + new_addr, + (new_addr != addr).then_some(new_addr as u64), + true, + )) + } + + /// The remembering arm's one remaining re-decode: a raw word that moved. + /// Out of line so the rare case does not grow the slot visit it sits in. + #[inline(never)] + fn revalidate_moved_raw(&self, bits: u64) -> Option { + self.ptrs.decode_bits(bits).map(|(addr, _, _)| addr) + } + pub(super) unsafe fn visit_slot_with_parent( &mut self, slot: *mut u64, @@ -100,22 +201,39 @@ impl CopyingNurseryCollector { self.weak_slots.push(slot); return; } - let bits = *slot; - if let Some(new_bits) = self.visit_value_bits(bits) { + // Asked BEFORE the visit: it reads only the parent and the slot's own + // address, never the child. Asked after, the optimizer duplicated the + // call into both decode arms and then stopped inlining it. + let remembering = !parent_header.is_null() + && !self.skip_remembering + && barrier_parent_needs_remembering( + (parent_header as *mut u8).add(GC_HEADER_SIZE) as usize, + external, + ); + let visited = self.visit_value_bits_child(*slot); + if let Some((_, Some(new_bits), _)) = visited { *slot = new_bits; } - if !parent_header.is_null() && !self.skip_remembering { - let parent_user = (parent_header as *mut u8).add(GC_HEADER_SIZE) as usize; - if barrier_parent_needs_remembering(parent_user, external) { - if let Some((child_addr, _, _)) = self.ptrs.decode_bits(*slot) { - // Keep old→malloc pages dirty alongside old→nursery: - // the malloc child is spared by this cycle's mark - // (mark_addr handles CopyingPointerKind::Malloc) but - // the NEXT minor's malloc sweep needs the edge again. - if crate::gc::barrier::remembered_child_needs_tracking(child_addr) { - self.sticky.remember_slot(parent_header, slot, external); - } - } + if !remembering { + return; + } + // The visit above already decoded this word; re-decoding `*slot` + // repeated it. Only a raw word that MOVED is validated again, which is + // all the re-decode could still reject. + let child = match visited { + Some((_, Some(new_bits), true)) => self.revalidate_moved_raw(new_bits), + other => other.map(|(addr, _, _)| addr).filter(|&addr| addr != 0), + }; + #[cfg(test)] + let child = + child.filter(|_| !copy_decode_sabotage::forgetting(copy_decode_sabotage::CHILD)); + if let Some(child_addr) = child { + // Keep old→malloc pages dirty alongside old→nursery: the malloc + // child is spared by this cycle's mark (mark_addr handles + // CopyingPointerKind::Malloc) but the NEXT minor's malloc sweep + // needs the edge again. + if crate::gc::barrier::remembered_child_needs_tracking(child_addr) { + self.sticky.remember_slot(parent_header, slot, external); } } } diff --git a/crates/perry-runtime/src/gc/tests/copy_slot_decode.rs b/crates/perry-runtime/src/gc/tests/copy_slot_decode.rs new file mode 100644 index 0000000000..68f6d1008c --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/copy_slot_decode.rs @@ -0,0 +1,141 @@ +//! The copying minor decodes each visited word ONCE (`visit_value_bits_child`): +//! a raw word's validating classification is the one the mark uses, and the +//! remembering arm reuses the child the visit decoded instead of re-decoding +//! the slot. Both halves are pinned by collections and what they leave in the +//! heap, and each has a sabotaged twin that must fail. + +use super::super::*; +use super::support::*; +use crate::gc::copying_parent_facts::copy_decode_sabotage::{Guard, CHILD, RAW_MARK}; + +fn string_bytes(addr: usize) -> Vec { + unsafe { + let s = addr as *const crate::StringHeader; + let data = (s as *const u8).add(std::mem::size_of::()); + std::slice::from_raw_parts(data, (*s).byte_len as usize).to_vec() + } +} + +/// A young string reachable ONLY through a RAW (untagged) word in a rooted +/// young object. Returns whether the minor evacuated it through that word. +fn raw_child_evacuated(sabotaged: bool) -> bool { + std::thread::spawn(move || { + let _guard = CopyingNurseryTestGuard::new(1); + let _triggers = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let _scan = ConservativeScanDisabledGuard::new(); + let _roots = ShadowAndGlobalRootResetGuard; + let (parent, fields) = unsafe { alloc_nursery_test_object(1) }; + let child = young_leaf(); + let expected = string_bytes(child); + unsafe { *fields = child as u64 }; + js_shadow_slot_set(0, ptr_bits(parent as usize)); + assert!( + crate::arena::pointer_in_nursery(child), + "premise: the child must be young, or there is nothing to evacuate" + ); + { + let _sabotage = sabotaged.then(|| Guard::arm(RAW_MARK)); + let _ = gc_collect_minor(); + } + let parent_after = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + assert_ne!( + parent_after, parent as usize, + "premise: the rooted parent moved" + ); + let word = unsafe { + *((parent_after as *const u8).add(std::mem::size_of::()) + as *const u64) + }; + // Checked before any read through `word`: a stale word names from-space. + word != child as u64 && string_bytes(word as usize) == expected + }) + .join() + .expect("raw-word decode test thread must not panic") +} + +#[test] +fn a_raw_word_is_marked_through_its_validating_classification() { + assert!( + raw_child_evacuated(false), + "the young child behind a raw word must be evacuated and the word rewritten" + ); +} + +#[test] +fn sabotaged_raw_mark_leaves_the_raw_word_stale() { + assert!( + !raw_child_evacuated(true), + "with the validated raw word dropped instead of marked, the child is not evacuated" + ); +} + +/// An OLD parent whose NaN-boxed slot holds a young child, handed to the minor +/// through the write barrier, then two minors: the second finds the edge only +/// if the first re-remembered it from the child its visit decoded. `Err` is +/// the collection thread's panic message. +fn old_edge_across_two_minors(sabotaged: bool) -> Result { + std::thread::spawn(move || { + let _guard = CopyingNurseryTestGuard::new(1); + let _tenuring = crate::gc::tenuring::set_survivals_for_test( + crate::gc::tenuring::GC_TENURING_SURVIVALS_MAX, + ); + let _triggers = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let _scan = ConservativeScanDisabledGuard::new(); + let _roots = ShadowAndGlobalRootResetGuard; + let (parent, fields) = unsafe { alloc_old_test_object(1) }; + let child = young_leaf(); + let expected = string_bytes(child); + unsafe { *fields = ptr_bits(child) }; + js_write_barrier_slot(ptr_bits(parent as usize), fields as u64, ptr_bits(child)); + assert!( + crate::arena::pointer_in_old_gen(parent as usize) + && crate::arena::pointer_in_nursery(child), + "premise: an old parent and a young child" + ); + let read = || unsafe { (*fields & POINTER_MASK) as usize }; + { + let _sabotage = sabotaged.then(|| Guard::arm(CHILD)); + let _ = gc_collect_minor(); + } + let first = read(); + assert!( + first != child && crate::arena::pointer_in_nursery(first), + "premise: the first minor copied the child within the nursery" + ); + let _ = gc_collect_minor(); + let second = read(); + second != first && string_bytes(second) == expected + }) + .join() + .map_err(|payload| { + payload + .downcast_ref::() + .cloned() + .or_else(|| payload.downcast_ref::<&str>().map(|s| s.to_string())) + .unwrap_or_default() + }) +} + +#[test] +fn an_old_parents_edge_is_remembered_from_the_child_the_visit_decoded() { + assert_eq!( + old_edge_across_two_minors(false), + Ok(true), + "the second minor must find and move the child through the remembered edge" + ); +} + +/// In a release build `restore_surviving_dirty_coverage` would re-add the page +/// the arm failed to remember, which is why a forgotten remembered-set entry +/// is invisible to a survival check alone. In the debug build `cargo test` +/// runs, the same walk cross-checks the dirty scan's per-slot re-remembering +/// and refuses the disagreement — that refusal is this twin's observable. +#[test] +fn sabotaged_remembering_arm_is_refused_by_the_coverage_cross_check() { + let outcome = old_edge_across_two_minors(true); + assert!( + matches!(&outcome, Err(message) if message.contains("restore_surviving_dirty_coverage")), + "with the decoded child forgotten, the coverage walk must report the \ + unremembered page; got {outcome:?}" + ); +} diff --git a/crates/perry-runtime/src/gc/tests/mod.rs b/crates/perry-runtime/src/gc/tests/mod.rs index 1ffdcb9893..3307fdea2b 100644 --- a/crates/perry-runtime/src/gc/tests/mod.rs +++ b/crates/perry-runtime/src/gc/tests/mod.rs @@ -17,6 +17,7 @@ mod census_block_windows; mod census_whole_block; mod concat_site; mod contract; +mod copy_slot_decode; mod copy_slot_hoists; mod copying; mod copying_side_tables; From cf3648941efa5083e6810525690f95afa88e29ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 17 Sep 2026 12:45:01 +0200 Subject: [PATCH 2/2] changelog: key the fragment to #10491 --- ...inor-single-decode.md => 10491-copying-minor-single-decode.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{10362-copying-minor-single-decode.md => 10491-copying-minor-single-decode.md} (100%) diff --git a/changelog.d/10362-copying-minor-single-decode.md b/changelog.d/10491-copying-minor-single-decode.md similarity index 100% rename from changelog.d/10362-copying-minor-single-decode.md rename to changelog.d/10491-copying-minor-single-decode.md