From a0f111c2f41f520e285285f134c476e777e03aa2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 3 Sep 2026 20:06:10 +0200 Subject: [PATCH] gc: price the idle reclaim by swept bytes, not old-gen occupancy (#9589) The reducer scored a full productive only when `old_gen_in_use_bytes` dropped. That number is the sum of the live old blocks' bump offsets, and a non-moving sweep cannot lower it: dead objects go back to the old-gen free list and the block keeps its offset, so only a whole-block release moves it. The unit tests passed because their litter fills blocks that end up entirely dead; every real workload does not. Measured on the compiled claude-code TUI, in a session with 279 collections behind it: two reducer fulls freed 2.37 MB and 0.51 MB while occupancy stood at 93.6 MB with 50 MB already on the old-gen free list, both scored unproductive, and the activity requirement had doubled twice inside a minute of idle. The reducer was switching itself off on exactly the workload it was built for. Price the full by the freed-bytes count the cycle already reports. The occupancy delta stays in the counters and on the diag line as the whole-block-release signal it actually is, next to a new `reusable=` field: the free-list residue that only compaction can return to the OS, which a budgeted (non-moving) cycle cannot do. Claude-Session: https://claude.ai/code/session_011qE5TRRJFzqxN44K34AnG2 --- changelog.d/9589-idle-reclaim-productivity.md | 1 + crates/perry-runtime/src/gc/idle_reclaim.rs | 45 ++++++++++++--- .../src/gc/tests/idle_reclaim.rs | 55 +++++++++++++++++++ 3 files changed, 93 insertions(+), 8 deletions(-) create mode 100644 changelog.d/9589-idle-reclaim-productivity.md diff --git a/changelog.d/9589-idle-reclaim-productivity.md b/changelog.d/9589-idle-reclaim-productivity.md new file mode 100644 index 0000000000..fd973578b7 --- /dev/null +++ b/changelog.d/9589-idle-reclaim-productivity.md @@ -0,0 +1 @@ +fix(gc): the idle-time reclaim now prices a full by what its sweep freed instead of by the old-gen occupancy delta (#9589). `arena::old_gen_in_use_bytes` sums the live old blocks' bump offsets, and a non-moving sweep cannot lower it — dead objects go back to the old-gen free list and the block keeps its offset — so the occupancy delta read ~0 for cycles that freed megabytes and the reducer backed off on every real workload. Measured on the compiled claude-code TUI: two reducer fulls freed 2.37 MB and 0.51 MB with occupancy flat at 93.6 MB and 50 MB already on the free list, and the activity requirement had doubled twice within a minute of idle. The `[gc-idle-reclaim] done` line now also reports the bar it was priced against and `reusable=`, the old-gen free-list residue that only compaction can return to the OS. diff --git a/crates/perry-runtime/src/gc/idle_reclaim.rs b/crates/perry-runtime/src/gc/idle_reclaim.rs index 273fa1e39d..eb7a32c729 100644 --- a/crates/perry-runtime/src/gc/idle_reclaim.rs +++ b/crates/perry-runtime/src/gc/idle_reclaim.rs @@ -58,9 +58,10 @@ //! 3. **Rate.** At least [`IDLE_RECLAIM_MIN_INTERVAL_MS`] since the reducer's //! own last full started. //! -//! **Productivity backoff.** A full that lowered old-gen occupancy by less than -//! [`IDLE_RECLAIM_PRODUCTIVE_PCT`] percent of what it started with, or by less -//! than [`IDLE_RECLAIM_PRODUCTIVE_MIN_BYTES`], doubles the activity requirement +//! **Productivity backoff.** A full whose SWEEP freed less than +//! [`IDLE_RECLAIM_PRODUCTIVE_PCT`] percent of the old-gen occupancy it started +//! with, or less than [`IDLE_RECLAIM_PRODUCTIVE_MIN_BYTES`], doubles the +//! activity requirement //! up to `2^`[`IDLE_RECLAIM_MAX_BACKOFF_SHIFT`] collections; a productive full //! resets it to one. This is what keeps a RETAINING idle heap (the TUI's //! six-second young-collection sawtooth) from paying a whole-heap mark per @@ -68,6 +69,19 @@ //! collections, and any burst — which produces many collections in a row — //! re-arms it promptly. //! +//! The price is the cycle's own freed-bytes count and NOT the change in +//! `arena::old_gen_in_use_bytes`, which is the sum of the live old blocks' +//! bump offsets: a non-moving sweep hands dead objects to the old-gen free +//! list and the block keeps its offset, so only a whole-block release moves +//! that number. Pricing on it scored every full on a real workload +//! unproductive — measured on the compiled claude-code TUI, two reducer fulls +//! freed 2.37 MB and 0.51 MB while occupancy stood still at 93.6 MB with +//! 50 MB already on the old-gen free list, and the reducer backed off to one +//! attempt per four collections inside a minute of idle. Returning that free +//! list to the OS needs compaction, which a budgeted (non-moving) cycle +//! cannot do; the `reusable=` field on the `[gc-idle-reclaim] done` line +//! reports how much is waiting for it. +//! //! **Work cap.** While a cycle is open the hook never spends more than //! [`IDLE_RECLAIM_MAX_WORK_MS_PER_SECOND`] of any wall-clock second stepping //! it; past that it lets the loop park for the rest of its budget. A healthy @@ -97,9 +111,9 @@ pub const IDLE_RECLAIM_QUIET_MS: u64 = 3_000; /// Minimum spacing between two reducer-started fulls. pub const IDLE_RECLAIM_MIN_INTERVAL_MS: u64 = 10_000; -/// A reducer full counts as productive when it lowered old-gen occupancy by at -/// least this many bytes AND by at least [`IDLE_RECLAIM_PRODUCTIVE_PCT`] percent -/// of the occupancy it started with. +/// A reducer full counts as productive when its sweep freed at least this many +/// bytes AND at least [`IDLE_RECLAIM_PRODUCTIVE_PCT`] percent of the old-gen +/// occupancy it started with. pub const IDLE_RECLAIM_PRODUCTIVE_MIN_BYTES: usize = 4 * 1024 * 1024; /// See [`IDLE_RECLAIM_PRODUCTIVE_MIN_BYTES`]. @@ -365,7 +379,14 @@ pub(super) fn note_cycle_completed(freed_bytes: u64) { let reclaimed = before.saturating_sub(after); OLD_RECLAIMED_BYTES.fetch_add(reclaimed as u64, Ordering::Relaxed); let bar = IDLE_RECLAIM_PRODUCTIVE_MIN_BYTES.max(before / 100 * IDLE_RECLAIM_PRODUCTIVE_PCT); - let productive = reclaimed >= bar; + // Price the full by what its sweep freed. `old_gen_occupancy` sums the + // live blocks' bump offsets, which a non-moving sweep cannot lower — + // it returns objects to the old-gen free list and the block keeps its + // offset — so the occupancy delta reads ~0 for a cycle that freed + // megabytes, and every real full scored unproductive (module docs). + // The delta stays in the counters and the trace as the whole-block + // release signal it actually is. + let productive = freed_bytes >= bar as u64; if productive { PRODUCTIVE.fetch_add(1, Ordering::Relaxed); st.backoff_shift = 0; @@ -379,7 +400,8 @@ pub(super) fn note_cycle_completed(freed_bytes: u64) { st.last_seen_external = external; if gc_diag_enabled() { eprintln!( - "[gc-idle-reclaim] done old_in_use={before}->{after} reclaimed_old={reclaimed} freed={freed_bytes} productive={productive} backoff_shift={}", + "[gc-idle-reclaim] done old_in_use={before}->{after} reclaimed_old={reclaimed} freed={freed_bytes} bar={bar} reusable={} productive={productive} backoff_shift={}", + old_free_bytes(), st.backoff_shift ); } @@ -526,6 +548,13 @@ pub(super) mod test_support { STATE.with(|s| *s.borrow_mut() = IdleReclaimState::default()); } + /// Pin the old-gen occupancy the in-flight full started with, so a + /// completion can be priced against a known `before` without reproducing + /// the allocation pattern that would produce one. + pub(crate) fn set_old_in_use_at_start(bytes: usize) { + STATE.with(|s| s.borrow_mut().old_in_use_at_start = bytes); + } + pub(crate) fn thread_attempts() -> u64 { STATE.with(|s| s.borrow().attempts) } diff --git a/crates/perry-runtime/src/gc/tests/idle_reclaim.rs b/crates/perry-runtime/src/gc/tests/idle_reclaim.rs index 8133dcfdbe..4b37368bac 100644 --- a/crates/perry-runtime/src/gc/tests/idle_reclaim.rs +++ b/crates/perry-runtime/src/gc/tests/idle_reclaim.rs @@ -419,3 +419,58 @@ fn idle_reclaim_full_reaches_the_allocator_purge() { ); } } + +/// A full is priced by what its SWEEP freed, not by the old-gen occupancy +/// delta. `arena::old_gen_in_use_bytes` is the sum of the live blocks' bump +/// offsets: a non-moving sweep returns dead objects to the old-gen free list +/// and the block keeps its offset, so a cycle that freed megabytes can leave +/// it unchanged. That is not a corner case — it is what the compiled TUI does +/// (2.37 MB freed, occupancy flat at 93.6 MB, scored unproductive, backoff to +/// one attempt per four collections inside a minute of idle). +#[test] +fn a_full_is_productive_on_swept_bytes_when_occupancy_cannot_move() { + let _reducer = IdleReclaimTestGuard::new(0); + // Price against the live occupancy, so the completion below sees no delta + // at all — the production shape, reproduced without the fragmentation. + let before = crate::arena::old_gen_in_use_bytes(); + set_old_in_use_at_start(before); + let bar = IDLE_RECLAIM_PRODUCTIVE_MIN_BYTES.max(before / 100 * IDLE_RECLAIM_PRODUCTIVE_PCT); + let productive_before = idle_reclaim_productive(); + + super::super::idle_reclaim::note_cycle_completed(bar as u64); + + assert_eq!( + idle_reclaim_productive(), + productive_before + 1, + "a sweep that freed the bar is productive however the block offsets read" + ); + assert_eq!( + idle_reclaim_backoff_shift(), + 0, + "a productive full resets the activity requirement" + ); +} + +/// The mirror: freeing less than the bar still backs off, so the pricing +/// change cannot be satisfied by calling every full productive. +#[test] +fn a_full_that_freed_less_than_the_bar_still_backs_off() { + let _reducer = IdleReclaimTestGuard::new(0); + let before = crate::arena::old_gen_in_use_bytes(); + set_old_in_use_at_start(before); + let bar = IDLE_RECLAIM_PRODUCTIVE_MIN_BYTES.max(before / 100 * IDLE_RECLAIM_PRODUCTIVE_PCT); + let productive_before = idle_reclaim_productive(); + + super::super::idle_reclaim::note_cycle_completed(bar as u64 - 1); + + assert_eq!( + idle_reclaim_productive(), + productive_before, + "one byte short of the bar is not productive" + ); + assert_eq!( + idle_reclaim_backoff_shift(), + 1, + "an unproductive full doubles the activity requirement" + ); +}