diff --git a/changelog.d/7914-promoted-page-run-description.md b/changelog.d/7914-promoted-page-run-description.md new file mode 100644 index 0000000000..16251b0794 --- /dev/null +++ b/changelog.d/7914-promoted-page-run-description.md @@ -0,0 +1,49 @@ +### GC: a promoted block's page-object list is described, not stored + +Whole-block in-place promotion (#7742/#7888) walked every promoted object to +stamp `GC_FLAG_TENURED`, and while there pushed each object's header address +into `OLD_GEN_PAGE_OBJECTS[page]` — 8 bytes per object, held for the life of the +process, so a later reader could answer "which objects live on this page". + +A promoted block is contiguous and parses linearly by `GcHeader::size`, which is +the parse `old_arena_walk_objects` (the old-gen sweep's own enumerator) already +performs over every old block. The list was a derivable fact being stored. It is +now recorded as one `PromotedPageRun { first_header, last_header, count }` per +page and expanded only if a reader actually asks for that page — +`gc-handoff/bench/retain.ts` asks for none of them. + +Peak RSS: `retain` 311.1 → 293.5 MB (−5.7 %), `retain_wide` 420.8 → 400.8 +(−4.7 %), `retain1` 108.8 → 99.4 (−8.6 %), `deeplist` 92.7 → 83.8 (−9.6 %). +`in_place_promotion` phase: `retain_wide` 41.3 → 34.6 ms, `retain` 18.2 → 17.6. +On the 11 corpus programs that never promote in place the phase is 0.0 ms in +both arms and RSS moves ≤ 0.2 MB. + +**What the measurement that motivated this also settled**, since both recorded +figures for `retain`'s GC share (62 % and 93 %) predate #7888 and are wrong: + +* `retain` is **33 %** GC pause, `retain1` 54 %, `retain_wide` 37 %, + `retain_wide1` 44 % — and **`shapes` is 7 %** (one minor, 4.6 ms, survival + 30‰; the 16 400-byte born-tenured cliff is gone, so it is a dispatch/string + benchmark now, not a GC one). +* A three-arm probe over the promotion walk found the `GC_FLAG_TENURED` stamp is + **free** (deleting it measures *slower* than keeping it on three of four + benchmarks), the page-index build is 50–67 % of the walk, and the residual + ~4.5 ns/object — finding each header in order to stamp it — is structural: + moving it means the generated write barrier testing a page instead of a header + bit, and with no contiguous heap reservation that is a hash probe on every + pointer store. + +Correctness is held structurally rather than by memory, because a reader that +skipped the expansion would see an empty page and the dirty scan it feeds would +lose every old→young edge out of those objects. +`every_page_object_reader_expands_promoted_runs` enumerates every function in +`arena/page_meta.rs` touching `OLD_GEN_PAGE_OBJECTS` and requires it to expand +first or carry a written exemption, with stale exemptions failing too (the shape +of #7624's `deferred_registration_flush_sites`); both halves of that gate fired +during development. A described run is valid only while the block's object +boundaries cannot move, so `GcCycleState::new_full` — the one constructor whose +sweep frees old-gen objects in place — expands every pending run before it +starts, `unregister_old_block_pages` discards rather than expands, and only +`PromotionLiveness::AssumeAllLive` pages may be described at all (a traced +promoting cycle's liveness lives in marks that `clear_marks` destroys, so that +path keeps the eager list unchanged). diff --git a/crates/perry-runtime/src/arena/mod.rs b/crates/perry-runtime/src/arena/mod.rs index 13ec129590..4f31575bed 100644 --- a/crates/perry-runtime/src/arena/mod.rs +++ b/crates/perry-runtime/src/arena/mod.rs @@ -24,6 +24,8 @@ mod walk; #[cfg(test)] mod tests; +#[cfg(test)] +mod tests_promoted_runs; // Cross-sibling shared types/thread-locals (used by sibling modules via // `use super::*;`). These are not part of the crate-public surface @@ -127,19 +129,21 @@ pub(crate) use stats::{old_gen_in_use_bytes_recomputed, old_gen_in_use_bytes_res // page_meta.rs (public + pub(crate) classification/page-meta API) pub(crate) use page_meta::{ classify_heap_generation, classify_heap_space, classify_heap_space_in_range, - generation_page_for_addr, old_arena_page_index_remove_object, - old_arena_source_blocks_for_pages, old_arena_walk_objects_on_pages, old_object_page_overlaps, - old_page_account_dirty_slot, old_page_account_dirty_slots, old_page_account_promoted_object, - old_page_account_swept_object, old_page_clear_dirty, old_page_mark_dirty, - old_page_meta_snapshot, old_page_summary, old_pages_begin_gc_cycle, - old_pages_reset_sweep_accounting, unregister_old_object_pages, HeapGeneration, HeapSpace, - OldArenaPageObjectCursor, OldArenaSourceBlockSelection, OldPageMeta, OldPageSummary, + generation_page_for_addr, materialize_all_promoted_page_runs, + old_arena_page_index_remove_object, old_arena_source_blocks_for_pages, + old_arena_walk_objects_on_pages, old_object_page_overlaps, old_page_account_dirty_slot, + old_page_account_dirty_slots, old_page_account_promoted_object, old_page_account_swept_object, + old_page_clear_dirty, old_page_mark_dirty, old_page_meta_snapshot, old_page_summary, + old_pages_begin_gc_cycle, old_pages_reset_sweep_accounting, unregister_old_object_pages, + HeapGeneration, HeapSpace, OldArenaPageObjectCursor, OldArenaSourceBlockSelection, OldPageMeta, + OldPageSummary, }; #[cfg(test)] pub(crate) use page_meta::{ deferred_old_page_registrations_len, generation_page_base, old_arena_page_index_clear_for_tests, old_page_meta_for_tests, - old_page_meta_snapshot_calls_for_tests, reset_old_page_meta_snapshot_calls_for_tests, + old_page_meta_snapshot_calls_for_tests, pending_promoted_page_runs, + reset_old_page_meta_snapshot_calls_for_tests, DEFERRED_OLD_PAGE_REGISTRATION_CAP, GENERATION_CLASS_SHIFT, GENERATION_PAGE_SIZE, }; diff --git a/crates/perry-runtime/src/arena/page_meta.rs b/crates/perry-runtime/src/arena/page_meta.rs index 394d3c5aca..833ac365f2 100644 --- a/crates/perry-runtime/src/arena/page_meta.rs +++ b/crates/perry-runtime/src/arena/page_meta.rs @@ -326,6 +326,16 @@ thread_local! { static OLD_GEN_PAGE_META: RefCell = RefCell::new(crate::fast_hash::new_ptr_hash_map()); + /// Promoted-block pages whose object list is DESCRIBED rather than stored — + /// see [`register_promoted_page_run`]. + static OLD_GEN_PAGE_PROMOTED_RUNS: RefCell> = + RefCell::new(crate::fast_hash::new_ptr_hash_map()); + + /// Monotone-within-a-window latch so the common case — a program that never + /// promotes a block in place — pays one `Cell` read per reader instead of a + /// hash probe per page. Same pattern as `PER_OBJECT_LAYOUTS_NONEMPTY`. + static OLD_GEN_PAGE_PROMOTED_RUNS_NONEMPTY: Cell = const { Cell::new(false) }; + pub(crate) static OLD_GEN_RECLAIM_REUSABLE_BYTES: Cell = const { Cell::new(0) }; pub(crate) static OLD_GEN_RECLAIM_POOLED_BYTES: Cell = const { Cell::new(0) }; pub(crate) static OLD_GEN_RECLAIM_RETURNED_BYTES: Cell = const { Cell::new(0) }; @@ -434,6 +444,17 @@ pub(crate) fn unregister_old_block_pages(pages: &[usize]) { index.remove(&page); } }); + // RUN REMOVER: DISCARD, never expand — the block backing these pages is + // going away, so a run's `first_header` no longer points at memory this + // arena owns. Expanding would parse freed pages. + if OLD_GEN_PAGE_PROMOTED_RUNS_NONEMPTY.with(Cell::get) { + OLD_GEN_PAGE_PROMOTED_RUNS.with(|runs| { + let mut runs = runs.borrow_mut(); + for &page in pages { + runs.remove(&page); + } + }); + } // #7187 Phase B: the other place a page's dirty stamp stops existing — the // metadata entry itself is gone. A cached page whose metadata was dropped // is no longer a complete recording, so drop the cache. @@ -562,7 +583,186 @@ pub(crate) fn retag_block_space( /// `bytes` is the number of bytes of the run that fall inside `page` (an object /// straddling a page boundary contributes its overlap to each page it touches), /// matching `update_old_page_meta_for_object`'s accounting exactly. -pub(crate) fn register_promoted_page_run(page: usize, headers: &[usize], bytes: usize) { +/// +/// # The list is DESCRIBED, not stored +/// +/// The run is recorded as `(first_header, last_header, count)` and expanded into +/// [`OLD_GEN_PAGE_OBJECTS`] only if some reader actually asks for this page — +/// see [`PromotedPageRun`]. The header addresses are recoverable by the same +/// linear parse `old_arena_walk_objects` already performs over every old block, +/// so storing them is storing a derivable fact. +/// +/// Measured on `gc-handoff/bench/retain.ts` (2.11 M promoted objects): building +/// the per-object list was **6.7 ms of the 18.2 ms** `in_place_promotion` phase +/// and **20 MB of permanent RSS**, and `retain` never reads one of those pages. +/// On `retain_wide` (2.94 M) it was 22.9 ms of 34.1 and 28 MB. +pub(crate) fn register_promoted_page_run( + page: usize, + first_header: usize, + last_header: usize, + count: usize, + bytes: usize, +) { + if count == 0 { + return; + } + // One promoted block authors a given page exactly once per cycle, but two + // blocks can share a page when block bases are not page aligned. Expanding + // the incumbent keeps "at most one pending run per page" true by + // construction rather than by an alignment argument. + // + // Taken and expanded OUTSIDE the borrow: `expand_promoted_run` touches a + // different thread-local today, and this keeps that from being a fact the + // next edit has to re-derive before it can add one line to the expansion. + let previous = OLD_GEN_PAGE_PROMOTED_RUNS.with(|runs| runs.borrow_mut().remove(&page)); + if let Some(previous) = previous { + expand_promoted_run(page, previous); + } + OLD_GEN_PAGE_PROMOTED_RUNS.with(|runs| { + runs.borrow_mut().insert( + page, + PromotedPageRun { + first_header, + last_header, + count, + }, + ); + }); + OLD_GEN_PAGE_PROMOTED_RUNS_NONEMPTY.with(|flag| flag.set(true)); + OLD_GEN_PAGE_META.with(|meta| { + let mut meta = meta.borrow_mut(); + let page_meta = meta + .entry(page) + .or_insert_with(|| OldPageMeta::zero_for_page(page)); + page_meta.allocated_bytes = page_meta.allocated_bytes.saturating_add(bytes); + page_meta.object_count = page_meta.object_count.saturating_add(count); + page_meta.live_bytes = page_meta.live_bytes.saturating_add(bytes); + page_meta.live_object_count = page_meta.live_object_count.saturating_add(count); + page_meta.refresh_policy_bits(); + }); +} + +/// A promoted-block page whose object list has not been built. +/// +/// `first_header`/`last_header` are INCLUSIVE header addresses of the first and +/// last object overlapping the page — the first may sit below the page base +/// (an object straddling in from the previous page), which is exactly the +/// population the eager list used to hold. Objects on an old block are +/// contiguous and ascending, so every object between those two also overlaps +/// the page: the pair is an exact description of the set, not an approximation. +/// +/// # Only [`PromotionLiveness::AssumeAllLive`] pages may be described +/// +/// The parse cannot reconstruct which objects were MARKED — `clear_marks` runs +/// after the promotion, so by expansion time the marks are gone. On a TRACED +/// promoting cycle the eager list therefore stays: it is the only place that +/// liveness exists. Restricting the description to the untraced path is what +/// makes "the parse yields exactly `count`" an exact claim rather than an +/// approximation, and it is the path that carries the cost anyway (4 of +/// `retain`'s 5 cycles, all of its promoted objects). +#[derive(Clone, Copy, Debug)] +struct PromotedPageRun { + first_header: usize, + last_header: usize, + count: usize, +} + +/// Expand one run into [`OLD_GEN_PAGE_OBJECTS`]. Caller must have already +/// removed it from [`OLD_GEN_PAGE_PROMOTED_RUNS`], so this cannot recurse and +/// cannot double-append. +/// +/// The parse is the one `old_arena_walk_objects` performs — hop by +/// `GcHeader::size`, stop on an implausible one. That walker is the old-gen +/// sweep's own enumerator, so "an old block parses linearly by header size" is +/// not a new invariant this introduces; it is the invariant the sweep already +/// rests on. What this adds is a bound taken at promotion time, which is why +/// runs are expanded before anything can reshape the block — see +/// [`materialize_all_promoted_page_runs`]. +fn expand_promoted_run(page: usize, run: PromotedPageRun) { + use crate::gc::GcHeader; + + let mut headers = Vec::with_capacity(run.count); + let mut addr = run.first_header; + while addr <= run.last_header { + let header = addr as *const GcHeader; + let total = unsafe { (*header).size } as usize; + if total < crate::gc::GC_HEADER_SIZE { + break; + } + // Same filter the producer applied: a non-arena-walkable object is + // hopped OVER, not indexed. Without this the expansion would hand + // readers headers the eager list never contained. + if crate::gc::gc_type_is_arena_walkable(unsafe { (*header).obj_type }) { + headers.push(addr); + } + addr += total; + } + debug_assert_eq!( + headers.len(), + run.count, + "a promoted page run did not re-parse to the object count recorded at \ + promotion (page {page:#x}, {:#x}..={:#x}). The block was reshaped while \ + its run was still pending, which means some path that frees, moves or \ + resizes an old-gen object reached it without expanding the run first.", + run.first_header, + run.last_header, + ); + OLD_GEN_PAGE_OBJECTS.with(|index| { + let mut index = index.borrow_mut(); + let slot = index.entry(page).or_insert_with(Vec::new); + if slot.is_empty() { + *slot = headers; + } else { + slot.extend_from_slice(&headers); + } + }); +} + +/// Expand any pending run covering one of `pages`. +/// +/// Every reader of [`OLD_GEN_PAGE_OBJECTS`] must call this for the pages it is +/// about to read, in the same way #7624 makes every reader flush the deferred +/// registration buffer. `promoted_page_run_materialization_sites` enumerates +/// the obligation from the source so a new reader cannot silently skip it. +pub(crate) fn materialize_promoted_page_runs(pages: impl IntoIterator) { + if !OLD_GEN_PAGE_PROMOTED_RUNS_NONEMPTY.with(Cell::get) { + return; + } + for page in pages { + let run = OLD_GEN_PAGE_PROMOTED_RUNS.with(|runs| runs.borrow_mut().remove(&page)); + if let Some(run) = run { + expand_promoted_run(page, run); + } + } +} + +/// Expand every pending run. +/// +/// Called before anything that can reshape an old-gen block — the full and +/// budgeted cycle constructors, whose sweep frees objects in place and whose +/// holes are then refilled by `old_free` with objects of a different size. A +/// run's `last_header` is an address remembered at promotion time; once +/// boundaries inside it can move, that address stops being a header boundary. +/// Expanding first keeps the run representation confined to the window in which +/// promoted blocks are immutable: between the promotion and the next old-gen +/// sweep. +pub(crate) fn materialize_all_promoted_page_runs() { + if !OLD_GEN_PAGE_PROMOTED_RUNS_NONEMPTY.with(Cell::get) { + return; + } + let pending: Vec<(usize, PromotedPageRun)> = + OLD_GEN_PAGE_PROMOTED_RUNS.with(|runs| runs.borrow_mut().drain().collect()); + OLD_GEN_PAGE_PROMOTED_RUNS_NONEMPTY.with(|flag| flag.set(false)); + for (page, run) in pending { + expand_promoted_run(page, run); + } +} + +/// Eager per-object registration for a promoted page whose liveness is +/// [`PromotionLiveness::Marked`] — a TRACED promoting cycle, where the marks +/// are the only record of which objects are live and they are cleared before +/// anything could re-derive them. Unchanged from the pre-description path. +pub(crate) fn register_promoted_page_headers(page: usize, headers: &[usize], bytes: usize) { if headers.is_empty() { return; } @@ -586,6 +786,12 @@ pub(crate) fn register_promoted_page_run(page: usize, headers: &[usize], bytes: }); } +/// Pending runs, for tests that must prove the subject actually ran. +#[cfg(test)] +pub(crate) fn pending_promoted_page_runs() -> usize { + OLD_GEN_PAGE_PROMOTED_RUNS.with(|runs| runs.borrow().len()) +} + pub(crate) fn unregister_block_generation(base: usize, size: usize) { if base == 0 || size == 0 { return; @@ -985,6 +1191,8 @@ pub(crate) fn unregister_old_object_pages(header_addr: usize, total_size: usize) // flush would leave the flush to resurrect this object. flush_deferred_old_page_registrations(); let overlaps = old_object_page_overlaps(header_addr, total_size); + // RUN REMOVER: as `old_arena_page_index_remove_object`. + materialize_promoted_page_runs(overlaps.iter().map(|&(page, _)| page)); let mut removed_pages = Vec::with_capacity(overlaps.len()); OLD_GEN_PAGE_OBJECTS.with(|index| { let mut index = index.borrow_mut(); @@ -1264,6 +1472,8 @@ pub(crate) fn old_arena_walk_objects_on_pages( // root scan runs before the remembered-set walk), so this cannot rely on // the cycle-start flush alone. flush_deferred_old_page_registrations(); + // RUN READER: a promoted page's list is built on demand. + materialize_promoted_page_runs(pages.iter().copied()); let mut headers = Vec::new(); let mut seen = crate::fast_hash::new_ptr_hash_set(); @@ -1301,6 +1511,10 @@ impl OldArenaPageObjectCursor { // between `new` and the last `next`; `next` debug-asserts that rather // than paying a thread-local check per object. flush_deferred_old_page_registrations(); + // RUN READER: same obligation, same window — the stepping window + // promotes nothing, so expanding every page's run once here covers + // every `next`. + materialize_promoted_page_runs(pages.iter().copied()); Self { pages: pages.iter().copied().collect(), page_cursor: 0, @@ -1349,6 +1563,10 @@ pub(crate) fn old_arena_page_index_remove_object(header_addr: usize, total_size: if overlaps.is_empty() { return; } + // RUN REMOVER: a removal against a page whose list is still described + // would silently no-op, and the later expansion would resurrect the + // object. Expand first, then remove from the real list. + materialize_promoted_page_runs(overlaps.iter().map(|&(page, _)| page)); OLD_GEN_PAGE_OBJECTS.with(|index| { let mut index = index.borrow_mut(); for (page, _) in overlaps { @@ -1400,6 +1618,11 @@ pub(crate) fn old_arena_page_index_clear_for_tests() { // would get a repopulated one if the pending burst were folded in first. DEFERRED_OLD_PAGE_REGISTRATIONS.with(|buf| buf.borrow_mut().clear()); OLD_GEN_PAGE_OBJECTS.with(|index| index.borrow_mut().clear()); + // DISCARD pending runs for the same reason the deferral buffer is + // discarded: a caller asking for an empty index must not get a + // repopulated one at the next read. + OLD_GEN_PAGE_PROMOTED_RUNS.with(|runs| runs.borrow_mut().clear()); + OLD_GEN_PAGE_PROMOTED_RUNS_NONEMPTY.with(|flag| flag.set(false)); } #[cfg(test)] diff --git a/crates/perry-runtime/src/arena/promote.rs b/crates/perry-runtime/src/arena/promote.rs index 2143a17363..275a982672 100644 --- a/crates/perry-runtime/src/arena/promote.rs +++ b/crates/perry-runtime/src/arena/promote.rs @@ -61,7 +61,8 @@ //! running total and reports it in the GC trace. use super::page_meta::{ - generation_page_base, register_promoted_page_run, retag_block_space, GENERATION_PAGE_SIZE, + generation_page_base, register_promoted_page_headers, register_promoted_page_run, + retag_block_space, GENERATION_PAGE_SIZE, }; use super::*; @@ -354,6 +355,26 @@ fn install_block_into(arena: &mut Arena, block: ArenaBlock) { arena.blocks.push(block); } +/// Hand one page's finished run to the index, either DESCRIBED (untraced +/// promotion — the parse can reconstruct it) or STORED (traced promotion — only +/// the about-to-be-cleared marks know which objects are live). +#[allow(clippy::too_many_arguments)] +fn flush_page_run( + describe: bool, + page: usize, + first: usize, + last: usize, + count: usize, + headers: &[usize], + bytes: usize, +) { + if describe { + register_promoted_page_run(page, first, last, count, bytes); + } else { + register_promoted_page_headers(page, headers, bytes); + } +} + /// Linear walk of one promoted block: stamp `GC_FLAG_TENURED` on every header, /// and register the live ones with the old-gen page index in per-page bulk /// runs. Returns `(objects, live_objects, live_bytes)`. @@ -372,11 +393,23 @@ fn stamp_and_index_block(block: &ArenaBlock, liveness: PromotionLiveness) -> (us let mut live_objects = 0usize; let mut live_bytes = 0usize; - // One page's worth of live headers, flushed whenever the page changes. The - // walk is in address order, so a page's headers are contiguous in it. + // One page's worth of live headers, flushed whenever the page changes. + // The walk is in address order, so a page's headers are a contiguous + // ascending run. + // + // On the UNTRACED path (`AssumeAllLive`) that run is DESCRIBED by its first + // and last address instead of stored — every walkable object between them + // is on the page, so the pair is exact and the list is re-derivable by the + // same parse the sweep already uses. On the TRACED path the marks are the + // only record of which objects are live and `clear_marks` destroys them, so + // the list is stored as before. See `register_promoted_page_run`. + let describe = matches!(liveness, PromotionLiveness::AssumeAllLive); let mut run_page: Option = None; - let mut run_headers: Vec = Vec::new(); + let mut run_first = 0usize; + let mut run_last = 0usize; + let mut run_count = 0usize; let mut run_bytes = 0usize; + let mut run_headers: Vec = Vec::new(); let mut offset = 0usize; while offset < block.offset { @@ -427,20 +460,42 @@ fn stamp_and_index_block(block: &ArenaBlock, liveness: PromotionLiveness) -> (us } if run_page != Some(page) { if let Some(previous) = run_page { - register_promoted_page_run(previous, &run_headers, run_bytes); + flush_page_run( + describe, + previous, + run_first, + run_last, + run_count, + &run_headers, + run_bytes, + ); } run_page = Some(page); - run_headers.clear(); + run_first = header_addr; + run_count = 0; run_bytes = 0; + run_headers.clear(); } - run_headers.push(header_addr); + run_last = header_addr; + run_count += 1; run_bytes += overlap_end - overlap_start; + if !describe { + run_headers.push(header_addr); + } } } offset = aligned + total; } if let Some(previous) = run_page { - register_promoted_page_run(previous, &run_headers, run_bytes); + flush_page_run( + describe, + previous, + run_first, + run_last, + run_count, + &run_headers, + run_bytes, + ); } debug_assert_eq!( offset, block.offset, diff --git a/crates/perry-runtime/src/arena/tests.rs b/crates/perry-runtime/src/arena/tests.rs index 0025eef817..f94c75a18e 100644 --- a/crates/perry-runtime/src/arena/tests.rs +++ b/crates/perry-runtime/src/arena/tests.rs @@ -24,7 +24,7 @@ fn general_block_offset(idx: usize) -> usize { ARENA.with(|a| unsafe { (&*a.get()).blocks[idx].offset }) } -fn run_with_fresh_arenas(test: impl FnOnce() + Send + 'static) { +pub(super) fn run_with_fresh_arenas(test: impl FnOnce() + Send + 'static) { std::thread::spawn(test) .join() .expect("arena test panicked"); @@ -1469,6 +1469,23 @@ fn deferred_registration_flush_sites() { 256 times per 1 MiB block — and cannot be needed, because nothing \ between the walk's start and its end allocates into old-gen", ), + ( + "register_promoted_page_headers", + "the TRACED promotion's eager arm, split out of \ + register_promoted_page_run. Same argument: one call per PAGE from \ + `finish_in_place_promotion`'s single linear walk, which flushes \ + once before the whole walk, and nothing between the walk's start \ + and its end allocates into old-gen", + ), + ( + "expand_promoted_run", + "expands a DESCRIBED promoted page into the object list. Every \ + caller has already flushed: the four readers/removers do so as \ + their #7624 obligation, `materialize_all_promoted_page_runs` runs \ + immediately after `old_pages_begin_gc_cycle`, and \ + `register_promoted_page_run` is inside the promotion walk covered \ + by the entry above", + ), ( "flush_deferred_old_page_registrations", "the flush entry point", diff --git a/crates/perry-runtime/src/arena/tests_promoted_runs.rs b/crates/perry-runtime/src/arena/tests_promoted_runs.rs new file mode 100644 index 0000000000..62b2ec2120 --- /dev/null +++ b/crates/perry-runtime/src/arena/tests_promoted_runs.rs @@ -0,0 +1,325 @@ +//! Promoted-block page runs: the page-object list for a whole-block promotion +//! is DESCRIBED (first/last header + count) and expanded only on demand. +//! +//! Two obligations are pinned here, in the same shape #7624 pins its own: +//! +//! * **behaviour** — a run must expand to exactly the header set the eager +//! per-object list used to hold, and the tests must prove the run path was +//! the one that ran (`pending_promoted_page_runs() > 0` before the read), +//! not merely that a read returned something; +//! * **coverage** — every reader and remover of `OLD_GEN_PAGE_OBJECTS` must +//! expand first, enumerated from the source so a reader added later cannot +//! silently skip it. + +use super::page_meta::{ + materialize_all_promoted_page_runs, register_promoted_page_run, unregister_old_block_pages, +}; +use super::*; +use crate::gc::{GcHeader, GC_HEADER_SIZE, GC_TYPE_STRING}; + +const OBJ: usize = 64; + +/// A real, parseable old-gen region: `count` back-to-back `GC_TYPE_STRING` +/// objects of `OBJ` bytes each, registered as an old block. +/// +/// Real memory, not the synthetic addresses `tests.rs` uses — a run is +/// expanded by PARSING the block, so a fake address would not survive the hop. +struct PromotedRegion { + _backing: Vec, + base: usize, + headers: Vec, +} + +fn promoted_region(count: usize) -> PromotedRegion { + let size = (count * OBJ).next_multiple_of(GENERATION_PAGE_SIZE); + let mut backing = vec![0u8; size + GENERATION_PAGE_SIZE]; + // Page-align so the run's page geometry is the same one a promoted arena + // block has. + let base = (backing.as_mut_ptr() as usize).next_multiple_of(GENERATION_PAGE_SIZE); + let headers: Vec = (0..count).map(|i| base + i * OBJ).collect(); + for &header in &headers { + unsafe { + *(header as *mut GcHeader) = GcHeader { + obj_type: GC_TYPE_STRING, + gc_flags: 0, + _reserved: 0, + size: OBJ as u32, + }; + } + } + register_block_space(base, size, HeapGeneration::Old, HeapSpace::Old); + PromotedRegion { + _backing: backing, + base, + headers, + } +} + +/// Register every page the region spans as one promoted run, the way +/// `finish_in_place_promotion`'s walk does. +fn register_region_runs(region: &PromotedRegion) -> crate::fast_hash::PtrHashSet { + let mut pages = crate::fast_hash::new_ptr_hash_set(); + let mut page_first: Vec<(usize, usize, usize, usize)> = Vec::new(); + for &header in ®ion.headers { + let page = generation_page_for_addr(header); + match page_first.last_mut() { + Some(entry) if entry.0 == page => { + entry.2 = header; + entry.3 += 1; + } + _ => page_first.push((page, header, header, 1)), + } + } + for (page, first, last, count) in page_first { + register_promoted_page_run(page, first, last, count, count * OBJ); + pages.insert(page); + } + pages +} + +fn walk(pages: &crate::fast_hash::PtrHashSet) -> Vec { + let mut seen = Vec::new(); + old_arena_walk_objects_on_pages(pages, |h| seen.push(h as usize)); + seen.sort_unstable(); + seen +} + +#[test] +fn a_promoted_run_expands_to_exactly_the_eager_header_list() { + super::tests::run_with_fresh_arenas(|| { + let region = promoted_region(200); + let pages = register_region_runs(®ion); + + // SUBJECT-LIVE: the eager list must NOT have been built. Without this + // the test would pass just as well against the per-object path it + // replaces, which is the #7024 shape. + assert!( + pending_promoted_page_runs() > 0, + "registration must DESCRIBE the run, not store the header list — \ + otherwise this test proves nothing about the new path" + ); + + let mut expected = region.headers.clone(); + expected.sort_unstable(); + assert_eq!( + walk(&pages), + expected, + "an expanded run must be exactly the header set the per-object \ + list held; a short parse is a page whose objects a dirty scan \ + would never visit — a missed old->young edge" + ); + assert_eq!( + pending_promoted_page_runs(), + 0, + "reading a page must consume its run, so the expansion is paid once" + ); + // Idempotent: a second read must not duplicate. + assert_eq!(walk(&pages), expected); + }); +} + +#[test] +fn page_meta_accounting_matches_the_object_count_without_expanding() { + super::tests::run_with_fresh_arenas(|| { + let region = promoted_region(200); + register_region_runs(®ion); + assert!(pending_promoted_page_runs() > 0); + let summary = old_page_summary(); + assert_eq!( + summary.object_count, 200, + "the run records its count eagerly: defrag page selection reads \ + object_count/live_bytes and must not have to expand to see them" + ); + assert_eq!(summary.live_object_count, 200); + }); +} + +#[test] +fn a_full_cycle_expands_every_pending_run_before_it_can_sweep() { + super::tests::run_with_fresh_arenas(|| { + let region = promoted_region(200); + let pages = register_region_runs(®ion); + assert!(pending_promoted_page_runs() > 0); + + // What `GcCycleState::new_full` calls. A run's bounds are addresses + // captured at promotion; once the sweep can free objects inside the + // block and `old_free` can refill the holes, those bounds stop being + // object boundaries. + materialize_all_promoted_page_runs(); + + assert_eq!(pending_promoted_page_runs(), 0); + let mut expected = region.headers.clone(); + expected.sort_unstable(); + assert_eq!(walk(&pages), expected); + }); +} + +#[test] +fn dropping_a_block_discards_its_run_instead_of_parsing_freed_pages() { + super::tests::run_with_fresh_arenas(|| { + let region = promoted_region(200); + let pages = register_region_runs(®ion); + let page_list: Vec = pages.iter().copied().collect(); + assert!(pending_promoted_page_runs() > 0); + + unregister_old_block_pages(&page_list); + + assert_eq!( + pending_promoted_page_runs(), + 0, + "the block is gone; a surviving run would have a later read parse \ + memory this arena no longer owns" + ); + assert!( + walk(&pages).is_empty(), + "an unregistered page must stay empty — a run must not resurrect it" + ); + drop(region); + }); +} + +/// A TRACED promotion may NOT be described: its liveness lives in marks that +/// `clear_marks` destroys before any expansion could read them, so the eager +/// list is the only record. This pins that the two paths stayed separate. +#[test] +fn a_traced_promotion_still_stores_its_header_list() { + super::tests::run_with_fresh_arenas(|| { + let region = promoted_region(64); + let page = generation_page_for_addr(region.base); + let mut pages = crate::fast_hash::new_ptr_hash_set(); + pages.insert(page); + + super::page_meta::register_promoted_page_headers( + page, + ®ion.headers, + region.headers.len() * OBJ, + ); + + assert_eq!( + pending_promoted_page_runs(), + 0, + "the Marked path must store, not describe — a described page would \ + re-parse to ALL walkable objects between the bounds, including the \ + unmarked ones the trace proved dead" + ); + let mut expected = region.headers.clone(); + expected.sort_unstable(); + assert_eq!(walk(&pages), expected); + }); +} + +/// The coverage half. Both tables are thread-locals private to `page_meta.rs`, +/// so the set of functions that touch `OLD_GEN_PAGE_OBJECTS` is enumerable from +/// the source: each must expand pending runs first, or carry a written reason. +/// +/// A name in `EXEMPT` that no longer touches the table also fails, so a fix +/// cannot leave a stale exemption behind. +#[test] +fn every_page_object_reader_expands_promoted_runs() { + const EXEMPT: &[(&str, &str)] = &[ + ("expand_promoted_run", "the expansion itself"), + ( + "unregister_old_block_pages", + "DISCARDS the run rather than expanding it — the backing block is \ + going away, so its bounds no longer address arena memory", + ), + ( + "old_arena_page_index_clear_for_tests", + "DISCARDS, for the same reason it discards the deferral buffer", + ), + ( + "register_old_object_pages", + "APPENDS a born-old object beyond the promoted run's bounds. The \ + two populations are disjoint and expansion unions them, so an \ + append needs no expansion; its `contains` dedup only guards \ + addresses it registered itself", + ), + ( + "flush_deferred_old_page_registrations_batch", + "as register_old_object_pages — appends beyond the run", + ), + ( + "register_promoted_page_headers", + "the TRACED promotion's eager path. It authors a page a described \ + run never covers (one promotion per page), and in the \ + two-blocks-share-a-page case the two populations are disjoint and \ + expansion unions them — the register_old_object_pages argument", + ), + ( + "next", + "OldArenaPageObjectCursor::next. `new` expands every page it will \ + step, and the budgeted stepping window marks without promoting, \ + so no run can appear mid-walk — the same window argument #7624 \ + makes for the deferral buffer, and `next` already debug-asserts it", + ), + ]; + + let src = std::fs::read_to_string( + std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src/arena/page_meta.rs"), + ) + .expect("page_meta.rs must be readable"); + + let mut bodies: Vec<(String, String)> = Vec::new(); + for line in src.lines() { + let trimmed = line.trim_start(); + if let Some(rest) = trimmed + .strip_prefix("pub(crate) fn ") + .or_else(|| trimmed.strip_prefix("pub fn ")) + .or_else(|| trimmed.strip_prefix("fn ")) + { + let name: String = rest + .chars() + .take_while(|c| c.is_alphanumeric() || *c == '_') + .collect(); + bodies.push((name, String::new())); + } + if let Some(last) = bodies.last_mut() { + last.1.push_str(line); + last.1.push('\n'); + } + } + + let exempt_names: Vec<&str> = EXEMPT.iter().map(|(n, _)| *n).collect(); + let mut offenders = Vec::new(); + let mut touching = std::collections::BTreeSet::new(); + for (name, body) in &bodies { + if !body.contains("OLD_GEN_PAGE_OBJECTS.with") { + continue; + } + touching.insert(name.as_str()); + if body.contains("materialize_promoted_page_runs(") + || body.contains("materialize_all_promoted_page_runs()") + { + continue; + } + if exempt_names.contains(&name.as_str()) { + continue; + } + offenders.push(name.clone()); + } + + assert!( + offenders.is_empty(), + "these functions in arena/page_meta.rs read or mutate \ + OLD_GEN_PAGE_OBJECTS without first expanding pending promoted page \ + runs: {offenders:?}.\n\ + A promoted page's object list is DESCRIBED until someone asks for it, \ + so a reader that skips the expansion sees an EMPTY page — and the \ + dirty scan that reader feeds would then never visit those objects, \ + losing every old->young edge out of them. Call \ + materialize_promoted_page_runs(pages), or add the function to EXEMPT \ + with the argument for why the description cannot be observed there." + ); + + let stale: Vec<&str> = exempt_names + .iter() + .copied() + .filter(|name| !touching.contains(name)) + .collect(); + assert!( + stale.is_empty(), + "these EXEMPT entries no longer touch OLD_GEN_PAGE_OBJECTS: {stale:?}. \ + Delete them — a stale exemption is an unexamined claim that would \ + cover a future function of the same name." + ); +} diff --git a/crates/perry-runtime/src/gc/cycle.rs b/crates/perry-runtime/src/gc/cycle.rs index b248e44310..682652ea1f 100644 --- a/crates/perry-runtime/src/gc/cycle.rs +++ b/crates/perry-runtime/src/gc/cycle.rs @@ -931,6 +931,9 @@ impl GcCycleState { let trace = GcCycleTrace::new(GcCollectionKind::Full, trigger); let start = Instant::now(); crate::arena::old_pages_begin_gc_cycle(); + // The one constructor that sweeps old-gen, so the one that invalidates + // a promoted run's bounds. See the fn's doc for why no minor needs it. + crate::arena::materialize_all_promoted_page_runs(); clear_mark_seeds(); // Allocate-black for the WHOLE cycle, from the first build slice on: // the mark barrier only engages at the END of BuildValidPointerSet diff --git a/scripts/addr_class_allowlist.txt b/scripts/addr_class_allowlist.txt index 91e95a9061..cbd021ed22 100644 --- a/scripts/addr_class_allowlist.txt +++ b/scripts/addr_class_allowlist.txt @@ -165,3 +165,5 @@ crates/perry-ext-events/src/lib.rs | const EVENT_EMITTER_HANDLE_ID_END | #7272: crates/perry-ext-net/src/jsvalue.rs | * | #7272: socket/server handle-vs-pointer discrimination before dereference; re-types the handle band instead of calling addr_class::is_handle_band, which perry-runtime does not export crates/perry-ext-ratelimit/src/lib.rs | (obj as usize) >= 0x100000 | #7272: same handle-vs-pointer guard before an ObjectHeader read crates/perry-ext-slugify/src/lib.rs | (obj as usize) < 0x100000 | #7272: same handle-vs-pointer guard before an ObjectHeader read +crates/perry-runtime/src/arena/page_meta.rs | let header = addr as *const GcHeader; | promoted-page-run expansion: `addr` starts at a `first_header` recorded by arena/promote.rs's linear block iteration (its grandfathered sibling entry above) and advances by `GcHeader::size` from there, so every address is a block-interior header, never a NaN-box payload; the parse stops at the first implausible size exactly as the arena walkers do +crates/perry-runtime/src/arena/tests_promoted_runs.rs | * | arena promoted-run tests: header addresses are offsets into a buffer the test itself allocated and initialised, never NaN-box payloads -- same discipline as the arena/tests.rs entry above