diff --git a/changelog.d/7449-recycled-block-pool.md b/changelog.d/7449-recycled-block-pool.md new file mode 100644 index 0000000000..a3bab2f8fc --- /dev/null +++ b/changelog.d/7449-recycled-block-pool.md @@ -0,0 +1,5 @@ +**Arena: recycled-block pool** (#7438 progress) — released 1 MB arena blocks were round-tripped through the process allocator, so each promoted-then-dropped cohort's replacement blocks landed in fresh mimalloc segments and the union of ever-dirtied pages grew with *cumulative promotion volume*: tree.ts peaked at 257.5 MiB committed scavenge-on vs 140.5 MiB scavenge-off for a ~35 MB live set, and a cap matrix showed the young-cap dial barely moves RSS (64/32/16 MB effective caps → 235/221/226 MB) while doubling wall time. + +Released blocks now enter a capped 64 MB thread-local pool and the single block-reservation funnel reuses them before minting fresh mappings. Pooled pages are `MADV_FREE`'d (the OS reclaims them under pressure; contents are undefined on reuse, which every consumer tolerates — blocks are bump-filled from offset 0 and re-registered by the adopting arena). The cap is measured, not guessed: no pool → 225 MB tree peak RSS, 64 MB → 190 MB, 128 MB → 210 MB (an oversized pool holds resident freed pages past the optimum). No collection decision changes — GC counters are byte-identical, so the pinned gc-ratchet baseline stands. Thread teardown still frees for real. + +Measured (dedicated M1 bench host, same-session A/B): tree.ts 225 → **190 MB** peak RSS at wall parity (9.85 s vs 9.96 s); deeplist 166 → 158 MB; churn/cycles/retain and the scavenge-off arm unchanged. Also relabels the sweep DIAG line that printed every non-general block as `longlived=` (it now says `non_general=` — the old label read as an 84 MB longlived leak on a workload whose longlived arena holds 1 MB). diff --git a/crates/perry-codegen/src/codegen/clone_suffix_tests.rs b/crates/perry-codegen/src/codegen/clone_suffix_tests.rs index 9af37e7eb9..4048953767 100644 --- a/crates/perry-codegen/src/codegen/clone_suffix_tests.rs +++ b/crates/perry-codegen/src/codegen/clone_suffix_tests.rs @@ -179,5 +179,8 @@ fn user_members_named_like_clone_suffixes_keep_their_own_symbols() { let add_clone = function_body(&ir, "perry_fn_clone_suffix_ts__add$typed_f64"); assert!(add_clone.contains("fadd"), "add's clone body is a + b"); let user_clone = function_body(&ir, "perry_fn_clone_suffix_ts__add__typed_f64$typed_f64"); - assert!(user_clone.contains("fmul"), "add__typed_f64's clone body is a * b"); + assert!( + user_clone.contains("fmul"), + "add__typed_f64's clone body is a * b" + ); } diff --git a/crates/perry-codegen/src/codegen/mod.rs b/crates/perry-codegen/src/codegen/mod.rs index afb6d40987..768e9c5c56 100644 --- a/crates/perry-codegen/src/codegen/mod.rs +++ b/crates/perry-codegen/src/codegen/mod.rs @@ -49,13 +49,13 @@ mod func_registry; mod function; // `pub(crate)` so `crate::linker` can read the inline-hot-small policy // (`inline_hot_small_enabled` / `inline_hot_small_hint_threshold`). +#[cfg(test)] +mod clone_suffix_tests; pub(crate) mod helpers; mod method; mod method_registry; mod module_globals_emit; #[cfg(test)] -mod clone_suffix_tests; -#[cfg(test)] mod number_exactness_tests; mod opts; mod spec_abi; diff --git a/crates/perry-runtime/src/arena/block.rs b/crates/perry-runtime/src/arena/block.rs index 0fe5ae523a..fcfc151cf1 100644 --- a/crates/perry-runtime/src/arena/block.rs +++ b/crates/perry-runtime/src/arena/block.rs @@ -59,6 +59,106 @@ fn block_size_for(min_size: usize) -> usize { /// that `reserve_arena_block` then runs allocates blocks of its own through the /// non-injectable path, so the injected refusal cannot be consumed by the wrong /// allocation. +// --------------------------------------------------------------------------- +// #7438: recycled-block pool. +// +// Block dealloc/realloc round-trips through the process allocator were the +// dominant term of tree.ts's scavenge-on peak RSS: every promoted-then-dropped +// cohort released its old-gen blocks and the next cohort's promotions landed +// in FRESH allocator segments, so the union of ever-dirtied pages grew with +// cumulative promotion volume (~230 MB resident for a ~35 MB live set) while +// a cap matrix showed the young-cap dial barely moves RSS at all (64/32/16 MB +// caps → 235/221/226 MB). Recycling released blocks bounds ever-dirtied pages +// at the CONCURRENT high-water instead. +// +// Pooled blocks are `MADV_FREE`d so the OS can take the pages under memory +// pressure; contents are undefined on reuse, which every consumer tolerates +// (blocks are bump-filled from offset 0 and re-registered by the arena that +// adopts them). The pool is capped; overflow falls through to real dealloc, +// and thread teardown (`Arena::drop`) never pools. +// --------------------------------------------------------------------------- + +/// Owns the pooled blocks, so that a thread exiting with a non-empty pool +/// releases them instead of leaking up to [`BLOCK_POOL_CAP_BYTES`]. +/// +/// The ownership has to live *here* rather than in a drain called from +/// `Arena::drop`: both are TLS destructors, their relative order is not +/// specified, and `LocalKey::with` panics once its own destructor has run — +/// so a drain could be skipped exactly when it is needed. A `Drop` on the +/// pool's own value is order-independent by construction. +/// +/// Matters for `perry/thread`: `spawn`/`parallelMap` give every agent its own +/// arena and GC, so each exiting agent thread would otherwise strand its +/// pooled blocks — unbounded growth across repeated spawns, in the one change +/// whose purpose is lowering RSS. +struct BlockPool(Vec<(*mut u8, usize)>); + +impl Drop for BlockPool { + fn drop(&mut self) { + for &(data, size) in &self.0 { + if data.is_null() || size == 0 { + continue; + } + let layout = Layout::from_size_align(size, 16).unwrap(); + unsafe { + // #4665, mirroring `Arena::drop`: test builds keep freed blocks + // mapped so unit tests holding raw GC pointers across a + // collection read stale bytes instead of faulting. + if !cfg!(test) { + std::alloc::dealloc(data, layout); + } + } + } + } +} + +thread_local! { + static BLOCK_POOL: RefCell = const { RefCell::new(BlockPool(Vec::new())) }; + static BLOCK_POOL_BYTES: Cell = const { Cell::new(0) }; +} + +/// Cap on pooled bytes: 64 MB, matching the young cap ceiling. Measured on +/// tree.ts (Mac mini M1, quiet): no pool -> 225 MB peak RSS; 64 MB pool -> +/// 190 MB; 128 MB pool -> 210 MB. Bigger is NOT better — pooled pages are +/// MADV_FREE'd but stay resident until the OS wants them, so an oversized +/// pool trades fresh-segment growth for held free pages past the optimum. +/// This is a cap, not a floor — the pool holds only blocks that were +/// actually released, and the OS can take every pooled page under pressure. +const BLOCK_POOL_CAP_BYTES: usize = 64 * 1024 * 1024; + +/// Offer a released block to the pool. Returns false (caller deallocs) when +/// the pool is full or the block is null. +pub(crate) fn block_pool_put(data: *mut u8, size: usize) -> bool { + if data.is_null() || size == 0 { + return false; + } + if BLOCK_POOL_BYTES.with(Cell::get).saturating_add(size) > BLOCK_POOL_CAP_BYTES { + return false; + } + #[cfg(unix)] + unsafe { + libc::madvise(data as *mut libc::c_void, size, libc::MADV_FREE); + } + BLOCK_POOL.with(|p| p.borrow_mut().0.push((data, size))); + BLOCK_POOL_BYTES.with(|c| c.set(c.get().saturating_add(size))); + true +} + +fn block_pool_take(size: usize) -> Option<*mut u8> { + let taken = BLOCK_POOL.with(|p| { + let mut pool = p.borrow_mut(); + let idx = pool.0.iter().rposition(|&(_, s)| s == size)?; + Some(pool.0.swap_remove(idx).0) + })?; + BLOCK_POOL_BYTES.with(|c| c.set(c.get().saturating_sub(size))); + Some(taken) +} + +#[cfg(test)] +pub(crate) fn block_pool_bytes_for_test() -> usize { + BLOCK_POOL_BYTES.with(Cell::get) +} + fn try_alloc_block(min_size: usize, injectable: bool) -> Option { let size = block_size_for(min_size); let layout = Layout::from_size_align(size, 16).unwrap(); @@ -68,6 +168,14 @@ fn try_alloc_block(min_size: usize, injectable: bool) -> Option { } #[cfg(not(test))] let _ = injectable; + if let Some(data) = block_pool_take(size) { + return Some(ArenaBlock { + data, + size, + offset: 0, + dead_cycles: 0, + }); + } let data = unsafe { alloc(layout) }; if data.is_null() { return None; diff --git a/crates/perry-runtime/src/arena/mod.rs b/crates/perry-runtime/src/arena/mod.rs index 4480e882ae..5dbaff7c99 100644 --- a/crates/perry-runtime/src/arena/mod.rs +++ b/crates/perry-runtime/src/arena/mod.rs @@ -29,14 +29,14 @@ pub(crate) use allocators::{ inactive_survivor_index, with_survivor_arena, with_survivor_arena_mut, }; pub(crate) use block::{ - arena_cell_alloc, old_gen_in_use_bytes_sub, Arena, ArenaBlock, ACTIVE_SURVIVOR, ARENA, - ARENA_TOTAL_BYTES, BLOCK_SIZE, FRESH_GENERAL_BLOCK_MIN_USED_BYTES, INLINE_STATE, + arena_cell_alloc, block_pool_put, old_gen_in_use_bytes_sub, Arena, ArenaBlock, ACTIVE_SURVIVOR, + ARENA, ARENA_TOTAL_BYTES, BLOCK_SIZE, FRESH_GENERAL_BLOCK_MIN_USED_BYTES, INLINE_STATE, LONGLIVED_ARENA, OLD_ARENA, OLD_GEN_IN_USE_BYTES, SURVIVOR_ARENA_0, SURVIVOR_ARENA_1, }; #[cfg(test)] pub(crate) use block::{ - force_next_block_alloc_failure, gc_trigger_arena_borrow_depth, gc_trigger_arena_calls, - reset_gc_trigger_arena_probe, + block_pool_bytes_for_test, force_next_block_alloc_failure, gc_trigger_arena_borrow_depth, + gc_trigger_arena_calls, reset_gc_trigger_arena_probe, }; pub(crate) use page_meta::{ address_span_overlaps_pages, register_block_space, register_old_object_pages, diff --git a/crates/perry-runtime/src/arena/reset.rs b/crates/perry-runtime/src/arena/reset.rs index dd11c5e153..4d4250d411 100644 --- a/crates/perry-runtime/src/arena/reset.rs +++ b/crates/perry-runtime/src/arena/reset.rs @@ -315,7 +315,7 @@ pub fn arena_reset_empty_blocks(block_has_live: &[bool]) -> ArenaResetStats { // #4665: in test builds keep freed blocks mapped (no munmap) so // unit tests holding raw GC pointers across a collection read stale // bytes instead of SIGSEGV-ing on an unmapped page. - if !cfg!(test) { + if !block_pool_put(block.data, block.size) && !cfg!(test) { std::alloc::dealloc(block.data, layout); } ARENA_TOTAL_BYTES.with(|t| t.set(t.get().saturating_sub(block.size))); @@ -591,7 +591,7 @@ impl ArenaResetEmptyBlocksState { // #4665: in test builds keep freed blocks mapped (no munmap) so // unit tests holding raw GC pointers across a collection read stale // bytes instead of SIGSEGV-ing on an unmapped page. - if !cfg!(test) { + if !block_pool_put(block.data, block.size) && !cfg!(test) { std::alloc::dealloc(block.data, layout); } ARENA_TOTAL_BYTES.with(|total| total.set(total.get().saturating_sub(size))); @@ -787,7 +787,7 @@ impl SurvivorArenaReclaimState { // #4665: in test builds keep freed blocks mapped (no munmap) so // unit tests holding raw GC pointers across a collection read stale // bytes instead of SIGSEGV-ing on an unmapped page. - if !cfg!(test) { + if !block_pool_put(block.data, block.size) && !cfg!(test) { std::alloc::dealloc(block.data, layout); } ARENA_TOTAL_BYTES.with(|total| total.set(total.get().saturating_sub(size))); @@ -1047,7 +1047,7 @@ impl OldArenaReclaimDeadBlocksState { // #4665: in test builds keep freed blocks mapped (no munmap) so // unit tests holding raw GC pointers across a collection read stale // bytes instead of SIGSEGV-ing on an unmapped page. - if !cfg!(test) { + if !block_pool_put(block.data, block.size) && !cfg!(test) { std::alloc::dealloc(block.data, layout); } ARENA_TOTAL_BYTES.with(|total| total.set(total.get().saturating_sub(size))); @@ -1142,7 +1142,7 @@ pub(crate) fn old_arena_reclaim_dead_blocks(block_has_live: &[bool]) -> ArenaRes // #4665: in test builds keep freed blocks mapped (no munmap) so // unit tests holding raw GC pointers across a collection read stale // bytes instead of SIGSEGV-ing on an unmapped page. - if !cfg!(test) { + if !block_pool_put(block.data, block.size) && !cfg!(test) { std::alloc::dealloc(block.data, layout); } ARENA_TOTAL_BYTES.with(|total| total.set(total.get().saturating_sub(size))); @@ -1245,7 +1245,7 @@ pub(crate) fn old_arena_reclaim_selected_dead_blocks( // #4665: in test builds keep freed blocks mapped (no munmap) so // unit tests holding raw GC pointers across a collection read stale // bytes instead of SIGSEGV-ing on an unmapped page. - if !cfg!(test) { + if !block_pool_put(block.data, block.size) && !cfg!(test) { std::alloc::dealloc(block.data, layout); } ARENA_TOTAL_BYTES.with(|total| total.set(total.get().saturating_sub(size))); @@ -1345,7 +1345,7 @@ fn reclaim_dead_survivor_arena_blocks( // #4665: in test builds keep freed blocks mapped (no munmap) so // unit tests holding raw GC pointers across a collection read stale // bytes instead of SIGSEGV-ing on an unmapped page. - if !cfg!(test) { + if !block_pool_put(block.data, block.size) && !cfg!(test) { std::alloc::dealloc(block.data, layout); } ARENA_TOTAL_BYTES.with(|total| total.set(total.get().saturating_sub(size))); diff --git a/crates/perry-runtime/src/arena/tests.rs b/crates/perry-runtime/src/arena/tests.rs index a404b6d688..d68af2088e 100644 --- a/crates/perry-runtime/src/arena/tests.rs +++ b/crates/perry-runtime/src/arena/tests.rs @@ -1103,3 +1103,68 @@ fn emergency_block_reclaim_runs_with_no_live_arena_borrow() { reallocate its `blocks` Vec underneath the borrow (#7022)" ); } + +/// #7438: a released block offered to the recycled-block pool is handed back +/// by the next same-size block reservation instead of a fresh allocator +/// mapping — the mechanism that bounds ever-dirtied pages at the concurrent +/// high-water instead of cumulative promotion volume. +#[test] +fn recycled_block_pool_reuses_released_blocks() { + let layout = std::alloc::Layout::from_size_align(BLOCK_SIZE, 16).unwrap(); + let raw = unsafe { std::alloc::alloc(layout) }; + assert!(!raw.is_null()); + let before = block_pool_bytes_for_test(); + assert!( + block_pool_put(raw, BLOCK_SIZE), + "pool must accept a block under its cap" + ); + assert_eq!(block_pool_bytes_for_test(), before + BLOCK_SIZE); + + // The reservation funnel must serve the pooled block back (LIFO) rather + // than minting a fresh mapping. + let block = crate::arena::block::reserve_arena_block(BLOCK_SIZE / 2); + assert_eq!( + block.data as usize, raw as usize, + "same-size reservation must reuse the pooled block" + ); + assert_eq!(block.size, BLOCK_SIZE); + assert_eq!(block.offset, 0); + assert_eq!(block_pool_bytes_for_test(), before); + // Hand it back to the allocator so the test doesn't leak the mapping. + unsafe { std::alloc::dealloc(block.data, layout) }; +} + +/// A thread exiting with a non-empty pool must run the pool's own `Drop` +/// rather than stranding its blocks. Before the `BlockPool` newtype the +/// thread-local held a bare `Vec<(*mut u8, usize)>`, so the TLS destructor +/// freed the Vec's buffer and leaked every block it pointed at — up to +/// `BLOCK_POOL_CAP_BYTES` per exiting thread, which `perry/thread`'s +/// `spawn`/`parallelMap` create routinely. +/// +/// The dealloc itself is `cfg!(test)`-skipped (#4665, exactly as in +/// `Arena::drop`), so this asserts the destructor RUNS and that pools are +/// per-thread; it cannot observe the free. Its value is that a regression to +/// a bare `Vec` — or a drain called from another TLS destructor, whose +/// ordering is unspecified — still has to keep this path alive. +#[test] +fn block_pool_is_per_thread_and_drops_with_its_thread() { + let before = block_pool_bytes_for_test(); + let handle = std::thread::spawn(|| { + // Fresh thread => fresh pool. + assert_eq!(block_pool_bytes_for_test(), 0); + let layout = std::alloc::Layout::from_size_align(BLOCK_SIZE, 16).unwrap(); + let raw = unsafe { std::alloc::alloc(layout) }; + assert!(!raw.is_null()); + assert!( + block_pool_put(raw, BLOCK_SIZE), + "pool should accept the block" + ); + assert_eq!(block_pool_bytes_for_test(), BLOCK_SIZE); + // Thread exits here with a non-empty pool: BlockPool::drop must run. + }); + handle + .join() + .expect("spawned thread must exit cleanly, not double-free"); + // The other thread's pool never touched ours. + assert_eq!(block_pool_bytes_for_test(), before); +} diff --git a/crates/perry-runtime/src/gc/oldgen.rs b/crates/perry-runtime/src/gc/oldgen.rs index 6aae75062f..327f837155 100644 --- a/crates/perry-runtime/src/gc/oldgen.rs +++ b/crates/perry-runtime/src/gc/oldgen.rs @@ -966,7 +966,7 @@ fn legacy_sweep_with_age_bump_and_old_reclaim_targets( .filter(|&i| block_has_live[i]) .count(); eprintln!( - "[gc] blocks: general={} ({} live), longlived={} ({} live), freed_bytes={} retained_forwarded_stub_bytes={} retained_forwarded_stub_objects={}", + "[gc] blocks: general={} ({} live), non_general={} ({} live, survivors+longlived+old), freed_bytes={} retained_forwarded_stub_bytes={} retained_forwarded_stub_objects={}", resettable_general_n, live_general, n_blocks - resettable_general_n, @@ -1324,7 +1324,7 @@ impl ArenaSweepObjectsState { .filter(|&i| self.block_has_live[i]) .count(); eprintln!( - "[gc] blocks: general={} ({} live), longlived={} ({} live), freed_bytes={} retained_forwarded_stub_bytes={} retained_forwarded_stub_objects={}", + "[gc] blocks: general={} ({} live), non_general={} ({} live, survivors+longlived+old), freed_bytes={} retained_forwarded_stub_bytes={} retained_forwarded_stub_objects={}", self.resettable_general_n, live_general, self.block_has_live.len() - self.resettable_general_n,