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..a9b71ae94b --- /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 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/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..561a6b9189 100644 --- a/crates/perry-runtime/src/gc/oldgen_defrag.rs +++ b/crates/perry-runtime/src/gc/oldgen_defrag.rs @@ -31,21 +31,41 @@ 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 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( 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 +90,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 +116,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 +250,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 +372,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 +}