From 0d92005429ca0388ff6206eba88bfdf287eeac1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 04:41:02 +0000 Subject: [PATCH 1/3] fix(gc): price the tiny-parse pressure guard by the productivity backoff Issue #9831 measured the ArenaBytes arm firing 51 times in one 66-delta claude-code reply, each collection freeing a median 131 KB, while the adaptive step sat saturated at 1 GiB. The issue located the discarded backoff in the arm's own re-arm arithmetic; correcting that (the issue's refuted branch) bought -10.8 % CPU for +22 % settled footprint and was rightly rejected. The arm's re-arm is not what re-fires it. Between two consecutive firings the arena grows a few hundred KB, against a trigger armed 16 MB (and below the ceiling, up to 128 MB) above the post-collection total. What pulls the trigger back down is the tiny-parse pressure guard: after every `JSON.parse` that grew the arena by <= 1 MB, `gc_bump_malloc_trigger` (and `gc_schedule_parse_boundary_collection_ if_pressure`, and the boundary collector they arm) tests the absolute `arena_in_use_bytes() >= 48 MB` and, if so, sets the trigger to "now". That threshold is a quantity no collection can lower below the live set, so on a program whose live set never drops under it every small parse -- one per SSE delta -- forced a minor at the next safepoint. The step those minors doubled was consulted by nothing. The guard now also requires the arena to have grown, since the last collection of any kind ended, by a headroom priced from the step: the step rescaled so that its power-on value (128 MB, the ceiling) buys the 16 MB floor, and each doubling the arm's ceiling clamp discards buys the guard one more doubling, bounded by the same ceiling. A productive collection halves the step and the guard keeps the cadence it always had; an unproductive one earns it room. The boundary collector re-prices a pending request so a collection that already satisfied it is not followed by a second one. Measured on the compiled claude-code TUI (cli_2.1.112.js, Linux, same perry binary, runtime-only A/B, 7 interleaved rounds, 3300-char streamed reply, chunk 50): turn CPU base 30.2-41.5 s (mean 35.1) fix 27.8-29.2 s (mean 28.6) post-turn RSS base 754-1057 MB (mean 803) fix 733-855 MB (mean 786) post-idle RSS base 527-1073 MB (mean 736) fix 517-843 MB (mean 722) peak RSS 1964-2062 MB both arms The fix wins CPU in every pair (-8 % to -30 %); footprint is flat within the base's own spread. The base arm is bimodal in both, which is what an absolute in-use threshold does. PERRY_GC_DIAG on one reply: copying minors 104 -> 84 (ArenaBytes 41 -> 13), old-gen fulls 19 -> 7, and the guard forced exactly one collection, after a genuine 16 MB of growth (`[gc-tiny-parse]` is the new witness line). test_memory_json_churn -- the guard's motivating shape -- is byte-identical in output and RSS in all four GC modes; 48/48 test_gap_gc_* and 8/8 test_gap_json_* pass. The arm's own arithmetic is left as it was and now says why. Claude-Session: https://claude.ai/code/session_015kqVkH6rHzfvXskGAj3tRv --- crates/perry-runtime/src/gc/policy.rs | 136 ++++++++++- crates/perry-runtime/src/gc/tests/mod.rs | 1 + .../src/gc/tests/tiny_parse_pressure.rs | 219 ++++++++++++++++++ scripts/gc_runtime_root_holders.json | 10 +- 4 files changed, 357 insertions(+), 9 deletions(-) create mode 100644 crates/perry-runtime/src/gc/tests/tiny_parse_pressure.rs diff --git a/crates/perry-runtime/src/gc/policy.rs b/crates/perry-runtime/src/gc/policy.rs index 0977d5e70a..830c6cc70c 100644 --- a/crates/perry-runtime/src/gc/policy.rs +++ b/crates/perry-runtime/src/gc/policy.rs @@ -367,6 +367,92 @@ pub(super) fn gc_suppressed_parse_is_tiny(parse_growth: usize) -> bool { parse_growth <= GC_SUPPRESSED_TINY_PARSE_BYTES } +/// #9831: how much the arena must have grown since the last collection ended +/// before tiny-parse churn may force another one — priced by the part of the +/// productivity backoff the `ArenaBytes` arm computes and then discards. +/// +/// `GC_STEP_BYTES` powers on at `GC_THRESHOLD_INITIAL_BYTES`, which equals the +/// trigger ceiling, and doubles on every collection that frees almost nothing. +/// The arm's own re-arm (`gc_finish_arena_trigger_collection`) clamps +/// `new_total + step` at the ceiling, so every doubling past the initial step +/// is backoff the arm has computed, stored, and cannot express. This rescales +/// the step so that its power-on value buys exactly the headroom floor, and +/// each discarded doubling buys the guard one more doubling — up to the same +/// ceiling, so a run of unproductive collections can never let the guard wait +/// longer than the arm itself would. A productive collection halves the step +/// toward its 16 MB floor, which maps below the headroom floor and clamps to +/// it: a churn loop whose collections pay keeps today's cadence. +pub(super) fn tiny_parse_pressure_headroom_bytes(step: usize) -> usize { + let floor = gc_trigger_headroom_floor_bytes(); + let ceiling = gc_trigger_absolute_ceiling_bytes(); + // 64-bit intermediate on purpose: `step` reaches 1 GiB and `floor` 16 MiB, + // whose product does not fit a 32-bit `usize` (watchOS/visionOS are ILP32; + // see `influx_driven_nursery_cap_bytes` for the same trap). + let scaled = (step as u64).saturating_mul(floor as u64) / (GC_THRESHOLD_INITIAL_BYTES as u64); + let scaled = scaled.min(usize::MAX as u64) as usize; + scaled.max(floor).min(ceiling.max(floor)) +} + +/// #9831: whether tiny-parse churn has earned a forced collection. +/// +/// The guard used to be `in_use >= in_use_trigger` alone — an absolute +/// threshold on a quantity no collection can lower below the live set. On a +/// program whose live set sits above it permanently (the compiled claude-code +/// TUI holds 59–297 MB of arena through one streamed reply) that made every +/// tiny `JSON.parse` the SSE stream consists of force a collection at the next +/// safepoint: 51 minors in one 66-delta reply, each freeing a median 131 KB, +/// while the adaptive step sat at its 1 GiB maximum saying "back off" and +/// nothing consulted it. The growth clause is what that step is for. +/// +/// `base` is `arena_in_use_bytes()` as the last collection ended +/// (`GC_TINY_PARSE_PRESSURE_BASE_BYTES`); `in_use` is the same reading now. +pub(super) fn tiny_parse_pressure_due_with( + in_use: usize, + in_use_trigger: usize, + base: usize, + step: usize, +) -> bool { + in_use >= in_use_trigger + && in_use >= base.saturating_add(tiny_parse_pressure_headroom_bytes(step)) +} + +/// The live [`tiny_parse_pressure_due_with`]: current base and step. +pub(super) fn tiny_parse_pressure_due(in_use: usize, in_use_trigger: usize) -> bool { + let base = GC_TINY_PARSE_PRESSURE_BASE_BYTES.with(Cell::get); + let step = GC_STEP_BYTES.with(Cell::get); + tiny_parse_pressure_due_with(in_use, in_use_trigger, base, step) +} + +/// The in-use reading the tiny-parse guard compares against in this collector +/// mode: the generational collector's guard sits higher than the full +/// mark-sweep one because a minor is the cheaper collection to force. +fn tiny_parse_in_use_trigger_for_mode() -> usize { + if gen_gc_enabled() { + gc_tiny_parse_in_use_trigger_dyn_bytes() + } else { + gc_tiny_parse_full_gc_in_use_trigger_dyn_bytes() + } +} + +/// `PERRY_GC_DIAG=1` witness that the guard is the arm that forced a +/// collection (CLAUDE.md: a gate must assert its subject was live). One line +/// per forced collection, tagged with which parse boundary asked for it. +fn diag_tiny_parse_forced_collection(site: &str, in_use: usize) { + if !crate::gc::gc_diag_enabled() { + return; + } + let base = GC_TINY_PARSE_PRESSURE_BASE_BYTES.with(Cell::get); + let step = GC_STEP_BYTES.with(Cell::get); + eprintln!( + "[gc-tiny-parse] forced collection site={} in_use={} base={} headroom={} step={}", + site, + in_use, + base, + tiny_parse_pressure_headroom_bytes(step), + step + ); +} + pub(super) fn gc_bump_arena_trigger_target( bytes_now: usize, step: usize, @@ -980,6 +1066,12 @@ thread_local! { /// reading and still escalate. Nursery garbage is not. pub(super) static GC_LAST_COLLECTION_POST_IN_USE_BYTES: Cell = const { Cell::new(0) }; + /// #9831: `arena_in_use_bytes()` as the most recent collection of ANY kind + /// ended — the base the tiny-parse pressure guard measures growth from. + /// The bump-offset reading rather than the live census above, because the + /// guard compares against `arena_in_use_bytes()` at every parse boundary, + /// and mixing the two would count every swept hole as growth. + pub(super) static GC_TINY_PARSE_PRESSURE_BASE_BYTES: Cell = const { Cell::new(0) }; /// Yield-adaptive backoff for major-GC pacing (#7726). /// /// `arena_growth_full_escalation_due` escalates a minor to a full once the @@ -1347,12 +1439,12 @@ pub fn gc_bump_malloc_trigger() { let is_tiny_parse = gc_bump_malloc_trigger_with_snapshot(current, bytes_now); if is_tiny_parse { let use_gen_gc = gen_gc_enabled(); - let in_use_trigger = if use_gen_gc { - gc_tiny_parse_in_use_trigger_dyn_bytes() - } else { - gc_tiny_parse_full_gc_in_use_trigger_dyn_bytes() - }; - if crate::arena::arena_in_use_bytes() < in_use_trigger { + // #9831: an absolute in-use guard alone forced a collection after + // EVERY tiny parse on a program whose live set never drops below it. + // The guard now also requires the arena to have grown past the + // productivity-priced headroom since the last collection ended. + let in_use = crate::arena::arena_in_use_bytes(); + if !tiny_parse_pressure_due(in_use, tiny_parse_in_use_trigger_for_mode()) { return; } if use_gen_gc { @@ -1361,6 +1453,7 @@ pub fn gc_bump_malloc_trigger() { return; } GC_SUPPRESSED_TINY_PARSE_COLLECTION_PENDING.with(|pending| pending.set(true)); + diag_tiny_parse_forced_collection("parse_end", in_use); GC_NEXT_TRIGGER_BYTES.with(|trigger| { if trigger.get() > bytes_now { trigger.set(bytes_now); @@ -1396,6 +1489,15 @@ pub fn gc_collect_pending_suppressed_parse() { GC_SUPPRESSED_TINY_PARSE_COLLECTION_PENDING.with(|pending| pending.set(true)); return; } + // #9831: the request was priced when it was made; a collection of any kind + // since then has moved the base, and re-pricing here is what keeps the + // boundary collection from stacking a second minor on top of it. A request + // nothing has satisfied is still due and still collects. + let in_use = crate::arena::arena_in_use_bytes(); + if !tiny_parse_pressure_due(in_use, tiny_parse_in_use_trigger_for_mode()) { + return; + } + diag_tiny_parse_forced_collection("parse_boundary", in_use); let total = crate::arena::arena_total_bytes(); GC_NEXT_TRIGGER_BYTES.with(|trigger| { @@ -1419,7 +1521,12 @@ pub fn gc_schedule_parse_boundary_collection_if_pressure() { if !gen_gc_enabled() { return; } - if crate::arena::arena_in_use_bytes() < gc_tiny_parse_in_use_trigger_dyn_bytes() { + // #9831: priced the same way as the post-parse guard above — see + // `tiny_parse_pressure_due_with`. + if !tiny_parse_pressure_due( + crate::arena::arena_in_use_bytes(), + gc_tiny_parse_in_use_trigger_dyn_bytes(), + ) { return; } GC_SUPPRESSED_TINY_PARSE_COLLECTION_PENDING.with(|pending| pending.set(true)); @@ -1911,6 +2018,8 @@ pub(super) fn pacing_arena_in_use_bytes() -> usize { pub(super) fn note_collection_finished_arena_occupancy(full: bool) { let bytes = pacing_arena_in_use_bytes(); GC_LAST_COLLECTION_POST_IN_USE_BYTES.with(|cell| cell.set(bytes)); + // #9831: the same moment, in the units the tiny-parse guard reads. + GC_TINY_PARSE_PRESSURE_BASE_BYTES.with(|cell| cell.set(crate::arena::arena_in_use_bytes())); super::arena_right_size::note_collection_finished(bytes, full); } @@ -2199,6 +2308,19 @@ fn gc_finish_arena_trigger_collection(pre_in_use: usize, outcome: GcCollectOutco // `new_total` so a workload whose post-GC live set already // approaches the ceiling doesn't thrash on every fresh // allocation. + // + // #9831: above `ceiling - floor` this arithmetic re-arms at `new_total + + // floor` whatever `step` says — the productivity backoff the branch above + // just computed is not visible to it. That is deliberate, and measured: + // pricing the arm's own headroom by the step bought -10.8 % turn CPU on + // the compiled claude-code TUI and cost +22 % settled footprint — a + // CPU-for-footprint trade this project does not accept (the issue's own + // refuted branch). The step is instead consumed where it was + // actually being discarded — the tiny-parse pressure guard + // (`tiny_parse_pressure_due_with`), which pulled this trigger down to + // "now" after every small `JSON.parse` on a heap above its in-use + // threshold and so re-fired this arm 51 times in one reply regardless of + // what this function armed. let stepped = new_total.saturating_add(step); let capped = stepped.min(gc_trigger_absolute_ceiling_bytes()); let floor = new_total.saturating_add(gc_trigger_headroom_floor_bytes()); diff --git a/crates/perry-runtime/src/gc/tests/mod.rs b/crates/perry-runtime/src/gc/tests/mod.rs index ac6d08f01e..1dd69f6e99 100644 --- a/crates/perry-runtime/src/gc/tests/mod.rs +++ b/crates/perry-runtime/src/gc/tests/mod.rs @@ -60,6 +60,7 @@ pub(super) mod support; mod teardown; mod telemetry_verifier; mod temp_roots; +mod tiny_parse_pressure; mod triggers; mod typed_layout_intact_residual; mod u8_inline_cache; diff --git a/crates/perry-runtime/src/gc/tests/tiny_parse_pressure.rs b/crates/perry-runtime/src/gc/tests/tiny_parse_pressure.rs new file mode 100644 index 0000000000..fd4be0507d --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/tiny_parse_pressure.rs @@ -0,0 +1,219 @@ +//! #9831: the tiny-parse pressure guard prices the collections it forces. +//! +//! The guard used to be an absolute `arena_in_use_bytes() >= 48 MB` test, so +//! on a program whose live set never drops below that it forced a collection +//! after EVERY tiny `JSON.parse` — the adaptive step's backoff was computed +//! by each of those collections and consulted by none of them. These tests +//! pin the pricing: the headroom the guard demands between collections is the +//! part of the step the `ArenaBytes` arm's ceiling clamp discards, and the +//! guard is not due until the arena has grown that much since the last +//! collection ended. +//! +//! Sabotage-proved: restoring the absolute guard (dropping the growth clause +//! from `tiny_parse_pressure_due_with`) fails +//! `the_absolute_guard_alone_is_the_bug_the_growth_clause_exists_for` and +//! `growth_past_the_headroom_is_due`'s boundary half while the rest pass; +//! pricing the headroom at the raw step (`floor.max(step.min(ceiling))`) +//! fails `power_on_step_buys_exactly_the_headroom_floor`. + +use super::super::heap_budget::{ + gc_trigger_absolute_ceiling_bytes, gc_trigger_headroom_floor_bytes, +}; +use super::super::policy::{ + tiny_parse_pressure_due, tiny_parse_pressure_due_with, tiny_parse_pressure_headroom_bytes, + GC_STEP_BYTES, GC_THRESHOLD_INITIAL_BYTES, GC_THRESHOLD_MAX_BYTES, + GC_TINY_PARSE_PRESSURE_BASE_BYTES, +}; + +const MB: usize = 1024 * 1024; + +/// Restores the two live cells the guard reads, so a test that moves them +/// cannot leak its state into the next one (the suite is single-threaded, but +/// the cells outlive the test). +struct LiveCellsGuard { + step: usize, + base: usize, +} + +impl LiveCellsGuard { + fn set(step: usize, base: usize) -> Self { + Self { + step: GC_STEP_BYTES.with(|cell| cell.replace(step)), + base: GC_TINY_PARSE_PRESSURE_BASE_BYTES.with(|cell| cell.replace(base)), + } + } +} + +impl Drop for LiveCellsGuard { + fn drop(&mut self) { + GC_STEP_BYTES.with(|cell| cell.set(self.step)); + GC_TINY_PARSE_PRESSURE_BASE_BYTES.with(|cell| cell.set(self.base)); + } +} + +#[test] +fn power_on_step_buys_exactly_the_headroom_floor() { + // The step powers on at the trigger ceiling. That is not evidence of an + // unproductive collection — nothing has run yet — so the guard keeps the + // cadence it always had: the headroom floor, not the ceiling. + assert_eq!( + tiny_parse_pressure_headroom_bytes(GC_THRESHOLD_INITIAL_BYTES), + gc_trigger_headroom_floor_bytes(), + "a step that has never been priced must buy the floor, not the ceiling" + ); +} + +#[test] +fn a_productive_collection_keeps_the_headroom_floor() { + // Productive collections halve the step toward its own 16 MB floor. Every + // step at or below the power-on value maps to the headroom floor. + let floor = gc_trigger_headroom_floor_bytes(); + for step in [16 * MB, 32 * MB, 64 * MB, GC_THRESHOLD_INITIAL_BYTES / 2] { + assert_eq!( + tiny_parse_pressure_headroom_bytes(step), + floor, + "step {step} is a productive reading and must keep the floor" + ); + } +} + +#[test] +fn each_doubling_the_ceiling_discards_doubles_the_headroom() { + let floor = gc_trigger_headroom_floor_bytes(); + let ceiling = gc_trigger_absolute_ceiling_bytes(); + let mut previous = tiny_parse_pressure_headroom_bytes(GC_THRESHOLD_INITIAL_BYTES); + for doublings in 1..=3u32 { + let step = GC_THRESHOLD_INITIAL_BYTES << doublings; + let headroom = tiny_parse_pressure_headroom_bytes(step); + let expected = (floor << doublings).min(ceiling); + assert_eq!( + headroom, expected, + "{doublings} discarded doubling(s) must buy floor << {doublings}, bounded by the ceiling" + ); + assert!( + headroom >= previous, + "backing off further must never shrink the headroom" + ); + if ceiling >= (floor << doublings) { + assert!( + headroom > previous, + "an unproductive collection must earn more headroom than the reading before it" + ); + } + previous = headroom; + } +} + +#[test] +fn headroom_is_bounded_by_the_absolute_ceiling() { + let ceiling = gc_trigger_absolute_ceiling_bytes(); + let saturated = tiny_parse_pressure_headroom_bytes(GC_THRESHOLD_MAX_BYTES); + assert!( + saturated <= ceiling, + "a saturated step ({saturated}) must not let the guard wait longer than the arm's ceiling ({ceiling})" + ); + // On the unconstrained desktop budget the saturated step (1 GiB, three + // doublings past the 128 MB initial) reaches the 128 MB ceiling exactly; + // under a small `PERRY_GC_HEAP_LIMIT` the ceiling is lower and the clamp + // binds earlier. Either way the saturated reading IS the ceiling. + if ceiling <= gc_trigger_headroom_floor_bytes() << 3 { + assert_eq!(saturated, ceiling); + } +} + +#[test] +fn below_the_in_use_trigger_is_never_due() { + let trigger = 48 * MB; + // Even with zero base and the most productive step, the guard stays off + // below its in-use trigger: small heaps are the regular arms' business. + assert!(!tiny_parse_pressure_due_with( + trigger - 1, + trigger, + 0, + 16 * MB + )); + assert!(!tiny_parse_pressure_due_with(0, trigger, 0, 16 * MB)); +} + +#[test] +fn the_absolute_guard_alone_is_the_bug_the_growth_clause_exists_for() { + // The measured shape: a 60 MB live set above the 48 MB trigger, a tiny + // parse that grew the arena by a few KB since the collection that just + // ran, and a step saturated at its maximum because those collections free + // nothing. The old guard said "collect" here after every parse. + let trigger = 48 * MB; + let base = 60 * MB; + let in_use = base + 4096; + assert!( + !tiny_parse_pressure_due_with(in_use, trigger, base, GC_THRESHOLD_MAX_BYTES), + "a few KB of growth past a collection that freed nothing must not force another" + ); + // Nor with a productive step: 4 KB is below the headroom floor too. + assert!(!tiny_parse_pressure_due_with( + in_use, + trigger, + base, + 16 * MB + )); +} + +#[test] +fn growth_past_the_headroom_is_due() { + let trigger = 48 * MB; + let base = 60 * MB; + for step in [16 * MB, GC_THRESHOLD_INITIAL_BYTES, GC_THRESHOLD_MAX_BYTES] { + let headroom = tiny_parse_pressure_headroom_bytes(step); + let boundary = base + headroom; + assert!( + tiny_parse_pressure_due_with(boundary, trigger, base, step), + "growth of exactly the headroom ({headroom}) at step {step} is due" + ); + assert!( + !tiny_parse_pressure_due_with(boundary - 1, trigger, base, step), + "one byte short of the headroom ({headroom}) at step {step} is not" + ); + } +} + +#[test] +fn the_live_predicate_reads_the_step_and_the_base() { + let trigger = 48 * MB; + let base = 60 * MB; + let floor = gc_trigger_headroom_floor_bytes(); + let ceiling = gc_trigger_absolute_ceiling_bytes(); + + // Saturated step: the guard waits for the ceiling's worth of growth. + let _cells = LiveCellsGuard::set(GC_THRESHOLD_MAX_BYTES, base); + if ceiling > floor { + assert!(!tiny_parse_pressure_due(base + floor, trigger)); + } + assert!(tiny_parse_pressure_due(base + ceiling.max(floor), trigger)); + + // Productive step: the floor is enough again. + GC_STEP_BYTES.with(|cell| cell.set(16 * MB)); + assert!(tiny_parse_pressure_due(base + floor, trigger)); + assert!(!tiny_parse_pressure_due(base + floor - 1, trigger)); +} + +#[test] +fn a_finished_collection_moves_the_base_to_the_post_collection_reading() { + use super::super::js_gc_collect; + // Whatever the base was, a completed collection re-baselines it to the + // arena's post-collection in-use reading — the same reading the guard + // compares against at the next parse boundary. Assert the identity of the + // two readings, not merely that the cell moved: a base recorded in other + // units (the live census) would count every swept hole as growth. + let _cells = LiveCellsGuard::set(GC_STEP_BYTES.with(|cell| cell.get()), usize::MAX); + js_gc_collect(); + let base = GC_TINY_PARSE_PRESSURE_BASE_BYTES.with(|cell| cell.get()); + assert_ne!( + base, + usize::MAX, + "a finished collection must record the base" + ); + assert_eq!( + base, + crate::arena::arena_in_use_bytes(), + "the base must be the post-collection `arena_in_use_bytes()` reading" + ); +} diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index b52c8629f5..0b2afbf149 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -270,7 +270,7 @@ "file": "crates/perry-runtime/src/gc/census.rs", "name": "PASS1_MARKED", "verdict": "non_moving_snapshot", - "why": "Real GC header addresses, deliberately untraced so the diagnostic does not keep its observed objects alive. Populated only at the end of mark propagation of a synchronous full cycle; consumed at sweep entry in the same run_to_completion invocation. The intervening full-cycle phases do not relocate or run JS callbacks. The Vec is used for membership comparisons and dropped with the census before sweep. Budgeted and minor cycles skip both boundaries. Pin re-audited 2026-09-05 after #9760 touched `gc/mod.rs`: that change is `mod heap_stats;` plus a `pub(crate) use` re-export and alters no mark/sweep control flow. `heap_stats()` is reached only from `js_bun_jsc_heap_stats` (the JS-facing `bun:jsc.heapStats()`), i.e. from mutator code, never inside a cycle, and its own module contract forbids allocation or collection during its walk. The mark-complete \u2192 sweep-entry window is unchanged. Re-audited 2026-09-05 (train125) after #9769 and #9771 touched pinned files. #9769 adds one `reg_scanner!` registration to `gc/mod.rs`; #9771 adds a feature-gated `alloc_census_init()` there and a feature-gated Rust-heap dump inside `take_census`. `alloc-census` is not in the default feature set, and decisively: `census_take_if_armed_at_full_sweep_start` does `PASS1_MARKED.with(|p| p.borrow_mut().take())` BEFORE calling `take_census`, so the snapshot has already left the thread-local by the time #9771's code runs \u2014 it cannot affect the window. Neither change alters mark/sweep control flow. Re-audited 2026-09-05 (train126) after #9755 restructured `gc/cycle.rs`. Its hunks are all root-scan machinery (`RootScanSubphase`, `RootScanCycleState`, the mutable-scanner iteration state), which runs BEFORE mark propagation completes; `gc/mod.rs` gains only a `mod young_log;` declaration. The bracketing is unchanged \u2014 `census_pass1_if_armed` is still inside `step_mark_propagation` and `census_take_if_armed_at_full_sweep_start` inside `step_sweep` \u2014 and a synchronous full mark-sweep still moves nothing between them.", + "why": "Real GC header addresses, deliberately untraced so the diagnostic does not keep its observed objects alive. Populated only at the end of mark propagation of a synchronous full cycle; consumed at sweep entry in the same run_to_completion invocation. The intervening full-cycle phases do not relocate or run JS callbacks. The Vec is used for membership comparisons and dropped with the census before sweep. Budgeted and minor cycles skip both boundaries. Pin re-audited 2026-09-05 after #9760 touched `gc/mod.rs`: that change is `mod heap_stats;` plus a `pub(crate) use` re-export and alters no mark/sweep control flow. `heap_stats()` is reached only from `js_bun_jsc_heap_stats` (the JS-facing `bun:jsc.heapStats()`), i.e. from mutator code, never inside a cycle, and its own module contract forbids allocation or collection during its walk. The mark-complete \u2192 sweep-entry window is unchanged. Re-audited 2026-09-05 (train125) after #9769 and #9771 touched pinned files. #9769 adds one `reg_scanner!` registration to `gc/mod.rs`; #9771 adds a feature-gated `alloc_census_init()` there and a feature-gated Rust-heap dump inside `take_census`. `alloc-census` is not in the default feature set, and decisively: `census_take_if_armed_at_full_sweep_start` does `PASS1_MARKED.with(|p| p.borrow_mut().take())` BEFORE calling `take_census`, so the snapshot has already left the thread-local by the time #9771's code runs \u2014 it cannot affect the window. Neither change alters mark/sweep control flow. Re-audited 2026-09-05 (train126) after #9755 restructured `gc/cycle.rs`. Its hunks are all root-scan machinery (`RootScanSubphase`, `RootScanCycleState`, the mutable-scanner iteration state), which runs BEFORE mark propagation completes; `gc/mod.rs` gains only a `mod young_log;` declaration. The bracketing is unchanged \u2014 `census_pass1_if_armed` is still inside `step_mark_propagation` and `census_take_if_armed_at_full_sweep_start` inside `step_sweep` \u2014 and a synchronous full mark-sweep still moves nothing between them. Re-audited 2026-09-06 after #9831 touched `gc/policy.rs`. Its hunks are (a) the tiny-parse pressure guard's pricing (`tiny_parse_pressure_headroom_bytes`, `tiny_parse_pressure_due*`, a `Cell` byte-count base) consulted from JSON.parse's mutator-side boundaries (`gc_bump_malloc_trigger`, `gc_collect_pending_suppressed_parse`, `gc_schedule_parse_boundary_collection_if_pressure`), none of which is reachable from inside a cycle, and (b) one extra `Cell` store in `note_collection_finished_arena_occupancy`, which runs from `publish_reclaim_outcome` in the Publish subphase \u2014 after `step_sweep` has already consumed the snapshot. Mark/sweep control flow between `census_pass1_if_armed` and `census_take_if_armed_at_full_sweep_start` is untouched.", "window": { "start": { "file": "crates/perry-runtime/src/gc/census.rs", @@ -288,7 +288,7 @@ "crates/perry-runtime/src/gc/census.rs": "8050a9d1ca15f783195ccfa5963089b60bc4a6c31d9ced5755537623e70e7e3c", "crates/perry-runtime/src/gc/cycle.rs": "763d552271b8e983a796b4e9648cd8ee984a0602b2b56aeefdb8713c0049c31f", "crates/perry-runtime/src/gc/mod.rs": "085c3dcde34a172aa2b96ee4500658abae77cd34ee7f0e2dfeee06ae5774a414", - "crates/perry-runtime/src/gc/policy.rs": "319ed42f1a985c88f6362657a08518077283fe5216d6055fc82343b34dec50f9", + "crates/perry-runtime/src/gc/policy.rs": "5626929989c5093109cd9322ad4ff3415bd1585ac33bfb2070b06cc4fa2a0207", "crates/perry-runtime/src/gc/progress.rs": "a5ad3971bbe4047229ca57325234780daa85921dbc778e1c08dff4ad07ccfb96" } } @@ -311,6 +311,12 @@ "verdict": "not_a_gc_pointer", "why": "#9772: releasable block BYTES the last idle selection promised \u2014 a size, not an address. A `Cell` compared against what the collection actually released." }, + { + "file": "crates/perry-runtime/src/gc/policy.rs", + "name": "GC_TINY_PARSE_PRESSURE_BASE_BYTES", + "verdict": "not_a_gc_pointer", + "why": "#9831: `arena_in_use_bytes()` (a sum of block bump offsets) recorded as each collection ends, read back by the tiny-parse pressure guard to price growth since then. A byte COUNT, never an address or a NaN-boxed value: written only from `note_collection_finished_arena_occupancy` and the test seam, read only by `tiny_parse_pressure_due`/`diag_tiny_parse_forced_collection`. Same shape as its frontier siblings `GC_LAST_COLLECTION_POST_IN_USE_BYTES`/`GC_STEP_BYTES`, with the verdict those still owe." + }, { "file": "crates/perry-runtime/src/gc/trace.rs", "name": "FORWARDED_STUB_MEMBERSHIP_RECOVERIES", From 644b9d36247a53150edb2c3b301e2a964ad8b870 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 04:42:35 +0000 Subject: [PATCH 2/3] docs(changelog): add the #9838 fragment Claude-Session: https://claude.ai/code/session_015kqVkH6rHzfvXskGAj3tRv --- .../9838-tiny-parse-pressure-pricing.md | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 changelog.d/9838-tiny-parse-pressure-pricing.md diff --git a/changelog.d/9838-tiny-parse-pressure-pricing.md b/changelog.d/9838-tiny-parse-pressure-pricing.md new file mode 100644 index 0000000000..dcd96c9b53 --- /dev/null +++ b/changelog.d/9838-tiny-parse-pressure-pricing.md @@ -0,0 +1,37 @@ +**The tiny-parse pressure guard now prices the collections it forces by the +adaptive step's productivity backoff (#9831).** On the compiled claude-code +TUI a 3300-character streamed reply spent 30–41 s of CPU in the base arm and +27.8–29.2 s with the fix (mean −19 %, every interleaved pair a win), with +post-turn and post-idle RSS flat within the base's own spread and peak RSS +unchanged. + +#9831 measured the `ArenaBytes` arm firing 51 times in one 66-delta reply, +each collection freeing a median 131 KB, while the adaptive step sat +saturated at 1 GiB — and located the discarded backoff in the arm's own +ceiling clamp. That clamp was not what re-fired the arm: between two firings +the arena grew a few hundred KB against a trigger armed 16–128 MB above the +post-collection total. What pulled the trigger down was the tiny-parse +pressure guard, which after every `JSON.parse` growing the arena by ≤ 1 MB +tested the absolute `arena_in_use_bytes() >= 48 MB` and, if it held, set the +trigger to "now". That is a quantity no collection can lower below the live +set, so on a heap that sits above it permanently every small parse (one per +SSE delta) forced a minor whose backoff nothing read — #9589's shape one +trigger over. + +The guard now also requires the arena to have grown, since the last +collection of any kind ended, by a headroom priced from the step: the step +rescaled so its power-on value buys the 16 MB headroom floor and each +doubling the arm's clamp discards buys one more doubling, bounded by the +trigger ceiling. A productive collection keeps today's cadence; an +unproductive one earns room. The parse-boundary collector re-prices a +pending request so a collection that already satisfied it is not followed by +a second. `PERRY_GC_DIAG=1` gains a `[gc-tiny-parse] forced collection …` +witness line. The arm's own arithmetic is unchanged and now documents why +(pricing it directly was measured at −10.8 % CPU for +22 % footprint, the +issue's refuted branch). + +Validation: `test_memory_json_churn.ts` (the guard's motivating shape) is +byte-identical in output and RSS in all four GC modes; 48/48 `test_gap_gc_*` +and 8/8 `test_gap_json_*` pass; nine new `gc::tests::tiny_parse_pressure` +tests pin the pricing and the predicate, sabotage-proved against both the +old absolute guard and a raw-step pricing. From 0846d672ebce34b7a0eae2001008e34779317756 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 10:07:32 +0200 Subject: [PATCH 3/3] fix(gc): re-arm the idle reclaimer on elapsed idle, not only on collections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A declined idle compaction is currently a terminal state. The reducer's activity gate wants `2^backoff` collections it did not start, and `external_collections()` subtracts only its own — so a COMPACTION is what registers as external. When the compactor's residue gate declines, no compaction runs, nothing registers, `since_attempt` never reaches 1, and the reducer never runs again. The decision removes the only event that could revisit it. Measured on the claude-code TUI, 400-char turn then 120 s idle, quiet host, both rounds of each arm: A settles 757/759 -> 512/527 MB; R ends the turn 19 MB BETTER at 738/742 and finishes at 748/748 — 221 MB worse. R's residue ratio is 23.68/23.67 % against a 25 % gate and starts zero compactions; A is at 25.94/25.95 % and starts two. Within-arm spread is 0.01-0.02 points, so this is a stable operating point just under a threshold, not a coin-flip. The largest piece of the loss is downstream: A right-sizes arena capacity 182.45 -> 81.79 MB across three observations, R holds 168.82 MB on one. This adds `StartReason::IdleElapsed`, extending the exemption that already sits twelve lines above it for the identical deadlock — `ArenaRightSize` bypasses the same gate because arena blocks need a second full observation an idle mutator will never produce (#9709). A requirement denominated in mutator collections cannot be met by a heap whose mutator is idle, which is exactly when the reducer is wanted. The constant was not the fix, and that is measured rather than asserted: the same R binary in a 5 s window DID clear the residue gate at 25.81 %, compacted, and released 0 (`kept_promise=false`). A's own second compaction releases 0 at 54.6 % residue. Half of A's compactions in that capture released nothing, aborting ~4x earlier on what looks like a pause budget. Lowering 25 -> 23 % would have bought a compaction that releases nothing and a `backoff_shift` bump. Anti-spin needs no new rule: the wait is `IDLE_RECLAIM_REARM_MS << backoff_shift`, the SAME shift that prices the activity arm, so an unproductive full doubles it — 15 s, 30 s, 60 s, 120 s, 240 s — and the arm is DISARMED at `IDLE_RECLAIM_MAX_BACKOFF_SHIFT` rather than merely slowed, so a heap with nothing to give is asked five bounded times and then not again until real activity resets the shift. A productive full resets it, so a heap still giving memory back keeps being asked every 15 s. Tests, both sabotage-proved and each failing on its own named assertion: `a_parked_heap_is_re_armed_by_elapsed_idle_alone` (no external collection anywhere in the test; asserts the REASON via a counter, not the attempt count) and `an_unproductive_elapsed_streak_doubles_the_wait_and_then_disarms`. Removing the arm fails the first; removing the backoff scaling fails "must not re-arm before the doubled wait"; removing the disarm fails "at the maximum shift the elapsed arm is disarmed". `cargo test -p perry-runtime --lib -- gc:: arena::` is green at 1,143 passed / 0 failed. NOT addressed here, and measured rather than assumed: after R's single reclaim, `[gc-general-reclaim] examined=66 released=0 has_live=39 aging=22` — 39 of 66 arena blocks hold a live object, against 3 of 65 in A. Only an evacuation can consolidate those, and whether an idle young evacuation is also needed is a separate change. Refs #9831. --- .../9831-idle-reclaim-elapsed-rearm.md | 73 +++++++++ crates/perry-runtime/src/gc/idle_reclaim.rs | 74 ++++++++- crates/perry-runtime/src/gc/mod.rs | 8 +- .../src/gc/tests/idle_reclaim.rs | 141 ++++++++++++++++++ 4 files changed, 286 insertions(+), 10 deletions(-) create mode 100644 changelog.d/9831-idle-reclaim-elapsed-rearm.md diff --git a/changelog.d/9831-idle-reclaim-elapsed-rearm.md b/changelog.d/9831-idle-reclaim-elapsed-rearm.md new file mode 100644 index 0000000000..0d9806f173 --- /dev/null +++ b/changelog.d/9831-idle-reclaim-elapsed-rearm.md @@ -0,0 +1,73 @@ +**A declined idle compaction is no longer a terminal state: the memory reducer +re-arms on elapsed idle as well as on mutator collections, so a heap that parks +1.3 points under the compactor's residue gate gets revisited instead of holding +221 MB until the next turn.** + +Measured on the compiled claude-code TUI, one 400-char turn then a 120 s idle +window, quiet host (load < 0.1), both rounds of each arm: + +| arm | after turn | after 120 s idle | +|---|---|---| +| A | 757 / 759 MB | **512 / 527 MB** | +| R | 738 / 742 MB | **748 / 748 MB** | + +R *ends the turn 19 MB better than A* and finishes 221 MB worse. The reclaimer's +own diagnostic says why, and it is a closed loop: + +1. **The compactor's residue gate declines**, reproducibly and narrowly. + `compaction_owed` gate 1 wants residue ≥ 25 % of old-gen occupancy; A is at + **25.94 / 25.95 %** and starts two compactions, R is at **23.68 / 23.67 %** + and starts none. Within-arm spread across rounds is 0.01–0.02 points: a + stable operating point just under a threshold, not a coin-flip. +2. **The decline removes the only event that could revisit it.** The reducer's + activity gate needs `2^backoff` collections *it did not start*, and + `external_collections()` subtracts only the reducer's own — so a **compaction + is what registers as external**. A's trace shows each one contributing + exactly +1 (`external_collections` 13 → 14 → 15 across three attempts, one + compaction between each). R stays at 9, `since_attempt` never reaches 1, and + there is no second attempt in the whole window. +3. So the heap parks, and the largest piece of the loss is downstream of that: + A right-sizes the arena from **182.45 MB of capacity to 81.79 MB** across its + three observations, while R holds **168.82 MB** on one. Roughly 87 MB of + capacity + 57 MB of young blocks + 38 MB of old-gen ≈ 182 of the 221 MB. + +**The fix extends an exemption that already exists twelve lines above it**, for +the identical deadlock: `StartReason::ArenaRightSize` bypasses the same gate +because arena blocks need a second full observation that an idle mutator will +never produce (#9709). This adds `StartReason::IdleElapsed` on the same +reasoning — a requirement denominated in *mutator collections* cannot be met by +a heap whose mutator is idle, which is precisely when the reducer is wanted. + +**Why the gate constant was not the fix, on measurement rather than principle.** +Lowering `IDLE_COMPACT_MIN_RESIDUE_PCT` from 25 to 23 would have let R start a +compaction — and the same R binary in a 5 s window *did* clear the gate, at +25.81 %, ran the compaction, and **released 0** (`kept_promise=false`, +`backoff_shift 0→1`). Nor is that peculiar to R: A's own second compaction +releases 0 at **54.6 %** residue. Half of A's compactions in this capture +released nothing, aborting ~4x earlier (`pause_us` 107k/161k against 442k) on +what looks like a budget. The knob is not merely forbidden; it does not work. + +**Anti-spin needs no new rule.** The elapsed wait is +`IDLE_RECLAIM_REARM_MS << backoff_shift` — the *same* shift that prices the +activity arm — so an unproductive full doubles it: 15 s, 30 s, 60 s, 120 s, +240 s. And the arm is **disarmed entirely at `IDLE_RECLAIM_MAX_BACKOFF_SHIFT`** +rather than merely slowed, because five unproductive attempts establish there is +nothing to give and an idle process must not pay a whole-heap mark forever. +A productive full resets the shift, so a heap still returning memory keeps being +asked every 15 s — which is the case this exists for. `IDLE_RECLAIM_REARM_MS` is +deliberately larger than `IDLE_RECLAIM_MIN_INTERVAL_MS` so the rate floor is +never the binding constraint and the two gates cannot be confused in a diag. + +Two tests, each sabotage-proved: a parked heap with **no** external collection +anywhere gets a second attempt at the wait and not before, identified by reason +rather than by attempt count; and an unproductive streak doubles the wait each +time and then stops. Removing the arm fails the first, removing the backoff +scaling fails the second's "must not re-arm before the doubled wait", and +removing the disarm fails its "at the maximum shift the elapsed arm is +disarmed". + +The young half of the loss is **not** addressed here and is measured, not +assumed: after R's single reclaim, `[gc-general-reclaim] examined=66 released=0 +has_live=39 aging=22` — 39 of 66 arena blocks hold a live object, against 3 of +65 in A, and only an evacuation can consolidate those. Whether an idle young +evacuation is also needed is a separate question and a separate change. diff --git a/crates/perry-runtime/src/gc/idle_reclaim.rs b/crates/perry-runtime/src/gc/idle_reclaim.rs index 8fbf67a92a..cf51815c77 100644 --- a/crates/perry-runtime/src/gc/idle_reclaim.rs +++ b/crates/perry-runtime/src/gc/idle_reclaim.rs @@ -51,9 +51,20 @@ //! 1. **Activity or arena debt.** Normally at least `2^backoff` collections //! the reducer did not start itself have completed since its last full. A //! collection is the signal that the mutator allocated enough to matter. -//! The exception is a bounded [`super::arena_right_size`] episode: arena -//! blocks need two full observations before their mappings can be returned, -//! and an idle heap cannot create the second through mutator activity. +//! There are two exceptions, and they are the same argument twice: a +//! requirement denominated in *mutator collections* cannot be met by a heap +//! whose mutator is idle, which is exactly when the reducer is wanted. +//! First, a bounded [`super::arena_right_size`] episode: arena blocks need +//! two full observations before their mappings can be returned, and an idle +//! heap cannot create the second through mutator activity. Second, elapsed +//! idle — see [`IDLE_RECLAIM_REARM_MS`] — because a *declined* follow-up +//! would otherwise be terminal (#9831): measured on the claude-code TUI, the +//! compactor's residue gate declines at 23.7 %, so no compaction runs, so no +//! collection is registered, so `since_attempt` stays 0 and the reducer +//! never runs again. The compaction IS the event that re-arms the reducer, +//! so declining one removes the only thing that could revisit the decision, +//! and the heap parks 221 MB above where the same workload settles when the +//! first compaction happens to fire. //! 2. **Quiet.** At least [`IDLE_RECLAIM_QUIET_MS`] since the last such //! collection was observed — a burst still in progress collects every few //! hundred milliseconds and must not be interleaved with a whole-heap mark. @@ -125,6 +136,23 @@ pub const IDLE_RECLAIM_PRODUCTIVE_PCT: usize = 5; /// exceeds `2^this` collections. pub const IDLE_RECLAIM_MAX_BACKOFF_SHIFT: u32 = 5; +/// Elapsed idle that substitutes for the activity requirement, at +/// `backoff_shift == 0`; the wait is `IDLE_RECLAIM_REARM_MS << backoff_shift`, +/// so it is the SAME backoff that prices the activity arm. +/// +/// This is the whole of the anti-spin argument and it needs no new rule: an +/// unproductive full doubles the wait, so a heap with nothing to give is asked +/// at 15 s, 30 s, 60 s, 120 s, 240 s and then — because the arm is disarmed at +/// [`IDLE_RECLAIM_MAX_BACKOFF_SHIFT`] — **not again until real mutator activity +/// resets the shift**. Five bounded attempts over ~8 minutes, then silence. A +/// PRODUCTIVE full resets the shift to zero, so a heap that is still giving +/// memory back keeps being asked every 15 s, which is the case this exists for. +/// +/// Larger than [`IDLE_RECLAIM_MIN_INTERVAL_MS`] on purpose, so the rate floor +/// is never the binding constraint on this arm and the two gates cannot be +/// confused for one another when reading a diag. +pub const IDLE_RECLAIM_REARM_MS: u64 = 15_000; + /// Most collector work the park hook will do in any one wall-clock second /// while a budgeted cycle is open; past this the loop parks instead. pub const IDLE_RECLAIM_MAX_WORK_MS_PER_SECOND: u64 = 500; @@ -147,6 +175,10 @@ enum StartReason { /// Sustained arena slack still needs full observations before empty blocks /// can be returned, even though the mutator has done nothing new. ArenaRightSize, + /// The activity requirement has not been met, but enough idle time has + /// passed that waiting for a mutator collection is waiting for something + /// that is not coming. See [`IDLE_RECLAIM_REARM_MS`]. + IdleElapsed, } impl StartReason { @@ -154,6 +186,7 @@ impl StartReason { match self { StartReason::Activity => "activity", StartReason::ArenaRightSize => "arena_right_size", + StartReason::IdleElapsed => "idle_elapsed", } } } @@ -247,6 +280,10 @@ static YIELDS: AtomicU64 = AtomicU64::new(0); static START_BLOCKED: AtomicU64 = AtomicU64::new(0); static WORK_CAPPED: AtomicU64 = AtomicU64::new(0); static POST_PURGES: AtomicU64 = AtomicU64::new(0); +/// Fulls started because idle time elapsed rather than because the mutator +/// collected. Counted so a test can assert WHICH arm started a full — the +/// attempt count alone cannot tell the two apart. +static IDLE_ELAPSED_STARTS: AtomicU64 = AtomicU64::new(0); /// Reducer fulls started in this process. pub fn idle_reclaim_attempts() -> u64 { @@ -300,6 +337,10 @@ pub fn idle_reclaim_post_purges() -> u64 { } /// Current unproductive-streak backoff shift on this thread. +pub fn idle_reclaim_elapsed_starts() -> u64 { + IDLE_ELAPSED_STARTS.load(Ordering::Relaxed) +} + pub fn idle_reclaim_backoff_shift() -> u32 { STATE.with(|s| s.borrow().backoff_shift) } @@ -366,10 +407,28 @@ fn start_reason(now: u64) -> Option { return Some(StartReason::ArenaRightSize); } let since_attempt = external.saturating_sub(st.external_at_last_attempt); - if since_attempt < (1u64 << st.backoff_shift) { - return None; + if since_attempt >= (1u64 << st.backoff_shift) { + return Some(StartReason::Activity); } - Some(StartReason::Activity) + // The activity requirement is denominated in collections the reducer + // did not start, and on a quiet heap the only such collections are the + // compactor's — which run only once the reducer has already moved the + // residue ratio past the compactor's own gate. When that gate declines, + // nothing else can move it, and the decline is permanent. Elapsed idle + // is the same requirement in the one unit a quiet heap still produces. + // + // Disarmed at the maximum shift rather than merely slowed: five + // unproductive attempts are enough to establish there is nothing to + // give, and after them this arm must stop entirely or an idle process + // pays a whole-heap mark forever. Real activity resets the shift (via a + // productive full) and re-enables it. + if st.attempts > 0 + && st.backoff_shift < IDLE_RECLAIM_MAX_BACKOFF_SHIFT + && now.saturating_sub(st.last_attempt_ms) >= (IDLE_RECLAIM_REARM_MS << st.backoff_shift) + { + return Some(StartReason::IdleElapsed); + } + None }) } @@ -384,6 +443,9 @@ fn note_started(now: u64, reason: StartReason) { if reason == StartReason::ArenaRightSize { super::arena_right_size::note_started(); } + if reason == StartReason::IdleElapsed { + IDLE_ELAPSED_STARTS.fetch_add(1, Ordering::Relaxed); + } if gc_diag_enabled() { let (_, right_size_fulls_remaining, _, usage) = super::arena_right_size::snapshot(); eprintln!( diff --git a/crates/perry-runtime/src/gc/mod.rs b/crates/perry-runtime/src/gc/mod.rs index cb0acd9a6b..c06ca1c5f9 100644 --- a/crates/perry-runtime/src/gc/mod.rs +++ b/crates/perry-runtime/src/gc/mod.rs @@ -60,12 +60,12 @@ pub use idle_compact::{ }; pub use idle_reclaim::{ idle_reclaim_attempts, idle_reclaim_backoff_shift, idle_reclaim_completions, - idle_reclaim_enabled_from_value, idle_reclaim_freed_bytes, idle_reclaim_old_reclaimed_bytes, - idle_reclaim_post_purges, idle_reclaim_productive, idle_reclaim_slices, - idle_reclaim_start_blocked, idle_reclaim_work_capped, idle_reclaim_yields, + idle_reclaim_elapsed_starts, idle_reclaim_enabled_from_value, idle_reclaim_freed_bytes, + idle_reclaim_old_reclaimed_bytes, idle_reclaim_post_purges, idle_reclaim_productive, + idle_reclaim_slices, idle_reclaim_start_blocked, idle_reclaim_work_capped, idle_reclaim_yields, IDLE_RECLAIM_MAX_BACKOFF_SHIFT, IDLE_RECLAIM_MAX_WORK_MS_PER_SECOND, IDLE_RECLAIM_MIN_INTERVAL_MS, IDLE_RECLAIM_PRODUCTIVE_MIN_BYTES, IDLE_RECLAIM_PRODUCTIVE_PCT, - IDLE_RECLAIM_QUIET_MS, IDLE_RECLAIM_SLICE_US, + IDLE_RECLAIM_QUIET_MS, IDLE_RECLAIM_REARM_MS, IDLE_RECLAIM_SLICE_US, }; pub(crate) use idle_reclaim::{park_hook as idle_reclaim_park_hook, ParkVerdict}; mod telemetry; diff --git a/crates/perry-runtime/src/gc/tests/idle_reclaim.rs b/crates/perry-runtime/src/gc/tests/idle_reclaim.rs index eacee0ba1f..63f609223d 100644 --- a/crates/perry-runtime/src/gc/tests/idle_reclaim.rs +++ b/crates/perry-runtime/src/gc/tests/idle_reclaim.rs @@ -223,6 +223,147 @@ fn sustained_arena_slack_gets_one_bounded_followup_without_mutator_activity() { ); } +#[test] +fn a_parked_heap_is_re_armed_by_elapsed_idle_alone() { + // #9831. The activity requirement is denominated in collections the + // reducer did not start. On a quiet heap the only such collections are the + // compactor's, and the compactor runs only once the reducer has already + // moved the residue ratio past the compactor's gate — so when that gate + // declines, the decline is permanent and the heap parks. Measured on the + // claude-code TUI: 23.7 % against a 25 % gate, one reclaim attempt, and + // 221 MB never returned. Elapsed idle must be able to start attempt 2 with + // no new collection anywhere. + // + // Sabotage: delete the `StartReason::IdleElapsed` arm from `start_reason` + // and this test fails at "a second attempt must start" — attempts stay 1 + // forever, which is exactly the production symptom. + let _guard = CopyingNurseryTestGuard::new(1); + let _triggers = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let _reducer = IdleReclaimTestGuard::new(0); + // Dead old-gen litter so the full is PRODUCTIVE and the shift stays 0: + // this test is about the re-arm, and the backoff is the next test's + // subject. (The guard has already pinned arena usage to live == capacity, + // so the `arena_right_size` arm cannot be what starts anything here.) + litter_old_gen_with_dead_promises(); + + // The ordinary activity arm starts attempt 1. This is the ONLY external + // collection in the whole test. + external_collection_observed_at(0); + set_test_now_ms(Some(IDLE_RECLAIM_QUIET_MS)); + assert!(resumes(idle_reclaim_park_hook(1000))); + drive_until_idle(IDLE_RECLAIM_QUIET_MS, 1000); + assert_eq!(thread_attempts(), 1); + assert_eq!( + idle_reclaim_backoff_shift(), + 0, + "the litter must have made this full productive, or the wait below is \ + doubled and this test is measuring the backoff instead of the re-arm" + ); + let elapsed_before = idle_reclaim_elapsed_starts(); + + // Past the rate floor (10 s) but short of the re-arm (15 s), so the ONLY + // thing that can hold the reducer here is the new gate. + assert!(IDLE_RECLAIM_REARM_MS > IDLE_RECLAIM_MIN_INTERVAL_MS); + set_test_now_ms(Some(IDLE_RECLAIM_QUIET_MS + IDLE_RECLAIM_REARM_MS - 1)); + assert!(parks(idle_reclaim_park_hook(1000))); + assert_eq!( + thread_attempts(), + 1, + "elapsed idle must not re-arm before the wait" + ); + + // At the wait: a second attempt, with no collection having happened. + let at = IDLE_RECLAIM_QUIET_MS + IDLE_RECLAIM_REARM_MS; + set_test_now_ms(Some(at)); + assert!( + resumes(idle_reclaim_park_hook(1000)), + "a second attempt must start on elapsed idle alone" + ); + assert_eq!(thread_attempts(), 2); + assert_eq!( + idle_reclaim_elapsed_starts(), + elapsed_before + 1, + "LIVE SUBJECT: the follow-up must identify ELAPSED IDLE as its reason, \ + not activity and not arena debt" + ); +} + +#[test] +fn an_unproductive_elapsed_streak_doubles_the_wait_and_then_disarms() { + // The anti-spin argument, and it needs no new rule: the elapsed arm is + // priced by the SAME `backoff_shift` as the activity arm, so a heap with + // nothing to give is asked at 15 s, 30 s, 60 s, 120 s, 240 s — and then not + // again, because the arm is disarmed at the maximum shift. Without the + // disarm an idle process would pay a whole-heap mark every 8 minutes + // forever. + // + // Two sabotages, one per guard. Drop `<< st.backoff_shift` from the wait + // and the "must not re-arm before the doubled wait" assertions fail. Drop + // the `backoff_shift < IDLE_RECLAIM_MAX_BACKOFF_SHIFT` term and the final + // assertion fails: the reducer keeps waking a heap that has already proved + // five times over that it has nothing to give. + let _guard = CopyingNurseryTestGuard::new(1); + let _triggers = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let _reducer = IdleReclaimTestGuard::new(0); + // No litter: every full is unproductive, so every attempt doubles the wait. + + external_collection_observed_at(0); + set_test_now_ms(Some(IDLE_RECLAIM_QUIET_MS)); + assert!(resumes(idle_reclaim_park_hook(1000))); + drive_until_idle(IDLE_RECLAIM_QUIET_MS, 1000); + assert_eq!(thread_attempts(), 1); + assert_eq!(idle_reclaim_backoff_shift(), 1); + + let elapsed_before = idle_reclaim_elapsed_starts(); + let mut now = IDLE_RECLAIM_QUIET_MS; + let mut attempts = 1; + for shift in 1..IDLE_RECLAIM_MAX_BACKOFF_SHIFT { + let wait = IDLE_RECLAIM_REARM_MS << shift; + set_test_now_ms(Some(now + wait - 1)); + assert!( + parks(idle_reclaim_park_hook(1000)), + "shift {shift}: must not re-arm before the doubled wait" + ); + assert_eq!(thread_attempts(), attempts); + + now += wait; + set_test_now_ms(Some(now)); + assert!( + resumes(idle_reclaim_park_hook(1000)), + "shift {shift}: the doubled wait has elapsed" + ); + attempts += 1; + assert_eq!(thread_attempts(), attempts); + drive_until_idle(now, 1000); + assert_eq!( + idle_reclaim_backoff_shift(), + shift + 1, + "still unproductive: the shift must keep growing" + ); + } + assert_eq!(idle_reclaim_backoff_shift(), IDLE_RECLAIM_MAX_BACKOFF_SHIFT); + assert_eq!( + idle_reclaim_elapsed_starts(), + elapsed_before + u64::from(IDLE_RECLAIM_MAX_BACKOFF_SHIFT - 1), + "every follow-up in the streak must have come from the elapsed arm" + ); + + // Disarmed. However long the heap stays idle, it is not asked again. + let far = now + (IDLE_RECLAIM_REARM_MS << IDLE_RECLAIM_MAX_BACKOFF_SHIFT) * 16; + set_test_now_ms(Some(far)); + assert!( + parks(idle_reclaim_park_hook(1000)), + "at the maximum shift the elapsed arm is disarmed: the hook must park, \ + not open another whole-heap mark on a heap that has nothing to give" + ); + assert_eq!( + thread_attempts(), + attempts, + "at the maximum shift the elapsed arm must stop entirely: a heap with \ + nothing to give gets no new collection" + ); +} + #[test] fn idle_reclaim_rate_floor_holds_between_two_owed_fulls() { let _guard = CopyingNurseryTestGuard::new(1);