From 15f42159564902c5f4f81cb73418230c7d8e86b5 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Tue, 28 Jul 2026 00:19:53 +0900 Subject: [PATCH] gc: throttle the interpreter safepoint on allocated bytes, not allocation count The safepoint ran a whole-heap old-gen major every `COLLECT_THRESHOLD` (65536) interpreter-routed allocations. Nothing upstream counts allocations: the allocator accumulates a byte total (`nursery_free = result + totalsize`, minimark.py:556-557), and a major is scheduled against `get_total_memory_used()` (incminimark.py:1264-1268) reaching a threshold that `set_major_threshold_from` (incminimark.py:575-594) re-derives from what survived the last one. A count cannot bound a heap, because one allocation is not one size -- 65536 boxed characters and 65536 frames are three orders of magnitude apart. `note_alloc` now takes the payload size that was allocated, and the safepoint compares accumulated bytes against a threshold set to the surviving old-gen total times `major_collection_threshold - 1` (incminimark.py:198's 1.82 default), floored at a `min_heap_size` (incminimark.py:307) of 8 MiB. The comparison is kept in that delta form on purpose: having allocated `b` bytes since the last major, the total is `live + b`, so incminimark's `live + b >= live * threshold` is exactly `b >= live * (threshold - 1)`. The per-dispatch test therefore stays a single atomic compare, as the counter was, and the heap-stats read happens only after a collection. Adds `try_gc_heap_stats` alongside `try_gc_jitframe_empty`, routed to `majit_gc::active_heap_stats`, whose doc already names the interpreter safepoint as its consumer. Measurements, all min-of-three: no workload regressed. `s.lower()` in a loop is unchanged (0.93s -> 1.00s under `PYRE_GC_INTERP=1`, 0.92s -> 0.79s without it) and its RSS stays bounded and flat. With the JIT off, so the interpreter allocation path this module targets is actually taken, a string-boxing loop routed through the managed allocator ran 2.55s against 2.72s for the immortal allocator. No claim is made here that this unblocks routing more sites through the managed constructors: measuring that needs a managed-allocator build on this same base for the count policy, which was not run. Assisted-by: Claude --- pyre/pyre-interpreter/src/pyframe.rs | 2 +- pyre/pyre-interpreter/src/pytraceback.rs | 2 +- pyre/pyre-jit/src/eval.rs | 8 +++ pyre/pyre-object/src/floatobject.rs | 2 +- pyre/pyre-object/src/gc_hook.rs | 33 +++++++++++ pyre/pyre-object/src/gc_interp.rs | 70 ++++++++++++++++++----- pyre/pyre-object/src/generator.rs | 2 +- pyre/pyre-object/src/interp_exceptions.rs | 2 +- pyre/pyre-object/src/intobject.rs | 2 +- pyre/pyre-object/src/longobject.rs | 4 +- pyre/pyre-object/src/unicodeobject.rs | 4 +- 11 files changed, 107 insertions(+), 24 deletions(-) diff --git a/pyre/pyre-interpreter/src/pyframe.rs b/pyre/pyre-interpreter/src/pyframe.rs index 8eba0a0c88d..e1191966856 100644 --- a/pyre/pyre-interpreter/src/pyframe.rs +++ b/pyre/pyre-interpreter/src/pyframe.rs @@ -595,7 +595,7 @@ impl FrameBox { debug_assert!(pyre_object::gc_hook::try_gc_owns_object( frame.locals_cells_stack_w as *mut u8 )); - pyre_object::gc_interp::note_alloc(); + pyre_object::gc_interp::note_alloc(std::mem::size_of::()); let ptr = raw as *mut PyFrame; unsafe { std::ptr::write(ptr, frame); diff --git a/pyre/pyre-interpreter/src/pytraceback.rs b/pyre/pyre-interpreter/src/pytraceback.rs index 8a337aea54a..c34497f1169 100644 --- a/pyre/pyre-interpreter/src/pytraceback.rs +++ b/pyre/pyre-interpreter/src/pytraceback.rs @@ -136,7 +136,7 @@ pub fn w_pytraceback_new( PYTRACEBACK_OBJECT_SIZE, ); if !raw.is_null() { - pyre_object::gc_interp::note_alloc(); + pyre_object::gc_interp::note_alloc(PYTRACEBACK_OBJECT_SIZE); let ptr = raw as *mut PyTraceback; unsafe { std::ptr::write(ptr, value); diff --git a/pyre/pyre-jit/src/eval.rs b/pyre/pyre-jit/src/eval.rs index 2606c6a0b4d..19560562740 100644 --- a/pyre/pyre-jit/src/eval.rs +++ b/pyre/pyre-jit/src/eval.rs @@ -141,6 +141,13 @@ fn pyre_object_gc_collect_oldgen_trampoline() { majit_gc::collect_oldgen_nonmoving(); } +/// Trampoline for the interpreter safepoint's `(oldgen_total, nursery_used)` +/// read, used to re-derive its next-major threshold from what survived a +/// collection (`set_major_threshold_from`, incminimark.py:575-594). +fn pyre_object_gc_heap_stats_trampoline() -> (usize, usize) { + majit_gc::active_heap_stats() +} + fn pyre_object_gc_set_enabled_trampoline(enabled: bool) { majit_gc::gc_set_enabled(enabled); } @@ -3463,6 +3470,7 @@ fn install_pyre_object_hooks() { ); pyre_object::register_gc_collect_hook(pyre_object_gc_collect_trampoline); pyre_object::gc_hook::register_gc_collect_oldgen_hook(pyre_object_gc_collect_oldgen_trampoline); + pyre_object::gc_hook::register_gc_heap_stats_hook(pyre_object_gc_heap_stats_trampoline); pyre_object::gc_hook::register_gc_set_enabled_hook(pyre_object_gc_set_enabled_trampoline); pyre_object::gc_hook::register_gc_finalizer_hooks( pyre_object_gc_register_finalizer_trampoline, diff --git a/pyre/pyre-object/src/floatobject.rs b/pyre/pyre-object/src/floatobject.rs index 4a47766f4b7..2c43ef65f15 100644 --- a/pyre/pyre-object/src/floatobject.rs +++ b/pyre/pyre-object/src/floatobject.rs @@ -61,7 +61,7 @@ pub fn w_float_new(value: f64) -> PyObjectRef { if crate::gc_interp::enabled() { let raw = crate::gc_hook::try_gc_alloc_stable_raw(W_FLOAT_GC_TYPE_ID, W_FLOAT_OBJECT_SIZE); if !raw.is_null() { - crate::gc_interp::note_alloc(); + crate::gc_interp::note_alloc(W_FLOAT_OBJECT_SIZE); unsafe { std::ptr::write(raw as *mut W_FloatObject, obj); return raw as PyObjectRef; diff --git a/pyre/pyre-object/src/gc_hook.rs b/pyre/pyre-object/src/gc_hook.rs index ca375f4571e..338b0ea2702 100644 --- a/pyre/pyre-object/src/gc_hook.rs +++ b/pyre/pyre-object/src/gc_hook.rs @@ -420,6 +420,39 @@ pub fn try_gc_jitframe_empty() -> bool { } } +/// Signature of the host-side `(oldgen_total, nursery_used)` byte-stats +/// callback. `oldgen_total` is `get_total_memory_used` +/// (incminimark.py:1264-1268). +pub type GcHeapStatsHookFn = fn() -> (usize, usize); + +majit_gc::global_hook!(static GC_HEAP_STATS_HOOK: GcHeapStatsHookFn); + +/// Install the heap-byte-stats callback. +pub fn register_gc_heap_stats_hook(hook: GcHeapStatsHookFn) { + GC_HEAP_STATS_HOOK.set(Some(hook)); +} + +/// Remove the heap-byte-stats callback. +pub fn clear_gc_heap_stats_hook() { + GC_HEAP_STATS_HOOK.set(None); +} + +/// Report `(oldgen_total, nursery_used)` in bytes via the installed hook. +/// `(0, 0)` when none is installed, which leaves the interpreter safepoint on +/// its `min_heap_size` floor. +/// +/// Reads the runtime-mutable `GC_HEAP_STATS_HOOK` fn-pointer cell, not a +/// build-time constant, so the JIT residualizes the call instead of tracing +/// into it (`@dont_look_inside`, the [`try_gc_jitframe_empty`] twin). It +/// cannot raise. +#[majit_macros::dont_look_inside] +pub fn try_gc_heap_stats() -> (usize, usize) { + match GC_HEAP_STATS_HOOK.get() { + Some(f) => f(), + None => (0, 0), + } +} + /// Signature of the host-side root-register callbacks. /// `slot` is a pointer to a slot holding a `PyObjectRef` /// (equivalently `*mut u8`); the GC treats it as a live root until diff --git a/pyre/pyre-object/src/gc_interp.rs b/pyre/pyre-object/src/gc_interp.rs index 6221f3b3fbd..6a5cbfe44a0 100644 --- a/pyre/pyre-object/src/gc_interp.rs +++ b/pyre/pyre-object/src/gc_interp.rs @@ -20,8 +20,10 @@ //! path dict/set/list/instances already use), so they become GC-tracked without //! the move hazard, and trigger a full mark-sweep at a bytecode-dispatch //! safepoint (loop top, where the only live refs are in the frame and reachable -//! through the registered `pyframe` root walker). The collection is throttled by -//! an allocation counter so the old-gen high-water stays bounded. +//! through the registered `pyframe` root walker). The collection is throttled +//! the way `set_major_threshold_from` (incminimark.py:575-594) throttles a +//! major: on bytes allocated since the last one, against a threshold re-derived +//! from what survived it, so the cost is amortised against heap growth. //! //! Gated off by default on native; enabled with `PYRE_GC_INTERP=1`. On wasm it //! is on by default — the env read returns nothing there, and the interp-path @@ -101,12 +103,37 @@ pub fn at_outermost_activation() -> bool { /// routing+collection while diagnosing root-completeness. static COLLECT_STATE: AtomicU8 = AtomicU8::new(0); -/// Number of interpreter-routed object allocations since the last collection. -static ALLOC_SINCE_GC: AtomicUsize = AtomicUsize::new(0); +/// Bytes of interpreter-routed object allocation since the last collection. +/// +/// The allocator accumulating a byte total is `nursery_free = result + +/// totalsize` (minimark.py:556-557); a count of allocations has no upstream +/// counterpart, and cannot bound anything because one allocation is not one +/// size. +static ALLOC_BYTES_SINCE_GC: AtomicUsize = AtomicUsize::new(0); + +/// Bytes that must be allocated before the next safepoint collection. +/// +/// `set_major_threshold_from` (incminimark.py:575-594) schedules the next major +/// at `get_total_memory_used() * major_collection_threshold`, so the major cost +/// is amortised against how much the heap grew rather than paid on a fixed +/// cadence. `threshold_reached` (incminimark.py:1288-1290) is the comparison. +/// +/// Held here as the equivalent *delta*: having allocated `b` bytes since the +/// last major, the total is `live + b`, so `live + b >= live * threshold` is +/// just `b >= live * (threshold - 1)`. That keeps the safepoint's per-dispatch +/// test a plain atomic compare and confines the heap-stats read to the far +/// rarer post-collection update. +static NEXT_MAJOR_BYTES: AtomicUsize = AtomicUsize::new(MIN_HEAP_BYTES); -/// Allocations between safepoint collections. At ~24-40 B per int/float this -/// bounds the dead-object high-water to a couple of MB. -const COLLECT_THRESHOLD: usize = 1 << 16; +/// `major_collection_threshold - 1` in hundredths (incminimark.py:198's 1.82 +/// default, as the growth delta 0.82). Integer maths keeps the safepoint free +/// of floating point. +const MAJOR_GROWTH_DELTA_PCT: usize = 82; + +/// Floor for [`NEXT_MAJOR_BYTES`], mirroring `min_heap_size` +/// (incminimark.py:307). Without it a small live heap would schedule the next +/// major almost immediately and the collection cost would dominate again. +const MIN_HEAP_BYTES: usize = 8 << 20; /// Whether `PYRE_GC_INTERP` routes int/float allocations through the GC and /// arms the dispatch-loop safepoint. Reads the env once, then caches. @@ -129,16 +156,22 @@ pub fn enabled() -> bool { } } -/// Account for one interpreter-routed allocation. Called from `w_int_new` / -/// `w_float_new` after a successful `try_gc_alloc_stable`. +/// Account for `size` bytes of interpreter-routed allocation. Called after a +/// successful `try_gc_alloc_stable` with the payload size that succeeded. +/// +/// Takes the size because the safepoint's budget is a byte budget: the +/// allocator's own accumulation is `nursery_free = result + totalsize` +/// (minimark.py:556-557). Counting allocations instead lets a workload that +/// boxes many small objects trigger a whole-heap major far more often than the +/// bytes it reclaims justify. /// -/// Touches the runtime-mutable `ALLOC_SINCE_GC` atomic; the value is not a -/// build-time constant, so the JIT residualises the call instead of tracing +/// Touches the runtime-mutable `ALLOC_BYTES_SINCE_GC` atomic; the value is not +/// a build-time constant, so the JIT residualises the call instead of tracing /// into it (`@dont_look_inside`, the [`enabled`] sibling). A `()` return has no /// discriminant to erase. #[majit_macros::dont_look_inside] -pub fn note_alloc() { - ALLOC_SINCE_GC.fetch_add(1, Ordering::Relaxed); +pub fn note_alloc(size: usize) { + ALLOC_BYTES_SINCE_GC.fetch_add(size, Ordering::Relaxed); } /// Dispatch-loop safepoint: when enough interpreter objects have accumulated, @@ -175,11 +208,18 @@ pub fn safepoint() { } if collect_enabled() && at_outermost_activation() - && ALLOC_SINCE_GC.load(Ordering::Relaxed) >= COLLECT_THRESHOLD + && ALLOC_BYTES_SINCE_GC.load(Ordering::Relaxed) >= NEXT_MAJOR_BYTES.load(Ordering::Relaxed) && crate::gc_hook::try_gc_jitframe_empty() { crate::gc_hook::try_gc_collect_oldgen(); - ALLOC_SINCE_GC.store(0, Ordering::Relaxed); + // `set_major_threshold_from` (incminimark.py:575-594) re-derives the + // next threshold from what survived, so a heap that keeps growing pays + // proportionally more between majors and a steady-state one stops + // collecting altogether. + let (oldgen_live, _nursery_used) = crate::gc_hook::try_gc_heap_stats(); + let next = (oldgen_live / 100).saturating_mul(MAJOR_GROWTH_DELTA_PCT); + NEXT_MAJOR_BYTES.store(next.max(MIN_HEAP_BYTES), Ordering::Relaxed); + ALLOC_BYTES_SINCE_GC.store(0, Ordering::Relaxed); } } diff --git a/pyre/pyre-object/src/generator.rs b/pyre/pyre-object/src/generator.rs index cea31a952a7..d2b3da2fe7c 100644 --- a/pyre/pyre-object/src/generator.rs +++ b/pyre/pyre-object/src/generator.rs @@ -160,7 +160,7 @@ fn w_generator_or_coroutine_new( let raw = crate::gc_hook::try_gc_alloc_stable_raw(W_GENERATOR_GC_TYPE_ID, W_GENERATOR_OBJECT_SIZE); if !raw.is_null() { - crate::gc_interp::note_alloc(); + crate::gc_interp::note_alloc(W_GENERATOR_OBJECT_SIZE); unsafe { std::ptr::write(raw as *mut GeneratorIterator, value); } diff --git a/pyre/pyre-object/src/interp_exceptions.rs b/pyre/pyre-object/src/interp_exceptions.rs index 09ae8e24ad7..a2cc9cfce36 100644 --- a/pyre/pyre-object/src/interp_exceptions.rs +++ b/pyre/pyre-object/src/interp_exceptions.rs @@ -597,7 +597,7 @@ fn w_exception_new_empty_impl(kind: ExcKind, immortal: bool) -> PyObjectRef { unsafe { std::ptr::write(raw as *mut W_BaseException, value); } - crate::gc_interp::note_alloc(); + crate::gc_interp::note_alloc(W_BASE_EXCEPTION_SIZE); crate::gc_hook::try_gc_write_barrier(raw); return raw as PyObjectRef; } diff --git a/pyre/pyre-object/src/intobject.rs b/pyre/pyre-object/src/intobject.rs index 5afa3069216..00ebcf90819 100644 --- a/pyre/pyre-object/src/intobject.rs +++ b/pyre/pyre-object/src/intobject.rs @@ -111,7 +111,7 @@ pub fn w_int_new(value: i64) -> PyObjectRef { if crate::gc_interp::enabled() { let raw = crate::gc_hook::try_gc_alloc_stable_raw(W_INT_GC_TYPE_ID, W_INT_OBJECT_SIZE); if !raw.is_null() { - crate::gc_interp::note_alloc(); + crate::gc_interp::note_alloc(W_INT_OBJECT_SIZE); unsafe { std::ptr::write(raw as *mut W_IntObject, obj); return raw as PyObjectRef; diff --git a/pyre/pyre-object/src/longobject.rs b/pyre/pyre-object/src/longobject.rs index 04d8ebb3e12..e2931feaaaf 100644 --- a/pyre/pyre-object/src/longobject.rs +++ b/pyre/pyre-object/src/longobject.rs @@ -149,11 +149,11 @@ pub fn w_long_from_raw(value: *mut BigInt) -> PyObjectRef { // until the wrapper is initialized and remembered below. let raw = crate::gc_hook::try_gc_alloc_stable_raw(W_LONG_GC_TYPE_ID, W_LONG_OBJECT_SIZE); if !raw.is_null() { - // Advance the dispatch-loop safepoint counter, as w_int_new / + // Charge the dispatch-loop safepoint's byte budget, as w_int_new / // w_float_new do for their stable allocs — otherwise a long-dominated // interpreter workload never reaches the safepoint threshold and the // dead old-gen long wrappers + their bigint payloads accumulate. - crate::gc_interp::note_alloc(); + crate::gc_interp::note_alloc(W_LONG_OBJECT_SIZE); unsafe { std::ptr::write( raw as *mut W_LongObject, diff --git a/pyre/pyre-object/src/unicodeobject.rs b/pyre/pyre-object/src/unicodeobject.rs index b83ab4ad558..279991f77f1 100644 --- a/pyre/pyre-object/src/unicodeobject.rs +++ b/pyre/pyre-object/src/unicodeobject.rs @@ -234,7 +234,9 @@ pub fn w_str_from_wtf8_managed(value: Wtf8Buf) -> PyObjectRef { let recovered = unsafe { (*value).clone() }; return w_str_from_wtf8_immortal(recovered); } - crate::gc_interp::note_alloc(); + // Both the header and the value box holding the WTF-8 bytes came from the + // collector, so the safepoint's byte budget accounts for both. + crate::gc_interp::note_alloc(W_UNICODE_OBJECT_SIZE + byte_len); unsafe { std::ptr::write(raw as *mut W_UnicodeObject, unicode); raw as PyObjectRef