From b93423d90d1227cc92fa4f7185732ca521533b43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 06:33:35 +0200 Subject: [PATCH 1/2] fix(gc): idle compaction selects whole blocks, so its prediction is achievable (#9772) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Old-gen memory is released a BLOCK at a time (`old_arena_reclaim_selected_dead_blocks`), but selection ranked individual 4 KB pages by fragmentation. The emptied pages were scattered across blocks that kept other live occupants, so the reclaim freed none of them: on the compiled claude-code TUI the pass chose 10,740 pages, predicted 44 MB of "releasable block bytes", ran 228 ms and released nothing. Selection now groups pages by their containing block (`arena::old_arena_block_ranges` / `old_arena_block_range_index`), skips blocks holding pinned bytes, ranks the rest cheapest-to-empty and takes whole blocks until the move budget. Every selected block therefore ends the pass with no live occupant, which is exactly what the reclaim tests, and `selected_releasable_block_bytes` becomes the sum of real block sizes instead of a sum of page granules nothing releases. Two-arm run, same binary, one env var apart (`PERRY_GC_IDLE_COMPACT_BLOCKS`): block-targeted selection predicted 52.4 MB -> released 46.6 MB, kept_promise=true, 50 of 50 targeted blocks released (`has_live=0`), old-gen in-use 120.4 MB -> 73.8 MB page-granular selection predicted 44.4 MB -> released 0 MB, kept_promise=false, no block reclaimed, 516 ms of pause Two defects that only became visible once the pass could be judged against its own prediction are fixed with it: * A pass that DECLINES to evacuate no longer drops the excluded pages' holes first. Filtering them is justified only by "this pass is about to empty and release these blocks"; doing it before the all-or-nothing movability check meant one immovable occupant anywhere in the selection cost the whole old-gen residue and returned nothing — measured `reusable` 40.7 MB -> 0.87 MB with `released=0` and `freed=296` bytes. * `IDLE_COMPACT_MOVE_BUDGET_BYTES` 8 MiB -> 1 MiB. That constant is a PAUSE budget and was calibrated on the #9644 fixture's ~14 ms per MiB moved; a real old generation runs the same pass at ~195 ms per MiB (8.39 MB across 50 blocks in 1.64 s), because its occupants are far smaller and far more numerous. 1 MiB keeps the pause at the 190-230 ms the pass already spent while returning nothing, so the change costs no additional pause budget. Counters, so a barren pass names its own obstacle instead of being silent: `[gc-old-block-reclaim] targeted/released/released_bytes/kept-by-reason`, `predicted=` and `kept_promise=` on `[gc-idle-compact] done`, and `broken_promises=` in the exit line. `[gc-general-reclaim]` does the same for the general arena's empty-block release, which is what proved the eden capacity in the census is working set in rotation rather than un-returned memory (12-16 blocks released per cycle, `has_live` the only real obstacle). Kill switch `PERRY_GC_IDLE_COMPACT_BLOCKS=0`. Claude-Session: https://claude.ai/code/session_01YPfnmWZmSpSWpmnoXvH8z2 --- .../9772-idle-compaction-block-selection.md | 1 + crates/perry-runtime/src/arena/mod.rs | 3 +- crates/perry-runtime/src/arena/page_meta.rs | 67 ++++++++ crates/perry-runtime/src/arena/reset.rs | 134 ++++++++++++++-- crates/perry-runtime/src/gc/idle_compact.rs | 22 ++- crates/perry-runtime/src/gc/oldgen.rs | 28 ++-- crates/perry-runtime/src/gc/oldgen_defrag.rs | 151 ++++++++++++++++-- 7 files changed, 364 insertions(+), 42 deletions(-) create mode 100644 changelog.d/9772-idle-compaction-block-selection.md diff --git a/changelog.d/9772-idle-compaction-block-selection.md b/changelog.d/9772-idle-compaction-block-selection.md new file mode 100644 index 0000000000..63912b6c59 --- /dev/null +++ b/changelog.d/9772-idle-compaction-block-selection.md @@ -0,0 +1 @@ +fix(gc): the idle old-generation compaction now selects whole BLOCKS, so the bytes it predicts are the bytes it can return (#9772). Old-gen memory is released a block at a time (`old_arena_reclaim_selected_dead_blocks`), but selection ranked individual 4 KB pages by fragmentation, and the emptied pages were scattered across blocks that kept other live occupants. Measured on the compiled claude-code TUI: the pass chose 10,740 pages, predicted 44 MB of "releasable block bytes", ran for 228 ms and released **nothing**; in a controlled two-arm run the same selection predicted 44.4 MB, spent 516 ms and released 0, because a page-granular prediction is not achievable by a block-granular reclaim. Selection now groups pages by their containing block (`arena::old_arena_block_ranges`), skips blocks holding pinned bytes, ranks the rest cheapest-to-empty and takes whole blocks, so every selected block ends the pass with no live occupant. Same workload, same binary, one env var apart: **released 46.6 MB of 52.4 MB predicted (`kept_promise=true`, 50 of 50 targeted blocks, `has_live=0`), old-gen occupancy 120.4 MB -> 73.8 MB**, against **0 MB released of 44.4 MB predicted** for the old selection. Two counters make a barren pass visible instead of silent: `[gc-old-block-reclaim]` reports targeted/released/kept-by-reason, and `[gc-idle-compact] done` now carries `predicted=` and `kept_promise=`, summarised as `broken_promises=` in the exit line. Two defects that only became visible once the pass could be judged against its own prediction are fixed with it: a pass that declines to evacuate no longer drops the excluded pages' holes first (it was destroying 40.7 MB of reusable free list and returning nothing), and `IDLE_COMPACT_MOVE_BUDGET_BYTES` is recalibrated 8 MiB -> 1 MiB against the rate this pass actually runs at on a real old generation (195 ms per MiB moved, not the 14 ms/MiB of the #9644 fixture), keeping the pause at the ~190-230 ms the unproductive pass already cost. Kill switch `PERRY_GC_IDLE_COMPACT_BLOCKS=0`. diff --git a/crates/perry-runtime/src/arena/mod.rs b/crates/perry-runtime/src/arena/mod.rs index 7c263b7437..ea700fc571 100644 --- a/crates/perry-runtime/src/arena/mod.rs +++ b/crates/perry-runtime/src/arena/mod.rs @@ -146,7 +146,8 @@ pub(crate) use page_meta::{ 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, record_arena_object_start, unregister_old_object_pages, HeapGeneration, HeapSpace, OldArenaPageObjectCursor, - OldArenaSourceBlockSelection, OldPageMeta, OldPageSummary, + old_arena_block_range_index, old_arena_block_ranges, OldArenaSourceBlockSelection, + OldPageMeta, OldPageSummary, }; #[cfg(test)] diff --git a/crates/perry-runtime/src/arena/page_meta.rs b/crates/perry-runtime/src/arena/page_meta.rs index e1116911f4..bc6984dff3 100644 --- a/crates/perry-runtime/src/arena/page_meta.rs +++ b/crates/perry-runtime/src/arena/page_meta.rs @@ -1538,6 +1538,45 @@ fn normalize_dirty_slots_for_epoch(mut page_meta: OldPageMeta, current_epoch: u6 page_meta } +/// Address ranges of the live old-generation blocks, as +/// `(base, end_exclusive, global_block_index, size)` sorted by base. +/// +/// Old-gen memory is released a BLOCK at a time (`old_arena_reclaim_*`), so a +/// pass that wants its bytes back has to reason in blocks. `OldPageMeta` is +/// page-granular and carries no block identity, which is why #9772's selection +/// could predict 44 MB of "releasable block bytes" from page granules and +/// release nothing. +pub(crate) fn old_arena_block_ranges() -> Vec<(usize, usize, usize, usize)> { + let old_block_start = longlived_end(); + OLD_ARENA.with(|arena| { + let arena = unsafe { &*arena.get() }; + let mut out: Vec<(usize, usize, usize, usize)> = arena + .blocks + .iter() + .enumerate() + .filter_map(|(i, block)| { + if block.data.is_null() || block.size == 0 { + return None; + } + let base = block.data as usize; + Some((base, base + block.size, old_block_start + i, block.size)) + }) + .collect(); + out.sort_unstable_by_key(|r| r.0); + out + }) +} + +/// Index into [`old_arena_block_ranges`] output for the block containing +/// `addr`, or `None` when the address is not in a live old-gen block. +pub(crate) fn old_arena_block_range_index( + ranges: &[(usize, usize, usize, usize)], + addr: usize, +) -> Option { + let idx = ranges.partition_point(|r| r.0 <= addr).checked_sub(1)?; + (addr < ranges[idx].1).then_some(idx) +} + pub(crate) fn old_arena_source_blocks_for_pages( selected_pages: &crate::fast_hash::PtrHashSet, ) -> OldArenaSourceBlockSelection { @@ -1896,3 +1935,31 @@ pub(crate) fn page_meta_census() -> Vec { }); rows } + +#[cfg(test)] +mod block_range_tests { + use super::old_arena_block_range_index; + + /// `old_arena_block_range_index` is the whole reason #9772's selection can + /// group pages by block, so it gets a test that can fail: gaps between + /// blocks must not be attributed to the block below them. + #[test] + fn block_range_lookup_respects_gaps_and_ends() { + // Two 1 MiB blocks with a 1 MiB hole between them. + let ranges = vec![ + (0x1000_0000, 0x1010_0000, 7, 0x10_0000), + (0x1020_0000, 0x1030_0000, 9, 0x10_0000), + ]; + assert_eq!(old_arena_block_range_index(&ranges, 0x1000_0000), Some(0)); + assert_eq!(old_arena_block_range_index(&ranges, 0x100F_FFFF), Some(0)); + // One past the end of block 0 is the gap, not block 0. + assert_eq!(old_arena_block_range_index(&ranges, 0x1010_0000), None); + assert_eq!(old_arena_block_range_index(&ranges, 0x1018_0000), None); + assert_eq!(old_arena_block_range_index(&ranges, 0x1020_0000), Some(1)); + assert_eq!(old_arena_block_range_index(&ranges, 0x102F_FFFF), Some(1)); + // Above every block, and below every block. + assert_eq!(old_arena_block_range_index(&ranges, 0x1030_0000), None); + assert_eq!(old_arena_block_range_index(&ranges, 0x0FFF_FFFF), None); + assert_eq!(old_arena_block_range_index(&[], 0x1000_0000), None); + } +} diff --git a/crates/perry-runtime/src/arena/reset.rs b/crates/perry-runtime/src/arena/reset.rs index 5414a6cebb..a5ba37fa86 100644 --- a/crates/perry-runtime/src/arena/reset.rs +++ b/crates/perry-runtime/src/arena/reset.rs @@ -533,6 +533,32 @@ enum GeneralResetSubphase { Done, } +/// Why a general-arena block was not released this cycle. Empty eden capacity +/// is the largest single piece of arena slack on the compiled claude-code TUI +/// (51-56 blocks holding 1-9 MB of objects), and `PERRY_GC_DIAG=1` could say +/// only that the blocks were still there. One counter per guard says which +/// guard is actually holding them. +#[derive(Clone, Copy, Default)] +struct GeneralDeallocDiag { + examined: usize, + no_snapshot: usize, + snapshot_moved: usize, + keep_window: usize, + has_live: usize, + in_use: usize, + aging: usize, + released: usize, +} + +enum DeallocReject { + NoSnapshot, + SnapshotMoved, + KeepWindow, + HasLive, + InUse, + Aging, +} + pub(crate) struct ArenaResetEmptyBlocksState { block_has_live: Vec, snapshots: Vec, @@ -542,6 +568,7 @@ pub(crate) struct ArenaResetEmptyBlocksState { reset_ranges: Vec<(usize, usize, usize)>, removed_ranges: Vec<(usize, usize)>, stats: ArenaResetStats, + diag: GeneralDeallocDiag, } impl ArenaResetEmptyBlocksState { @@ -555,6 +582,7 @@ impl ArenaResetEmptyBlocksState { reset_ranges: Vec::new(), removed_ranges: Vec::new(), stats: ArenaResetStats::default(), + diag: GeneralDeallocDiag::default(), } } @@ -649,38 +677,58 @@ impl ArenaResetEmptyBlocksState { &mut self, block_idx: usize, ) -> Option<(usize, usize, ArenaBlockRelease)> { + self.diag.examined += 1; + let outcome = self.dealloc_block_inner(block_idx); + match &outcome { + Ok(_) => self.diag.released += 1, + Err(DeallocReject::NoSnapshot) => self.diag.no_snapshot += 1, + Err(DeallocReject::SnapshotMoved) => self.diag.snapshot_moved += 1, + Err(DeallocReject::KeepWindow) => self.diag.keep_window += 1, + Err(DeallocReject::HasLive) => self.diag.has_live += 1, + Err(DeallocReject::InUse) => self.diag.in_use += 1, + Err(DeallocReject::Aging) => self.diag.aging += 1, + } + outcome.ok() + } + + fn dealloc_block_inner( + &mut self, + block_idx: usize, + ) -> Result<(usize, usize, ArenaBlockRelease), DeallocReject> { let snapshot = self.snapshots.get(block_idx).copied().unwrap_or_default(); if snapshot.data == 0 { - return None; + return Err(DeallocReject::NoSnapshot); } ARENA.with(|arena| unsafe { let arena = &mut *arena.get(); let current = arena.current; let keep_low = current.saturating_sub(4); - let block = arena.blocks.get_mut(block_idx)?; + let Some(block) = arena.blocks.get_mut(block_idx) else { + return Err(DeallocReject::NoSnapshot); + }; if block.data.is_null() || block.data as usize != snapshot.data || block.size != snapshot.size { - return None; + return Err(DeallocReject::SnapshotMoved); } if block_idx == current || (block_idx >= keep_low && block_idx <= current) { block.dead_cycles = 0; - return None; + return Err(DeallocReject::KeepWindow); } if self.block_has_live.get(block_idx).copied().unwrap_or(false) { block.dead_cycles = 0; - return None; + return Err(DeallocReject::HasLive); } if block.offset != 0 { block.dead_cycles = 0; - return None; + return Err(DeallocReject::InUse); } block.dead_cycles = block.dead_cycles.saturating_add(1); if block.dead_cycles < GENERAL_DEALLOC_DEAD_CYCLES { - return None; + return Err(DeallocReject::Aging); } let base = block.data as usize; @@ -694,7 +742,7 @@ impl ArenaResetEmptyBlocksState { block.offset = 0; block.dead_cycles = 0; self.changed = true; - Some((base, size, release)) + Ok((base, size, release)) }) } @@ -717,6 +765,21 @@ impl ArenaResetEmptyBlocksState { ..self.stats }; + if crate::gc::gc_diag_enabled() && self.diag.examined > 0 { + let d = self.diag; + eprintln!( + "[gc-general-reclaim] examined={} released={} rejected: no_snapshot={} \ + snapshot_moved={} keep_window={} has_live={} in_use={} aging={}", + d.examined, + d.released, + d.no_snapshot, + d.snapshot_moved, + d.keep_window, + d.has_live, + d.in_use, + d.aging, + ); + } if !self.changed { return; } @@ -1011,6 +1074,20 @@ impl SurvivorArenaReclaimDeadBlocksState { } } +/// #9772: a compaction that evacuates pages and then releases nothing is +/// indistinguishable, from the outside, from one that had nothing to do. These +/// count why each TARGETED old block survived its reclaim, so an unproductive +/// pass names its own obstacle instead of costing a pause silently. +#[derive(Clone, Copy, Default)] +struct OldReclaimDiag { + targeted: usize, + no_snapshot: usize, + snapshot_moved: usize, + has_live: usize, + released: usize, + released_bytes: usize, +} + pub(crate) struct OldArenaReclaimDeadBlocksState { block_has_live: Vec, snapshots: Vec, @@ -1019,6 +1096,8 @@ pub(crate) struct OldArenaReclaimDeadBlocksState { subphase: RegionReclaimSubphase, changed: bool, stats: ArenaResetStats, + diag: OldReclaimDiag, + targeted_mode: bool, } impl OldArenaReclaimDeadBlocksState { @@ -1042,11 +1121,13 @@ impl OldArenaReclaimDeadBlocksState { Self { block_has_live: block_has_live.to_vec(), snapshots: snapshots.to_vec(), + targeted_mode: selected_old_blocks.is_some(), selected_old_blocks, cursor: 0, subphase: RegionReclaimSubphase::Reclaim, changed: false, stats: ArenaResetStats::default(), + diag: OldReclaimDiag::default(), } } @@ -1067,6 +1148,22 @@ impl OldArenaReclaimDeadBlocksState { } RegionReclaimSubphase::Finish => { self.finish(); + if crate::gc::gc_diag_enabled() && self.targeted_mode { + let d = self.diag; + eprintln!( + "[gc-old-block-reclaim] targeted={} released={} released_bytes={} \ + kept: has_live={} snapshot_moved={} no_snapshot={} \ + pooled_bytes={} deallocated_bytes={}", + d.targeted, + d.released, + d.released_bytes, + d.has_live, + d.snapshot_moved, + d.no_snapshot, + self.stats.pooled_bytes, + self.stats.deallocated_bytes, + ); + } OLD_GEN_RECLAIM_REUSABLE_BYTES .with(|bytes| bytes.set(self.stats.reusable_bytes)); OLD_GEN_RECLAIM_POOLED_BYTES.with(|bytes| bytes.set(self.stats.pooled_bytes)); @@ -1096,15 +1193,22 @@ impl OldArenaReclaimDeadBlocksState { return; } + self.diag.targeted += 1; let snapshot = self.snapshots.get(block_idx).copied().unwrap_or_default(); if snapshot.data == 0 { + self.diag.no_snapshot += 1; return; } + let diag = &mut self.diag; + let block_has_live = &self.block_has_live; + let changed = &mut self.changed; + let stats = &mut self.stats; OLD_ARENA.with(|arena| unsafe { let arena = &mut *arena.get(); let original_current = arena.current; let Some(block) = arena.blocks.get_mut(local_idx) else { + diag.no_snapshot += 1; return; }; if block.data.is_null() @@ -1112,12 +1216,16 @@ impl OldArenaReclaimDeadBlocksState { || block.size != snapshot.size || block.offset != snapshot.offset { + diag.snapshot_moved += 1; return; } - if self.block_has_live.get(block_idx).copied().unwrap_or(false) { + if block_has_live.get(block_idx).copied().unwrap_or(false) { + diag.has_live += 1; block.dead_cycles = 0; return; } + diag.released += 1; + diag.released_bytes += block.size; let base = block.data as usize; let size = block.size; @@ -1131,16 +1239,16 @@ impl OldArenaReclaimDeadBlocksState { crate::gc::old_free_filter_range(base, size); if used != 0 { - self.stats.reset_blocks = self.stats.reset_blocks.saturating_add(1); + stats.reset_blocks = stats.reset_blocks.saturating_add(1); } block.clear_object_starts(); block.offset = 0; block.dead_cycles = 0; old_gen_in_use_bytes_sub(used); - self.changed = true; + *changed = true; if local_idx == original_current { - self.stats.reusable_bytes = self.stats.reusable_bytes.saturating_add(used); + stats.reusable_bytes = stats.reusable_bytes.saturating_add(used); return; } @@ -1152,7 +1260,7 @@ impl OldArenaReclaimDeadBlocksState { block.object_starts = Box::new([]); block.offset = 0; block.dead_cycles = 0; - self.stats.record_block_release(size, release); + stats.record_block_release(size, release); }); } diff --git a/crates/perry-runtime/src/gc/idle_compact.rs b/crates/perry-runtime/src/gc/idle_compact.rs index f054cd8d07..ae00f0aa6d 100644 --- a/crates/perry-runtime/src/gc/idle_compact.rs +++ b/crates/perry-runtime/src/gc/idle_compact.rs @@ -300,6 +300,15 @@ pub(super) fn maybe_compact(now: u64) -> bool { let after_occupancy = crate::arena::old_gen_in_use_bytes(); let after_residue = residue_bytes(); let released = before_occupancy.saturating_sub(after_occupancy); + // #9772: judge the pass against its OWN prediction. Selecting whole blocks + // makes `predicted` achievable by construction, so a pass that returns far + // less than it promised is a defect, not a quiet no-op — it has spent a + // mutator pause on a process already far above node's idle CPU. + let predicted = super::oldgen_defrag::last_idle_predicted_release_bytes(); + let kept_promise = predicted == 0 || released.saturating_mul(2) >= predicted; + if !kept_promise { + BROKEN_PROMISES.fetch_add(1, Ordering::Relaxed); + } let productive = released >= IDLE_COMPACT_PRODUCTIVE_MIN_BYTES; ATTEMPTS.fetch_add(1, Ordering::Relaxed); @@ -322,6 +331,7 @@ pub(super) fn maybe_compact(now: u64) -> bool { if gc_diag_enabled() { eprintln!( "[gc-idle-compact] done old_in_use={before_occupancy}->{after_occupancy} released={released} \ + predicted={predicted} kept_promise={kept_promise} \ reusable={before_residue}->{after_residue} freed={freed} pause_us={pause_us} \ productive={productive} backoff_shift={}", st.backoff_shift @@ -336,14 +346,24 @@ pub(super) fn maybe_compact(now: u64) -> bool { true } +/// Idle compactions that released less than half the block bytes their own +/// selection predicted (#9772). +static BROKEN_PROMISES: AtomicU64 = AtomicU64::new(0); + +/// See [`BROKEN_PROMISES`]. +pub fn idle_compact_broken_promises() -> u64 { + BROKEN_PROMISES.load(Ordering::Relaxed) +} + /// `PERRY_GC_DIAG=1` exit line. pub(super) fn emit_diag() { eprintln!( - "[gc-idle-compact] enabled={} attempts={} productive={} released_bytes={} \ + "[gc-idle-compact] enabled={} attempts={} productive={} broken_promises={} released_bytes={} \ pause_us_total={} pause_us_max={} wake_declined={} backoff_shift={}", idle_compact_enabled(), idle_compact_attempts(), idle_compact_productive(), + idle_compact_broken_promises(), idle_compact_released_bytes(), idle_compact_pause_us_total(), idle_compact_pause_us_max(), diff --git a/crates/perry-runtime/src/gc/oldgen.rs b/crates/perry-runtime/src/gc/oldgen.rs index 3dea59757f..2a331eaa89 100644 --- a/crates/perry-runtime/src/gc/oldgen.rs +++ b/crates/perry-runtime/src/gc/oldgen.rs @@ -1852,17 +1852,6 @@ pub(super) fn evacuate_selected_old_pages_collecting( // source blocks and evacuate the block all-or-nothing. Dead old objects // remain indexed until a full trace proves them dead, so conservatively // copying them here preserves the same minor-GC retention contract. - // Every hole on a page this pass is evacuating is unusable for the rest - // of it, and the block is released at the end. Drop them once, so the - // per-allocation exclusion scan in `old_free_take_exact` has nothing to - // walk: it is a linear scan of the size bucket, and with the fragmented - // pages excluded it used to fail over the whole bucket for every moved - // object. See `old_free_filter_pages` for the measurement. - let dropped_holes = crate::gc::old_free_filter_pages(excluded_pages); - if crate::gc::gc_diag_enabled() && dropped_holes > 0 { - eprintln!("[gc-old-page-defrag] dropped_excluded_holes_bytes={dropped_holes}"); - } - let mut source_headers = Vec::new(); crate::arena::old_arena_walk_objects_on_pages(excluded_pages, |header_ptr| { source_headers.push(header_ptr as *mut GcHeader); @@ -1880,9 +1869,26 @@ pub(super) fn evacuate_selected_old_pages_collecting( && !is_conservatively_pinned(header) }); if source_headers.is_empty() || !source_block_is_movable { + // #9772: a declined pass must not also DESTROY the free list. Dropping + // the excluded pages' holes is only justified by "this pass is about to + // empty and release these blocks"; doing it before the all-or-nothing + // movability check meant one immovable occupant anywhere in the + // selection cost the whole old-gen residue and returned nothing. + // Measured on the compiled claude-code TUI: `reusable` 40.7 MB -> + // 0.87 MB, `released=0`, 189 ms of pause, and the bytes were neither + // returned to the OS nor available to the next allocation. return evacuated; } + // Every hole on a page this pass is evacuating is unusable for the rest of + // it, and the block is released at the end. Drop them once, so the + // per-allocation exclusion scan in `old_free_take_exact` has nothing to + // walk (see `old_free_filter_pages` for the #9644 measurement). + let dropped_holes = crate::gc::old_free_filter_pages(excluded_pages); + if crate::gc::gc_diag_enabled() && dropped_holes > 0 { + eprintln!("[gc-old-page-defrag] dropped_excluded_holes_bytes={dropped_holes}"); + } + for header in source_headers { unsafe { let user_ptr = (header as *mut u8).add(GC_HEADER_SIZE); diff --git a/crates/perry-runtime/src/gc/oldgen_defrag.rs b/crates/perry-runtime/src/gc/oldgen_defrag.rs index c56313a9b0..46b15ff198 100644 --- a/crates/perry-runtime/src/gc/oldgen_defrag.rs +++ b/crates/perry-runtime/src/gc/oldgen_defrag.rs @@ -31,21 +31,39 @@ pub(super) fn old_page_defrag_skipped_for_pin(meta: crate::arena::OldPageMeta) - meta.allocated_bytes > 0 && meta.live_bytes > 0 && meta.dead_bytes > 0 && meta.pinned_bytes > 0 } -/// Live bytes one idle compaction will move before it stops selecting pages. +/// Live bytes one idle compaction will move before it stops selecting. /// -/// The pass is linear in moved objects once the free-list pathology is gone -/// (`gc/old_free.rs::old_free_filter_pages`): the #9644 fixture moved 9.4 MB -/// in 235,241 objects in 132 ms, i.e. ~0.56 us per object. A budget keeps the -/// pause bounded on a heap far larger than that fixture's — the candidate -/// pages are sorted most-fragmented-first, so the bytes this leaves behind are -/// the least profitable ones, and the next idle compaction takes them. -pub(super) const IDLE_COMPACT_MOVE_BUDGET_BYTES: usize = 8 * 1024 * 1024; +/// This is a PAUSE budget, and it is calibrated against the measured rate of +/// the pass it bounds. The #9644 fixture moved 9.4 MB in 235,241 objects in +/// 132 ms once the free-list pathology was gone +/// (`gc/old_free.rs::old_free_filter_pages`), which is where 8 MiB came from. +/// On the compiled claude-code TUI the same 8 MiB budget moved 8.39 MB across +/// 50 blocks in **1.64 s** — ~195 ms per MiB, twelve times the fixture's rate, +/// because a real old generation's occupants are far smaller and far more +/// numerous than that corpus's. 1 MiB holds the pause near the 190-230 ms the +/// pass already cost before #9772 made it productive, so the change returns +/// tens of megabytes for the pause budget the old pass spent returning +/// nothing. Selection is cheapest-block-first, so what a pass leaves behind is +/// what the next one takes. +pub(super) const IDLE_COMPACT_MOVE_BUDGET_BYTES: usize = 1024 * 1024; pub(super) fn select_old_page_defrag_pages_from_snapshot( snapshot: &[crate::arena::OldPageMeta], force: bool, ) -> OldPageDefragSelection { let mut selection = OldPageDefragSelection::default(); + // #9772: the idle compaction's release unit is a BLOCK, so selecting the + // globally most-fragmented PAGES predicts bytes it cannot return — the + // emptied pages are scattered over blocks that keep other live occupants, + // and `old_arena_reclaim_selected_dead_blocks` frees none of them. It + // picked 10,740 pages promising 44 MB, ran 228 ms and released 0 on the + // compiled claude-code TUI. Selecting whole blocks, cheapest-to-empty + // first, makes the prediction achievable by construction: every selected + // block ends the pass with no live occupant, which is exactly what the + // reclaim tests. + if idle_compact_armed() && idle_compact_block_selection_enabled() { + return select_whole_blocks(snapshot, selection); + } let mut candidates = Vec::new(); for &meta in snapshot { if old_page_defrag_skipped_for_pin(meta) { @@ -70,15 +88,8 @@ pub(super) fn select_old_page_defrag_pages_from_snapshot( .then_with(|| a.page_base.cmp(&b.page_base)) }); - // The idle compaction pays for its pass with a mutator pause, so it takes - // the most profitable pages and stops. Every other caller selects the - // whole candidate set as before. - let move_budget = idle_compact_armed().then_some(IDLE_COMPACT_MOVE_BUDGET_BYTES); + // Every non-idle caller takes the whole candidate set, as before. for meta in candidates { - if move_budget.is_some_and(|budget| selection.selected_live_bytes >= budget) { - selection.budget_stopped = true; - break; - } let page = crate::arena::generation_page_for_addr(meta.page_base); if selection.pages.insert(page) { selection.page_order.push(page); @@ -103,6 +114,22 @@ pub(super) fn select_old_page_defrag_pages_from_snapshot( crate::perry_thread_local! { /// Set for the duration of one `gc/idle_compact.rs` collection. static IDLE_COMPACT_ARMED: std::cell::Cell = const { std::cell::Cell::new(false) }; + /// Releasable block bytes the last idle selection promised (#9772). + static LAST_IDLE_PREDICTED_RELEASE: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// `PERRY_GC_IDLE_COMPACT_BLOCKS` — ON by default. `=0`/`off`/`false` restores +/// the pre-#9772 page-granular selection, which predicts releasable bytes it +/// cannot return. Present so the two selections can be compared in one binary. +fn idle_compact_block_selection_enabled() -> bool { + static ENABLED: std::sync::OnceLock = std::sync::OnceLock::new(); + *ENABLED.get_or_init(|| crate::gc::env_default_on_enabled("PERRY_GC_IDLE_COMPACT_BLOCKS")) +} + +/// Block bytes the most recent idle-compaction selection predicted it could +/// hand back. `gc/idle_compact.rs` checks the pass against it. +pub(super) fn last_idle_predicted_release_bytes() -> usize { + LAST_IDLE_PREDICTED_RELEASE.with(|c| c.get()) } fn idle_compact_armed() -> bool { @@ -221,6 +248,12 @@ pub(super) fn select_old_page_defrag_pages(force: bool) -> OldPageDefragSelectio } let snapshot = crate::arena::old_page_meta_snapshot(); let selection = select_old_page_defrag_pages_from_snapshot(&snapshot, force); + if idle_compact_armed() { + // #9772: publish what this pass PROMISED, so the pass that consumes it + // can be judged against its own prediction instead of reporting a + // pause and no bytes. + LAST_IDLE_PREDICTED_RELEASE.with(|c| c.set(selection.selected_releasable_block_bytes)); + } if idle_compact_armed() && crate::gc::gc_diag_enabled() { let dead: usize = snapshot.iter().map(|m| m.dead_bytes).sum(); let live: usize = snapshot.iter().map(|m| m.live_bytes).sum(); @@ -337,3 +370,89 @@ mod tests { let _ = enabled; } } + +/// Block-granular selection for the idle compaction (#9772). +/// +/// Groups every old page with live bytes by its containing arena block, drops +/// blocks that hold pinned bytes (those can never be emptied), ranks the rest +/// by how much live data must move to empty them, and takes whole blocks until +/// [`IDLE_COMPACT_MOVE_BUDGET_BYTES`] of live bytes is committed. +/// `selected_releasable_block_bytes` is then the sum of the selected blocks' +/// sizes — memory the reclaim actually hands back — rather than a sum of page +/// granules nothing releases. +fn select_whole_blocks( + snapshot: &[crate::arena::OldPageMeta], + mut selection: OldPageDefragSelection, +) -> OldPageDefragSelection { + let ranges = crate::arena::old_arena_block_ranges(); + if ranges.is_empty() { + return selection; + } + #[derive(Default, Clone)] + struct BlockAcc { + live_bytes: usize, + dead_bytes: usize, + pinned: bool, + pages: Vec, + } + let mut blocks: Vec = vec![BlockAcc::default(); ranges.len()]; + for &meta in snapshot { + if meta.allocated_bytes == 0 { + continue; + } + let Some(bi) = crate::arena::old_arena_block_range_index(&ranges, meta.page_base) else { + continue; + }; + let acc = &mut blocks[bi]; + acc.live_bytes = acc.live_bytes.saturating_add(meta.live_bytes); + acc.dead_bytes = acc.dead_bytes.saturating_add(meta.dead_bytes); + acc.pinned |= meta.pinned_bytes > 0; + acc.pages + .push(crate::arena::generation_page_for_addr(meta.page_base)); + } + + let mut order: Vec = (0..blocks.len()) + .filter(|&i| { + let b = &blocks[i]; + // A block with no live occupant is already the ordinary sweep's + // job; a pinned one can never be emptied by moving. + !b.pinned && b.live_bytes > 0 && b.dead_bytes > 0 && !b.pages.is_empty() + }) + .collect(); + selection.candidate_pages = order.iter().map(|&i| blocks[i].pages.len()).sum(); + selection.skipped_pinned_pages = blocks.iter().filter(|b| b.pinned).map(|b| b.pages.len()).sum(); + // Cheapest to empty first; among equals prefer the one that gives back the + // most dead bytes. + order.sort_unstable_by(|&a, &b| { + blocks[a] + .live_bytes + .cmp(&blocks[b].live_bytes) + .then_with(|| blocks[b].dead_bytes.cmp(&blocks[a].dead_bytes)) + .then_with(|| ranges[a].0.cmp(&ranges[b].0)) + }); + + for bi in order { + if selection.selected_live_bytes >= IDLE_COMPACT_MOVE_BUDGET_BYTES { + selection.budget_stopped = true; + break; + } + let acc = &blocks[bi]; + for &page in &acc.pages { + if selection.pages.insert(page) { + selection.page_order.push(page); + selection.selected_pages = selection.selected_pages.saturating_add(1); + } + } + selection.selected_live_bytes = selection + .selected_live_bytes + .saturating_add(acc.live_bytes); + selection.selected_reclaimable_bytes = selection + .selected_reclaimable_bytes + .saturating_add(acc.dead_bytes); + // The whole block comes back once its live occupants are gone. + selection.selected_releasable_block_bytes = selection + .selected_releasable_block_bytes + .saturating_add(ranges[bi].3); + } + selection +} From 4d63254e4c797e2cb40689d915627738ca353307 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 07:26:00 +0200 Subject: [PATCH 2/2] fix(gc): correct the move-budget rationale against the confirming measurement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 8 MiB -> 1 MiB change was justified as buying back pause at a measured ~195 ms per MiB moved. Three interleaved pairs on the compiled claude-code TUI refute that: cutting the budget eightfold moved the selection from ~50 blocks to ~15 and left the pause unchanged (1,070 ms mean against the page-granular arm's 1,044 ms, 515-1,375 ms spread tracking machine load, not the arm). So the pass is dominated by fixed per-pass cost — the old-page meta snapshot, the walk over the selected blocks' pages, the sweep — and the budget bounds the moved volume rather than the pause. The claim the measurement supports is the one the table shows: ~15 MB of whole blocks returned per pass for the same pause the barren pass already spent. Lowering that fixed cost is separate work. Claude-Session: https://claude.ai/code/session_01YPfnmWZmSpSWpmnoXvH8z2 --- .../9772-idle-compaction-block-selection.md | 2 +- crates/perry-runtime/src/gc/oldgen_defrag.rs | 26 ++++++++++--------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/changelog.d/9772-idle-compaction-block-selection.md b/changelog.d/9772-idle-compaction-block-selection.md index 63912b6c59..a9b71ae94b 100644 --- a/changelog.d/9772-idle-compaction-block-selection.md +++ b/changelog.d/9772-idle-compaction-block-selection.md @@ -1 +1 @@ -fix(gc): the idle old-generation compaction now selects whole BLOCKS, so the bytes it predicts are the bytes it can return (#9772). Old-gen memory is released a block at a time (`old_arena_reclaim_selected_dead_blocks`), but selection ranked individual 4 KB pages by fragmentation, and the emptied pages were scattered across blocks that kept other live occupants. Measured on the compiled claude-code TUI: the pass chose 10,740 pages, predicted 44 MB of "releasable block bytes", ran for 228 ms and released **nothing**; in a controlled two-arm run the same selection predicted 44.4 MB, spent 516 ms and released 0, because a page-granular prediction is not achievable by a block-granular reclaim. Selection now groups pages by their containing block (`arena::old_arena_block_ranges`), skips blocks holding pinned bytes, ranks the rest cheapest-to-empty and takes whole blocks, so every selected block ends the pass with no live occupant. Same workload, same binary, one env var apart: **released 46.6 MB of 52.4 MB predicted (`kept_promise=true`, 50 of 50 targeted blocks, `has_live=0`), old-gen occupancy 120.4 MB -> 73.8 MB**, against **0 MB released of 44.4 MB predicted** for the old selection. Two counters make a barren pass visible instead of silent: `[gc-old-block-reclaim]` reports targeted/released/kept-by-reason, and `[gc-idle-compact] done` now carries `predicted=` and `kept_promise=`, summarised as `broken_promises=` in the exit line. Two defects that only became visible once the pass could be judged against its own prediction are fixed with it: a pass that declines to evacuate no longer drops the excluded pages' holes first (it was destroying 40.7 MB of reusable free list and returning nothing), and `IDLE_COMPACT_MOVE_BUDGET_BYTES` is recalibrated 8 MiB -> 1 MiB against the rate this pass actually runs at on a real old generation (195 ms per MiB moved, not the 14 ms/MiB of the #9644 fixture), keeping the pause at the ~190-230 ms the unproductive pass already cost. Kill switch `PERRY_GC_IDLE_COMPACT_BLOCKS=0`. +fix(gc): the idle old-generation compaction now selects whole BLOCKS, so the bytes it predicts are the bytes it can return (#9772). Old-gen memory is released a block at a time (`old_arena_reclaim_selected_dead_blocks`), but selection ranked individual 4 KB pages by fragmentation, and the emptied pages were scattered across blocks that kept other live occupants. Measured on the compiled claude-code TUI: the pass chose 10,740 pages, predicted 44 MB of "releasable block bytes", ran for 228 ms and released **nothing**; in a controlled two-arm run the same selection predicted 44.4 MB, spent 516 ms and released 0, because a page-granular prediction is not achievable by a block-granular reclaim. Selection now groups pages by their containing block (`arena::old_arena_block_ranges`), skips blocks holding pinned bytes, ranks the rest cheapest-to-empty and takes whole blocks, so every selected block ends the pass with no live occupant. Same workload, same binary, one env var apart: **released 46.6 MB of 52.4 MB predicted (`kept_promise=true`, 50 of 50 targeted blocks, `has_live=0`), old-gen occupancy 120.4 MB -> 73.8 MB**, against **0 MB released of 44.4 MB predicted** for the old selection. Two counters make a barren pass visible instead of silent: `[gc-old-block-reclaim]` reports targeted/released/kept-by-reason, and `[gc-idle-compact] done` now carries `predicted=` and `kept_promise=`, summarised as `broken_promises=` in the exit line. Two defects that only became visible once the pass could be judged against its own prediction are fixed with it: a pass that declines to evacuate no longer drops the excluded pages' holes first (it was destroying 40.7 MB of reusable free list and returning nothing), and `IDLE_COMPACT_MOVE_BUDGET_BYTES` is 8 MiB -> 1 MiB, which moves ~15 blocks per pass instead of ~50. Three interleaved pairs show the pause is unchanged by that (1,070 ms mean against the old selection's 1,044 ms, spread 515-1,375 ms tracking machine load), so the pass is dominated by fixed per-pass cost rather than by moving: the measured claim is that this returns ~15 MB per pass for the same pause the barren pass already spent, not that it made the pass cheaper. Kill switch `PERRY_GC_IDLE_COMPACT_BLOCKS=0`. diff --git a/crates/perry-runtime/src/gc/oldgen_defrag.rs b/crates/perry-runtime/src/gc/oldgen_defrag.rs index 46b15ff198..561a6b9189 100644 --- a/crates/perry-runtime/src/gc/oldgen_defrag.rs +++ b/crates/perry-runtime/src/gc/oldgen_defrag.rs @@ -33,18 +33,20 @@ pub(super) fn old_page_defrag_skipped_for_pin(meta: crate::arena::OldPageMeta) - /// Live bytes one idle compaction will move before it stops selecting. /// -/// This is a PAUSE budget, and it is calibrated against the measured rate of -/// the pass it bounds. The #9644 fixture moved 9.4 MB in 235,241 objects in -/// 132 ms once the free-list pathology was gone -/// (`gc/old_free.rs::old_free_filter_pages`), which is where 8 MiB came from. -/// On the compiled claude-code TUI the same 8 MiB budget moved 8.39 MB across -/// 50 blocks in **1.64 s** — ~195 ms per MiB, twelve times the fixture's rate, -/// because a real old generation's occupants are far smaller and far more -/// numerous than that corpus's. 1 MiB holds the pause near the 190-230 ms the -/// pass already cost before #9772 made it productive, so the change returns -/// tens of megabytes for the pause budget the old pass spent returning -/// nothing. Selection is cheapest-block-first, so what a pass leaves behind is -/// what the next one takes. +/// This bounds how much a single pass MOVES. 8 MiB came from the #9644 +/// fixture (9.4 MB in 235,241 objects in 132 ms once the free-list pathology +/// was gone, `gc/old_free.rs::old_free_filter_pages`). +/// +/// Measured on the compiled claude-code TUI, cutting it to 1 MiB moved the +/// selection from ~50 blocks to ~15 and left the pause UNCHANGED — three +/// interleaved pairs gave a 1,070 ms mean against the old selection's +/// 1,044 ms, with a 515-1,375 ms spread that tracks machine load rather than +/// the arm. So this pass is dominated by fixed per-pass cost (the old-page +/// meta snapshot, the walk over the selected blocks' pages, the sweep), not by +/// moving, and the budget's job is bounding the moved volume rather than +/// buying back pause. 1 MiB is enough to release ~15 MB of whole blocks per +/// pass; selection is cheapest-block-first, so what one pass leaves behind is +/// what the next one takes. Lowering the fixed cost is separate work. pub(super) const IDLE_COMPACT_MOVE_BUDGET_BYTES: usize = 1024 * 1024; pub(super) fn select_old_page_defrag_pages_from_snapshot(