fix(gc): idle compaction selects whole blocks, so its prediction is achievable (#9772)

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`.

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)]

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<usize> {
+    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<usize>,
 ) -> OldArenaSourceBlockSelection {
@@ -1896,3 +1935,31 @@ pub(crate) fn page_meta_census() -> Vec<crate::gc::census::SideTableRow> {
     });
     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);
+    }
+}

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<bool>,
     snapshots: Vec<ArenaBlockSnapshot>,
@@ -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<bool>,
     snapshots: Vec<ArenaBlockSnapshot>,
@@ -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,28 +1193,39 @@ 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()
                 || block.data as usize != snapshot.data
                 || 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);
         });
     }