diff --git a/Cargo.lock b/Cargo.lock index 2b478f20d94..d6b1759bb5c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1992,6 +1992,7 @@ dependencies = [ "indexmap", "libc", "majit-ir", + "region", ] [[package]] diff --git a/majit/gate-triage.md b/majit/gate-triage.md index b0045c369b6..57f35f8e225 100644 --- a/majit/gate-triage.md +++ b/majit/gate-triage.md @@ -212,7 +212,7 @@ cover the condition they diagnose. - Read sites: 2 — `majit/majit-gc/src/nursery.rs`, `majit/majit-gc/src/oldgen.rs` - Accessor: `new()` -- What it does: **UNRECORDED** — no doc comment describes the gate. Read off the sites, not quoted: the two reads initialise `poison_on_reset` (`nursery.rs`) and `poison_on_alloc` (`oldgen.rs`). +- What it does: fill recycled nursery and old-gen memory with a poison word instead of zeroes, so an allocation path that relies on its memory arriving zeroed fails where it reads rather than later. The two reads initialise `poison_on_reset` (`nursery.rs`) and `poison_on_alloc` (`oldgen.rs`). The nursery half of this is upstream's `gc_nursery_debug` (`PYPY_GC_NURSERY_DEBUG`), which selects `arena_reset` mode 3; that name is read separately and additively, so either spelling turns the fill on and neither turns the other off. - Retirement condition: **UNRECORDED** — owed by this gate's owner. ### `MAJIT_GC_STRESS` diff --git a/majit/majit-backend-cranelift/src/compiler.rs b/majit/majit-backend-cranelift/src/compiler.rs index c468045f3bf..3df33c30350 100644 --- a/majit/majit-backend-cranelift/src/compiler.rs +++ b/majit/majit-backend-cranelift/src/compiler.rs @@ -482,7 +482,7 @@ fn register_active_hooks(supports_guard_gc_type: bool) { alloc_nursery_collecting_typed_rooted_via_active_runtime, )); majit_gc::set_active_alloc_oldgen_typed(Some(alloc_oldgen_typed_via_active_runtime)); - majit_gc::set_active_collect_full(Some(collect_full_via_active_runtime)); + majit_gc::set_active_collect_generation(Some(collect_generation_via_active_runtime)); majit_gc::set_active_collect_step(Some(collect_step_via_active_runtime)); majit_gc::set_active_get_objects(Some(get_objects_via_active_runtime)); majit_gc::set_active_get_referents(Some(get_referents_via_active_runtime)); @@ -501,6 +501,9 @@ fn register_active_hooks(supports_guard_gc_type: bool) { majit_gc::set_active_heap_stats(Some(heap_stats_via_active_runtime)); majit_gc::set_active_gc_memory_stats(Some(gc_memory_stats_via_active_runtime)); majit_gc::set_active_major_threshold_reached(Some(major_threshold_reached_via_active_runtime)); + majit_gc::set_active_minor_collections_since_major(Some( + minor_collections_since_major_via_active_runtime, + )); majit_gc::set_active_root_hooks( Some(gc_add_root_via_active_runtime), Some(gc_remove_root_via_active_runtime), @@ -1852,13 +1855,13 @@ fn alloc_oldgen_typed_via_active_runtime(type_id: u32, size: usize) -> GcRef { with_cranelift_gc(|gc| gc.alloc_oldgen_typed(type_id, size)).unwrap_or(GcRef(0)) } -/// User-level `gc.collect()` trampoline — drives `GcAllocator::collect_full` -/// on the active cranelift-owned GC. Mirrors the dynasm equivalent; +/// User-level `gc.collect(n)` trampoline — drives +/// `GcAllocator::collect_generation` on the active cranelift-owned GC. Mirrors the dynasm equivalent; /// safety constraints apply (caller must be at a safepoint where every /// live PyObjectRef is rooted — Rust-stack PyObjectRef in nursery would /// dangle after the embedded minor cycle). -fn collect_full_via_active_runtime() { - with_cranelift_gc(|gc| gc.collect_full()); +fn collect_generation_via_active_runtime(generation: i64) { + with_cranelift_gc(|gc| gc.collect_generation(generation)); } fn collect_step_via_active_runtime() -> majit_gc::GcStepTransition { @@ -1985,7 +1988,7 @@ fn total_memory_pressure_via_active_runtime() -> isize { /// Non-moving old-gen-only major trampoline — sweeps dead old-gen objects /// without moving the nursery, so the interpreter safepoint can drive it under -/// an active JIT (non-empty nursery). Unlike [`collect_full_via_active_runtime`] +/// an active JIT (non-empty nursery). Unlike [`collect_generation_via_active_runtime`] /// it runs no minor, so a Rust-stack nursery PyObjectRef cannot dangle. fn collect_oldgen_nonmoving_via_active_runtime() { with_cranelift_gc(|gc| gc.collect_oldgen_nonmoving()); @@ -2018,6 +2021,10 @@ fn major_threshold_reached_via_active_runtime() -> bool { with_cranelift_gc(|gc| gc.major_threshold_reached()).unwrap_or(false) } +fn minor_collections_since_major_via_active_runtime() -> usize { + with_cranelift_gc(|gc| gc.minor_collections_since_major()).unwrap_or(0) +} + /// Host-side root-register trampoline. Bridges /// `majit_gc::gc_add_root` to the active cranelift-owned GC's /// `RootSet`. diff --git a/majit/majit-backend-dynasm/src/runner.rs b/majit/majit-backend-dynasm/src/runner.rs index a7c6df1d2e7..dfc2a0b6d97 100644 --- a/majit/majit-backend-dynasm/src/runner.rs +++ b/majit/majit-backend-dynasm/src/runner.rs @@ -329,7 +329,7 @@ fn register_active_hooks(supports_guard_gc_type: bool) { dynasm_alloc_nursery_collecting_typed_rooted, )); majit_gc::set_active_alloc_oldgen_typed(Some(dynasm_alloc_oldgen_typed)); - majit_gc::set_active_collect_full(Some(dynasm_collect_full)); + majit_gc::set_active_collect_generation(Some(dynasm_collect_generation)); majit_gc::set_active_collect_step(Some(dynasm_collect_step)); majit_gc::set_active_get_objects(Some(dynasm_get_objects)); majit_gc::set_active_get_referents(Some(dynasm_get_referents)); @@ -348,6 +348,7 @@ fn register_active_hooks(supports_guard_gc_type: bool) { majit_gc::set_active_heap_stats(Some(dynasm_heap_stats)); majit_gc::set_active_gc_memory_stats(Some(dynasm_gc_memory_stats)); majit_gc::set_active_major_threshold_reached(Some(dynasm_major_threshold_reached)); + majit_gc::set_active_minor_collections_since_major(Some(dynasm_minor_collections_since_major)); majit_gc::set_active_root_hooks(Some(dynasm_gc_add_root), Some(dynasm_gc_remove_root)); majit_gc::set_active_gc_owns_object(Some(dynasm_gc_owns_object)); majit_gc::set_active_gc_is_nursery_object(Some(dynasm_gc_is_nursery_object)); @@ -659,18 +660,18 @@ fn bh_alloc_struct(sizedescr: &majit_translate::jitcode::BhDescr) -> *mut libc:: ptr } -/// User-level `gc.collect()` trampoline — drives `GcAllocator::collect_full` -/// on the active dynasm-owned GC. PyPy's `pypy/module/gc/interp_gc.py:7-26` -/// runs `rgc.collect()` from app-level `gc.collect`; this is the dynasm -/// backend's edge of that path. Safety: callers must be at a safepoint +/// User-level `gc.collect(n)` trampoline — drives +/// `GcAllocator::collect_generation` on the active dynasm-owned GC. +/// `interp_gc.py collect` runs `rgc.collect()` from app-level `gc.collect`; +/// this is the dynasm backend's edge of that path. Safety: callers must be at a safepoint /// where every live PyObjectRef is either in a registered root, on the /// Python value stack, or on the shadow stack — Rust-stack PyObjectRef /// in nursery would dangle after the embedded minor cycle. -fn dynasm_collect_full() { - if gc_box::with_mut(|g| g.collect_full()).is_some() { +fn dynasm_collect_generation(generation: i64) { + if gc_box::with_mut(|g| g.collect_generation(generation)).is_some() { return; } - majit_gc::gc_sync::gc_op(|g| g.collect_full()); + majit_gc::gc_sync::gc_op(|g| g.collect_generation(generation)); } fn dynasm_collect_step() -> majit_gc::GcStepTransition { @@ -807,7 +808,7 @@ fn dynasm_total_memory_pressure() -> isize { /// Non-moving old-gen-only major. Reclaims stable-allocated interp int/float /// without moving the nursery, so the interpreter safepoint can fire it under -/// an active JIT (nursery non-empty) — unlike [`dynasm_collect_full`], whose +/// an active JIT (nursery non-empty) — unlike [`dynasm_collect_generation`], whose /// embedded minor would relocate a Rust-stack nursery PyObjectRef. fn dynasm_collect_oldgen_nonmoving() { if gc_box::with_mut(|g| g.collect_oldgen_nonmoving()).is_some() { @@ -848,6 +849,13 @@ fn dynasm_major_threshold_reached() -> bool { majit_gc::gc_sync::gc_op(|g| g.major_threshold_reached()) } +fn dynasm_minor_collections_since_major() -> usize { + if let Some(r) = gc_box::with_mut(|g| g.minor_collections_since_major()) { + return r; + } + majit_gc::gc_sync::gc_op(|g| g.minor_collections_since_major()) +} + /// Host-side root-register trampoline. Bridges /// `majit_gc::gc_add_root` to the active backend's `RootSet`. /// diff --git a/majit/majit-backend-wasm/src/lib.rs b/majit/majit-backend-wasm/src/lib.rs index f7857e04fce..9383157949d 100644 --- a/majit/majit-backend-wasm/src/lib.rs +++ b/majit/majit-backend-wasm/src/lib.rs @@ -1011,12 +1011,15 @@ fn register_active_hooks(supports_guard_gc_type: bool) { majit_gc::set_active_get_typeids_list(Some(wasm_get_typeids_list)); majit_gc::set_active_add_memory_pressure(Some(wasm_add_memory_pressure)); majit_gc::set_active_total_memory_pressure(Some(wasm_total_memory_pressure)); - majit_gc::set_active_collect_full(Some(wasm_collect_full)); + majit_gc::set_active_collect_generation(Some(wasm_collect_generation)); majit_gc::set_active_collect_step(Some(wasm_collect_step)); majit_gc::set_active_collect_oldgen(Some(wasm_collect_oldgen_nonmoving)); majit_gc::set_active_heap_stats(Some(active_gc_heap_stats)); majit_gc::set_active_gc_memory_stats(Some(active_gc_memory_stats)); majit_gc::set_active_major_threshold_reached(Some(active_gc_major_threshold_reached)); + majit_gc::set_active_minor_collections_since_major(Some( + active_gc_minor_collections_since_major, + )); majit_gc::set_active_finalizer_hooks( Some(wasm_register_finalizer), Some(wasm_finalizer_next_dead), @@ -1069,6 +1072,12 @@ pub fn active_gc_major_threshold_reached() -> bool { with_wasm_active_gc(|gc| gc.major_threshold_reached()).unwrap_or(false) } +/// Minor collections the active GC has run since its last major, or `0` when +/// none is installed. +pub fn active_gc_minor_collections_since_major() -> usize { + with_wasm_active_gc(|gc| gc.minor_collections_since_major()).unwrap_or(0) +} + /// Diagnostic: `(minor_collections, major_collections)` of the active GC, or /// `(0, 0)` when none is installed. Companion to [`active_gc_heap_stats`]. pub fn active_gc_collection_counts() -> (usize, usize) { @@ -1169,16 +1178,17 @@ fn jf_top_addr() -> Option { .filter(|&addr| addr != 0) } -/// `majit_gc::CollectFullFn` installed by `register_active_hooks`. Drives -/// `gc.collect()` (`interp_gc.py`) through the active GC. Without it -/// `majit_gc::collect_full` has no hook to dispatch to and silently returns, +/// `majit_gc::CollectGenerationFn` installed by `register_active_hooks`. Drives +/// `gc.collect(n)` (`interp_gc.py`) through the active GC. Without it +/// `majit_gc::collect_generation` has no hook to dispatch to and silently +/// returns, /// so no major cycle ever runs on this backend and /// `deal_with_objects_with_finalizers` — which lives inside the major — never /// executes: no `__del__`, no generator `finally`, not even under an explicit -/// `gc.collect()`. Mirrors dynasm's `dynasm_collect_full` and cranelift's -/// `collect_full_via_active_runtime`. -fn wasm_collect_full() { - with_wasm_active_gc_mut(|gc| gc.collect_full()); +/// `gc.collect()`. Mirrors dynasm's `dynasm_collect_generation` and cranelift's +/// `collect_generation_via_active_runtime`. +fn wasm_collect_generation(generation: i64) { + with_wasm_active_gc_mut(|gc| gc.collect_generation(generation)); } fn wasm_collect_step() -> majit_gc::GcStepTransition { diff --git a/majit/majit-gc/Cargo.toml b/majit/majit-gc/Cargo.toml index 13a96c08a50..f267827a275 100644 --- a/majit/majit-gc/Cargo.toml +++ b/majit/majit-gc/Cargo.toml @@ -42,5 +42,11 @@ indexmap = { workspace = true } libc = { workspace = true } majit-ir = { workspace = true } +# `llarena.arena_protect` is posix `mprotect` and nt `VirtualProtect`; +# `llarena.has_protect` is false everywhere else, which is why this is not a +# dependency of the wasm32 build. +[target.'cfg(not(target_arch = "wasm32"))'.dependencies] +region = { workspace = true } + [dev-dependencies] majit-ir = { workspace = true, features = ["test-support"] } diff --git a/majit/majit-gc/src/collector.rs b/majit/majit-gc/src/collector.rs index fb2e09b1118..83c3d408752 100644 --- a/majit/majit-gc/src/collector.rs +++ b/majit/majit-gc/src/collector.rs @@ -1,11 +1,11 @@ -/// MiniMarkGC — the core collector implementing the GcAllocator trait. -/// -/// A generational copying collector with: -/// - Bump-pointer nursery for young objects -/// - ArenaCollection old gen plus rawmalloc fallback, with incremental major sweep -/// - Write barrier with remembered set for old-to-young pointers -/// -/// Modeled after incminimark's minor/major collection. +//! MiniMarkGC — the core collector implementing the GcAllocator trait. +//! +//! A generational copying collector with: +//! - Bump-pointer nursery for young objects +//! - ArenaCollection old gen plus rawmalloc fallback, with incremental major sweep +//! - Write barrier with remembered set for old-to-young pointers +//! +//! Modeled after incminimark's minor/major collection. use majit_ir::GcRef; use std::collections::VecDeque; use std::sync::RwLock; @@ -251,6 +251,15 @@ pub struct GcConfig { /// incminimark.py:275: card_page_indices (0 disables card marking). /// Must be a power of two. pub card_page_indices: u32, + /// gc/base.py `post_setup` — `PYPY_GC_DEBUG`, the level the collector's + /// own self-checks are gated on. 0 is off; 1 installs the rotating + /// nurseries; 2 additionally runs `debug_check_consistency` after every + /// minor collection. + pub debug: u8, + /// incminimark.py `gc_nursery_debug` — `PYPY_GC_NURSERY_DEBUG`. Fill the + /// recycled nursery with garbage rather than zeroes, and rotate the arena + /// when the debug ring is installed. + pub gc_nursery_debug: bool, /// translationoption.py `taggedpointers` (default off). When set, /// a small `int` may be stored as an unboxed immediate with an odd /// low bit; the collector must then skip such fields rather than read @@ -258,12 +267,16 @@ pub struct GcConfig { pub taggedpointers: bool, } +/// incminimark.py `post_setup` allocates this many spare nurseries, so a +/// retired arena is not handed back until the whole ring has turned. +pub const DEBUG_ROTATING_NURSERIES: usize = 6; + /// The variables [`GcConfig`] and [`MiniMarkGC::with_config`] resolve against, /// for an embedder that has to hand its environment over rather than share it. /// /// Published here so such a host does not keep its own copy of the list in step -/// with the collector; `PYPY_GC_DEBUG` and the tracing knobs are absent because -/// nothing in this file reads them. +/// with the collector; the tracing knobs are absent because nothing in this +/// file reads them. pub const GC_ENV_NAMES: &[&str] = &[ "PYPY_GC_NURSERY", "PYPY_GC_MAX_PINNED", @@ -273,6 +286,8 @@ pub const GC_ENV_NAMES: &[&str] = &[ "PYPY_GC_MIN", "PYPY_GC_MAX", "PYPY_GC_MAX_DELTA", + "PYPY_GC_NURSERY_DEBUG", + "PYPY_GC_DEBUG", ]; /// Environment an embedder supplies because the platform gives the process @@ -747,6 +762,10 @@ impl Default for GcConfig { debug_tiny_nursery, large_object_threshold: LARGE_OBJECT_THRESHOLD, card_page_indices: 128, + debug: read_uint_from_env("PYPY_GC_DEBUG") + .unwrap_or(0) + .min(u8::MAX as usize) as u8, + gc_nursery_debug: read_uint_from_env("PYPY_GC_NURSERY_DEBUG").is_some_and(|v| v != 0), taggedpointers: false, } } @@ -996,6 +1015,22 @@ pub struct MiniMarkGC { pub minor_collections: usize, /// Count of major collections performed. pub major_collections: usize, + /// [`minor_collections`](Self::minor_collections) as it stood when the last + /// major *cycle* ended, so the difference is the minors run since. + /// + /// No incminimark counterpart: it exists for `gc.get_count`, whose second + /// element 3.14 defines as the generation-0 collections run since + /// generation 1 was last collected. A major here collects both older + /// generations, so it is what resets that count. + /// + /// Sampled at the FINALIZING -> SCANNING transition and not in + /// `finish_incremental_cycle`, which is a sweep-to-finalize seam rather + /// than the end: `do_collect_full` runs a minor before every remaining + /// step, so one more still runs after that seam, and sampling there leaves + /// `gc.collect()` reporting a minor the collection had not yet finished + /// running. Measured, not assumed — see + /// `minors_accumulate_until_a_major_finishes`. + minor_collections_at_major_end: usize, /// `incminimark.py:self.hooks`, supplied by the translated standalone /// target and restricted to its allocation-free low-level surface. hooks: GcHooks, @@ -1128,8 +1163,6 @@ pub struct MiniMarkGC { /// shadow instead of a fresh allocation. Cleared after each /// minor collection. nursery_objects_shadows: AddressMap, - /// Registry of compiled code regions for GC root scanning. - pub compiled_code_registry: CompiledCodeRegistry, /// llsupport/gc.py:563 vtable→typeid mapping. RPython derives this /// arithmetically from the GC `type_info_group` base; pyre's GC /// keeps an explicit table because frontends register vtables @@ -1199,7 +1232,15 @@ impl MiniMarkGC { // nursery_size * major_collection_threshold. min_heap_size = min_heap_size.max(nursery_size as f64 * major_collection_threshold); - let nursery = Nursery::new(config.nursery_size); + let mut nursery = Nursery::new(config.nursery_size); + // incminimark.py `post_setup`: under `PYPY_GC_DEBUG` a retired nursery + // is protected rather than reused, so a pointer left behind in one + // faults on the next read instead of being answered by whatever was + // allocated over it. + if config.debug != 0 { + nursery.install_debug_rotating_nurseries(DEBUG_ROTATING_NURSERIES); + } + nursery.set_nursery_debug(config.gc_nursery_debug); // incminimark.py:516-528. `nonlarge_max + 1` is the large-object // cutoff; pyre stores that cutoff directly in the configuration. let max_number_of_pinned_objects = @@ -1234,6 +1275,7 @@ impl MiniMarkGC { config, minor_collections: 0, major_collections: 0, + minor_collections_at_major_end: 0, hooks: GcHooks, stat_ac_arenas_count: 0, stat_rawmalloced_total_size: 0, @@ -1265,7 +1307,6 @@ impl MiniMarkGC { max_number_of_pinned_objects, pinned_objects_in_nursery: 0, nursery_objects_shadows: AddressMap::default(), - compiled_code_registry: CompiledCodeRegistry::new(), vtable_to_type_id: AddressMap::default(), _infobits_offset: 0, _infobits_offset_plus: 0, @@ -2483,12 +2524,19 @@ impl MiniMarkGC { /// The clear an old-gen block owes when it stands in for a nursery bump. /// - /// WASM-ONLY ADAPTATION, paired with `Nursery::reset`: wasm skips the GC - /// rewrite, so its JIT code emits none of the `clear_gc_fields` stores that - /// carry `emit_raw_memclear`'s job there, and reads recycled bytes as - /// initialized. Its nursery arm hands out zero-filled memory, so a stand-in - /// for that arm owes the same. Every other target leaves the block as - /// `external_malloc` does. + /// WASM-ONLY ADAPTATION, paired with `Nursery::reset`, which states what + /// the zero-fill stands in for and what retiring it would take. In short: + /// wasm runs no part of the GC rewrite, so its compiled code emits none of + /// the `clear_gc_fields` stores that carry `emit_raw_memclear`'s job + /// there, and reads recycled bytes as initialized. Its nursery arm hands + /// out zero-filled memory, so a stand-in for that arm owes the same. Every + /// other target leaves the block as `external_malloc` does. + /// + /// Only the spill owes it. A request that asks for old-gen outright + /// arrives through `alloc_oldgen_typed`, which takes + /// [`alloc_in_oldgen_clear`](Self::alloc_in_oldgen_clear) and clears on + /// every target — so a wasm `New` against a `non_moving` descr is covered + /// without this arm. #[inline] fn clear_nursery_substitute(obj: GcRef, total_size: usize) { if cfg!(target_arch = "wasm32") { @@ -3028,10 +3076,6 @@ impl MiniMarkGC { } else { crate::shadow_stack::walk_my_extra_areas(&mut visit_extra_area); } - crate::walk_active_extra_roots(&mut |gcref| { - self.drag_out_root(gcref); - }); - // Multi-registrar walker fan-out (rd_consts const-pool, etc.). crate::shadow_stack::walk_extra_roots(|gcref| { self.drag_out_root(gcref); @@ -3254,6 +3298,13 @@ impl MiniMarkGC { } self.refresh_published_nursery_top(); + // incminimark.py `_minor_collection`: `if self.DEBUG >= 2` — the whole + // heap is walked, so it is gated a level above the rotating nurseries + // rather than on `PYPY_GC_DEBUG` being set at all. + if self.config.debug >= 2 { + self.debug_check_consistency(); + } + // incminimark.py `self.root_walker.finished_minor_collection()`, // the callback framework.py:135-138 reads out of `_jit2gc`: after the // nursery is reset and accounted for, and before the timing and the @@ -4073,10 +4124,12 @@ impl MiniMarkGC { /// whose all-ones flag region fakes every flag (IGNORE_FINALIZER /// included), so a raw read there would silently drop a finalizer entry. /// - /// Latent today: every finalizer-queue registrant (instances, generators) - /// is stable-allocated, so a queued object is never in the nursery. It - /// becomes load-bearing when instance allocation converges back to the - /// movable nursery (see `alloc_instance_object`). + /// Load-bearing today. Instances and generators are stable-allocated, but + /// `list_descr_new` takes its header from `w_list_new` — the collecting + /// *nursery* arm — and then calls `maybe_register_finalizer` for a + /// builtin-layout subclass, so a `class L(list)` with `__del__` puts a + /// nursery header on the queue. It gets more so when instance allocation + /// converges back to the movable nursery (see `alloc_instance_object`). fn get_possibly_forwarded_header(&self, obj_addr: usize) -> *const GcHeader { let hdr = unsafe { header_of(obj_addr) }; if self.is_nursery_object_start(obj_addr) && unsafe { (*hdr).is_forwarded() } { @@ -5109,10 +5162,6 @@ impl MiniMarkGC { crate::shadow_stack::walk_my_extra_areas(&mut visit_extra_area); } - crate::walk_active_extra_roots(&mut |gcref| { - result.push((*gcref, "active_extra_root")); - }); - crate::shadow_stack::walk_extra_roots(|gcref| { result.push((*gcref, "extra_root")); }); @@ -5787,6 +5836,9 @@ impl MiniMarkGC { // incminimark.py:2623-2631: recursive collections from a // handler must see a collector ready to start a new scan. self.gc_state = GcState::Scanning; + // The cycle is over here, which is what `gc.get_count`'s + // second element counts from. + self.minor_collections_at_major_end = self.minor_collections; self.execute_finalizer_triggers(); } } @@ -5803,15 +5855,101 @@ impl MiniMarkGC { } } - /// incminimark.py:1316-1319 debug invariant. + /// incminimark.py `debug_check_consistency`. + /// + /// Self-gated on the debug level rather than gated at its call sites, as + /// upstream is: the body opens with `if self.DEBUG:`, so a run that did + /// not ask for the checks pays one load and the checks are real + /// assertions rather than `debug_assert!`s that a release build drops. + /// `PYPY_GC_DEBUG` is the only way to arm them, and a run that sets it is + /// asking to be aborted on a broken invariant. fn debug_check_consistency(&self) { + if self.config.debug == 0 { + return; + } + assert!( + self.oldgen.young_rawmalloced_is_empty(), + "young raw-malloced objects in a major collection" + ); + assert!( + self.young_objects_with_weakrefs.is_empty(), + "young objects with weakrefs in a major collection" + ); if self.oldgen.rawmalloc_sweep_pending() { - debug_assert_eq!( + assert_eq!( self.gc_state, GcState::Sweeping, "raw_malloc_might_sweep must be empty outside SWEEPING" ); } + self.debug_check_reachable(); + } + + /// gc/base.py `debug_check_consistency`'s heap half — enumerate every root + /// and trace the whole reachable graph, checking each object once. + /// + /// Upstream keeps its seen set and pending stack as GC-side `AddressDict` / + /// `AddressStack` because it has no other allocator; here they are ordinary + /// Rust containers, which is the same structure without the bookkeeping. + fn debug_check_reachable(&self) { + let mut seen: std::collections::HashSet = std::collections::HashSet::new(); + let mut pending: Vec = Vec::new(); + let record = + |addr: usize, seen: &mut std::collections::HashSet, pending: &mut Vec| { + if seen.insert(addr) { + self.debug_check_object(addr); + pending.push(addr); + } + }; + for root in self.enumerate_root_walker_values() { + if !root.is_null() { + record(root.0, &mut seen, &mut pending); + } + } + while let Some(obj_addr) = pending.pop() { + let type_id = unsafe { (*header_of(obj_addr)).type_id() }; + if (type_id as usize) >= self.types.len() { + continue; + } + let mut children: Vec = Vec::new(); + unsafe { + self.types.get(type_id).for_each_gc_ptr(obj_addr, |slot| { + let child = *slot; + if !child.is_null() { + children.push(child.0); + } + }); + } + for child in children { + record(child, &mut seen, &mut pending); + } + } + } + + /// incminimark.py `debug_check_object`: after a collection nothing is left + /// in the nursery but the pinned objects, and neither of the two flags the + /// collection itself uses may survive it. + fn debug_check_object(&self, obj_addr: usize) { + let hdr = unsafe { &*header_of(obj_addr) }; + if self.is_pinned(GcRef(obj_addr)) { + assert!( + self.is_in_nursery(obj_addr), + "pinned object not in nursery at {obj_addr:#x}" + ); + return; + } + assert!( + !self.is_in_nursery(obj_addr), + "object in nursery after collection at {obj_addr:#x}" + ); + assert!( + !hdr.has_flag(flags::VISITED_RMY), + "GCFLAG_VISITED_RMY after collection at {obj_addr:#x}" + ); + assert!( + !hdr.has_flag(flags::PINNED), + "GCFLAG_PINNED outside the nursery after collection at {obj_addr:#x}" + ); } /// Perform one incremental marking step. @@ -6293,9 +6431,11 @@ impl MiniMarkGC { /// and are therefore born black. /// /// pyre's stack root sets are mutated with no write barrier and hold - /// pre-cycle objects: a JitFrame lives in the old gen so its pointer stays - /// valid across a collecting call while compiled code stores Refs into its - /// gcmap slots, the blackhole register banks and resume-construction roots + /// pre-cycle objects: an off-GC JitFrame is `alloc_zeroed` memory outside + /// the heap entirely, so its pointer stays valid across a collecting call + /// while compiled code stores Refs into its gcmap slots (the nursery-built + /// frames get their pointer back from `_reload_frame_if_necessary` + /// instead), the blackhole register banks and resume-construction roots /// are plain slices, and `seed_major_root` arms a newly seeded old root /// into the remembered set only once — the next minor drains that set and /// nothing re-arms it. A black root can therefore come to hold the only @@ -7460,6 +7600,22 @@ impl MiniMarkGC { /// A move within one object carries its items across card boundaries while /// the card bits stay where they are, so the per-card record is no longer /// true of the object and only the whole-object record is. + /// + /// Nothing calls this outside tests, and nothing could act on it if it + /// did: `CARDS_SET` is only ever set behind a `HAS_CARDS` test, and the + /// one non-test place that sets `HAS_CARDS`, `alloc_in_oldgen_with_cards`, + /// has no production caller. So the guard above rejects every object pyre + /// can build today. + /// + /// It stops rejecting them the moment a production caller of that + /// allocator lands, and the sites that owe this call then are the item + /// moves in `pyre_object::listobject`'s `W_ListObject` — `object_insert`, + /// `object_remove` and `object_drain` each shift items with a bare + /// `ptr::copy`. Upstream reaches this barrier from those operations + /// through `rgc.ll_arraymove`, which `ll_insert_nonneg`, `ll_pop_zero`, + /// `ll_delitem_nonneg` and `ll_listdelslice_startstop` all call; pyre's + /// list is not an rtyper-lowered list, so it has no such seam and the + /// calls have to be written at those three sites. pub fn writebarrier_before_move(&mut self, array_addr: usize) { if self.config.card_page_indices == 0 { return; @@ -7771,6 +7927,17 @@ impl MiniMarkGC { previous_end = header_addr + object_size; } self.nursery.reset_range(previous_end, nursery_end); + // `_minor_collection` rotates only on the no-pinned-objects arm: a + // pinned object stays where it is, and the arena holding it is about + // to be made inaccessible. + if self.config.gc_nursery_debug && barriers.is_empty() { + self.nursery.debug_rotate(); + } + // Read the arena back rather than reusing the entry values: a rotation + // moved it, and both the barrier below and the `nursery_free` at the + // tail of this function have to name the arena now installed. + let nursery_start = self.nursery.start_ptr() as usize; + let nursery_end = nursery_start + self.nursery.size(); barriers.push_back(nursery_end); self.nursery_barriers = barriers; @@ -7835,204 +8002,12 @@ impl MiniMarkGC { !hdr.is_forwarded() && hdr.has_flag(flags::PINNED) } - /// Free memory associated with invalidated JIT compiled code. - /// - /// `code_ptr` and `size` identify the compiled code region to release. - /// The region is looked up and removed from the compiled code registry - /// so the GC no longer scans it for root references. - pub fn jit_free(&mut self, code_ptr: usize, size: usize) { - // Find and remove any compiled code region that matches the given range. - self.compiled_code_registry - .regions - .retain(|r| !(r.code_start == code_ptr && r.code_size == size)); - } - /// Number of objects in the remembered set (for testing / diagnostics). pub fn remembered_set_len(&self) -> usize { self.remembered_set.len() } } -/// Safepoint GC map: records which frame slots contain GC references -/// at a specific program point (guard or call site). -/// -/// The Cranelift backend builds these during compilation and stores them -/// alongside the compiled code. During collection, the GC uses them to -/// find live references on the stack. -#[derive(Debug, Clone)] -pub struct SafepointMap { - /// Map from code offset to GcMap. - pub entries: Vec, -} - -/// A single safepoint entry. -#[derive(Debug, Clone)] -pub struct SafepointEntry { - /// Offset in the compiled code (bytes from function start). - pub code_offset: u32, - /// Bitmap of which frame slots contain GC references. - pub gc_map: crate::GcMap, -} - -impl SafepointMap { - pub fn new() -> Self { - SafepointMap { - entries: Vec::new(), - } - } - - /// Add a safepoint entry. - pub fn add(&mut self, code_offset: u32, gc_map: crate::GcMap) { - self.entries.push(SafepointEntry { - code_offset, - gc_map, - }); - } - - /// Look up the GcMap for a given code offset. - pub fn lookup(&self, code_offset: u32) -> Option<&crate::GcMap> { - self.entries - .iter() - .find(|e| e.code_offset == code_offset) - .map(|e| &e.gc_map) - } -} - -impl Default for SafepointMap { - fn default() -> Self { - Self::new() - } -} - -/// Registry of compiled code regions and their safepoint maps. -/// -/// When the GC needs to scan the stack during collection, it uses the return -/// address to find which compiled code region is active, then looks up the -/// safepoint map to determine which frame slots contain GC references. -/// -/// From rpython/jit/backend/llsupport/gc.py GcRootMap_asmgcc / GcRootMap_shadowstack. -pub struct CompiledCodeRegistry { - /// Compiled code regions, sorted by start address for binary search. - regions: Vec, -} - -/// A single compiled code region with its safepoint map. -#[derive(Debug, Clone)] -pub struct CompiledCodeRegion { - /// Start address of the compiled code. - pub code_start: usize, - /// Size of the compiled code in bytes. - pub code_size: usize, - /// Safepoint map for this region. - pub safepoint_map: SafepointMap, - /// Frame size in slots (each slot = 8 bytes). - pub frame_size_slots: u32, - /// JitCellToken number for identification. - pub loop_token: u64, -} - -impl CompiledCodeRegistry { - pub fn new() -> Self { - CompiledCodeRegistry { - regions: Vec::new(), - } - } - - /// Register a compiled code region. - pub fn register(&mut self, region: CompiledCodeRegion) { - self.regions.push(region); - // Keep sorted by code_start for binary search - self.regions.sort_by_key(|r| r.code_start); - } - - /// Unregister a compiled code region (e.g., when invalidating a loop). - pub fn unregister(&mut self, loop_token: u64) { - self.regions.retain(|r| r.loop_token != loop_token); - } - - /// Look up a compiled code region containing the given return address. - /// - /// Returns the region and the offset within it. - pub fn find_region(&self, return_addr: usize) -> Option<(&CompiledCodeRegion, u32)> { - // Binary search for the region containing this address - let idx = self - .regions - .binary_search_by(|r| { - if return_addr < r.code_start { - std::cmp::Ordering::Greater - } else if return_addr >= r.code_start + r.code_size { - std::cmp::Ordering::Less - } else { - std::cmp::Ordering::Equal - } - }) - .ok()?; - - let region = &self.regions[idx]; - let offset = (return_addr - region.code_start) as u32; - Some((region, offset)) - } - - /// Scan a compiled frame for GC references using the safepoint map. - /// - /// Given a return address (from the call stack) and the frame base pointer, - /// enumerates all frame slots that contain GC references. - /// - /// # Safety - /// `frame_base` must point to a valid JIT frame with at least - /// `region.frame_size_slots` slots. - pub unsafe fn scan_frame( - &self, - return_addr: usize, - frame_base: *const usize, - ) -> Vec<*mut GcRef> { - let mut roots = Vec::new(); - - let (region, offset) = match self.find_region(return_addr) { - Some(r) => r, - None => return roots, - }; - - let gc_map = match region.safepoint_map.lookup(offset) { - Some(map) => map, - None => return roots, - }; - - // Enumerate all slots marked as GC references - for word_idx in 0..gc_map.ref_bitmap.len() { - let mut bits = gc_map.ref_bitmap[word_idx]; - while bits != 0 { - let bit = bits.trailing_zeros() as usize; - let slot_idx = word_idx * 64 + bit; - - if slot_idx < region.frame_size_slots as usize { - let slot_ptr = unsafe { frame_base.add(slot_idx) } as *mut GcRef; - roots.push(slot_ptr); - } - - bits &= bits - 1; // Clear lowest set bit - } - } - - roots - } - - /// Number of registered regions. - pub fn len(&self) -> usize { - self.regions.len() - } - - pub fn is_empty(&self) -> bool { - self.regions.is_empty() - } -} - -impl Default for CompiledCodeRegistry { - fn default() -> Self { - Self::new() - } -} - impl Default for MiniMarkGC { fn default() -> Self { Self::new() @@ -8164,6 +8139,10 @@ impl GcAllocator for MiniMarkGC { (self.minor_collections, self.major_collections) } + fn minor_collections_since_major(&self) -> usize { + self.minor_collections - self.minor_collections_at_major_end + } + fn get_write_barrier_descr(&self) -> Option { let mut descr = crate::WriteBarrierDescr::for_current_gc(); if self.card_page_shift == 0 { @@ -8230,6 +8209,10 @@ impl GcAllocator for MiniMarkGC { self.do_collect_full(); } + fn collect_generation(&mut self, generation: i64) { + self.do_collect(generation); + } + fn collect_step(&mut self) -> crate::GcStepTransition { self.collect_step() } @@ -8412,10 +8395,6 @@ impl GcAllocator for MiniMarkGC { self.gc_step() } - fn jit_free(&mut self, code_ptr: usize, size: usize) { - self.jit_free(code_ptr, size); - } - fn pin(&mut self, obj: GcRef) -> bool { self.pin(obj) } @@ -8717,6 +8696,62 @@ mod tests { }) } + /// incminimark.py `_minor_collection`: on the `gc_nursery_debug` arm with + /// no pinned survivor, the recycled arena is retired and the next one in + /// the ring takes its place. + #[test] + fn a_minor_collection_rotates_the_nursery_under_gc_nursery_debug() { + if !crate::nursery::HAS_PROTECT { + return; + } + let mut gc = MiniMarkGC::with_config(GcConfig { + nursery_size: 4096, + large_object_threshold: 2048, + debug: 1, + gc_nursery_debug: true, + ..GcConfig::default() + }); + assert_eq!( + gc.nursery.debug_rotating_nurseries(), + DEBUG_ROTATING_NURSERIES + ); + + let before = gc.nursery.start_ptr() as usize; + gc.do_collect_nursery(); + let after = gc.nursery.start_ptr() as usize; + + assert_ne!(before, after, "the minor collection retired the arena"); + assert_eq!( + gc.nursery.free_ptr() as usize, + after, + "allocation resumes at the start of the arena now installed" + ); + assert_eq!( + gc.published_nursery_top.load(Ordering::Relaxed), + after + gc.nursery.size(), + "compiled code reads the published top, which must follow" + ); + } + + /// `post_setup` allocates the ring only `if self.DEBUG and + /// llarena.has_protect`, so an ordinary run keeps its one arena and + /// `debug_rotate_nursery` finds nothing to rotate to. + #[test] + fn no_ring_is_allocated_without_the_debug_level() { + let mut gc = MiniMarkGC::with_config(GcConfig { + nursery_size: 4096, + large_object_threshold: 2048, + gc_nursery_debug: true, + ..GcConfig::default() + }); + assert_eq!(gc.config.debug, 0); + assert_eq!(gc.nursery.debug_rotating_nurseries(), 0); + + let before = gc.nursery.start_ptr() as usize; + gc.do_collect_nursery(); + assert_eq!(gc.nursery.start_ptr() as usize, before); + } + /// incminimark.py:1890-1893 `free_young_rawmalloced_objects`: an /// oversized object nothing reached is freed by the minor, not carried to /// the next major. @@ -11504,210 +11539,6 @@ mod tests { ); } - // ── SafepointMap tests ── - - #[test] - fn test_safepoint_map_register_and_lookup() { - let mut smap = SafepointMap::new(); - - let mut gc_map_0 = crate::GcMap::new(); - gc_map_0.set_ref(0); - gc_map_0.set_ref(3); - - let mut gc_map_1 = crate::GcMap::new(); - gc_map_1.set_ref(1); - gc_map_1.set_ref(7); - - smap.add(100, gc_map_0); - smap.add(200, gc_map_1); - - // Lookup existing entries. - let found_0 = smap.lookup(100).unwrap(); - assert!(found_0.is_ref(0)); - assert!(found_0.is_ref(3)); - assert!(!found_0.is_ref(1)); - - let found_1 = smap.lookup(200).unwrap(); - assert!(found_1.is_ref(1)); - assert!(found_1.is_ref(7)); - assert!(!found_1.is_ref(0)); - - // Lookup non-existent offset returns None. - assert!(smap.lookup(999).is_none()); - } - - #[test] - fn test_safepoint_map_empty() { - let smap = SafepointMap::new(); - assert!(smap.lookup(0).is_none()); - assert!(smap.entries.is_empty()); - } - - // ── CompiledCodeRegistry tests ── - - #[test] - fn test_compiled_code_registry_register_and_find() { - let mut registry = CompiledCodeRegistry::new(); - assert!(registry.is_empty()); - - let mut smap = SafepointMap::new(); - let mut gc_map = crate::GcMap::new(); - gc_map.set_ref(0); - gc_map.set_ref(2); - smap.add(16, gc_map); - - registry.register(CompiledCodeRegion { - code_start: 0x1000, - code_size: 0x100, - safepoint_map: smap, - frame_size_slots: 4, - loop_token: 42, - }); - - assert_eq!(registry.len(), 1); - - // Address inside the region. - let (region, offset) = registry.find_region(0x1010).unwrap(); - assert_eq!(region.loop_token, 42); - assert_eq!(offset, 0x10); - - // Address at the start. - let (region, offset) = registry.find_region(0x1000).unwrap(); - assert_eq!(region.loop_token, 42); - assert_eq!(offset, 0); - - // Address outside the region. - assert!(registry.find_region(0x900).is_none()); - assert!(registry.find_region(0x1100).is_none()); - } - - #[test] - fn test_compiled_code_registry_multiple_regions() { - let mut registry = CompiledCodeRegistry::new(); - - registry.register(CompiledCodeRegion { - code_start: 0x1000, - code_size: 0x100, - safepoint_map: SafepointMap::new(), - frame_size_slots: 4, - loop_token: 1, - }); - registry.register(CompiledCodeRegion { - code_start: 0x3000, - code_size: 0x200, - safepoint_map: SafepointMap::new(), - frame_size_slots: 8, - loop_token: 2, - }); - registry.register(CompiledCodeRegion { - code_start: 0x2000, - code_size: 0x80, - safepoint_map: SafepointMap::new(), - frame_size_slots: 2, - loop_token: 3, - }); - - assert_eq!(registry.len(), 3); - - // Each region should be findable. - assert_eq!(registry.find_region(0x1050).unwrap().0.loop_token, 1); - assert_eq!(registry.find_region(0x2040).unwrap().0.loop_token, 3); - assert_eq!(registry.find_region(0x3100).unwrap().0.loop_token, 2); - - // Gap between regions returns None. - assert!(registry.find_region(0x1200).is_none()); - } - - #[test] - fn test_compiled_code_registry_unregister() { - let mut registry = CompiledCodeRegistry::new(); - - registry.register(CompiledCodeRegion { - code_start: 0x1000, - code_size: 0x100, - safepoint_map: SafepointMap::new(), - frame_size_slots: 4, - loop_token: 10, - }); - registry.register(CompiledCodeRegion { - code_start: 0x2000, - code_size: 0x100, - safepoint_map: SafepointMap::new(), - frame_size_slots: 4, - loop_token: 20, - }); - - assert_eq!(registry.len(), 2); - - registry.unregister(10); - assert_eq!(registry.len(), 1); - assert!(registry.find_region(0x1050).is_none()); - assert_eq!(registry.find_region(0x2050).unwrap().0.loop_token, 20); - } - - #[test] - fn test_compiled_code_registry_safepoint_lookup_for_root_scanning() { - let mut registry = CompiledCodeRegistry::new(); - - let mut smap = SafepointMap::new(); - let mut gc_map = crate::GcMap::new(); - gc_map.set_ref(0); - gc_map.set_ref(2); - smap.add(0x20, gc_map); - - registry.register(CompiledCodeRegion { - code_start: 0x5000, - code_size: 0x200, - safepoint_map: smap, - frame_size_slots: 4, - loop_token: 99, - }); - - // Simulate finding a return address and looking up the safepoint map. - let return_addr = 0x5020; - let (region, offset) = registry.find_region(return_addr).unwrap(); - let gc_map = region.safepoint_map.lookup(offset).unwrap(); - - // Verify the GC map identifies the correct slots. - assert!(gc_map.is_ref(0), "slot 0 should be a GC ref"); - assert!(!gc_map.is_ref(1), "slot 1 should not be a GC ref"); - assert!(gc_map.is_ref(2), "slot 2 should be a GC ref"); - assert!(!gc_map.is_ref(3), "slot 3 should not be a GC ref"); - } - - #[test] - fn test_scan_frame_enumerates_gc_ref_slots() { - let mut registry = CompiledCodeRegistry::new(); - - let mut smap = SafepointMap::new(); - let mut gc_map = crate::GcMap::new(); - gc_map.set_ref(0); - gc_map.set_ref(2); - smap.add(0x10, gc_map); - - registry.register(CompiledCodeRegion { - code_start: 0xA000, - code_size: 0x100, - safepoint_map: smap, - frame_size_slots: 4, - loop_token: 77, - }); - - // Allocate a fake frame on the stack. - let frame: [usize; 4] = [111, 222, 333, 444]; - let frame_base = frame.as_ptr(); - - let return_addr = 0xA010; - let roots = unsafe { registry.scan_frame(return_addr, frame_base) }; - - // Should find slots 0 and 2. - assert_eq!(roots.len(), 2); - unsafe { - assert_eq!(*(roots[0] as *const usize), 111); - assert_eq!(*(roots[1] as *const usize), 333); - } - } - // ── Incremental marking tests ── #[test] @@ -12060,11 +11891,117 @@ mod tests { gc.roots.clear(); } + /// `gc.get_count`'s second element: minors accumulate and a finished major + /// is what resets them. + /// + /// Driven through `do_collect_full`, the operation `gc.collect()` runs, so + /// the state the interpreter reports afterwards is the state asserted + /// here. Stepping by hand would count differently: the reset lives in + /// `finish_incremental_cycle`, and a full collection runs minors around + /// its steps. + #[test] + fn minors_accumulate_until_a_major_finishes() { + let mut gc = test_gc(4096); + let tid = gc.register_type(TypeInfo::simple(16)); + assert_eq!(gc.minor_collections_since_major(), 0); + + gc.do_collect_nursery(); + gc.do_collect_nursery(); + assert_eq!(gc.minor_collections_since_major(), 2); + + // A survivor, so the major has something to trace. + let mut root = gc.alloc_with_type(tid, 16); + unsafe { gc.roots.add(&mut root) }; + + gc.do_collect_full(); + assert!(gc.major_collections >= 1, "a full collection runs a major"); + assert_eq!( + gc.minor_collections_since_major(), + 0, + "a finished major resets the minors counted since the last one" + ); + + gc.do_collect_nursery(); + assert_eq!(gc.minor_collections_since_major(), 1); + gc.roots.clear(); + } + + /// A GC armed for the debug checks, with the rotation ring the level also + /// installs. + fn debug_gc(nursery_size: usize) -> MiniMarkGC { + MiniMarkGC::with_config(GcConfig { + nursery_size, + large_object_threshold: nursery_size / 2, + debug: 1, + ..GcConfig::default() + }) + } + + /// The heap half of the check reaches an object through the root walk, so + /// a flag the collection should have cleared is caught on the object + /// rather than only on the collector's own lists. + /// + /// This is what makes the walk non-vacuous: without it a root enumeration + /// that returned nothing would pass every run. + #[test] + #[should_panic(expected = "GCFLAG_VISITED_RMY after collection")] + fn the_debug_walk_reaches_a_promoted_object() { + let mut gc = debug_gc(4096); + let tid = gc.register_type(TypeInfo::simple(16)); + let obj = gc.alloc_with_type(tid, 16); + let mut root = obj; + unsafe { gc.roots.add(&mut root) }; + gc.do_collect_nursery(); + assert!(!gc.is_in_nursery(root.0), "the object promoted"); + + unsafe { (*header_of(root.0)).set_flag(flags::VISITED_RMY) }; + gc.debug_check_consistency(); + } + + /// The same walk over a heap nothing has broken reports nothing, so the + /// test above is failing on the flag rather than on the walk itself. + #[test] + fn the_debug_walk_passes_a_clean_heap() { + let mut gc = debug_gc(4096); + let tid = gc.register_type(TypeInfo::simple(16)); + let obj = gc.alloc_with_type(tid, 16); + let mut root = obj; + unsafe { gc.roots.add(&mut root) }; + gc.do_collect_nursery(); + gc.debug_check_consistency(); + gc.roots.clear(); + } + + /// Without the level the body returns before the first assertion, so the + /// same broken flag goes unreported. + #[test] + fn the_debug_checks_are_off_without_the_level() { + let mut gc = test_gc(4096); + assert_eq!(gc.config.debug, 0); + let tid = gc.register_type(TypeInfo::simple(16)); + let obj = gc.alloc_with_type(tid, 16); + let mut root = obj; + unsafe { gc.roots.add(&mut root) }; + gc.do_collect_nursery(); + + unsafe { (*header_of(root.0)).set_flag(flags::VISITED_RMY) }; + gc.debug_check_consistency(); + unsafe { (*header_of(root.0)).clear_flag(flags::VISITED_RMY) }; + gc.roots.clear(); + } + + /// No `#[cfg(debug_assertions)]`: `debug_check_consistency` self-gates on + /// the debug level and asserts for real, so the check survives a release + /// build — which is what makes `PYPY_GC_DEBUG` worth setting there. #[test] - #[cfg(debug_assertions)] #[should_panic(expected = "raw_malloc_might_sweep must be empty outside SWEEPING")] fn rawmalloc_sweep_candidates_require_sweeping_state() { - let mut gc = test_gc(4096); + let mut gc = MiniMarkGC::with_config(GcConfig { + nursery_size: 4096, + large_object_threshold: 2048, + debug: 1, + ..GcConfig::default() + }); let tid = gc.register_type(TypeInfo::simple(16)); let raw_size = gc.oldgen.small_request_threshold() + std::mem::size_of::(); gc.alloc_in_oldgen_clear(tid, raw_size); @@ -12098,99 +12035,6 @@ mod tests { gc.roots.clear(); } - // ── GC stress tests ── - - #[test] - #[cfg(debug_assertions)] - fn test_gc_stress_with_safepoint_scanning() { - // Register a compiled code region with a safepoint map, then - // allocate objects under pressure so nursery collections fire. - // After collection, verify that roots discovered via scan_frame - // point to valid, promoted objects. - - let ptr_size = std::mem::size_of::(); - let mut gc = test_gc(512); // small nursery to force frequent collections - let tid = gc.register_type(TypeInfo::with_gc_ptrs(ptr_size * 2, vec![0, ptr_size])); - - // Build a compiled code registry with a safepoint map marking - // frame slots 0 and 2 as GC references. - let mut registry = CompiledCodeRegistry::new(); - let mut smap = SafepointMap::new(); - let mut gc_map = crate::GcMap::new(); - gc_map.set_ref(0); - gc_map.set_ref(2); - smap.add(0x50, gc_map); - - registry.register(CompiledCodeRegion { - code_start: 0x1000, - code_size: 0x100, - safepoint_map: smap, - frame_size_slots: 4, - loop_token: 1, - }); - - // Simulate a JIT frame: slots 0 and 2 hold GcRefs, slots 1 and 3 - // hold non-pointer data. - let obj_a = gc.alloc_with_type(tid, ptr_size * 2); - let obj_b = gc.alloc_with_type(tid, ptr_size * 2); - unsafe { - *(obj_a.0 as *mut GcRef) = GcRef::NULL; - *((obj_a.0 + ptr_size) as *mut GcRef) = GcRef::NULL; - *(obj_b.0 as *mut GcRef) = GcRef::NULL; - *((obj_b.0 + ptr_size) as *mut GcRef) = GcRef::NULL; - } - - let frame: [usize; 4] = [obj_a.0, 0xDEAD, obj_b.0, 0xBEEF]; - - // Register frame slots as GC roots (simulating what the backend does - // at a safepoint). - let roots_from_frame = unsafe { registry.scan_frame(0x1050, frame.as_ptr()) }; - assert_eq!(roots_from_frame.len(), 2); - - // Register the scanned slots as roots with the GC. - for root_ptr in &roots_from_frame { - unsafe { - gc.roots.add(*root_ptr); - } - } - - // Allocate many objects to force multiple nursery collections. - for i in 0..200 { - let filler = gc.alloc_with_type(tid, ptr_size * 2); - unsafe { - *(filler.0 as *mut u64) = i as u64; - } - } - assert!( - gc.minor_collections > 0, - "should have triggered nursery collections" - ); - - // Read back the GcRefs from the frame slots (the GC may have updated - // them when it promoted the objects). - let ref_a = GcRef(frame[0]); - let ref_b = GcRef(frame[2]); - - // The original nursery objects should have been forwarded. - // The frame slots must now point to valid (non-nursery) addresses. - assert!(!ref_a.is_null()); - assert!(!ref_b.is_null()); - assert!( - !gc.is_in_nursery(ref_a.0), - "object A should have been promoted out of nursery" - ); - assert!( - !gc.is_in_nursery(ref_b.0), - "object B should have been promoted out of nursery" - ); - - // Verify non-GC slots are untouched. - assert_eq!(frame[1], 0xDEAD); - assert_eq!(frame[3], 0xBEEF); - - gc.roots.clear(); - } - /// With `set_stress_collect(true)`, `alloc_with_type` forces a full /// collection on every allocation. A large nursery is used so that no /// collection would fire naturally across these allocations — the @@ -13121,7 +12965,7 @@ cache size\t: 8192 kB\n"; gc.roots.clear(); } - // ── Pin / Unpin / jit_free tests ── + // ── Pin / Unpin tests ── #[test] fn test_pin_prevents_nursery_move() { @@ -13378,42 +13222,6 @@ cache size\t: 8192 kB\n"; gc.roots.clear(); } - #[test] - fn test_jit_free_unregisters_code() { - let mut gc = test_gc(4096); - - let smap = SafepointMap::new(); - gc.compiled_code_registry.register(CompiledCodeRegion { - code_start: 0x1000, - code_size: 256, - safepoint_map: smap, - frame_size_slots: 4, - loop_token: 1, - }); - - let smap2 = SafepointMap::new(); - gc.compiled_code_registry.register(CompiledCodeRegion { - code_start: 0x2000, - code_size: 512, - safepoint_map: smap2, - frame_size_slots: 8, - loop_token: 2, - }); - - assert_eq!(gc.compiled_code_registry.len(), 2); - - // Free the first region. - gc.jit_free(0x1000, 256); - - assert_eq!(gc.compiled_code_registry.len(), 1); - assert!(gc.compiled_code_registry.find_region(0x1050).is_none()); - assert!(gc.compiled_code_registry.find_region(0x2050).is_some()); - - // Free the second region. - gc.jit_free(0x2000, 512); - assert_eq!(gc.compiled_code_registry.len(), 0); - } - #[test] fn test_incremental_cycle_root_walk_skips_non_gc_roots() { let mut gc = test_gc(4096); diff --git a/majit/majit-gc/src/gcreftracer.rs b/majit/majit-gc/src/gcreftracer.rs index f9167675b5e..f32f1a5fef8 100644 --- a/majit/majit-gc/src/gcreftracer.rs +++ b/majit/majit-gc/src/gcreftracer.rs @@ -42,6 +42,14 @@ //! `asmmemmgr_gcreftracers` (or `CompiledLoopToken` itself) is GC-traced, //! this collapses to a managed `GCREFTRACER` GcStruct + custom trace //! hook, matching upstream exactly. +//! +//! One upstream duty does not carry over. `free_loop_and_bridges` calls +//! `clear_gcref_tracer`, which zeroes `array_length`, because upstream's +//! slot array is reserved inside the code block (`reserve_gcref_table`) +//! and is freed with it — a tracer outliving the block would otherwise +//! hand the collector freed memory. Here the slots are the table's own +//! `Box`, allocated and released with it, so the array cannot outlive its +//! storage and there is nothing to turn off. use std::cell::Cell; use std::sync::{Arc, RwLock, Weak}; @@ -73,14 +81,21 @@ pub struct GcTable { unsafe impl Send for GcTable {} unsafe impl Sync for GcTable {} -/// Live per-loop tables, walked as GC roots. The strong reference is -/// held by `CompiledLoopToken.asmmemmgr_gcreftracers` (parity -/// `gcreftracers.append(tracer)`, `x86/assembler.py`); the registry -/// keeps only a `Weak`, so when the loop token is freed the table drops -/// and its registry entry becomes dangling — deregistration needs no -/// explicit `free_loop` hook (neither backend's `free_loop` clears -/// `asmmemmgr_gcreftracers`; both rely on `Arc` -/// drop). +/// Live per-loop tables, walked as GC roots. A strong reference is held by +/// `CompiledLoopToken.asmmemmgr_gcreftracers` (parity +/// `gcreftracers.append(tracer)`, `x86/assembler.py`); the registry keeps +/// only a `Weak`, and a `Weak` that stops upgrading is the whole of +/// deregistration. No `free_loop` clears `asmmemmgr_gcreftracers`, and +/// nothing dispatches `Backend::free_loop` at all — the release is `Arc` +/// drop, driven by the memory manager retiring a token in +/// `try_to_free_some_loops`. +/// +/// That drop is not always the CLT's. On cranelift a bridge's table is +/// pinned a second time by every `BridgeData` that can dispatch to the +/// bridge, so it outlives the token it was registered against for as long +/// as a fail descr in another token holds one. The table dies with its +/// last strong holder, whichever that is; on dynasm the CLT is the only +/// one. static LIVE_GC_TABLES: RwLock>> = RwLock::new(Vec::new()); /// Test-only lock modeling the stop-the-world invariant that no table is diff --git a/majit/majit-gc/src/lib.rs b/majit/majit-gc/src/lib.rs index a70c8958fb6..1ebc5a547a1 100644 --- a/majit/majit-gc/src/lib.rs +++ b/majit/majit-gc/src/lib.rs @@ -670,6 +670,23 @@ pub trait GcAllocator: Send { /// Trigger a full collection. fn collect_full(&mut self); + /// `incminimark.py collect(gen=2)`: "Do a minor (gen=0), start a major + /// (gen=1), or do a full major (gen>=2) collection." + /// + /// The generation argument is the same one [`get_objects`](Self::get_objects) + /// takes, and the two agree on what a generation is: 0 is the nursery and + /// anything above it is what a minor collection has already promoted out of + /// it. This is what app-level `gc.collect(n)` reaches, so the dispatch has + /// to happen behind whichever hook the active backend installed rather than + /// on `gc_sync`'s singleton — the wasm backend keeps its GC somewhere that + /// singleton does not reach. + /// + /// A collector with no state machine has no generation to select, so the + /// default answers every one of them with the whole collection. + fn collect_generation(&mut self, _generation: i64) { + self.collect_full(); + } + /// `incminimark.py collect_step`: perform one minor collection /// and exactly one major-collection state transition, independently of /// the automatic-collection enabled flag. @@ -912,9 +929,6 @@ pub trait GcAllocator: Send { false } - /// Free memory associated with invalidated JIT compiled code. - fn jit_free(&mut self, _code_ptr: usize, _size: usize) {} - /// Pin a nursery object so it won't move during minor collection. /// Returns true if pinning succeeded. fn pin(&mut self, _obj: GcRef) -> bool { @@ -975,6 +989,16 @@ pub trait GcAllocator: Send { (0, 0) } + /// Minor collections run since the last major finished. + /// + /// Not derivable from [`collection_counts`](Self::collection_counts): that + /// pair is cumulative, and the caller cannot subtract a snapshot it was + /// never handed. `gc.get_count`'s second element is this number. Default + /// `0` for stub allocators, which run no collections to count. + fn minor_collections_since_major(&self) -> usize { + 0 + } + /// Whether a JIT inline nursery bump of `type_id` is equivalent to /// `alloc_with_type`'s fast path: the type registers no destructor and is /// not a weakref (either would need a side-list push at allocation, i.e. @@ -1320,6 +1344,9 @@ impl GcAllocator for GcHandle { fn collect_full(&mut self) { gc_sync::gc_op(|gc| gc.collect_full()) } + fn collect_generation(&mut self, generation: i64) { + gc_sync::gc_op(|gc| gc.collect_generation(generation)) + } fn collect_step(&mut self) -> GcStepTransition { gc_sync::gc_op(|gc| gc.collect_step()) } @@ -1439,9 +1466,6 @@ impl GcAllocator for GcHandle { fn gc_step(&mut self) -> bool { gc_sync::gc_op(|gc| gc.gc_step()) } - fn jit_free(&mut self, code_ptr: usize, size: usize) { - gc_sync::gc_op(|gc| gc.jit_free(code_ptr, size)) - } fn pin(&mut self, obj: GcRef) -> bool { gc_sync::gc_op(|gc| gc.pin(obj)) } @@ -1469,6 +1493,9 @@ impl GcAllocator for GcHandle { fn collection_counts(&self) -> (usize, usize) { gc_sync::gc_query_reentrant(|gc| gc.collection_counts()) } + fn minor_collections_since_major(&self) -> usize { + gc_sync::gc_query_reentrant(|gc| gc.minor_collections_since_major()) + } fn type_alloc_is_plain(&self, type_id: u32) -> bool { gc_sync::gc_query_reentrant(|gc| gc.type_alloc_is_plain(type_id)) } @@ -1554,48 +1581,6 @@ pub trait GcRewriter: Send { } } -/// Stack map — records which frame slots contain GC references at a safepoint. -/// -/// At each guard (potential GC safepoint), the backend records a stack map -/// so the GC can find all live references in compiled code. -#[derive(Debug, Clone)] -pub struct GcMap { - /// Bitmap: bit N is set if frame slot N contains a GC reference. - pub ref_bitmap: Vec, -} - -impl GcMap { - pub fn new() -> Self { - GcMap { - ref_bitmap: Vec::new(), - } - } - - pub fn set_ref(&mut self, slot: usize) { - let word = slot / 64; - let bit = slot % 64; - if word >= self.ref_bitmap.len() { - self.ref_bitmap.resize(word + 1, 0); - } - self.ref_bitmap[word] |= 1u64 << bit; - } - - pub fn is_ref(&self, slot: usize) -> bool { - let word = slot / 64; - let bit = slot % 64; - if word >= self.ref_bitmap.len() { - return false; - } - (self.ref_bitmap[word] >> bit) & 1 != 0 - } -} - -impl Default for GcMap { - fn default() -> Self { - Self::new() - } -} - // ───────────────────────────────────────────────────────────────────── // Process-global active GC allocator hooks // ───────────────────────────────────────────────────────────────────── @@ -1657,7 +1642,6 @@ pub type TypeidIsObjectFn = fn(typeid: u32) -> Option; /// Process-global callback that checks whether a type id indexes the active /// collector's registered type table. pub type IsRegisteredTypeIdFn = fn(typeid: u32) -> bool; -pub type ExtraRootWalkerFn = fn(&mut dyn FnMut(&mut GcRef)); /// Process-global callback that answers `rgc.can_move(gcref)` /// (rpython/rlib/rgc.py:229) for the currently active backend's GC. The @@ -1675,7 +1659,6 @@ global_hook!(static ACTIVE_IS_REGISTERED_TYPE_ID: IsRegisteredTypeIdFn); global_hook!(static ACTIVE_CAN_MOVE: CanMoveFn); static ACTIVE_SUPPORTS_GUARD_GC_TYPE: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); -global_hook!(static ACTIVE_EXTRA_ROOT_WALKER: ExtraRootWalkerFn); /// Bundle of callbacks the metainterp / executor can reach through /// process-global cells. Mirrors the fan-out of methods RPython's optimizer @@ -1756,19 +1739,6 @@ pub fn override_gc_guard_hooks_for_test(hooks: ActiveGcGuardHooks) -> GuardHooks GuardHooksTestGuard { prev, _lock: lock } } -/// Install a process-global callback that exposes non-shadow-stack roots -/// owned by the embedding runtime. -pub fn set_active_extra_root_walker(walker: Option) { - ACTIVE_EXTRA_ROOT_WALKER.set(walker); -} - -/// Walk the active runtime's extra GC roots. -pub fn walk_active_extra_roots(visitor: &mut dyn FnMut(&mut GcRef)) { - if let Some(f) = ACTIVE_EXTRA_ROOT_WALKER.get() { - f(visitor); - } -} - /// Hand the collector the payload address of every jitframe that is currently /// a live DEADFRAME, for the visitor to trace. pub type LiveDeadFrameWalkerFn = fn(&mut dyn FnMut(usize)); @@ -2461,30 +2431,35 @@ pub unsafe fn alloc_fast_nursery_collecting_typed_rooted( } } -/// Process-global callback that runs a full mark-sweep collection cycle -/// on the active backend's GC (`GcAllocator::collect_full`). Used by -/// `pypy/module/gc/interp_gc.py collect` ports — i.e. user-level -/// `gc.collect()` reaches the live GC through this trampoline. Returns -/// silently when no backend has installed a hook (callers treat -/// it as a no-op). -pub type CollectFullFn = fn(); +/// Process-global callback that runs `GcAllocator::collect_generation` on the +/// active backend's GC. App-level `gc.collect(n)` reaches the live GC through +/// this trampoline, carrying the generation it was given: the backends do not +/// share one GC, so the dispatch cannot happen on `gc_sync`'s singleton. +/// Returns silently when no backend has installed a hook (callers treat it as +/// a no-op). +pub type CollectGenerationFn = fn(i64); -global_hook!(static ACTIVE_COLLECT_FULL: CollectFullFn); +global_hook!(static ACTIVE_COLLECT_GENERATION: CollectGenerationFn); -/// Install the active backend's full-collection trampoline. Pass -/// `None` to clear. -pub fn set_active_collect_full(hook: Option) { - ACTIVE_COLLECT_FULL.set(hook); +/// Install the active backend's collection trampoline. Pass `None` to clear. +pub fn set_active_collect_generation(hook: Option) { + ACTIVE_COLLECT_GENERATION.set(hook); } -/// Trigger a full mark-sweep collection on the active backend's GC. +/// Run `incminimark.py collect(gen)` on the active backend's GC. /// No-op when no backend has installed a hook. -pub fn collect_full() { - if let Some(f) = ACTIVE_COLLECT_FULL.get() { - f(); +pub fn collect_generation(generation: i64) { + if let Some(f) = ACTIVE_COLLECT_GENERATION.get() { + f(generation); } } +/// [`collect_generation`] at the generation that means "all of it". Named +/// separately because `majit-translate`'s gctransform knows this symbol. +pub fn collect_full() { + collect_generation(2); +} + /// Active-backend trampoline for `incminimark.py collect_step`. pub type CollectStepFn = fn() -> GcStepTransition; @@ -2798,6 +2773,29 @@ pub fn active_major_threshold_reached() -> bool { } } +/// Process-global callback for [`GcAllocator::minor_collections_since_major`], +/// installed by whichever backend owns the GC. The interpreter's `gc` module +/// asks through this rather than through `gc_sync`'s singleton, because the +/// wasm backend keeps its GC somewhere that singleton does not reach. +pub type MinorCollectionsSinceMajorFn = fn() -> usize; + +global_hook!(static ACTIVE_MINOR_COLLECTIONS_SINCE_MAJOR: MinorCollectionsSinceMajorFn); + +/// Install the minors-since-major callback for the active backend. +pub fn set_active_minor_collections_since_major(hook: Option) { + ACTIVE_MINOR_COLLECTIONS_SINCE_MAJOR.set(hook); +} + +/// Minor collections the active backend's GC has run since its last major. +/// `0` when no backend has installed a hook, which is also the truthful +/// answer for a process that has collected nothing. +pub fn active_minor_collections_since_major() -> usize { + match ACTIVE_MINOR_COLLECTIONS_SINCE_MAJOR.get() { + Some(f) => f(), + None => 0, + } +} + /// Process-global callback that reports whether a raw address is owned /// by the active backend's GC heap. Used by host-side allocators /// (`pyre-object`'s `dealloc_items_block`) to discriminate @@ -3105,15 +3103,6 @@ pub fn gc_register_finalizer(fq_index: usize, obj: GcRef, trigger: FinalizerTrig } } -/// rgc.py `collect(gen)` — the internal entry that names how much work to do, -/// as opposed to `gc.collect()`, which asks for all of it. `gen < 0` is a -/// minor with no major progress at all, `0` a minor plus whatever major step -/// the accounting calls for, `1` that plus starting a cycle if none is running, -/// and `>= 2` a full collection. -pub fn gc_collect_gen(generation: i64) { - gc_sync::gc_op(|gc| gc.do_collect(generation)); -} - /// Whether a collection could still deliver a finalizer — whether /// `deal_with_objects_with_finalizers` has anything registered to pass over, or /// `rawrefcount` is live. diff --git a/majit/majit-gc/src/nursery.rs b/majit/majit-gc/src/nursery.rs index 50e98e678eb..990d296d60e 100644 --- a/majit/majit-gc/src/nursery.rs +++ b/majit/majit-gc/src/nursery.rs @@ -1,11 +1,11 @@ -/// Bump-pointer nursery allocator. -/// -/// A fixed-size memory region where young objects are allocated by -/// advancing a free pointer. When the nursery is full, a minor -/// collection copies live objects out. -/// -/// Layout: [header0|payload0|header1|payload1|...|free...top] -/// ^nursery_start ^free ^top +//! Bump-pointer nursery allocator. +//! +//! A fixed-size memory region where young objects are allocated by +//! advancing a free pointer. When the nursery is full, a minor +//! collection copies live objects out. +//! +//! Layout: [header0|payload0|header1|payload1|...|free...top] +//! ^nursery_start ^free ^top use std::alloc::{self, Layout}; use std::ptr; @@ -54,6 +54,72 @@ pub const TRANSLATION_NURSERY_SIZE: usize = 896 * 1024; /// on its own. pub const DEFAULT_NURSERY_SIZE: usize = 4 * 1024 * 1024; +/// Whether an arena can be made inaccessible on this target. +/// +/// `llarena.has_protect` is true for posix and nt and false for everything +/// else, which is what decides whether `post_setup` allocates the rotating +/// nurseries at all. +#[cfg(not(target_arch = "wasm32"))] +pub const HAS_PROTECT: bool = true; +/// See the posix/nt constant above; wasm32 is upstream's `else` arm. +#[cfg(target_arch = "wasm32")] +pub const HAS_PROTECT: bool = false; + +/// The granularity [`protect_arena`] works in. +#[cfg(not(target_arch = "wasm32"))] +fn page_size() -> usize { + region::page::size() +} +/// wasm32 never protects, so the value only has to be a plausible power of two. +#[cfg(target_arch = "wasm32")] +fn page_size() -> usize { + 65536 +} + +/// The whole pages a nursery of `size` bytes occupies. +/// +/// Protection is applied to whole pages, so an arena sharing its last page +/// with another allocation could not be made inaccessible without taking that +/// one with it. Rounding the request up is what makes the page ours to +/// protect. +fn arena_bytes(size: usize) -> usize { + let page = page_size(); + size.div_ceil(page) * page +} + +/// The layout every arena is allocated and freed with. +fn arena_layout(size: usize) -> Layout { + Layout::from_size_align(arena_bytes(size), page_size()).expect("invalid nursery layout") +} + +/// `llarena.arena_malloc(..., zero=True)` for one nursery-sized arena. +fn alloc_arena(size: usize) -> *mut u8 { + let layout = arena_layout(size); + let start = unsafe { alloc::alloc_zeroed(layout) }; + if start.is_null() { + alloc::handle_alloc_error(layout); + } + start +} + +/// `llarena.arena_protect`. +/// +/// A failure is dropped rather than reported, as `llimpl_protect` drops it +/// ("ignore potential errors"): the protection is a debugging aid and a host +/// that refuses it must still run the program. +#[cfg(not(target_arch = "wasm32"))] +fn protect_arena(start: *mut u8, size: usize, inaccessible: bool) { + let protection = if inaccessible { + region::Protection::NONE + } else { + region::Protection::READ_WRITE + }; + let _ = unsafe { region::protect(start, arena_bytes(size), protection) }; +} +/// wasm32 has no `arena_protect`; see [`HAS_PROTECT`]. +#[cfg(target_arch = "wasm32")] +fn protect_arena(_start: *mut u8, _size: usize, _inaccessible: bool) {} + /// Nursery memory region with bump-pointer allocation. /// /// incminimark.py:324-325 parity: nursery_free and nursery_top live in @@ -70,6 +136,14 @@ pub struct Nursery { /// llarena.py mode-3 parity: poison recycled nursery bytes so tests /// expose allocation paths that incorrectly rely on zero-filled memory. poison_on_reset: bool, + /// incminimark.py `debug_rotating_nurseries` — spare arenas, each + /// inaccessible while it waits its turn. + /// + /// Empty unless `PYPY_GC_DEBUG` asked for them. Their point is that a + /// pointer into a retired nursery faults when it is read, instead of being + /// answered by whichever object was later allocated over it: the reuse is + /// what hides a missing root, not the staleness. + rotating: Vec<*mut u8>, } // Safety: The nursery owns its memory exclusively and only one thread accesses it. @@ -80,11 +154,7 @@ impl Nursery { /// self.nursery_free = self.nursery /// self.nursery_top = self.nursery + self.nursery_size pub fn new(size: usize) -> Self { - let layout = Layout::from_size_align(size, 16).expect("invalid nursery layout"); - let start = unsafe { alloc::alloc_zeroed(layout) }; - if start.is_null() { - alloc::handle_alloc_error(layout); - } + let start = alloc_arena(size); let top = unsafe { start.add(size) }; let ptrs = Box::new(NurseryPtrs { free: start, top }); let poison_on_reset = std::env::var_os("MAJIT_GC_NURSERY_POISON").is_some(); @@ -93,7 +163,64 @@ impl Nursery { size, ptrs, poison_on_reset, + rotating: Vec::new(), + } + } + + /// incminimark.py `post_setup` — allocate `count` further arenas and + /// protect them, so [`Self::debug_rotate`] has a ring to draw from. + /// + /// Upstream allocates six. The count is a parameter only so a test can ask + /// for a shorter ring; a host reads it from `PYPY_GC_DEBUG`. + pub fn install_debug_rotating_nurseries(&mut self, count: usize) { + if !HAS_PROTECT { + return; + } + for _ in 0..count { + let arena = alloc_arena(self.size); + protect_arena(arena, self.size, true); + self.rotating.push(arena); + } + } + + /// incminimark.py `debug_rotate_nursery`. + /// + /// Retire the current arena to the back of the ring — inaccessible — and + /// take the one at the front. Reports whether a ring was installed. + /// + /// The caller owns the precondition upstream states by calling this only + /// where `nursery_barriers` is still empty: nothing may be living in the + /// retired arena, because reading it now faults. + pub fn debug_rotate(&mut self) -> bool { + if self.rotating.is_empty() { + return false; } + let old = self.start; + protect_arena(old, self.size, true); + let new = self.rotating.remove(0); + self.rotating.push(old); + protect_arena(new, self.size, false); + self.start = new; + // `debug_rotate_nursery` sets `nursery` and `nursery_top`, and the + // `nursery_free = nursery` its caller performs at the end of + // `_minor_collection` lands on the arena installed here. + self.ptrs.free = new; + self.ptrs.top = unsafe { new.add(self.size) }; + true + } + + /// How many spare arenas the rotation ring holds. + pub fn debug_rotating_nurseries(&self) -> usize { + self.rotating.len() + } + + /// `_minor_collection` under `gc_nursery_debug` resets the recycled range + /// in `arena_reset` mode 3, the one that fills it with garbage. + /// + /// Additive because the arena reads `MAJIT_GC_NURSERY_POISON` for itself: + /// either spelling selects the mode, and neither turns the other off. + pub fn set_nursery_debug(&mut self, on: bool) { + self.poison_on_reset |= on; } /// incminimark.py malloc_fixedsize parity: @@ -130,11 +257,27 @@ impl Nursery { /// sites initialize their own GC-pointer fields. Poison mode mirrors /// llarena.py mode 3 for detecting violations of that contract. /// - /// WASM-ONLY ADAPTATION: majit-backend-wasm/src/codegen.rs - /// documents that wasm skips the GC rewrite, so its JIT code has no - /// `clear_gc_fields` stores and still requires recycled nursery bytes to - /// be zero-filled. Delete this target branch once wasm runs the rewrite - /// or its inline allocation paths explicitly initialize GC fields. + /// WASM-ONLY ADAPTATION, paired with `MiniMarkGC::clear_nursery_substitute`. + /// The wasm backend runs no part of `GcRewriterImpl` — the omission is + /// total rather than selective by allocation shape — and lowers `New`, + /// `NewArray`, the `non_moving` old-gen routing and the write barrier in + /// its own codegen instead. Two of the pass's zeroing duties go with it: + /// the `clear_gc_fields` NULL stores that follow `handle_new`, and the + /// clear half of `NewArrayClear`, which wasm lowers exactly like + /// `NewArray` — `wasm_jit_alloc_array` stamps the length and nothing + /// else. Zero-filling the recycled bytes is what makes both hold. The + /// `ZeroArray` that pass would have emitted never arrives, and the wasm + /// codegen declines a trace carrying one rather than lean on this arm. + /// The rewrite module's other half, `remove_ref_constants`, does run on + /// wasm, so "skips the GC rewrite" names `GcRewriterImpl` and not the + /// module. + /// + /// Deleting this arm takes either the whole pass — which additionally + /// needs a `ZeroArray` lowering and a descr-carrying `GC_LOAD`/`GC_STORE` + /// lowering, the arm that panics today — or explicit initialization at + /// four sites: the `New` and `NewArray` inline nursery bumps and the + /// `wasm_jit_alloc` / `wasm_jit_alloc_array` helpers. + /// `clear_nursery_substitute` goes at the same time, not before. pub fn reset(&mut self) { self.reset_range(self.start as usize, self.start as usize + self.size); self.ptrs.free = self.start; @@ -253,8 +396,17 @@ impl Nursery { impl Drop for Nursery { fn drop(&mut self) { - let layout = Layout::from_size_align(self.size, 16).unwrap(); + let layout = arena_layout(self.size); + // Unprotect a parked arena before handing it back: the allocator + // writes its own bookkeeping into the block it reclaims, and an + // inaccessible one would fault inside `dealloc`. + for &arena in &self.rotating { + protect_arena(arena, self.size, false); + } unsafe { + for &arena in &self.rotating { + alloc::dealloc(arena, layout); + } alloc::dealloc(self.start, layout); } } @@ -264,6 +416,76 @@ impl Drop for Nursery { mod tests { use super::*; + /// Protection works in whole pages, so an arena that did not start on one + /// could not be made inaccessible without taking a neighbour with it. + #[test] + fn an_arena_starts_on_a_page_boundary() { + let nursery = Nursery::new(4096); + assert_eq!(nursery.start_ptr() as usize % page_size(), 0); + } + + /// `debug_rotate_nursery` hands out the ring's front arena and sends the + /// retired one to the back, so the count never changes and no address is + /// reused until the whole ring has turned. + #[test] + fn rotating_hands_out_a_fresh_arena_and_keeps_the_ring_full() { + if !HAS_PROTECT { + return; + } + let mut nursery = Nursery::new(4096); + nursery.install_debug_rotating_nurseries(2); + assert_eq!(nursery.debug_rotating_nurseries(), 2); + + // The JIT hardcodes these two addresses, so a rotation that moved them + // would leave compiled code bumping a pointer pair nothing reads. + let free_slot = nursery.free_addr(); + let top_slot = nursery.top_addr(); + + let first = nursery.start_ptr() as usize; + assert!(nursery.debug_rotate()); + let second = nursery.start_ptr() as usize; + assert_ne!(second, first, "a rotation must hand out a different arena"); + assert_eq!( + nursery.debug_rotating_nurseries(), + 2, + "the retired arena takes the place of the one taken" + ); + assert_eq!( + nursery.free_ptr() as usize, + second, + "the bump pointer follows the arena" + ); + assert_eq!(nursery.top_ptr() as usize, second + nursery.size()); + assert_eq!(nursery.free_addr(), free_slot, "the pointer pair stays put"); + assert_eq!(nursery.top_addr(), top_slot); + + assert!(nursery.debug_rotate()); + let third = nursery.start_ptr() as usize; + assert_ne!(third, second); + assert_ne!( + third, first, + "two spares means three arenas before a repeat" + ); + + assert!(nursery.debug_rotate()); + assert_eq!( + nursery.start_ptr() as usize, + first, + "and then the ring comes round" + ); + } + + /// `debug_rotate_nursery` opens with `if self.debug_rotating_nurseries:` — + /// without `PYPY_GC_DEBUG` there is nothing to rotate to and the arena + /// stays. + #[test] + fn rotating_without_a_ring_leaves_the_arena_alone() { + let mut nursery = Nursery::new(4096); + let before = nursery.start_ptr() as usize; + assert!(!nursery.debug_rotate()); + assert_eq!(nursery.start_ptr() as usize, before); + } + #[test] fn test_nursery_create() { let nursery = Nursery::new(4096); diff --git a/majit/majit-gc/src/oldgen.rs b/majit/majit-gc/src/oldgen.rs index 24fad5658a5..102b08744c5 100644 --- a/majit/majit-gc/src/oldgen.rs +++ b/majit/majit-gc/src/oldgen.rs @@ -205,6 +205,13 @@ impl OldGen { /// incminimark.py:1219-1221, the second half of `is_young_object`. #[inline] + /// Whether the young raw-malloced generation is empty. + /// + /// `debug_check_consistency` asserts it is, outside a minor collection. + pub fn young_rawmalloced_is_empty(&self) -> bool { + self.young_rawmalloced_objects.is_empty() + } + pub fn young_rawmalloced_contains(&self, obj_addr: usize) -> bool { !self.young_rawmalloced_payloads.is_empty() && self.young_rawmalloced_payloads.contains(&obj_addr) diff --git a/majit/majit-gc/src/shadow_stack.rs b/majit/majit-gc/src/shadow_stack.rs index c844ec41ff7..8451d026e1e 100644 --- a/majit/majit-gc/src/shadow_stack.rs +++ b/majit/majit-gc/src/shadow_stack.rs @@ -1,22 +1,23 @@ -/// Shadow stack for GC root tracking in compiled JIT code. -/// -/// RPython reference: rpython/jit/backend/llsupport/gc.py GcRootMap_shadowstack -/// -/// Two stacks: -/// 1. GcRef shadow stack — individual GC refs (legacy, for non-jitframe roots) -/// 2. JitFrame shadow stack — jitframe pointers (RPython _call_header_shadowstack) -/// -/// Protocol for jitframe shadow stack (assembler.py:1122-1136): -/// Entry: inline MOVs push [is_minor=1, jf_ptr] to root stack -/// Per-call: push_gcmap writes jf_gcmap; pop_gcmap clears it -/// GC: walk_jf_roots → read jf_gcmap → trace ref slots -/// Exit: pop_jf_to(depth) — _call_footer_shadowstack -/// -/// The jitframe shadow stack uses a per-thread flat memory array with a -/// root_stack_top pointer, matching RPython's per-thread ShadowStackPool. -/// Compiled code manipulates the current thread's root_stack_top with inline -/// load/store instructions (no function calls), exactly as in -/// assembler.py:1122-1136. +//! Shadow stack for GC root tracking in compiled JIT code. +//! +//! RPython reference: rpython/jit/backend/llsupport/gc.py GcRootMap_shadowstack +//! +//! Two stacks: +//! 1. GcRef shadow stack — individual GC refs (legacy, for non-jitframe roots) +//! 2. JitFrame shadow stack — jitframe pointers (RPython _call_header_shadowstack) +//! +//! Protocol for jitframe shadow stack (`_call_header_shadowstack` / +//! `_call_footer_shadowstack`): +//! Entry: inline MOVs push [is_minor=1, jf_ptr] to root stack +//! Per-call: push_gcmap writes jf_gcmap; pop_gcmap clears it +//! GC: walk_jf_roots → read jf_gcmap → trace ref slots +//! Exit: pop_jf_to(depth) — _call_footer_shadowstack +//! +//! The jitframe shadow stack uses a per-thread flat memory array with a +//! root_stack_top pointer, matching RPython's per-thread ShadowStackPool. +//! Compiled code manipulates the current thread's root_stack_top with inline +//! load/store instructions (no function calls), exactly as in +//! `_call_header_shadowstack`. use std::cell::{Cell, RefCell}; use std::sync::{Mutex, OnceLock, RwLock}; @@ -344,7 +345,17 @@ unsafe impl Send for MutatorEntry {} static MUTATOR_REGISTRY: Mutex> = Mutex::new(Vec::new()); /// Register the current thread's TLS root structures for STW root walks. -/// Unregistration is supplied by the caller's RAII destructor (pyre-jit's `GcMutatorRegistration` thread-local, armed in `init_gc_subsystem`, whose `Drop` calls [`unregister_mutator`]); callers must arm that pairing, while an API-level return guard is a tracked follow-up. +/// Unregistration is the caller's, and the pairing is armed rather than +/// returned: `pyre_interpreter::module::thread`'s `RuntimeThread` thread-local +/// calls [`unregister_mutator`] from its `Drop`, and `enter_runtime_thread` +/// arms it by touching `RUNTIME_THREAD` immediately after this call. That +/// order is what makes it correct — this function is where the thread first +/// touches all five root structures, so their destructors are registered +/// before `RuntimeThread`'s and therefore run after it, and the registry entry +/// naming them is gone before any of them is destroyed. A guard returned from +/// here would carry the pairing in the type system instead, but it would have +/// to be stored in a thread-local to survive the call, which is the same +/// arming with an extra step. pub fn register_mutator() { let thread_id = std::thread::current().id(); let shadow_stack = SHADOW_STACK.with(|stack| stack as *const _); @@ -560,13 +571,16 @@ pub fn push(gcref: GcRef) -> usize { /// needs. A plain `assert!` and not `debug_assert!` — this crate is /// extracted to LLBC, where debug assertions are compiled in. /// -/// TODO: Rust drops thread-locals in reverse order on -/// thread exit, and a TLS-owned `Drop` (e.g. `JitDriver`'s) may call -/// this during its own teardown. If -/// `SHADOW_STACK`'s destructor has already fired, `.with()` panics with -/// `AccessError`. RPython has no analogous hazard — the GIL thread does -/// not tear down TLS mid-run. Silently do nothing so the exiting thread -/// proceeds. +/// `try_with` and not `with`. Rust registers a thread-local's destructor when +/// the thread first touches it and runs the registered destructors in reverse, +/// so anything reached from inside another thread-local's destructor can run +/// after `SHADOW_STACK`'s own has fired, and `with` answers that with an +/// `AccessError` panic. Nothing pops from a thread-local destructor today — +/// the guards that pop are stack locals, and the one destructor that does run +/// at thread exit, `pyre_interpreter::module::thread`'s `RuntimeThread`, +/// unregisters the mutator rather than popping — so this is a standing +/// precaution and not a live path. RPython has no analogous hazard at all: +/// the GIL thread does not tear its thread-locals down mid-run. pub fn pop_to(depth: usize) { let _ = SHADOW_STACK.try_with(|ss| { let mut ss = ss.borrow_mut(); @@ -579,10 +593,20 @@ pub fn pop_to(depth: usize) { }); } -/// Like `pop_to` but silently no-ops if the shadow stack thread-local -/// is already torn down (program shutdown). Drop callers should use this -/// because the TLS drop order between `JitDriver` and `SHADOW_STACK` is -/// not deterministic in Rust. +/// [`pop_to`] without the balance assertion. +/// +/// Not the teardown-tolerant half of a pair: both reach the thread-local +/// through `try_with` and both no-op once it is destroyed. The assert is the +/// whole difference, and `Vec::truncate` saturates, so a depth above the +/// current one leaves the stack alone rather than shortening it. +/// +/// The callers that want it are all `Drop` impls — `FrameRoot` in both the +/// jit and the call-jit evaluator, `FrameAnchor`, and the `RootGuard`s inside +/// `gc_sync`'s `gc_op_with_root` and `gc_op_add_root` — where an assertion +/// that fails while an unwind is already running aborts the process and takes +/// the original panic's report with it. That is a choice per site, not a rule +/// about destructors: `ExportedState::release_roots` and +/// `Trace::release_roots` are `Drop` paths that keep the assert. pub fn try_pop_to(depth: usize) { let _ = SHADOW_STACK.try_with(|ss| { let mut ss = ss.borrow_mut(); @@ -773,7 +797,7 @@ pub fn walk_all_roots(mut visitor: impl FnMut(&mut GcRef)) { /// Current depth of the GcRef shadow stack. /// -/// TODO: see `pop_to` for the TLS-teardown rationale. +/// Answers through `try_with` for the reason [`pop_to`] gives. /// Returns 0 when the TLS has been destroyed; callers running under /// Drop (e.g. `ExportedState::release_roots`) observe an empty /// stack instead of panicking on the destroyed key. diff --git a/majit/majit-gc/src/trace.rs b/majit/majit-gc/src/trace.rs index b7b2a477c4d..5cfb1ec6442 100644 --- a/majit/majit-gc/src/trace.rs +++ b/majit/majit-gc/src/trace.rs @@ -1,28 +1,32 @@ -/// Object tracing for GC reference discovery. -/// -/// During collection, the GC needs to find all GC references within -/// a live object so it can update them (for copying collection) or -/// mark the targets (for mark-sweep). -/// -/// Instead of a closure-based trace approach (which causes lifetime issues -/// with the borrow checker), we use an offset-based approach: each type -/// declares the offsets of its GC pointer fields relative to the object -/// payload start. The collector reads/writes GcRef values at these offsets -/// directly. +//! Object tracing for GC reference discovery. +//! +//! During collection, the GC needs to find all GC references within +//! a live object so it can update them (for copying collection) or +//! mark the targets (for mark-sweep). +//! +//! Instead of a closure-based trace approach (which causes lifetime issues +//! with the borrow checker), we use an offset-based approach: each type +//! declares the offsets of its GC pointer fields relative to the object +//! payload start. The collector reads/writes GcRef values at these offsets +//! directly. use majit_ir::GcRef; /// One `gctypelayout.GCData.TYPE_INFO` entry of the materialized /// type-info group (gc.py:592, x86/assembler.py:1924-1943). /// /// Mirrors the shape of `TYPE_INFO` that `genop_guard_guard_is_object` -/// reads: `infobits` at offset 0 carries `T_IS_RPYTHON_INSTANCE`. The -/// rest of the struct is reserved so the size matches -/// `rffi.sizeof(GCData.TYPE_INFO) = 16` on 64-bit majit. (RPython's -/// 64-bit `TYPE_INFO` carries additional fields majit does not yet need -/// — `customdata`, `fixedsize`, `ofstoptrs` — totalling 32 bytes in the -/// C backend. majit only consults `infobits` from this struct and keeps -/// the other fields out of this layout; the remaining padding is still -/// present so the per-entry stride stays at a power of two.) +/// reads: `infobits` at offset 0 carries `T_IS_RPYTHON_INSTANCE`. +/// +/// This row is *smaller* than upstream's, not equal to it. `GCData.TYPE_INFO` +/// is four words — `infobits`, `customdata`, `fixedsize`, `ofstoptrs` — so 32 +/// bytes on 64-bit, and `VARSIZE_TYPE_INFO` extends it to eight; upstream's +/// per-entry size is not even uniform, since `get_type_id` allocates the +/// narrow struct for a fixed-size type and the wide one for a varsize type. +/// majit consults only `infobits` and keeps the other fields out of this +/// layout, so the row is one word of content. The reserved word is not there +/// to match a size: it is there so `TypeEntry`'s stride stays a power of two, +/// which is what lets the backend address the table by a shift. Deleting it +/// gives a stride of 24 and the shift no longer applies. #[repr(C)] #[derive(Copy, Clone, Default)] pub struct TypeInfoLayout { diff --git a/pyre/extra_tests/snippets/gc_get_count_reports_minors_since_the_major.py b/pyre/extra_tests/snippets/gc_get_count_reports_minors_since_the_major.py new file mode 100644 index 00000000000..f8dd07a3824 --- /dev/null +++ b/pyre/extra_tests/snippets/gc_get_count_reports_minors_since_the_major.py @@ -0,0 +1,40 @@ +# pyre-check: gate=1 +"""`gc.get_count`'s second element counts collections of the youngest generation. + +3.14's three elements are not the same kind of number: element 0 counts tracked +containers allocated and not yet freed, while elements 1 and 2 count +*collections* -- generation-0 collections since generation 1 was collected, and +generation-1 collections since generation 2 was. Collecting a generation zeroes +its own count and every younger one, so the counts move under `gc.collect(n)` +without depending on how much the script happened to allocate. + +Only element 1 is pinned here. Element 0 is an allocation count and element 2 +needs a generation-1-only collection, and an implementation is free to answer +either differently -- see the `get_count` entry in `module/gc` for what pyre +answers and why. +""" + +import gc + +counts = gc.get_count() +assert isinstance(counts, tuple), counts +assert len(counts) == 3, counts +assert all(isinstance(value, int) for value in counts), counts + +# The default generation is the oldest one, and collecting it zeroes every +# count below it as well. +gc.collect() +assert gc.get_count() == (0, 0, 0), gc.get_count() + +# One collection of the youngest generation is one thing for element 1 to +# count. Reading it back is what says the argument was honoured: a `collect` +# that ignored its generation and ran the oldest one would leave this at zero. +gc.collect(0) +assert gc.get_count()[1] == 1, gc.get_count() + +gc.collect(0) +assert gc.get_count()[1] == 2, gc.get_count() + +# ...and collecting the oldest generation zeroes it again. +gc.collect() +assert gc.get_count() == (0, 0, 0), gc.get_count() diff --git a/pyre/pyre-interpreter/src/builtins.rs b/pyre/pyre-interpreter/src/builtins.rs index a88775a2df1..3149ea5a47b 100644 --- a/pyre/pyre-interpreter/src/builtins.rs +++ b/pyre/pyre-interpreter/src/builtins.rs @@ -17285,7 +17285,12 @@ pub(crate) unsafe fn fileio_writebuf( let Ok(view) = w_memoryview_new_with_flags(obj, 0x0001) else { return Err(type_error(obj)); }; - let _ = pyre_object::gc_roots::pin_root(view); + // The exporter's own `bf_getbuffer` ran to build this view, and + // the recursive call can run more Python, so the view is pinned + // across it -- with a bracket, since a pin with nothing to pop it + // leaves the slot on the shadow stack for good. + let _roots = pyre_object::gc_roots::push_roots(); + let view = pyre_object::gc_roots::pin_root(view); let (data, owner, _) = fileio_writebuf(view)?; return Ok((data, owner, true)); } diff --git a/pyre/pyre-interpreter/src/eval.rs b/pyre/pyre-interpreter/src/eval.rs index d8fda93f9c7..1b626748860 100644 --- a/pyre/pyre-interpreter/src/eval.rs +++ b/pyre/pyre-interpreter/src/eval.rs @@ -2223,9 +2223,11 @@ fn eval_loop(frame: &mut PyFrame) -> PyResult { // user code (a side-effecting getter / dunder / module top level). crate::call::bump_frame_entry_count(); // Count this interpreter activation so the JIT eval loop's GC safepoint - // fires only at the outermost activation (PYRE_GC_INTERP root-completeness): - // a nested `eval_loop_jit` running under this one observes depth > 1 and - // skips collection. No-op when the flag is off. + // stops firing once the nesting is deep enough to hide a root + // (PYRE_GC_INTERP root-completeness): module level and one called + // function's loop still collect, and an `eval_loop_jit` re-entered from + // inside an opcode handler observes depth >= 3 and skips collection. + // No-op when the flag is off. let _eval_activation = pyre_object::gc_interp::EvalActivationGuard::enter(); if _eval_activation.armed() { // Publish the process-stable configuration in the breaker word once diff --git a/pyre/pyre-interpreter/src/module/gc/mod.rs b/pyre/pyre-interpreter/src/module/gc/mod.rs index f55394b2d35..670a11e522e 100644 --- a/pyre/pyre-interpreter/src/module/gc/mod.rs +++ b/pyre/pyre-interpreter/src/module/gc/mod.rs @@ -2,6 +2,15 @@ //! //! Partial port of `interp_gc.py`. Explicit collection runs the complete //! RPython collection, then drains the finalizer queue synchronously. +//! +//! Part of this module answers to 3.14 alone. `moduledef.py` binds no +//! `get_count`, `set_threshold`/`get_threshold`, `set_debug`/`get_debug` or +//! `freeze`/`unfreeze`/`get_freeze_count`, so those have no implementation to +//! follow and each one below states what it answers and why. Nothing grades +//! them either: `test_gc` is an implementation-detail module on both axes — +//! `lib-python/conftest.py`'s testmap skips it and `cpython_tests/run.py` +//! carries that skip forward — so the assertions that would pin these live in +//! `extra_tests/snippets/` instead. use pyre_object::*; use rustpython_wtf8::Wtf8; @@ -17,18 +26,38 @@ pub mod hook; /// call so callers that toggle and re-read the state stay consistent. static GC_ENABLED: AtomicBool = AtomicBool::new(true); -/// The collector debug word. PyPy does not expose this frontend knob, but -/// the 3.14 observable contract requires the value to be interpreter-owned and -/// shared by all threads. The moving collector has no -/// refcount-cycle diagnostic stream to toggle; the word is nevertheless kept -/// exactly so callers can bracket a collection and restore the prior flags. +/// The collector debug word. +/// +/// `[3.14-spec]` PyPy exposes no such knob, and 3.14 requires the value to be +/// interpreter-owned and shared by all threads, so the word is kept exactly: +/// a caller can bracket a collection and restore the prior flags. It drives +/// nothing, and `DEBUG_SAVEALL` is the flag that shows why. 3.14 retains what +/// the *cyclic* collector found unreachable, which is a small set precisely +/// because refcounting already reclaimed the acyclic garbage before it ran. +/// Here there is no refcount, so the population reaching the sweep's +/// free-or-keep callback is everything that died since the last major — +/// hundreds of objects across a dozen type ids inside the single collection +/// `test_saveall` brackets, where it expects one — and no filter at that +/// callback can recover the distinction, because a would-this-have-died-by- +/// refcount answer is never computed. The remaining flags describe a +/// per-object cycle report this collector likewise does not produce. static GC_DEBUG: AtomicI64 = AtomicI64::new(0); -/// The collection thresholds `gc.get_threshold()` reports. pyre's collector -/// has no generational allocation counters to drive, so the values are only -/// remembered: `set_threshold` stores what it was given and `get_threshold` -/// hands the same tuple back, which is the part of the pair's behaviour a -/// caller can observe. All three are kept, including the third, whose round +/// The collection thresholds `gc.get_threshold()` reports. +/// +/// `[3.14-spec]` A remembered round trip, where PyPy binds no threshold +/// surface at all. The values drive nothing, and the reason is a unit +/// mismatch rather than an absence: what schedules a collection here is a byte +/// reading — `get_total_memory_used` against `next_major_collection_threshold` +/// — while `threshold0` is a count of container allocations, and the only +/// knob retunable after construction, `set_max_heap_size`, is a byte ceiling +/// too. An old-gen live-*object* count does exist (`live_objects`, kept by +/// the arena collection), so the honest statement is that no knob shares +/// `threshold0`'s unit, not that nothing is counted. Pointing a count at a +/// byte knob would silently mean something neither 3.14 nor PyPy means. So +/// `set_threshold` stores what it was given and `get_threshold` hands the same +/// tuple back, which is the part of the pair's behaviour a caller can +/// observe. All three are kept, including the third, whose round /// trip 3.14 preserves even though its own incremental collector sizes no /// third generation. The initial values are the ones a fresh interpreter /// starts with. @@ -1389,14 +1418,19 @@ crate::py_module! { }, inline_functions: { fn collect( - #[default(w_int_new(0))] generation: PyObjectRef, + #[default(w_int_new(NUM_GENERATIONS - 1))] generation: PyObjectRef, ) -> Result { - // `interp_gc.py collect` unwraps the optional generation as an - // int and ignores its value, because PyPy's frontend has no - // generations to select between. This one reports three, so the - // argument is bounded the way `gc_collect_impl` bounds it; the - // value is still ignored below, since every collection here is a - // full one. + // `interp_gc.py collect` unwraps the optional generation as an int + // and then ignores it, because the frontend it belongs to has no + // generations to select between. This one does: `NUM_GENERATIONS` + // publishes the mapping, `get_objects` already selects on it, and + // `get_count` reports per generation. So the argument is bounded + // the way `gc_collect_impl` bounds it and then passed on to + // `incminimark.py collect(gen)`, whose generations are the same + // ones -- a minor at 0, a started major at 1, a full major at 2. + // + // The default is the oldest generation, so a bare `gc.collect()` + // is the full collection it has always been. let generation = crate::baseobjspace::int_w( crate::baseobjspace::space_index(generation)?, )?; @@ -1405,7 +1439,7 @@ crate::py_module! { } crate::baseobjspace::clear_method_cache(); crate::objspace::std::mapdict::clear_map_attr_cache(); - pyre_object::gc_hook::try_gc_collect(); + pyre_object::gc_hook::try_gc_collect(generation); run_finalizers_now(); run_cpyext_deallocs_now(); // The return value is the caller-observable axis and is an int. @@ -1654,8 +1688,38 @@ crate::py_module! { .map(|slot| w_int_new(slot.load(Ordering::Relaxed))) .collect(), )), + // `gc_get_count_impl` reads three fields and only the first is an + // object count: element 0 is tracked-container allocations minus + // deallocations since generation 0 was collected, while elements 1 and + // 2 count *collections* -- generation-0 collections since generation 1 + // was collected, and generation-1 collections since generation 2 was. + // Collecting a generation zeroes its own count and every younger one. + // + // Under the generation mapping `NUM_GENERATIONS` already publishes -- + // 0 is the nursery, 1 the generation this collector keeps empty, 2 + // what is not in the nursery -- a minor collection is the generation-0 + // one and a major collects both older generations at once. So element + // 1 is the minors run since the last major, which is what `collect(0)` + // moves. Element 2 is exact at zero: `collect(1)` is `collect(0)` plus + // "start the major now if one is not already running", so asking for + // the middle generation runs no collection of its own for element 2 to + // count -- there is no middle generation holding anything to reclaim. + // + // Element 0 stays zero because no counter can be truthful here. The + // allocation seam is keyed by a majit type id and nothing else + // (`try_gc_alloc(type_id, payload_size)`), and the tracked predicate is + // not a function of that key -- `cpython_object_is_gc` reaches the + // object's type and, for a type object, the object itself, so one type + // id covers both a tracked heap type and an untracked static one. + // Even a decidable bit would undercount: every backend emits the + // nursery bump inline and merges several objects into one, so compiled + // code allocates without passing any counter site, and a virtualized + // allocation is removed outright. Counting by walking instead is what + // `gc.get_objects` costs, four orders of magnitude above this call. "get_count" / 0 = |_| Ok(w_tuple_new(vec![ - w_int_new(0), w_int_new(0), w_int_new(0), + w_int_new(0), + w_int_new(majit_gc::active_minor_collections_since_major() as i64), + w_int_new(0), ])), "get_debug" / 0 = |_| Ok(w_int_new(GC_DEBUG.load(Ordering::Relaxed))), "set_debug" / 1 = |args| { @@ -1727,11 +1791,18 @@ crate::py_module! { "is_finalized" / 1 = |args| Ok(w_bool_from( majit_gc::gc_finalizer_has_run(args[0] as usize), )), - // CPython 3.14 `gc.freeze()` moves the surviving objects into a + // `[3.14-spec]` `gc.freeze()` moves the surviving objects into a // permanent generation that later collections skip; it is a pre-fork - // hint, not a semantic guarantee. The collector has no permanent - // generation, so freezing and unfreezing are no-ops and the frozen - // count is always the truthful zero. + // hint, not a semantic guarantee. PyPy binds none of the three. The + // collector has no permanent generation, so freezing and unfreezing + // are no-ops and the frozen count is the truthful zero. + // + // Rooting the live set instead would not be that operation under + // another name: a frozen object in 3.14 is skipped by the cyclic + // collector but still reclaimed by refcount, while a rooted one is + // immortal until `unfreeze` and has its `__del__` deferred until then. + // That is a third behaviour, matching neither side, and the whole live + // set is what it would apply to. "freeze" / 0 = |_| Ok(w_none()), "unfreeze" / 0 = |_| Ok(w_none()), "get_freeze_count" / 0 = |_| Ok(w_int_new(0)), diff --git a/pyre/pyre-interpreter/src/module/sys/vm.rs b/pyre/pyre-interpreter/src/module/sys/vm.rs index ea694dd08c3..15a274ba1dd 100644 --- a/pyre/pyre-interpreter/src/module/sys/vm.rs +++ b/pyre/pyre-interpreter/src/module/sys/vm.rs @@ -3597,9 +3597,9 @@ fn error_is_exception(err: &crate::PyError) -> bool { ); if !err.exc_object.is_null() && !w_exception.is_null() { let _roots = pyre_object::gc_roots::push_roots(); - let _ = pyre_object::gc_roots::pin_root(err.exc_object); + let exc_object = pyre_object::gc_roots::pin_root(err.exc_object); let w_exception = pyre_object::gc_roots::pin_root(w_exception); - return crate::baseobjspace::isinstance(err.exc_object, w_exception).unwrap_or(false); + return crate::baseobjspace::isinstance(exc_object, w_exception).unwrap_or(false); } // `exc_kind_matches(kind, "Exception")` spelled over `PyErrorKind`: every // variant descends from `Exception` except the two `BaseException` ones. diff --git a/pyre/pyre-jit/src/eval.rs b/pyre/pyre-jit/src/eval.rs index 85ab8e986ed..e264a58589b 100644 --- a/pyre/pyre-jit/src/eval.rs +++ b/pyre/pyre-jit/src/eval.rs @@ -112,16 +112,17 @@ unsafe fn pyre_object_gc_alloc_collecting_rooted_trampoline( } } -/// `gc.collect()` (interp_gc.py) trampoline. Bridges -/// pyre-object's `try_gc_collect` to `majit_gc::collect_full`, which -/// fans out to the active backend's `dynasm_collect_full` / -/// `collect_full_via_active_runtime`. pyre-object intentionally has +/// `gc.collect(n)` (interp_gc.py) trampoline. Bridges pyre-object's +/// `try_gc_collect` to `majit_gc::collect_generation`, which fans out to the +/// active backend's `dynasm_collect_generation` / +/// `collect_generation_via_active_runtime`. pyre-object intentionally has /// no majit-gc dep, hence the indirection lives here. /// /// # Safety hazard (documented gap) /// -/// `do_collect_full` always runs a minor cycle first; the nursery is -/// moving. Any live PyObjectRef held on the Rust stack of the +/// Every generation runs a minor cycle — `do_collect_full` starts with one +/// and generation 0 is one — and the nursery is moving. Any live +/// PyObjectRef held on the Rust stack of the /// bytecode interpreter that is NOT registered as a GC root (via /// `pyframe_root_walker` / shadow stack / `try_gc_add_root`) will /// dangle after collection. pyre's interpreter has no shadowstack @@ -130,8 +131,8 @@ unsafe fn pyre_object_gc_alloc_collecting_rooted_trampoline( /// segfault on the next memory access. The trampoline is wired up, but /// safe enablement is not yet implemented: it requires a shadowstack /// pass that registers every live PyObjectRef as a GC root. -fn pyre_object_gc_collect_trampoline() { - majit_gc::collect_full(); +fn pyre_object_gc_collect_trampoline(generation: i64) { + majit_gc::collect_generation(generation); } fn pyre_object_gc_collect_step_trampoline() -> (u8, u8) { diff --git a/pyre/pyre-object/src/dictmultiobject.rs b/pyre/pyre-object/src/dictmultiobject.rs index 9b6ca13f68f..9d71cb28db7 100644 --- a/pyre/pyre-object/src/dictmultiobject.rs +++ b/pyre/pyre-object/src/dictmultiobject.rs @@ -2796,7 +2796,7 @@ unsafe fn scan_dict_key_reentrant( let stored_slot = crate::gc_roots::shadow_stack_len(); let stored_obj = crate::gc_roots::pin_root(stored_obj); let key_slot = crate::gc_roots::shadow_stack_len(); - let _ = crate::gc_roots::pin_root(key.obj); + key.obj = crate::gc_roots::pin_root(key.obj); let equal = dict_keys_equal(stored_obj, key.obj); obj = crate::gc_roots::shadow_stack_get(obj_slot); diff --git a/pyre/pyre-object/src/gc_hook.rs b/pyre/pyre-object/src/gc_hook.rs index 252bdb6a994..5be7bdddce9 100644 --- a/pyre/pyre-object/src/gc_hook.rs +++ b/pyre/pyre-object/src/gc_hook.rs @@ -198,7 +198,10 @@ majit_gc::global_hook!(static GC_ALLOC_COLLECTING_HOOK: GcAllocHookFn); /// Unlike [`register_gc_alloc_hook`] (no-collect), the backend routes this to a /// nursery allocator that runs a minor collection when the nursery is full. Only /// for callers that hold no unrooted GC pointer across the allocation and run at -/// a JIT safepoint (gcmap-rooted) — i.e. the elidable bigint payload helpers. +/// a JIT safepoint (gcmap-rooted). The elidable bigint payload helpers were the +/// first; the rooted sibling now also carries every list header +/// (`w_list_new_with_strategy`), `w_weakref_new`, and builtin `str()`'s +/// `w_str_from_wtf8_managed_collecting`. pub fn register_gc_alloc_collecting_hook(hook: GcAllocHookFn) { GC_ALLOC_COLLECTING_HOOK.set(Some(hook)); } @@ -260,30 +263,31 @@ pub unsafe fn try_gc_alloc_collecting_rooted( Some(result) } -/// Signature of the host-side full-collection callback. Used by +/// Signature of the host-side collection callback. Used by /// `pypy/module/gc/interp_gc.py collect` ports — i.e. user-level -/// `gc.collect()` reaches the live GC through this hook. -pub type GcCollectHookFn = fn(); +/// `gc.collect(n)` reaches the live GC through this hook, carrying the +/// generation it was asked for. +pub type GcCollectHookFn = fn(i64); majit_gc::global_hook!(static GC_COLLECT_HOOK: GcCollectHookFn); -/// Install the full-collection callback. Overwrites any previously-installed -/// hook. +/// Install the collection callback. Overwrites any previously-installed hook. pub fn register_gc_collect_hook(hook: GcCollectHookFn) { GC_COLLECT_HOOK.set(Some(hook)); } -/// Remove the full-collection callback. +/// Remove the collection callback. pub fn clear_gc_collect_hook() { GC_COLLECT_HOOK.set(None); } -/// Trigger a full mark-sweep collection via the installed hook. No-op -/// when no hook is installed. +/// Run `incminimark.py collect(gen)` via the installed hook: a minor at +/// generation 0, a started major at 1, a full major at 2 and above. No-op when +/// no hook is installed. #[majit_macros::dont_look_inside] -pub fn try_gc_collect() { +pub fn try_gc_collect(generation: i64) { if let Some(f) = GC_COLLECT_HOOK.get() { - f(); + f(generation); } } diff --git a/pyre/pyre-object/src/gc_interp.rs b/pyre/pyre-object/src/gc_interp.rs index 09c93e6bf39..9b9a2007f9b 100644 --- a/pyre/pyre-object/src/gc_interp.rs +++ b/pyre/pyre-object/src/gc_interp.rs @@ -291,9 +291,10 @@ pub fn would_collect() -> bool { /// the suspended frame's live refs by expanding each jitframe on the JF shadow /// stack through `trace_libc_jitframe`. That is the same basis on which /// upstream collects with compiled frames on the stack. The safepoint fires -/// only at the outermost eval activation ([`at_outermost_activation`]) so a -/// Python callback nested inside native module code — whose Rust-stack roots -/// the pyframe walker cannot see — never triggers it. +/// at the first two eval activations and no deeper +/// ([`at_outermost_activation`]), so a Python callback re-entered from inside +/// an opcode handler — whose Rust-stack roots the pyframe walker cannot see — +/// never triggers it. /// /// Dispatches to the installed threshold and collection hooks, neither a /// build-time constant, so the JIT residualizes the call instead of tracing diff --git a/pyre/pyre-object/src/gc_roots.rs b/pyre/pyre-object/src/gc_roots.rs index 6ed01ac9d1a..541c61e62bc 100644 --- a/pyre/pyre-object/src/gc_roots.rs +++ b/pyre/pyre-object/src/gc_roots.rs @@ -31,22 +31,42 @@ //! // pop_roots(livevars). //! ``` //! -//! ## Phase plan +//! ## What this scaffold stands in for //! -//! - **Phase 2a** — no-op stub. The API surface existed but the body -//! was empty. -//! - **Phase 2b** — TLS shadow-stack body. [`push_roots`] snapshots -//! the thread-local shadow stack length into a [`RootScope`]; -//! [`pin_root`] appends a [`PyObjectRef`]; [`Drop`] truncates the -//! stack back to the saved length. Mirrors -//! `rpython/memory/gctransform/shadowstack.py walk_stack_root`. -//! - **Phase 2c (this commit)** — expose [`walk_shadow_stack`] -//! so the backend GC can visit pinned roots during nursery -//! collection. `pyre-jit::eval` registers a thin -//! `pyre-object`-to-`majit-gc` adapter through -//! `majit_gc::shadow_stack::register_extra_root_walker`; pinned -//! pointers are now observable to the active `MiniMarkGC` -//! instance and survive across collections. +//! Upstream writes no bracket by hand. The transformer inserts +//! `push_roots` / `pop_roots` around the operations that can collect, +//! across the whole translated graph, so an author cannot forget one and a +//! reviewer never has to look for it. pyre has no such pass over Rust +//! code: every bracket is written by hand, and their count is the size of +//! what an automatic transform would replace. From the repo root, +//! excluding this module: +//! +//! ```text +//! rg -o 'push_roots\(\)' --glob '!target' --glob '!**/gc_roots.rs' pyre/ majit/ | wc -l # 1376 +//! rg -o 'pin_root\(' --glob '!target' --glob '!**/gc_roots.rs' pyre/ majit/ | wc -l # 2604 +//! rg -o 'shadow_stack_get\(' --glob '!target' --glob '!**/gc_roots.rs' pyre/ majit/ | wc -l # 3571 +//! rg -l 'push_roots\(\)' --glob '!target' --glob '!**/gc_roots.rs' pyre/ majit/ | wc -l # 163 +//! ``` +//! +//! A forgotten bracket is not a compile error, so nothing downstream +//! assumes the set is complete. `crate::gc_interp` allocates interpreter +//! boxes born-old through `try_gc_alloc_stable`, and the safepoint major +//! is `MiniMarkGC::do_collect_oldgen_nonmoving`, which leaves the nursery +//! byte-for-byte intact. Both exist because an unrooted [`PyObjectRef`] on +//! the Rust stack of a bytecode handler would dangle across a moving +//! minor. Ordinary moving-nursery allocation for the interpreter waits on +//! the pass, not on this module. +//! +//! ## State +//! +//! [`push_roots`] snapshots the thread-local shadow-stack length into a +//! [`RootScope`]; [`pin_root`] appends a [`PyObjectRef`]; [`Drop`] +//! truncates back to the saved length (`shadowstack.py walk_stack_root`). +//! [`walk_shadow_stack`] hands the pinned roots to the collector: +//! `pyre-jit::eval` registers a `pyre-object`-to-`majit-gc` adapter +//! through `majit_gc::shadow_stack::register_extra_root_walker`, so a +//! pinned pointer is visible to the active `MiniMarkGC` and is read back +//! forwarded. use std::alloc::{Layout, alloc_zeroed, dealloc}; use std::cell::Cell; diff --git a/pyre/pyre-object/src/setobject.rs b/pyre/pyre-object/src/setobject.rs index b1e346358d3..0b31c5ad1b9 100644 --- a/pyre/pyre-object/src/setobject.rs +++ b/pyre/pyre-object/src/setobject.rs @@ -507,7 +507,7 @@ unsafe fn scan_set_key_reentrant( let stored_slot = crate::gc_roots::shadow_stack_len(); let stored_obj = crate::gc_roots::pin_root(stored_obj); let key_slot = crate::gc_roots::shadow_stack_len(); - let _ = crate::gc_roots::pin_root(key.obj); + key.obj = crate::gc_roots::pin_root(key.obj); let equal = crate::dictmultiobject::dict_keys_equal(stored_obj, key.obj); let stored_obj = crate::gc_roots::shadow_stack_get(stored_slot); @@ -1002,10 +1002,10 @@ unsafe fn w_set_contains_key_for_update( if stored.hash == key.hash { let _roots = crate::gc_roots::push_roots(); let stored_slot = crate::gc_roots::shadow_stack_len(); - let _ = crate::gc_roots::pin_root(stored.obj); + let stored_obj = crate::gc_roots::pin_root(stored.obj); let key_slot = crate::gc_roots::shadow_stack_len(); - let _ = crate::gc_roots::pin_root(key.obj); - let equal = crate::dictmultiobject::dict_keys_equal(stored.obj, key.obj); + key.obj = crate::gc_roots::pin_root(key.obj); + let equal = crate::dictmultiobject::dict_keys_equal(stored_obj, key.obj); let stored_obj = crate::gc_roots::shadow_stack_get(stored_slot); key.obj = crate::gc_roots::shadow_stack_get(key_slot); if crate::dictmultiobject::take_dict_key_error() { @@ -1069,10 +1069,10 @@ unsafe fn w_set_remove_key_for_update( if stored.hash == key.hash { let _roots = crate::gc_roots::push_roots(); let stored_slot = crate::gc_roots::shadow_stack_len(); - let _ = crate::gc_roots::pin_root(stored.obj); + let stored_obj = crate::gc_roots::pin_root(stored.obj); let key_slot = crate::gc_roots::shadow_stack_len(); - let _ = crate::gc_roots::pin_root(key.obj); - let equal = crate::dictmultiobject::dict_keys_equal(stored.obj, key.obj); + key.obj = crate::gc_roots::pin_root(key.obj); + let equal = crate::dictmultiobject::dict_keys_equal(stored_obj, key.obj); let stored_obj = crate::gc_roots::shadow_stack_get(stored_slot); key.obj = crate::gc_roots::shadow_stack_get(key_slot); if crate::dictmultiobject::take_dict_key_error() { diff --git a/pyre/pyrex/src/lib.rs b/pyre/pyrex/src/lib.rs index ff4c58c5956..6cb0f6f5eb9 100644 --- a/pyre/pyrex/src/lib.rs +++ b/pyre/pyrex/src/lib.rs @@ -1532,7 +1532,7 @@ fn run_atexit_callbacks(canonical: pyre_object::PyObjectRef, ec_ptr: *const PyEx /// bumped `finalizer_trigger_count` is that sweep reporting what it found. fn collect_and_run_finalizers(ec_ptr: *const PyExecutionContext) -> bool { let triggers_before = pyre_interpreter::executioncontext::finalizer_trigger_count(); - pyre_object::gc_hook::try_gc_collect(); + pyre_object::gc_hook::try_gc_collect(2); let made_finalizable = pyre_interpreter::executioncontext::finalizer_trigger_count() != triggers_before; if !ec_ptr.is_null() {