gc: keep short-lived lists young and match PyPy shutdown - #1158
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
WalkthroughThis change makes list allocation, mutation, and draining safe across object relocation. It updates namespace-cell folding for movable values. Runtime shutdown removes forced collection and global finalizers. Validation records reflect updated results. ChangesMovable list GC handling
JIT namespace-cell folding
Runtime shutdown
Validation records
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Runtime
participant Threads
participant Atexit
participant Streams
Runtime->>Threads: join threads
Runtime->>Atexit: run atexit callbacks
Runtime->>Runtime: mark finalization
Runtime->>Streams: flush streams
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/ae162219abf4334f44f6b9b3823eae15041cb4a5/pyre-object/src/listobject.rs#L990
Once this allocator returns nursery-resident headers, existing mutation paths can retain stale receiver pointers when a backing-store growth triggers collection. For example, object_push/object_insert call w_list_grow_items_block, which roots and relocates the list internally but returns only the relocated element; their pre-call self reference is then used for the item store, length update, and barrier. This was safe with the old stable header allocation, but under nursery pressure an append or insert at capacity can now write through the evacuated address, corrupting the list or causing memory unsafety. Porting the movable allocation therefore also requires threading/reloading the relocated list through every allocating mutator.
AGENTS.md reference: AGENTS.md:L231-L233
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit 0b49f0d). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patchNone. 2. Other mismatches introduced by this patch
3. Pre-existing mismatches (already present before this patch)
4. Structural adaptations
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pyre/pyre-object/src/listobject.rs (2)
997-1018: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winReload
items_blockbefore the null fallback.If
try_gc_alloc_collecting_rootedperforms a collection and returns null, Lines 1007-1018 store the pre-collectionitems_blockaddress in the boxed list. Line 1022 reloads the shadow-stack slot only on the non-null path. The fallback can retain a stale moving-GC pointer.Move the
block_rootreload before theraw.is_null()branch.Proposed fix
let ListStorage { int_items, float_items, .. } = storage; + if let Some(s) = block_root { + items_block = crate::gc_roots::shadow_stack_get(s) as *mut ItemsBlock; + } if raw.is_null() { let boxed = Box::new(W_ListObject { ob_header: header, allocated: items.len() as isize, length, items: items_block, strategy, int_items, float_items, w_slots: PY_NULL, }); return Box::into_raw(boxed) as PyObjectRef; } - if let Some(s) = block_root { - items_block = crate::gc_roots::shadow_stack_get(s) as *mut ItemsBlock; - }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyre/pyre-object/src/listobject.rs` around lines 997 - 1018, Move the `block_root` reload to occur before the `raw.is_null()` branch in the list construction flow, so the fallback `W_ListObject` uses the post-collection `items_block` pointer. Keep the existing non-null allocation path unchanged apart from removing its now-redundant reload.
937-956: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the allocation comments.
Line 937 names
alloc_list_items_block, but this constructor callsalloc_list_items_block_gcat Line 899. Lines 948-951 also describetry_gc_alloc_stableas the header path. The header now usestry_gc_alloc_collecting_rooted. Update the full comment block to describe the current relocation boundary.Proposed comment correction
- // below (`alloc_list_items_block`, the collecting header allocation) so the + // below (`alloc_list_items_block_gc`, the collecting header allocation) so the - // `try_gc_alloc_stable` header alloc — the only allocation that can - // relocate it, since the typed-block allocs precede it. + // collecting header allocation. That allocation can relocate the rooted + // nursery block after the typed-block allocations complete.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyre/pyre-object/src/listobject.rs` around lines 937 - 956, Update the allocation comments surrounding build_list_storage and the W_ListObject header allocation to reference alloc_list_items_block_gc, which is called by this constructor, and try_gc_alloc_collecting_rooted, which is the current collecting header path. Revise the relocation-boundary description to match the actual allocation order and behavior, without changing the pinning logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@pyre/pyre-object/src/listobject.rs`:
- Around line 997-1018: Move the `block_root` reload to occur before the
`raw.is_null()` branch in the list construction flow, so the fallback
`W_ListObject` uses the post-collection `items_block` pointer. Keep the existing
non-null allocation path unchanged apart from removing its now-redundant reload.
- Around line 937-956: Update the allocation comments surrounding
build_list_storage and the W_ListObject header allocation to reference
alloc_list_items_block_gc, which is called by this constructor, and
try_gc_alloc_collecting_rooted, which is the current collecting header path.
Revise the relocation-boundary description to match the actual allocation order
and behavior, without changing the pinning logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3a07fb2a-9097-4197-a852-ec83c3e8f552
📒 Files selected for processing (5)
pyre/pyre-interpreter/src/baseobjspace.rspyre/pyre-jit/tests/gc_stress.rspyre/pyre-object/src/listobject.rspyre/pyre-object/src/lltype.rspyre/pyrex/src/lib.rs
ae16221 to
47542a7
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47542a755b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| crate::gc_hook::try_gc_alloc_collecting_rooted( | ||
| W_LIST_GC_TYPE_ID, | ||
| W_LIST_OBJECT_SIZE, | ||
| &mut allocation_root, | ||
| &mut needs_write_barrier, | ||
| ) |
There was a problem hiding this comment.
Allocating every W_ListObject in the moving nursery invalidates the existing mutation paths that retain a raw header reference across GC safepoints. For example, w_list_append creates list before calling w_list_append_inner and then calls list.sync_allocated; the inner path calls barriers and may resize or switch strategies, all of which can park for a concurrent minor collection that forwards the rooted obj while leaving list pointing at the evacuated header. The subsequent writes then target stale memory, so ordinary list mutation can corrupt the heap under free-threaded collection. The movable-header change therefore needs the corresponding GC-transformed refactor throughout list operations: retain/reload the owner slot after every possible safepoint rather than carrying &mut W_ListObject across it.
AGENTS.md reference: AGENTS.md:L231-L233
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs`:
- Around line 8974-8982: Validate the movable-pointer folding change in
emit_namespace_cell_fold by running cargo check --features dynasm, cargo test
--features dynasm, and all eight benchmarks. Record any benchmark regressions
and retain the parity-correct implementation unless validation reveals a
functional failure.
In `@pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs`:
- Around line 12615-12617: Update the comment above emit_module_dict_cell_fold
to remove IntMutableCell from the unfoldable-case description and describe only
null or strategy-switched entries or failed guards. Preserve the presence check
near the builtins fallback so any present global continues to shadow builtins.
In `@pyre/pyre-object/src/listobject.rs`:
- Around line 989-998: Reload the rooted items block before branching on the
allocation result: move the block_root reload in the list allocation flow ahead
of the raw.is_null() fallback check. Ensure the fallback boxed header uses the
reloaded items_block after try_gc_alloc_collecting_rooted may collect, while
preserving the existing successful-allocation path.
In `@pyre/pyrex/src/lib.rs`:
- Around line 1127-1128: Update finalize_runtime to iterate the existing shared
built-in module owner, invoking each started module’s shutdown hook before
pyre_interpreter::module::_io::flush_all_streams(). Preserve the RPython/PyPy
shutdown order and storage semantics, and do not add a separate registry.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d37b6080-3ec6-49a2-9848-28271e71f004
📒 Files selected for processing (7)
pyre/pyre-interpreter/src/baseobjspace.rspyre/pyre-jit-trace/src/jitcode_dispatch/mod.rspyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rspyre/pyre-jit/tests/gc_stress.rspyre/pyre-object/src/listobject.rspyre/pyre-object/src/lltype.rspyre/pyrex/src/lib.rs
| // `emit_module_dict_cell_fold` returns `false` for BOTH an absent name and | ||
| // a present-but-unfoldable one (`IntMutableCell` / movable / strategy | ||
| // switched). Only an ABSENT name may fall through to the builtins fold — a | ||
| // a present-but-unfoldable one (`IntMutableCell` / strategy switched). | ||
| // Only an ABSENT name may fall through to the builtins fold — a |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove IntMutableCell from the unfoldable-case description.
pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs now passes every non-null stored value, including IntMutableCell, to emit_namespace_cell_fold. Its is_int_cell path can return Ok(true). Therefore, the changed comment is stale. Describe null or strategy-switched entries, or a failed guard, instead. Keep the presence check at Line 12620 because a present global must continue to shadow builtins.
Proposed comment update
- // a present-but-unfoldable one (`IntMutableCell` / strategy switched).
+ // a present-but-unfoldable one (null / strategy switched, or a failed guard).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // `emit_module_dict_cell_fold` returns `false` for BOTH an absent name and | |
| // a present-but-unfoldable one (`IntMutableCell` / movable / strategy | |
| // switched). Only an ABSENT name may fall through to the builtins fold — a | |
| // a present-but-unfoldable one (`IntMutableCell` / strategy switched). | |
| // Only an ABSENT name may fall through to the builtins fold — a | |
| // `emit_module_dict_cell_fold` returns `false` for BOTH an absent name and | |
| // a present-but-unfoldable one (null / strategy switched, or a failed guard). | |
| // Only an ABSENT name may fall through to the builtins fold — a |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs` around lines 12615 -
12617, Update the comment above emit_module_dict_cell_fold to remove
IntMutableCell from the unfoldable-case description and describe only null or
strategy-switched entries or failed guards. Preserve the presence check near the
builtins fallback so any present global continues to shadow builtins.
| // alive. | ||
| pyre_interpreter::module::_io::flush_all_streams(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Invoke shutdown hooks for every started built-in module.
Line 1128 flushes _io streams only. finalize_runtime does not iterate the started built-in modules and call their shutdown hooks. The new documentation states that it does. PyPy ObjSpace.finish() performs this iteration after thread shutdown and exit callbacks. As written, shutdown hooks for started modules other than _io never run. Restore the RPython order by using the existing shared module owner and invoking each started module's shutdown hook before stream flushing. Do not introduce a side registry. (github.com)
As per coding guidelines: “Before choosing a Rust collection or thread-local, locate the corresponding RPython/PyPy owner and preserve its storage shape and semantics,” and “Port RPython/PyPy code with strict line-by-line structural parity.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyrex/src/lib.rs` around lines 1127 - 1128, Update finalize_runtime to
iterate the existing shared built-in module owner, invoking each started
module’s shutdown hook before
pyre_interpreter::module::_io::flush_all_streams(). Preserve the RPython/PyPy
shutdown order and storage semantics, and do not add a separate registry.
Source: Coding guidelines
47542a7 to
1832ad9
Compare
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/1832ad9d3650ec6200f14261f9c5d00c685faa8c/pyre-object/src/listobject.rs#L2016-L2018
Fresh evidence beyond the earlier append comment: this revision roots obj in w_list_insert, but then keeps the derived list reference while IntArray::insert/FloatArray::insert may grow through the GC allocator. When a full-capacity typed list is inserted into while another thread triggers collection, the root slot is forwarded but list still addresses the evacuated header, so the grow installs the new block and updates metadata through stale memory. Reload the receiver from root_base around the growth rather than deriving one long-lived &mut W_ListObject here.
https://github.com/youknowone/pyre/blob/1832ad9d3650ec6200f14261f9c5d00c685faa8c/pyre-object/src/listobject.rs#L65-L66
For exact built-in lists, w_class is always the single get_instantiate(&LIST_TYPE) object, so this hashes every ordinary list to the same mutex. In free-threaded workloads, operations on completely unrelated lists now serialize, eliminating the previous 64-way striping and causing severe contention in list-heavy parallel code. The stable lock key needs to remain instance-specific rather than using the shared class identity.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
pyre/pyre-interpreter/src/jit_fnaddr.rs (1)
3880-3887: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtend the test to cover the newly matched member.
The test asserts only the
list_write_barrieraddress. The predicate at Lines 328-329 gainedcurrent_gc_ref, and the registration at Lines 1779-1786 supplies it. A future edit that drops either half leaves this test green.Add an assertion for the
current_gc_refbinding.💚 Proposed test addition
let barrier = bindings["pyre_object::listobject::list_write_barrier"]; assert!(is_list_write_barrier(barrier as usize)); + let gc_ref = bindings["pyre_object::listobject::current_gc_ref"]; + assert!(is_list_write_barrier(gc_ref as usize)); let nlocals = bindings["pyre_interpreter::pyframe::PyFrame::nlocals"];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyre/pyre-interpreter/src/jit_fnaddr.rs` around lines 3880 - 3887, Extend is_list_write_barrier_matches_registered_barrier to retrieve the pyframe::PyFrame::current_gc_ref binding from jit_trace_fnaddrs and assert that is_list_write_barrier returns true for its address, while preserving the existing positive and negative assertions.pyre/pyre-object/src/listobject.rs (1)
1068-1101: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winBoth allocators build a
PyObjectheader before a point this PR now treats as collecting, and never refreshheader.w_class. Each header capturesget_instantiate(&TYPE)and is stored bystd::ptr::writeonly after that point.listobject.rsreplaced the non-moving stable allocation withtry_gc_alloc_collecting_rooted, andfunction.rsmovedtry_gc_write_barrier_managedahead of the stores because the barrier can park behind a collection. The prior guarantee that captured payload pointers cannot go stale applies only totry_gc_alloc_stable_raw/try_gc_alloc_stable, so it no longer covers either site. Resolve the premise once: either establish that builtin type instantiates are immortal and non-moving, or root and refreshheader.w_classat both sites.
pyre/pyre-object/src/listobject.rs#L1068-L1101: rootheader.w_classacrosstry_gc_alloc_collecting_rootedand re-read it before thestd::ptr::write, or document the immortality ofget_instantiate(&LIST_TYPE).pyre/pyre-object/src/function.rs#L61-L75: refreshheader.w_classin the same loop that refreshesw_function,w_self, andw_class, or document the immortality ofget_instantiate(&METHOD_TYPE).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyre/pyre-object/src/listobject.rs` around lines 1068 - 1101, Ensure the PyObject header type pointers remain valid across collecting allocations: in pyre/pyre-object/src/listobject.rs:1068-1101, root header.w_class through try_gc_alloc_collecting_rooted and refresh it before std::ptr::write; in pyre/pyre-object/src/function.rs:61-75, refresh header.w_class alongside w_function, w_self, and w_class in the existing reload loop. Alternatively, document and establish that get_instantiate(&LIST_TYPE) and get_instantiate(&METHOD_TYPE) are immortal and non-moving, with no direct code change required at either site if that guarantee is proven.Source: Learnings
pyre/pyre-interpreter/src/baseobjspace.rs (1)
16059-16067: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winThe arity-1 zip path drops two steps the neighbouring paths perform.
itemis read fromnextat Line 16065 and handed straight tow_tuple_newat Line 16066.w_tuple_newallocates. A collection inside that allocation relocatesitemafter it was read. Every other arm in this function pins the pulled item first and reads it back from its slot, including the arity-2 path at Lines 16105-16106 and 16121-16126.The arity-1 path also never calls
w_zip_set_iteration_progress, while the arity-2 path stamps progress before each pull. A partially consumed arity-1ziptherefore reports stale progress to__reduce__/__setstate__.Add the pin and the progress stamp.
🛡️ Proposed fix for the arity-1 path
if length == 1 { let iterator = pyre_object::w_list_getitem( pyre_object::gc_roots::shadow_stack_get(iterators_slot), 0, ) .unwrap(); - let item = next(iterator)?; - return Ok(pyre_object::w_tuple_new(vec![item])); + pyre_object::gc_roots::pin_root(iterator); + let iterator_slot = pyre_object::gc_roots::shadow_stack_len() - 1; + zo::w_zip_set_iteration_progress( + pyre_object::gc_roots::shadow_stack_get(obj_slot), + 0, + ); + let item = next(pyre_object::gc_roots::shadow_stack_get(iterator_slot))?; + pyre_object::gc_roots::pin_root(item); + let item_slot = pyre_object::gc_roots::shadow_stack_len() - 1; + return Ok(pyre_object::w_tuple_new(vec![ + pyre_object::gc_roots::shadow_stack_get(item_slot), + ])); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyre/pyre-interpreter/src/baseobjspace.rs` around lines 16059 - 16067, Update the length-1 branch of the zip iteration function to stamp progress via w_zip_set_iteration_progress before pulling the item, then pin the result in the shadow stack and read it back after w_tuple_new allocation, matching the neighboring arity-2 paths. Preserve the existing single-item tuple result and iterator advancement behavior.pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs (1)
6223-6229: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winReject a null
CodeObjectpointer before dereference.Line 6227 dereferences
raw_code.frame_raw_codereturnsSomewhenw_codeis non-null, even ifw_code_get_ptr(w_code)is null. A gateway builtin or test fixture can therefore cause undefined behavior in this new path.Make
frame_raw_codereturnNonewhenw_code_get_ptris null.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs` around lines 6223 - 6229, Update frame_raw_code to return None when w_code_get_ptr yields a null pointer, before constructing its Some result. Preserve the existing non-null path so callers such as the decode_instruction_at check never dereference a null CodeObject pointer.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyre/cpython_tests/baseline.json`:
- Line 1072: Restore baseline PASS entries for test.test_struct and
test.test_threading after fixing their failures, so the default runner continues
executing both regression tests; alternatively, place any intentional failures
in a separate required gate rather than leaving them excluded from the PASS
baseline.
In `@pyre/pyre-interpreter/src/jit_fnaddr.rs`:
- Around line 328-329: Rename the predicates is_list_write_barrier and
is_idempotent_gc_barrier to names describing idempotent GC-liveness handling
rather than write barriers, and update all call sites accordingly. Preserve
their existing matching behavior, including current_gc_ref.
In `@pyre/pyre-object/src/listobject.rs`:
- Around line 62-66: Replace the w_class-based stripe key in the list lock
acquisition path with a stable, per-instance list identity stored in the list
header and assigned monotonically at allocation, so relocation preserves the
lock mapping while unrelated lists distribute across LIST_LOCKS. Update all
relevant list creation paths to initialize this identity and use it for
indexing; do not retain the class key.
- Around line 1616-1624: Reload the list after converters may trigger GC before
de-specialization: in pyre/pyre-object/src/listobject.rs:1616-1624 and
:1648-1660, refresh obj with current_gc_ref and rederive list before
switch_to_object_strategy; in :1441-1447, :1486-1492, :2048-2054, and
:2087-2093, rederive list from shadow_stack_get(root_base) before
switch_to_object_strategy. Ensure all six arms pass the post-relocation list
reference.
---
Outside diff comments:
In `@pyre/pyre-interpreter/src/baseobjspace.rs`:
- Around line 16059-16067: Update the length-1 branch of the zip iteration
function to stamp progress via w_zip_set_iteration_progress before pulling the
item, then pin the result in the shadow stack and read it back after w_tuple_new
allocation, matching the neighboring arity-2 paths. Preserve the existing
single-item tuple result and iterator advancement behavior.
In `@pyre/pyre-interpreter/src/jit_fnaddr.rs`:
- Around line 3880-3887: Extend is_list_write_barrier_matches_registered_barrier
to retrieve the pyframe::PyFrame::current_gc_ref binding from jit_trace_fnaddrs
and assert that is_list_write_barrier returns true for its address, while
preserving the existing positive and negative assertions.
In `@pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs`:
- Around line 6223-6229: Update frame_raw_code to return None when
w_code_get_ptr yields a null pointer, before constructing its Some result.
Preserve the existing non-null path so callers such as the decode_instruction_at
check never dereference a null CodeObject pointer.
In `@pyre/pyre-object/src/listobject.rs`:
- Around line 1068-1101: Ensure the PyObject header type pointers remain valid
across collecting allocations: in pyre/pyre-object/src/listobject.rs:1068-1101,
root header.w_class through try_gc_alloc_collecting_rooted and refresh it before
std::ptr::write; in pyre/pyre-object/src/function.rs:61-75, refresh
header.w_class alongside w_function, w_self, and w_class in the existing reload
loop. Alternatively, document and establish that get_instantiate(&LIST_TYPE) and
get_instantiate(&METHOD_TYPE) are immortal and non-moving, with no direct code
change required at either site if that guarantee is proven.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e9741168-2faf-4327-8cb3-924f1bd41383
📒 Files selected for processing (10)
pyre/bench/synth/bytes_split_whitespace_maxsplit.cranelift.jitstatspyre/bench/synth/bytes_split_whitespace_maxsplit.dynasm.jitstatspyre/cpython_tests/baseline.jsonpyre/pyre-interpreter/src/baseobjspace.rspyre/pyre-interpreter/src/jit_fnaddr.rspyre/pyre-jit-trace/src/jitcode_dispatch/mod.rspyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rspyre/pyre-object/src/function.rspyre/pyre-object/src/listobject.rspyre/pyrex/src/lib.rs
| }, | ||
| "test.test_struct": { | ||
| "dynasm": "PASS" | ||
| "dynasm": "FAIL" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- baseline entries ---'
rg -n -C 2 '"(test\.test_struct|test\.test_threading|dynasm)"' pyre/cpython_tests/baseline.json
printf '%s\n' '--- runner structure ---'
sed -n '1,260p' pyre/cpython_tests/run.py
printf '%s\n' '--- relevant diff ---'
git diff -- pyre/cpython_tests/baseline.json pyre/cpython_tests/run.pyRepository: youknowone/pyre
Length of output: 47758
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- repository state ---'
git status --short
git show --stat --oneline HEAD
printf '%s\n' '--- baseline change in HEAD ---'
git diff HEAD^ HEAD -- pyre/cpython_tests/baseline.json
printf '%s\n' '--- runner baseline and gate references ---'
rg -n -C 5 'DEFAULT_BASELINE|baseline|strict-baseline|update-baseline|PASS|REGRESSION|status' pyre/cpython_tests/run.pyRepository: youknowone/pyre
Length of output: 14422
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import json
import subprocess
from pathlib import Path
baseline_path = "pyre/cpython_tests/baseline.json"
current = json.loads(Path(baseline_path).read_text())
previous = json.loads(subprocess.check_output(
["git", "show", f"HEAD^:{baseline_path}"], text=True
))
modules = current["modules"]
for module in ("test.test_struct", "test.test_threading"):
before = previous["modules"][module]["dynasm"]
after = modules[module]["dynasm"]
assert (before, after) == ("PASS", "FAIL"), (module, before, after)
source = Path("pyre/cpython_tests/run.py").read_text()
assert "gate_pass_only = not (args.full or args.update_baseline or args.strict_baseline)" in source
assert "if gate_pass_only and exp != \"PASS\":" in source
assert "if exp == \"PASS\" and status != \"PASS\":" in source
selected = {
module for module, entry in modules.items()
if entry.get("dynasm") == "PASS"
}
assert "test.test_struct" not in selected
assert "test.test_threading" not in selected
print("previous statuses: PASS, PASS")
print("current statuses: FAIL, FAIL")
print(f"default dynasm gate selects {len(selected)} PASS modules")
print("both downgraded modules are excluded from the default gate")
print("the runner reports regressions only for executed results")
PYRepository: youknowone/pyre
Length of output: 373
Keep both CPython tests regression-protected.
test.test_struct and test.test_threading changed from PASS to FAIL. The default runner executes only baseline-PASS modules, so it now excludes both tests and cannot report future regressions. Restore PASS after fixing the failures, or run intentional failures in a separate required gate.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/cpython_tests/baseline.json` at line 1072, Restore baseline PASS entries
for test.test_struct and test.test_threading after fixing their failures, so the
default runner continues executing both regression tests; alternatively, place
any intentional failures in a separate required gate rather than leaving them
excluded from the PASS baseline.
| || path.ends_with("::listobject::current_gc_ref") | ||
| || *path == "pyre_object::current_gc_ref" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find every consumer of is_list_write_barrier and inspect what it concludes from a true result.
set -euo pipefail
rg -n -C 20 'is_list_write_barrier' --type=rustRepository: youknowone/pyre
Length of output: 153
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- predicate and related symbols ---'
rg -n -C 12 'is_list_write_barrier|current_gc_ref|list_write_barrier|prepare_list_ref_store' pyre/pyre-interpreter/src/jit_fnaddr.rs pyre --type=rust || true
printf '%s\n' '--- file outline ---'
ast-grep outline pyre/pyre-interpreter/src/jit_fnaddr.rs --match 'is_list_write_barrier' --view expanded || true
printf '%s\n' '--- direct identifier references across tracked Rust files ---'
git grep -n -E 'is_list_write_barrier|current_gc_ref|list_write_barrier|prepare_list_ref_store' -- '*.rs' || trueRepository: youknowone/pyre
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- exact consumers ---'
git grep -n -C 8 'is_list_write_barrier' -- '*.rs'
printf '%s\n' '--- residual execution around classification ---'
sed -n '2425,2525p' pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs
sed -n '5635,5715p' pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs
printf '%s\n' '--- list helper implementations and call sites ---'
rg -n -C 10 'fn (current_gc_ref|list_write_barrier|prepare_list_ref_store)|current_gc_ref\(|list_write_barrier\(|prepare_list_ref_store\(' pyre --type=rust
printf '%s\n' '--- all current_gc_ref references ---'
git grep -n -C 5 'current_gc_ref' -- '*.rs'Repository: youknowone/pyre
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- every use of the local classification ---'
rg -n -C 12 'is_idempotent_gc_barrier|idempotent_gc_barrier|body_effect|body effect' pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs
printf '%s\n' '--- current_gc_ref implementation and direct call context ---'
sed -n '900,930p' pyre/pyre-object/src/listobject.rs
sed -n '1555,1590p' pyre/pyre-object/src/listobject.rs
printf '%s\n' '--- GC current-object-address implementation ---'
rg -n -C 12 'try_gc_current_object_address|current_object_address' pyre --type=rustRepository: youknowone/pyre
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
fnaddr = Path("pyre/pyre-interpreter/src/jit_fnaddr.rs").read_text()
residual = Path("pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs").read_text()
listobj = Path("pyre/pyre-object/src/listobject.rs").read_text()
# Enumerate executable consumers, excluding the definition and test references.
consumers = [
(m.start(), residual[max(0, m.start()-120):m.start()+180])
for m in re.finditer(r"is_list_write_barrier\s*\(", residual)
]
print("residual consumer count:", len(consumers))
for _, context in consumers:
print(context.replace("\n", " "))
# Extract the helper body and report whether it invokes a write barrier.
m = re.search(
r"pub\s+fn\s+current_gc_ref\s*\([^)]*\)\s*->\s*\*mut\s+PyObject\s*\{(.*?)\n\}",
listobj,
re.S,
)
assert m, "current_gc_ref body not found"
body = m.group(1)
print("current_gc_ref calls list_write_barrier:", "list_write_barrier" in body)
print("current_gc_ref calls try_gc_current_object_address:",
"try_gc_current_object_address" in body)
# Show the only classification uses in the consumer.
uses = re.findall(r"!is_idempotent_gc_barrier", residual)
print("classification exclusion count:", len(uses))
assert len(consumers) == 1
assert "list_write_barrier" not in body
assert "try_gc_current_object_address" in body
PYRepository: youknowone/pyre
Length of output: 619
Rename the predicate to reflect its broader role. current_gc_ref only resolves a relocated pointer. It does not run a write barrier or remember the list. Rename is_list_write_barrier and is_idempotent_gc_barrier to describe idempotent GC-liveness handling.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyre-interpreter/src/jit_fnaddr.rs` around lines 328 - 329, Rename the
predicates is_list_write_barrier and is_idempotent_gc_barrier to names
describing idempotent GC-liveness handling rather than write barriers, and
update all call sites accordingly. Preserve their existing matching behavior,
including current_gc_ref.
| // A nursery list can move while its guard is held. Stripe on its stable | ||
| // class identity, not the movable instance address, so every operation on | ||
| // one list continues to acquire the same lock after collection. | ||
| let w_class = (*obj).w_class; | ||
| let lock = LIST_LOCKS[(w_class as usize >> 4) & (LIST_LOCKS.len() - 1)].get(); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Lock striping now collapses every plain list onto one lock.
w_class is get_instantiate(&LIST_TYPE) for every exact list instance. The stripe index therefore resolves to a single entry of LIST_LOCKS for all plain lists, so LIST_LOCKS degenerates from a 256-way stripe to one global list lock. Concurrent append / setitem / setslice on unrelated lists now serialize, and before_external_block is entered far more often.
The stated motivation is correct: an instance-address key returns a different lock after the list moves, so two operations on the same logical list can take different locks. The address key is unsound. But w_class is too coarse as the replacement.
Store a stable per-list identity that survives relocation (for example a monotonically assigned id in the list header, stamped at allocation) and stripe on that. Report the measured contention impact from the eight benchmarks if you keep the class key.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyre-object/src/listobject.rs` around lines 62 - 66, Replace the
w_class-based stripe key in the list lock acquisition path with a stable,
per-instance list identity stored in the list header and assigned monotonically
at allocation, so relocation preserves the lock mapping while unrelated lists
distribute across LIST_LOCKS. Update all relevant list creation paths to
initialize this identity and use it for indexing; do not retain the class key.
| } else if is_float_strategy_item(value) && integer_to_int_or_float(list) { | ||
| let obj = current_gc_ref(obj); | ||
| let value = current_gc_ref(value); | ||
| w_list_append_inner(obj, value); | ||
| } else { | ||
| switch_to_object_strategy(list); | ||
| let obj = switch_to_object_strategy(list); | ||
| let value = current_gc_ref(value); | ||
| let list = &mut *(obj as *mut W_ListObject); | ||
| list.object_push(value); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Six de-specialize arms pass a possibly stale list into switch_to_object_strategy. Each arm is guarded by is_float_strategy_item(value) && integer_to_int_or_float(list) or by the float_to_int_or_float(list) twin. If a converter installs fresh typed storage and then returns false, it has already run a GC allocation, so the reference reaching switch_to_object_strategy can be pre-relocation. The sibling redispatch arms already reload through current_gc_ref or through shadow_stack_get(root_base); the de-specialize arms do not.
pyre/pyre-object/src/listobject.rs#L1616-L1624: reloadobjthroughcurrent_gc_refand rederivelistbefore callingswitch_to_object_strategy.pyre/pyre-object/src/listobject.rs#L1648-L1660: apply the same reload in the Float arm beforeswitch_to_object_strategy.pyre/pyre-object/src/listobject.rs#L1441-L1447: rederivelistfromshadow_stack_get(root_base)beforeswitch_to_object_strategy(list).pyre/pyre-object/src/listobject.rs#L1486-L1492: rederivelistfromshadow_stack_get(root_base)beforeswitch_to_object_strategy(list).pyre/pyre-object/src/listobject.rs#L2048-L2054: rederivelistfromshadow_stack_get(root_base)beforeswitch_to_object_strategy(list).pyre/pyre-object/src/listobject.rs#L2087-L2093: rederivelistfromshadow_stack_get(root_base)beforeswitch_to_object_strategy(list).
📍 Affects 1 file
pyre/pyre-object/src/listobject.rs#L1616-L1624(this comment)pyre/pyre-object/src/listobject.rs#L1648-L1660pyre/pyre-object/src/listobject.rs#L1441-L1447pyre/pyre-object/src/listobject.rs#L1486-L1492pyre/pyre-object/src/listobject.rs#L2048-L2054pyre/pyre-object/src/listobject.rs#L2087-L2093
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyre-object/src/listobject.rs` around lines 1616 - 1624, Reload the list
after converters may trigger GC before de-specialization: in
pyre/pyre-object/src/listobject.rs:1616-1624 and :1648-1660, refresh obj with
current_gc_ref and rederive list before switch_to_object_strategy; in
:1441-1447, :1486-1492, :2048-2054, and :2087-2093, rederive list from
shadow_stack_get(root_base) before switch_to_object_strategy. Ensure all six
arms pass the post-relocation list reference.
1832ad9 to
142a9a4
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 142a9a4995
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }, | ||
| "test.test_struct": { | ||
| "dynasm": "PASS" | ||
| "dynasm": "FAIL" |
There was a problem hiding this comment.
Changing test.test_struct and test.test_threading from PASS to FAIL removes both modules from the default regression run: pyre/cpython_tests/run.py only selects baseline-PASS modules at lines 499-519. Consequently, the two regressions recorded by this commit are silently deselected in CI rather than fixed or reported; retain their PASS expectations and address the failures instead of weakening the gate.
Useful? React with 👍 / 👎.
| let w_class = (*obj).w_class; | ||
| let lock = LIST_LOCKS[(w_class as usize >> 4) & (LIST_LOCKS.len() - 1)].get(); |
There was a problem hiding this comment.
For ordinary lists, every instance has the same w_class, so this expression maps all exact lists to one LIST_LOCKS entry. In free-threaded workloads, unrelated getitem, append, pop, and length operations on otherwise independent lists therefore serialize globally, negating the 256-way striping and allowing one contended list to stall every other list; use a stable per-instance identity rather than the shared class identity.
Useful? React with 👍 / 👎.
142a9a4 to
0b49f0d
Compare
| }, | ||
| "test.test_struct": { | ||
| "dynasm": "PASS" | ||
| "dynasm": "FAIL" |
There was a problem hiding this comment.
do not accept this regression
`finalize_runtime` ended after the `ObjSpace.finish()` phases, so nothing held only by a namespace was finalized: `struct.x = C()` whose class defines `__del__` printed nothing at exit, and a `__main__` global's `__del__` did not run either. Restore `collect_and_run_finalizers`, `release_frees_nothing` and the `__main__` newest-to-oldest release loop that #1158 removed. A `__del__` reading a module global needs a collection to run while the remaining names are still bound; `test_start_new_thread_at_finalization` reads `_thread` and otherwise sees `None`. Port `finalize_modules` / `_PyModule_ClearDict` for the namespaces `__main__` does not reach. `release_sys_modules_for_shutdown` snapshots `sys.modules` in insertion order and detaches every entry except `sys` and `builtins`, which the unraisable path still reads while the released modules are finalized. `clear_shutdown_modules` then walks the snapshot newest-first, skipping those two, and clears each module dict in two name passes -- a single leading underscore first, then every name but `__builtins__` -- assigning `None` rather than deleting. One collection follows the whole walk: a sweep per module costs a full mark-and-sweep for each of the ~100 modules a bare `import unittest` loads, which measured 905ms of teardown against 39ms for the single sweep, and `test_regrtest` spends it once per subprocess. `test.test_struct` and `test.test_threading` return to PASS in the baseline; `test_struct_cleans_up_at_runtime_shutdown` and `test_start_new_thread_at_finalization` are the tests they cover. Assisted-by: Claude
`finalize_runtime` ended after the `ObjSpace.finish()` phases, so nothing held only by a namespace was finalized: `struct.x = C()` whose class defines `__del__` printed nothing at exit, and a `__main__` global's `__del__` did not run either. Restore `collect_and_run_finalizers`, `release_frees_nothing` and the `__main__` newest-to-oldest release loop that #1158 removed. A `__del__` reading a module global needs a collection to run while the remaining names are still bound; `test_start_new_thread_at_finalization` reads `_thread` and otherwise sees `None`. Port `finalize_modules` / `_PyModule_ClearDict` for the namespaces `__main__` does not reach. `release_sys_modules_for_shutdown` snapshots `sys.modules` in insertion order and detaches every entry except `sys` and `builtins`, which the unraisable path still reads while the released modules are finalized. `clear_shutdown_modules` then walks the snapshot newest-first, skipping those two, and clears each module dict in two name passes -- a single leading underscore first, then every name but `__builtins__` -- assigning `None` rather than deleting. One collection follows the whole walk: a sweep per module costs a full mark-and-sweep for each of the ~100 modules a bare `import unittest` loads, which measured 905ms of teardown against 39ms for the single sweep, and `test_regrtest` spends it once per subprocess. `test.test_struct` and `test.test_threading` return to PASS in the baseline; `test_struct_cleans_up_at_runtime_shutdown` and `test_start_new_thread_at_finalization` are the tests they cover. Assisted-by: Claude
…le teardown (#1187) * jit: fold the builtins fallback for module-scope LOAD_NAME `try_walker_load_global_cell_fold` folds a name that misses the module dict and resolves through `get_builtin().getdictvalue`. `try_walker_load_name_cell_fold` ended at `emit_module_dict_cell_fold`, so module-scope `LOAD_NAME` of such a name residualized `bh_load_name_fn` on every iteration. Move that leg into `emit_builtins_cell_fold` and call it from both folds. The guard sequence is unchanged: the name must be absent from the module dict, whose `version?` is pinned so a later shadowing insert fails GUARD_NOT_INVALIDATED, and `emit_namespace_cell_fold` pins the builtins dict's own `version?`. A 2.4M-iteration module-scope `total + len(s)` loop compiles to 21 ops with no `call_may_force`, matching the same loop with `len` bound to a module global; it recorded 30 ops and one `call_may_force` before. Assisted-by: Claude * bench/synth: gate the module-scope LOAD_NAME builtins cell fold The hot loop reads `len` at module scope, where the name misses the module dict and resolves through the frame's builtin module. The trailing `len = lambda x: 100` plus a second loop pins the invalidation: the module dict `version?` bump has to be seen, so the second loop prints 40000000. Output matches CPython and PyPy. Ceiling 8 against measured 1.6x dynasm, 2.1x cranelift, 1.9x wasm. The pypy denominator sits near the execution floor, so the ratios moved by about a quarter between runs; the residual form this gates measured about 140x. The shape is load-bearing: inside a function the read compiles to LOAD_GLOBAL, which folded already, and a module-scope `del` of a global drops `mc_entered` to 0 and runs the loop interpreted. Assisted-by: Claude * stdlib: address hashlib and shutdown parity reviews * interpreter: clear module globals at shutdown `finalize_runtime` ended after the `ObjSpace.finish()` phases, so nothing held only by a namespace was finalized: `struct.x = C()` whose class defines `__del__` printed nothing at exit, and a `__main__` global's `__del__` did not run either. Restore `collect_and_run_finalizers`, `release_frees_nothing` and the `__main__` newest-to-oldest release loop that #1158 removed. A `__del__` reading a module global needs a collection to run while the remaining names are still bound; `test_start_new_thread_at_finalization` reads `_thread` and otherwise sees `None`. Port `finalize_modules` / `_PyModule_ClearDict` for the namespaces `__main__` does not reach. `release_sys_modules_for_shutdown` snapshots `sys.modules` in insertion order and detaches every entry except `sys` and `builtins`, which the unraisable path still reads while the released modules are finalized. `clear_shutdown_modules` then walks the snapshot newest-first, skipping those two, and clears each module dict in two name passes -- a single leading underscore first, then every name but `__builtins__` -- assigning `None` rather than deleting. One collection follows the whole walk: a sweep per module costs a full mark-and-sweep for each of the ~100 modules a bare `import unittest` loads, which measured 905ms of teardown against 39ms for the single sweep, and `test_regrtest` spends it once per subprocess. `test.test_struct` and `test.test_threading` return to PASS in the baseline; `test_struct_cleans_up_at_runtime_shutdown` and `test_start_new_thread_at_finalization` are the tests they cover. Assisted-by: Claude * bench/synth/str_fstring: re-record the cranelift jit-stats baseline guard_failures 659 -> 658. Assisted-by: Claude * check.py: run the vendored CPython suite only on --cpython-suite The suite ran twice per CI round: once inside `pyre/check.py` on the macos-latest leg, and once in the dedicated `cpython-tests` job. It dominates this script's wall time, so make it opt-in -- `--cpython-suite` replaces `--no-cpython-suite` and no CI job passes it, leaving the dedicated job as the only place CI pays for the run. The stage itself is unchanged and still skips off darwin-arm64. Its docstring no longer claims the CI job pins `runs-on: macos-latest`, which `ci: make the CPython gate host-aware` changed to ubuntu-24.04. Assisted-by: Claude
1 participant
Summary
Evidence
With PYRE_NO_JIT=1 and N=2,000,000, sampled every 0.2 seconds under a 341,000 KB hard RSS cap:
The PyPy oracle leaves a module-global del unrun at shutdown while still running atexit callbacks, matching baseobjspace.py ObjSpace.finish.
Validation
Summary by CodeRabbit
Bug Fixes
Shutdown