jit: stop the frame-chain walkers from forcing; keep last_instr current across residual calls - #841
Conversation
WalkthroughThe PR updates GC nursery publication, Result exception lowering, warning and frame handling, hashing, JIT dispatch, specialized tuple construction, and local scratchpad exclusions. ChangesGC nursery publication
Translator Result lowering
Interpreter runtime behavior
Hashing and object representation
JIT dispatch and residual execution
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Backend
participant GcAllocator
participant GCStore
participant NurseryCheck
Backend->>GcAllocator: install thread-local allocator
Backend->>GCStore: disarm published singleton nursery
GcAllocator->>GCStore: publish nursery metadata on singleton install
NurseryCheck->>GCStore: read armed nursery metadata
GCStore-->>NurseryCheck: return bounds and tagged-pointer setting
sequenceDiagram
participant Interpreter
participant FrameWalk
participant force_frame
participant UserFrame
Interpreter->>FrameWalk: request visible frame chain
FrameWalk-->>Interpreter: return force-free frame links
Interpreter->>force_frame: materialize selected frame
force_frame-->>UserFrame: provide materialized frame fields
Possibly related issues
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/70a68163c6d4741f2c0bf335aa6d4793dae58276/pyre-interpreter/src/executioncontext.rs#L423-L427
When sys.settrace() or sys.setprofile() enables a hook while JIT-compiled code is running, force_all_frames() still relies exclusively on these two walkers to force each virtualizable frame; the upstream implementation explicitly relies on reading f_back during this walk to fail the following GUARD_NOT_FORCED. Removing every force_frame call without adding one in force_all_frames() allows assembler execution to continue and can omit trace/profile events or leave is_being_profiled on a stale materialization. Keep the general warning walk force-free, but explicitly force each frame in this consumer.
AGENTS.md reference: AGENTS.md:L14-L20
https://github.com/youknowone/pyre/blob/70a68163c6d4741f2c0bf335aa6d4793dae58276/pyre-interpreter/src/module/_warnings/mod.rs#L50
If application code removes or replaces sys.modules['_warnings'] and the old module becomes otherwise unreachable, the global root walker no longer visits this captured namespace: STATE_NS is only an integer and is not a registered GC root. A later major collection can therefore reclaim or move managed values stored in the dict, notably the new_version() instance, after which current_version() dereferences stale state during the next warning. Preserve the upstream interpreter-owned State with an explicit rooted owner/root-walker entry rather than only caching its raw address.
AGENTS.md reference: AGENTS.md:L148-L155
https://github.com/youknowone/pyre/blob/70a68163c6d4741f2c0bf335aa6d4793dae58276/pyre-interpreter/src/builtins.rs#L9163-L9165
For the always-reachable empty string, _hash_str deliberately returns 0, but this new substitution changes hash("") to 29742. RPython's ll_strhash returns zero directly for an empty string and applies its nonzero cache sentinel only inside _ll_strhash for nonempty inputs; additionally, the raw-string dict hook hash_str_bytes("") still returns zero, so the two hash entry points now disagree for the same key. Special-case empty strings before applying the memo-slot sentinel (and use the upstream sentinel for the rare nonempty zero digest).
AGENTS.md reference: AGENTS.md:L194-L196
ℹ️ 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 ac4ba4c). 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
|
|
Re-verified against the current base ( Correctness spot-checks on both backends: Two earlier local runs failed a boundary perf bench (
— commented by Claude |
|
Rebased onto the current Added a commit from the Codex parity review. The review flagged
That value is SipHash-2-4 under the all-zero key over empty input ( The commit also moves the zero-digest substitute from the str caller into For the record on the review's other two points: finding 1 ("empty strings should hash as 0, per
Verification on — commented by Claude |
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/8ca19b19da79075da3b4a6ca9e8f8c71ab097c8e/pyre-interpreter/src/executioncontext.rs#L447-L449
When sys.settrace() or sys.setprofile() enables a callback while JIT-compiled code is active, ExecutionContext::force_all_frames still relies on these walkers to force every frame so the following GUARD_NOT_FORCED exits the compiled trace. Making both walkers force-free without adding an explicit force_frame(frame) in force_all_frames means the walk now only materializes vrefs and mutates debugdata; it no longer marks the virtualizable as forced, so the active trace can continue without the newly installed tracing/profiling callbacks. Preserve forcing specifically in that consumer.
AGENTS.md reference: AGENTS.md:L194-L196
ℹ️ 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".
|
Fixed the one finding from the latest Codex parity review that lands on new code in this PR, and it was a real latent defect.
Not exercised by the corpus — check.py was green before and after — so this is a latent fix, not a repro. The review's other two points are noted but not addressed here:
Final verification on Low-load numbers from that run: — commented by Claude |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10bf87dbc8
ℹ️ 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".
| if let Some(vable_ref) = ctx.trace_ctx.standard_virtualizable_box() | ||
| && let Some(idx) = info.static_field_index_by_name("last_instr") | ||
| { |
There was a problem hiding this comment.
When this path runs inside an inlined non-portal callee, ctx.vstack_cur_pypc is a coordinate in the callee's bytecode, but standard_virtualizable_box() still selects the portal/caller frame. The recording path explicitly avoids this by targeting INLINE_CONCRETE_FRAME; the emitted trace instead writes the callee PC into the caller and leaves the callee stale. A compiled inline residual that walks frames (for example, a custom iterator that emits a warning) can therefore report/deduplicate against the wrong line, and its immediate resume snapshot carries a caller frame with an invalid callee coordinate. Thread the current inline frame OpRef here rather than collapsing the write onto the portal virtualizable.
AGENTS.md reference: AGENTS.md:L32-L42
Useful? React with 👍 / 👎.
| pub unsafe fn w_str_set_hash(obj: PyObjectRef, hash: i64) { | ||
| unsafe { (*(obj as *mut W_UnicodeObject)).hash = hash } |
There was a problem hiding this comment.
When two free-threaded callers hash the same previously unhashed shared string concurrently, they both read and write this plain i64 through raw pointers. Equal written values do not make this safe in Rust: an unsynchronized read/write or write/write race is undefined behavior. Use an atomic slot (or synchronize cache initialization) before relying on this memoization in the no-GIL runtime.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31f6d42f99
ℹ️ 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".
| let payload = crate::front::result_exc::tyref_result_ok(output, llbc); | ||
| let kind_src = payload.as_ref().unwrap_or(output); |
There was a problem hiding this comment.
tyref_result_ok accepts every Result<T, E>, but only Result<T, PyError> is lowered to the residual-call ABI that returns T; other error carriers remain ordinary Result ADTs. For example, the opaque _AsDouble function returns Result<f64, RBigIntError>, so this change stamps its FUNC.RESULT as f64 while its callers still receive a Result reference, producing a call-descriptor/ABI mismatch when bigint-to-float code is translated. Gate this unwrapping with the existing tyref_is_result_of_pyerror predicate.
AGENTS.md reference: AGENTS.md:L14-L20
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (1)
pyre/pyre-interpreter/src/executioncontext.rs (1)
423-463: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winRestores missing forcing in
force_all_frames.
force_all_frames(false)is thesettracecall site and no longer forces any visited frames becausegettopframe_nohidden/getnextframe_nohiddenare explicitly force-free. This leaves already-virtualized stack frames un-materialized when tracing is installed mid-execution. Add aforce_frame(frame)hop at the start of theforce_all_framesloop so consumers still get the intended materialization.Apply to: lines 975-985.
🤖 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/executioncontext.rs` around lines 423 - 463, Update force_all_frames so each iteration calls force_frame on the current frame before processing or advancing through the frame chain. Preserve the existing traversal behavior while ensuring the settrace path materializes every visited frame, including frames returned by gettopframe_nohidden and getnextframe_nohidden.
🤖 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 `@majit/majit-backend-wasm/src/lib.rs`:
- Around line 334-336: Add a Wasm-specific nursery-query function alongside
wasm_gc_owns_object and register it through ACTIVE_GC_IS_NURSERY_OBJECT before
calling majit_gc::disarm_published_nursery(). Ensure gc_is_nursery_object uses
the active Wasm hook for per-thread nursery objects rather than a stale backend
or singleton allocator hook.
In `@pyre/pyre-interpreter/src/module/_warnings/mod.rs`:
- Around line 628-654: Reload the category value from category_slot after every
allocating helper in this warning-processing block. In the else branch, re-read
category_slot for the tuple’s final category after call_function_impl_result; in
the true branch, replace the .unwrap_or(category) fallback with a post-operation
shadow-stack reload so no stale local category pointer survives allocation.
Preserve the existing category/type selection behavior otherwise.
In `@pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs`:
- Around line 1381-1385: Update the tuple guard in the wrappeditems path around
the arg_concretes[2] match to accept only the exact generic tuple
representation, excluding SPECIALISED_TUPLE_* values, before
tuple_wrappeditems_descr() is accessed; preserve rejection of null and non-tuple
references.
In `@pyre/pyre-object/src/unicodeobject.rs`:
- Line 25: Update the documented object layout comment near the Wtf8 object
definition to include index_storage between w_slots and hash, matching the
actual field order. Keep the existing layout notation and all other
documentation unchanged.
---
Outside diff comments:
In `@pyre/pyre-interpreter/src/executioncontext.rs`:
- Around line 423-463: Update force_all_frames so each iteration calls
force_frame on the current frame before processing or advancing through the
frame chain. Preserve the existing traversal behavior while ensuring the
settrace path materializes every visited frame, including frames returned by
gettopframe_nohidden and getnextframe_nohidden.
🪄 Autofix (Beta)
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: 166a479b-49e1-4c11-b45a-5c2e29e61271
📒 Files selected for processing (28)
.gitignoremajit/majit-backend-cranelift/src/compiler.rsmajit/majit-backend-dynasm/src/runner.rsmajit/majit-backend-wasm/src/lib.rsmajit/majit-gc/src/collector.rsmajit/majit-gc/src/gc_sync.rsmajit/majit-gc/src/lib.rsmajit/majit-translate/src/front/checked_arith.rsmajit/majit-translate/src/front/mir.rsmajit/majit-translate/src/front/result_exc.rspyre/pyre-interpreter/src/builtins.rspyre/pyre-interpreter/src/call.rspyre/pyre-interpreter/src/executioncontext.rspyre/pyre-interpreter/src/host_seam.rspyre/pyre-interpreter/src/importing.rspyre/pyre-interpreter/src/jit_fnaddr.rspyre/pyre-interpreter/src/module/_warnings/mod.rspyre/pyre-interpreter/src/module/sys/vm.rspyre/pyre-interpreter/src/module/thread/mod.rspyre/pyre-interpreter/src/objspace/descroperation.rspyre/pyre-interpreter/src/pyframe.rspyre/pyre-interpreter/src/warn.rspyre/pyre-jit-trace/src/helpers.rspyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rspyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rspyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rspyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rspyre/pyre-object/src/unicodeobject.rs
| // Per-thread allocator: its nursery is not the singleton's, so the | ||
| // process-wide published range can no longer answer `is_nursery_object`. | ||
| majit_gc::disarm_published_nursery(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Register Wasm’s nursery-query hook before disarming.
After Line 336, gc_is_nursery_object must use ACTIVE_GC_IS_NURSERY_OBJECT. Wasm does not register that hook, so it can consult a stale backend hook or the singleton allocator instead. This can skip forwarding for a moved Wasm nursery object and retain a stale GcRef. Add and register a Wasm-specific nursery query alongside wasm_gc_owns_object.
🤖 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 `@majit/majit-backend-wasm/src/lib.rs` around lines 334 - 336, Add a
Wasm-specific nursery-query function alongside wasm_gc_owns_object and register
it through ACTIVE_GC_IS_NURSERY_OBJECT before calling
majit_gc::disarm_published_nursery(). Ensure gc_is_nursery_object uses the
active Wasm hook for per-thread nursery objects rather than a stale backend or
singleton allocator hook.
| let category = pyre_object::gc_roots::shadow_stack_get(category_slot); | ||
| let (text, message, category) = if crate::baseobjspace::isinstance(input_message, warning)? { | ||
| ( | ||
| crate::builtins::builtin_str(&[pyre_object::gc_roots::shadow_stack_get( | ||
| input_message_slot, | ||
| )])?, | ||
| pyre_object::gc_roots::shadow_stack_get(input_message_slot), | ||
| crate::typedef::r#type(pyre_object::gc_roots::shadow_stack_get(input_message_slot)) | ||
| .map(|p| p.as_ptr()) | ||
| .unwrap_or(category), | ||
| ) | ||
| } else { | ||
| let input_message = pyre_object::gc_roots::shadow_stack_get(input_message_slot); | ||
| let text = if unsafe { is_str(input_message) || is_bytes(input_message) } { | ||
| input_message | ||
| let (text, message, category) = | ||
| if unsafe { crate::baseobjspace::isinstance_w(input_message, warning) } { | ||
| ( | ||
| crate::builtins::builtin_str(&[pyre_object::gc_roots::shadow_stack_get( | ||
| input_message_slot, | ||
| )])?, | ||
| pyre_object::gc_roots::shadow_stack_get(input_message_slot), | ||
| crate::typedef::r#type(pyre_object::gc_roots::shadow_stack_get(input_message_slot)) | ||
| .map(|p| p.as_ptr()) | ||
| .unwrap_or(category), | ||
| ) | ||
| } else { | ||
| crate::builtins::builtin_str(&[input_message])? | ||
| let input_message = pyre_object::gc_roots::shadow_stack_get(input_message_slot); | ||
| let text = if unsafe { is_str(input_message) || is_bytes(input_message) } { | ||
| input_message | ||
| } else { | ||
| crate::builtins::builtin_str(&[input_message])? | ||
| }; | ||
| let text_slot = pin_root_slot(text); | ||
| let instance = crate::call::call_function_impl_result( | ||
| pyre_object::gc_roots::shadow_stack_get(category_slot), | ||
| &[pyre_object::gc_roots::shadow_stack_get(input_message_slot)], | ||
| )?; | ||
| let text = pyre_object::gc_roots::shadow_stack_get(text_slot); | ||
| (text, instance, category) | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Stale category pointer reused across an allocating call — GC-safety violation.
category is read once at line 628 (shadow_stack_get(category_slot)) before the if/else. In the else branch — the common warnings.warn(str_message, category) path — crate::call::call_function_impl_result(...) at lines 648-651 can allocate and trigger a collection, yet the branch's final tuple element at line 653 reuses that pre-call category local instead of re-reading category_slot. This function's own doc comment states: RPython keeps these values in livevars across every allocating helper. Native Rust locals are invisible to the moving collector, so mirror that lifetime with the established temporary shadow-stack bracket. The .unwrap_or(category) fallback in the true branch (line 638) has the same latent issue (lower likelihood since r#type() rarely returns None).
If the collector ever relocates the object category_slot points to during that call (e.g. a freshly-defined custom Warning subclass not yet promoted out of the nursery), this returns/propagates a dangling pointer that later gets re-pinned and dereferenced (get_filter, already_warned, show_warning, etc.).
🛡️ Proposed fix — reload `category` from the shadow stack in both branches
let (text, message, category) =
if unsafe { crate::baseobjspace::isinstance_w(input_message, warning) } {
(
crate::builtins::builtin_str(&[pyre_object::gc_roots::shadow_stack_get(
input_message_slot,
)])?,
pyre_object::gc_roots::shadow_stack_get(input_message_slot),
crate::typedef::r#type(pyre_object::gc_roots::shadow_stack_get(input_message_slot))
.map(|p| p.as_ptr())
- .unwrap_or(category),
+ .unwrap_or_else(|| pyre_object::gc_roots::shadow_stack_get(category_slot)),
)
} else {
let input_message = pyre_object::gc_roots::shadow_stack_get(input_message_slot);
let text = if unsafe { is_str(input_message) || is_bytes(input_message) } {
input_message
} else {
crate::builtins::builtin_str(&[input_message])?
};
let text_slot = pin_root_slot(text);
let instance = crate::call::call_function_impl_result(
pyre_object::gc_roots::shadow_stack_get(category_slot),
&[pyre_object::gc_roots::shadow_stack_get(input_message_slot)],
)?;
let text = pyre_object::gc_roots::shadow_stack_get(text_slot);
- (text, instance, category)
+ (text, instance, pyre_object::gc_roots::shadow_stack_get(category_slot))
};📝 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.
| let category = pyre_object::gc_roots::shadow_stack_get(category_slot); | |
| let (text, message, category) = if crate::baseobjspace::isinstance(input_message, warning)? { | |
| ( | |
| crate::builtins::builtin_str(&[pyre_object::gc_roots::shadow_stack_get( | |
| input_message_slot, | |
| )])?, | |
| pyre_object::gc_roots::shadow_stack_get(input_message_slot), | |
| crate::typedef::r#type(pyre_object::gc_roots::shadow_stack_get(input_message_slot)) | |
| .map(|p| p.as_ptr()) | |
| .unwrap_or(category), | |
| ) | |
| } else { | |
| let input_message = pyre_object::gc_roots::shadow_stack_get(input_message_slot); | |
| let text = if unsafe { is_str(input_message) || is_bytes(input_message) } { | |
| input_message | |
| let (text, message, category) = | |
| if unsafe { crate::baseobjspace::isinstance_w(input_message, warning) } { | |
| ( | |
| crate::builtins::builtin_str(&[pyre_object::gc_roots::shadow_stack_get( | |
| input_message_slot, | |
| )])?, | |
| pyre_object::gc_roots::shadow_stack_get(input_message_slot), | |
| crate::typedef::r#type(pyre_object::gc_roots::shadow_stack_get(input_message_slot)) | |
| .map(|p| p.as_ptr()) | |
| .unwrap_or(category), | |
| ) | |
| } else { | |
| crate::builtins::builtin_str(&[input_message])? | |
| let input_message = pyre_object::gc_roots::shadow_stack_get(input_message_slot); | |
| let text = if unsafe { is_str(input_message) || is_bytes(input_message) } { | |
| input_message | |
| } else { | |
| crate::builtins::builtin_str(&[input_message])? | |
| }; | |
| let text_slot = pin_root_slot(text); | |
| let instance = crate::call::call_function_impl_result( | |
| pyre_object::gc_roots::shadow_stack_get(category_slot), | |
| &[pyre_object::gc_roots::shadow_stack_get(input_message_slot)], | |
| )?; | |
| let text = pyre_object::gc_roots::shadow_stack_get(text_slot); | |
| (text, instance, category) | |
| }; | |
| let category = pyre_object::gc_roots::shadow_stack_get(category_slot); | |
| let (text, message, category) = | |
| if unsafe { crate::baseobjspace::isinstance_w(input_message, warning) } { | |
| ( | |
| crate::builtins::builtin_str(&[pyre_object::gc_roots::shadow_stack_get( | |
| input_message_slot, | |
| )])?, | |
| pyre_object::gc_roots::shadow_stack_get(input_message_slot), | |
| crate::typedef::r#type(pyre_object::gc_roots::shadow_stack_get(input_message_slot)) | |
| .map(|p| p.as_ptr()) | |
| .unwrap_or_else(|| pyre_object::gc_roots::shadow_stack_get(category_slot)), | |
| ) | |
| } else { | |
| let input_message = pyre_object::gc_roots::shadow_stack_get(input_message_slot); | |
| let text = if unsafe { is_str(input_message) || is_bytes(input_message) } { | |
| input_message | |
| } else { | |
| crate::builtins::builtin_str(&[input_message])? | |
| }; | |
| let text_slot = pin_root_slot(text); | |
| let instance = crate::call::call_function_impl_result( | |
| pyre_object::gc_roots::shadow_stack_get(category_slot), | |
| &[pyre_object::gc_roots::shadow_stack_get(input_message_slot)], | |
| )?; | |
| let text = pyre_object::gc_roots::shadow_stack_get(text_slot); | |
| (text, instance, pyre_object::gc_roots::shadow_stack_get(category_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/module/_warnings/mod.rs` around lines 628 - 654,
Reload the category value from category_slot after every allocating helper in
this warning-processing block. In the else branch, re-read category_slot for the
tuple’s final category after call_function_impl_result; in the true branch,
replace the .unwrap_or(category) fallback with a post-operation shadow-stack
reload so no stale local category pointer survives allocation. Preserve the
existing category/type selection behavior otherwise.
| /// | ||
| /// Layout: | ||
| /// `[ob_type | w_class | value:*mut Wtf8Buf | byte_len | len | w_slots]` | ||
| /// `[ob_type | w_class | value:*mut Wtf8Buf | byte_len | len | w_slots | hash]` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the documented object layout in sync.
Line 25 omits index_storage, so it says hash follows w_slots even though the field follows index_storage. Update the layout comment to avoid misleading future unsafe offset or GC work.
Proposed documentation fix
-/// `[ob_type | w_class | value:*mut Wtf8Buf | byte_len | len | w_slots | hash]`
+/// `[ob_type | w_class | value:*mut Wtf8Buf | byte_len | len | w_slots | index_storage:*mut Utf8IndexStorage | hash]`📝 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.
| /// `[ob_type | w_class | value:*mut Wtf8Buf | byte_len | len | w_slots | hash]` | |
| /// `[ob_type | w_class | value:*mut Wtf8Buf | byte_len | len | w_slots | index_storage:*mut Utf8IndexStorage | hash]` |
🤖 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/unicodeobject.rs` at line 25, Update the documented
object layout comment near the Wtf8 object definition to include index_storage
between w_slots and hash, matching the actual field order. Keep the existing
layout notation and all other documentation unchanged.
… read The `BaseException.args` arm of `try_walker_specialize_load_attr` built its symbolic tuple with `emit_object_tuple_inline`, which is unconditionally array-backed, while stamping the record-time concrete with `w_tuple_new`, which routes `len == 2` to `makespecialisedtuple2`. An arity-2 read therefore left the emitted shape disagreeing with its own concrete. `walker_guard_exc_match_tuple_items` dispatches on the match target's concrete `ob_type`, so an `except <tuple>:` whose target came from `e.args` took the `spec_oo` branch and guarded for a layout the trace never builds. The loop aborted instead of compiling: `loops_compiled 1`, `loops_aborted 5` against `2` / `0` for both the arity-3 and the literal-tuple control. `guard_failures` stayed flat only because no loop compiled. `emit_specialised_tuple_oo_inline` emits the `W_SpecialisedTupleObject_oo` shape (`NewWithVtable` + `w_class` + inline `value0` / `value1`), mirroring the `spec_ii` emit in `try_walker_specialize_newtuple`. The arm asks `newtuple` which shape the read produces rather than reproducing its dispatch, and settles it before recording any guard so the `Cls_ii` / `Cls_ff` decline — reachable only from an Object-strategy `args_w` holding two plain ints or two plain floats — leaves no pinned class behind. `excs = err.args` at arity 2 feeding `except excs:`: 0.89s -> 0.05s user, matching the arity-3 and literal-tuple controls at 0.05s. check.py dynasm 308/308 + cranelift 308/308. Assisted-by: Claude
…ew findings `fbw_reorder_call_kw_args` searched all of `varnames[..co_argcount]` for a keyword name, so a keyword could bind a positional-only parameter: `def f(x, /)` called as `f(x=1)` is a TypeError the interpreter raises, but the fold placed the value in slot 0. Instrumenting the binder shows it reaching that bind, so only a downstream decline kept the wrong answer off the output. Reject a match inside the positional-only range. `fbw_callee_scope_is_positional_only` replaces the keyword fold's inline flag check and now also gates the star-call path, which had none: `co_argcount` counts neither `*args` nor `**kwargs` nor keyword-only params, so `def f(a, *, b=5)` passed the arity check against a 1-tuple while `b` was never bound. `fbw_unpack_call_function_ex_args` read `W_Tuple.wrappeditems` off the star argument with no type check. That descr resolves to a structural field index, so an object whose slot 0 matches in offset, size and type can hit the same cache entry; `f(*some_list)` is ordinary Python. Pin the concrete to a tuple first, as the kwnames path does. `fbw_callee_body_replay_safety` read operand 0 of every `setfield_gc*` as the target ref register, but `setfield_gc_i/iid`, `setfield_gc_r/ird`, `setfield_gc_v/iid` and `setfield_gc_v/ird` address the target by raw int, so the freshness set was indexed with an int register number. Accept only the `r<value>d` shapes. `body_branch_targets` discovered joins from `L` operands, but `switch/id` carries its case targets in the descr, so a body containing one could keep a freshness claim alive across a switch join. Report no target set for such a body, which classifies it Dirty. check.py dynasm 311/311 + cranelift 311/311; call_function_ex_star and call_kw_hot_loop still fold at 0.06s. Assisted-by: Claude
`gc_current_object_address` asked `gc_owns_object`, which walks the old-generation arena index and the rawmalloc address set before the header read. A forwarding stub is only ever installed at a nursery address -- `copy_nursery_object` stamps it on the young object's own header, and the major collection is mark-and-sweep -- so the nursery range test answers the same question. Every root pin and every root reload asks it. Interleaved warm runs, aarch64, user time: startup 0.13s -> 0.05s, import_from_hot 1.97s -> 1.46s, str_fstring 2.90s -> 2.29s, getattribute_override_no_bind 1.52s -> 1.28s, calls_closures 0.96s -> 0.81s, dict_set 1.57s -> 1.45s. Assisted-by: Claude
`try_hash_value` fell through to its generic slot tail for an exact str / int / bool / float / bytes and called the builtin `__hash__` through the call protocol: two MRO lookups, a Vec allocation and an int box per key. Every ObjectKey-keyed container operation ran it -- the Object- and Unicode-strategy dicts behind `sys.modules`, module namespaces and every set -- where `UnicodeDictStrategy` upstream stores unwrapped keys and never reaches an app-level call. Those six types are not heap types, so an exact instance can only reach the digest `hash_value` already computes. The tuple arm takes the `stack_check` the leaf dispatch used to supply, so a nest deep enough to exhaust the C stack still raises RecursionError. Assisted-by: Claude
`warn_category` looked for `sys.modules['warnings']` and, on a miss, wrote the message straight to stderr with neither a filter nor a registry. Nothing imports `warnings` for a bare script, so every interpreter warning took that path: `~` on a bool inside a 1.5M iteration loop printed 1,500,000 lines / 510 MB, where cpython prints one and pypy prints none. `space.warn` now hands the message to the native `do_warn` with `stacklevel - 1`, as `baseobjspace.warn` does, so the filters, the module `__warningregistry__` and `catch_warnings(record=True)` observe the event whether or not the `warnings` wrapper is loaded. The two callers that were passing a `warnings.warn` stacklevel are rescaled to that convention; `~True` in a script now reports the same file, line and source line as cpython. A warning raised while `sys.modules['_warnings']` is absent or rebound still degrades to the unfiltered write rather than raising out of the operator that issued it. In `_warnings`: `do_warn` no longer folds in `get_category`, matching `interp_warnings.do_warn`, whose two callers resolve the category themselves; the two `space.isinstance_w` sites use the type-only test instead of the `isinstance()` builtin's `__instancecheck__` protocol; and the `State` names are read from the module namespace dict when `sys.modules` still holds the module. synth/arith_int_bool runs the machinery 1.5M times and goes 2.05s -> 11.6s against a 20s timeout; it carries no max-pypy-ratio. Assisted-by: Claude
…ounds `is_nursery_object_start` is `addr != 0`, the tagged-immediate filter and a range test against `nursery.start` / `nursery.size` — fields written only by `Nursery::new` (`reset` rewinds `ptrs.free` and nothing else). Reaching them went through the `ACTIVE_GC_IS_NURSERY_OBJECT` hook — a thread-local resolve, a `RefCell` borrow and a trait-object dispatch — or, when no box is installed, through `gc_sync`'s `gc_op` protocol. The latter is the production shape: `install_gc_standalone` registers the hooks and leaves the box empty. `pyre_object::gc_roots::pin_root` and `shadow_stack_get` ask on every pin and every slot read. `GcAllocator` gains `nursery_bounds` and `taggedpointers`, defaulting to `None` / `false` so a stub allocator never arms the path. `store_singleton` and `replace_singleton_leaking_old` publish the pair; `gc_is_nursery_object` answers inline while armed. `install_gc_box` in the dynasm, cranelift and wasm backends calls `disarm_published_nursery`, which latches so a later `store_singleton` cannot re-arm it: a per-thread allocator has its own nursery, which one process-wide pair cannot describe. Measured on a 300k-iteration loop issuing one `space.warn` per iteration: 1.11s -> 0.99s, interleaved, 3/3 runs. Assisted-by: Claude
… a list `rstr.py:395-412 ll_strhash` keeps the digest in the string header and recomputes it only while the slot still reads zero (`jit.conditional_call_elidable(s.hash, LLHelpers._ll_strhash, s)`); `unicodeobject.py:339-343 hash_w` reaches it through `compute_hash(self._utf8)`. `W_UnicodeObject` gains the field. Zero doubles as "not computed", so a digest that lands on zero is stored as the same substitute every time (`rstr.py:409-410`) and stays consistent for the value. All five constructors write it and the new setter is the only other writer; `W_UNICODE_OBJECT_SIZE` is `size_of`, so the raw-alloc paths and the JIT array descrs follow the layout. `hash_value`'s tuple arm collected the element digests into a `Vec` before folding. `tupleobject.py:409-420 _descr_hash_unroll` folds them straight into the accumulator; `_hash_tuple_xx_iter` takes the iterator and the slice form delegates to it. Assisted-by: Claude
* `descroperation::invert` re-wrapped its bool-deprecation message on every `~True`. RPython wraps a string literal once at translation time, so the message becomes a `PrebuiltText` cell and `warn_category_w` takes the already-wrapped message; `warn_category` keeps the per-call form. * `_warnings` read its `State` fields back through `sys.modules['_warnings']` — a dict probe plus a module/getattr fallback per access, and the state disappeared once app code rebound the name. Upstream reads them off `space.fromcache(State)`, which no Python name can reach; the module namespace is captured once instead, after every field is stored, so `state_is_readable` never reports a half-filled State. * `type_descr_call_impl` reloaded its pinned arguments into a throwaway `Vec` before extending the vector it hands to `__new__` and `__init__`. Both call sites now fill their vector from the slots directly. * `exc_base_exception_init` built a second `args_w` list over the objects the list from `__new__` already held (`interp_exceptions.py:123-124`, `:277-282`). It keeps the existing storage when that storage already holds exactly those objects; `args` is read out as a fresh tuple and the storage is never handed out, so its identity is not observable. Measured interleaved, 3 runs each, on a 300k-iteration loop issuing one `space.warn` per iteration: the exception-args change 1.17s -> 1.02s. Profile share of one warning: `_hash_str` 8.4% -> 0.2%, `check_sys_modules` 3.5% -> 0, `w_str_new` 4.3% -> 1.1%. Assisted-by: Claude
Probe scripts, profile dumps and baseline binaries are written under `scratchpad/` and are not part of the tree. Assisted-by: Claude
A scoped `Result<T, PyError>` callee returns `T` after the exception-link lowering, and the residual-call ABI likewise returns `T` with the error routed through `BH_LAST_EXC_VALUE`. Both ends of `getcalldescr`'s `RESULT == FUNC.RESULT` check were instead reading the whole `Result` ADT, which projects to `Ref` for every `T`; the two agreed while both were wrong. front/result_exc.rs: `collapse_pos0_read` returns the type of the `__pos_0` FieldRead it deletes, and the `?`-diamond narrows the producing `OpKind::Call`'s `result_ty` to it. The call op had kept the `Ref` stamp taken from its Rust `dest.ty` while the value flowing out of it was the unwrapped payload. Adds `tyref_result_ok`, sharing the `Ok` slot lookup with `tyref_result_ok_is_unit`. front/mir.rs: `dont_look_inside_return_token` projects the `Ok` payload before computing the token, so `Result<(), PyError>` reaches the unit arm. The `OBJECTPTR` carve-out keeps reading the declared output. front/checked_arith.rs: comment only. `_warnings::do_warn_explicit` did not build: `warned = update_registry()?` has payload `bool`, so the `Ref`-stamped call result entered a merge column whose other four predecessors carry `ConstBool(false)`; `union(Bool, Ref)` yields `Unknown`, the cleared binding is backfilled to `GcRef`, and `encode_regorconst_source` rejects the renaming. `#[elidable] bigint_truediv -> Result<f64, PyError>` is the callee-token case. check.py: dynasm 322/322, cranelift 322/322. Assisted-by: Claude
`emit_constant`'s `FnPath` arm materialises a function-item constant as a 0-arg `Call` on the callee's real path and stamped the result `Int`. A function pointer is `Ptr(FuncType)`, whose `getkind` is `r` — the same reason the `Str` arm above it declares a Ref. Stamp `Ref(None)`. Reusing the real path also puts the define in front of `getcalldescr`'s `RESULT == FUNC.RESULT` check (`call.py`), which reads it as a genuine 0-arg call to that function. That check is gated on the callee's graph carrying a `FUNC.RESULT` token, stamped only for `dont_look_inside` / `elidable_residual` / dyn-indirect callees, so the mismatch stayed dormant until `w_dict_new` became `dont_look_inside`: `_warnings::setup_context` threads it through `Option::unwrap_or_else`, and the build aborted with "calling a function with return type Ref, but the actual return type is Int". check.py: dynasm 329/329, cranelift 329/329. Assisted-by: Claude
`ExecutionContext::gettopframe_nohidden` and `getnextframe_nohidden` called `force_frame` on every frame they walked. `executioncontext.py` has no force in either walk: upstream emits `jit_force_virtualizable` per redirected field access (`rvirtualizable.hook_access_field`) and `jtransform` deletes it again in every graph the codewriter looks inside. Forcing in the walk escaped the traced virtualizable for every frame-walking helper, so a residual call that reached one raised `VableEscapedDuringResidualCall`. Both walks are now force-free and the consumers force instead: `sys._getframe` (which also forces `gettopframe` before the walk, so an inlined callee's frame is materialised before the walk picks a frame), `sys._current_frames`, `PyFrame::fget_f_back` (both `self` and the result), and the coroutine-origin walk. `force_frame` is now `pub`. Nothing else kept the heap frame's `last_instr` current once the walk stopped forcing, so `try_execute_residual_call_via_executor` records a `SetfieldGc` of the virtualizable's `last_instr` alongside the `mirror_vable_static_to_boxes` box half already there, from the same constant. Marks four interpreter functions `dont_look_inside` and registers their addresses in `jit_fnaddr`: `builtins::lookup_exc_class` (reads the `EXC_CLASS_REGISTRY` static), `host_seam::emit_stdout` / `emit_stderr` (host stdio handles), and a new `descroperation::bool_invert_deprecation_text` accessor wrapping the `PrebuiltText` static. Each of those statics failed the front-end lift and, transitively, the lift of every caller; `warn::warn_category_w` and `descroperation::invert` now get jitcode. Ports `vable_after_residual_call`'s `debug_print` under `PYRE_FBW_DEBUG_ABORT`, naming the callee that forced the virtualizable. check.py: dynasm 331/331, cranelift 331/331. `synth/arith_int_bool` 11.6s -> 4.09s (dynasm, measured under load). Known divergence: `~bool` in a traced loop reports one extra DeprecationWarning at the loop-entry line. The compiled path is correct; the recording walk leaves the live frame's `last_instr` at the resume coordinate. Assisted-by: Claude
…residual The recording walk dispatches opcodes itself rather than through `execute_opcode_step`, so nothing advanced the live frame's `last_instr` and it still named the last resume point while a residual ran concretely. A frame reader inside the callee therefore saw the wrong line: `_warnings` keyed its registry on it and re-issued a warning the interpreted run had already deduplicated, so `~bool` in a traced loop reported two DeprecationWarnings — one at the `~` statement, one at the loop-entry line — where CPython reports one. `LiveLastInstrGuard` sets the field to the executing pc for the duration of the residual and restores it after, because `last_instr` is also the resume coordinate: `flush_walk_end_state_to_frame_inner` writes `resume_py_pc - 1` and `ActiveFrameEscapeGuard` is entered with the same pc, so the two meanings differ by exactly one and can only coexist while the residual runs. The guard publishes onto `INLINE_CONCRETE_FRAME` when one is set. Inside an inline sub-walk `vstack_cur_pypc` is in the callee's code, so writing it onto the walk's virtualizable strands that frame's replay on a stack depth it never had (`synth/getframe_inlined_callee_own_frame`). `capture_escape_flush_undo` captures the value the guard displaced rather than the field, so withdrawing a committed escape restores the resume coordinate. The guard skips its own restore when a flush committed onto the same frame. check.py: dynasm 331/331, cranelift 331/331. `synth/arith_int_bool` 2.65s dynasm / 2.77s cranelift. `raise_catch` 1.4x dynasm at the same load where the parent commit measured 1.5x. Assisted-by: Claude
`_hash_str` returned 0 for empty input without running siphash. `ll_strhash`'s
`return 0` arm is a null-pointer check (`if s:`), not an empty-string case, so
upstream digests `b""` like any other value: SipHash-2-4 under the all-zero key
gives `0x1e924b9d737700d7`, which is what PyPy reports for `hash("")` under
`PYTHONHASHSEED=0`.
Measured before: `hash("")` was 29742 (the zero-substitute, since the
short-circuit fed 0 into it) and `hash(b"")` / `hash(memoryview(b""))` were 0,
against 2202906307356721367 for all three on PyPy. Every non-empty digest
already matched PyPy byte-for-byte and is unchanged.
Also moves the zero-digest substitute from the str caller into `_hash_str`,
which is where `_ll_strhash` applies it, and corrects its value from 29742 to
upstream's 29872897. Applying it inside the digest keeps bytes and memoryview
in agreement with the value a str caches for the same bytes.
check.py: dynasm 326/326, cranelift 326/326.
Assisted-by: Claude
…learing it `PUBLISHED_LAST_INSTR` is a single TLS slot, but the guard is entered per residual call and a residual can run user code that records a nested walk whose own residual enters a second guard. The inner drop cleared the slot, so a still-live outer publication became invisible and `capture_escape_flush_undo` would snapshot the executing pc as the outer frame's resume coordinate. The guard now saves the displaced entry and puts it back on drop, the same discipline `InlineConcreteFrameGuard` and `ResidualFrameChainGuard` already use for their thread-locals. check.py: dynasm 326/326, cranelift 326/326. Assisted-by: Claude
`mirror_vable_static_to_boxes` and `vable_setfield_descr` both name the walk's virtualizable, but inside an inline sub-walk `vstack_cur_pypc` indexes the callee's code. Compiled code therefore stamped a callee pc onto the caller's frame, and `offset2lineno` resolved it against the caller's code object: a warning raised from an inlined callee reported a line picked by where that byte offset happened to land. Gate both halves on the predicate `LiveLastInstrGuard` already uses to retarget the concrete store. Assisted-by: Claude
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac4ba4ca73
ℹ️ 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".
| target: CallTarget::FunctionPath { segments }, | ||
| args: vec![], | ||
| result_ty: ValueType::Int, | ||
| result_ty: ValueType::Ref(None), |
There was a problem hiding this comment.
When a FnDef constant remains live across a guard or collection, this Ref stamp makes register splitting and GC maps treat its executable address as a GcRef, so the collector may inspect a code address as an object. The cited upstream rule actually says the opposite of the new comment: rpython/jit/metainterp/history.py:64-68 maps Ptr(FuncType) to int because the pointee has raw GC kind. Preserve that integer-bank representation (ideally with a real function-pointer constant rather than changing the synthetic call's result kind).
AGENTS.md reference: AGENTS.md:L194-L196
Useful? React with 👍 / 👎.
1 participant
What
ExecutionContext::gettopframe_nohiddenandgetnextframe_nohiddencalledforce_frameon every frame they walked.executioncontext.pyhas no force in either walk — upstream emitsjit_force_virtualizableper redirected field access (rvirtualizable.hook_access_field) andjtransformdeletes it again in every graph the codewriter looks inside, so only graphs the JIT cannot see keep the call.Forcing inside the walk escaped the traced virtualizable for every frame-walking helper, so any residual call that reached one raised
VableEscapedDuringResidualCall. Concretely:~boolemits a DeprecationWarning,_warnings::setup_contextwalks the frame chain, and that walk aborted the trace — so a loop containing~boolnever compiled at all.Both walks are now force-free and the consumers force instead:
sys._getframe,sys._current_frames,PyFrame::fget_f_back, and the coroutine-origin walk.Follow-on:
last_instrNothing else kept the live frame's
last_instrcurrent once the walk stopped forcing, and that field has two incompatible required values at a residual-call site:vstack_cur_pypceval.rs frame.last_instr = pc; the existingmirror_vable_static_to_boxesbox halfresume_py_pc - 1flush_walk_end_state_to_frame_inner, withActiveFrameEscapeGuardpassing that same pcExactly off by one, always. The old code never hit the conflict because forcing during the walk always ended in
ABORT_ESCAPE, so the resume coordinate never had to survive the residual.SetfieldGcoflast_instrrecorded from the same constant as the box mirror, so shadow and heap cannot disagree.LiveLastInstrGuardpublishes the executing pc for the residual's duration and restores it afterwards. It targetsINLINE_CONCRETE_FRAMEwhen one is set — inside an inline sub-walkvstack_cur_pypcis in the callee's code, and writing it onto the walk's virtualizable strands that frame's replay on a stack depth it never had.Without this,
~boolin a traced loop reported two DeprecationWarnings (one at the~statement, one at the loop-entry line) where CPython reports one.Lift fixes
Four interpreter functions are marked
dont_look_insidewith their addresses registered injit_fnaddr:builtins::lookup_exc_class(reads theEXC_CLASS_REGISTRYstatic),host_seam::emit_stdout/emit_stderr(host stdio handles), and a newdescroperation::bool_invert_deprecation_textaccessor wrapping aPrebuiltTextstatic. Each static failed the front-end lift and, transitively, the lift of every caller — which is howwarn::warn_category_wanddescroperation::invertlost their jitcode.Also ports
vable_after_residual_call'sdebug_printunderPYRE_FBW_DEBUG_ABORT, naming the callee that forced the virtualizable.Review follow-ups
Two fixes from the review of this branch:
LiveLastInstrGuardnow saves and restores the previous publication instead of clearing it on drop. The guards inresidual_call.rsnest — a residual runs user code that records a nested walk whose own residual enters the guard again — so clearing on the inner drop hid a still-live outer publication. Every sibling (InlineConcreteFrameGuard.previous,ResidualFrameChainGuard.previous_published,ActiveFrameEscapeGuard.prev) already saved-and-restored; being the odd one out was the tell. The corpus does not cover the nesting, so this was latent rather than reproducible.mirror_vable_static_to_boxesandvable_setfield_descrboth namedstandard_virtualizable_box()— the walk's virtualizable — while usingvstack_cur_pypc, which inside a sub-walk indexes the callee's code. Compiled code therefore stamped a foreign pc onto the caller's frame andoffset2linenoresolved it against the caller's code object.LiveLastInstrGuardalready retargeted the concrete store toINLINE_CONCRETE_FRAME; the IR emission did not. Both halves are now gated on the same predicate.Verifying this area needs care — two natural-looking instruments are unsound. The warning dedup registry keys on lineno, so 99 999 compiled iterations that all report the same wrong line show up as one extra histogram entry, indistinguishable from a single stray recording event; and
warnings.simplefilter("always")makesshow_warningdo stderr I/O every iteration, which stops the loop compiling (Total # of loops: 0) so the measurement is of the interpreter. The instrument that works is a countingshowwarningwith no I/O, plus confirmingTotal # of loops > 0.Numbers
synth/arith_int_bool: 11.6s → 2.65s dynasm, 2.77s cranelift (cpython 1.00s, pypy 0.03s — PyPy has no~booldeprecation at all, so this bench's pypy ratio is structurally unwinnable; CPython is the honest comparison).Warning attribution matches CPython exactly in the flat shape on both backends (
[(19, 100000)]). It does not match when the warning is raised from a JIT-inlined callee: the frame chain has no callee frame, so the warning is attributed to the caller. This divergence is known and deliberately left here.Closing it is not a matter of forcing in the consumer. That was built and measured: correct (
[(14,5),(15,100005)]vs CPython[(15,100000)]) but 15.97/17.17/15.24 s against 4.61/6.81/7.05 s without — i.e. straight back to the regression this PR removes, because the force lands as a live may-force residual inside the (still lifted)setup_context, escapes the virtualizable and aborts every trace. It is "correct" only because nothing compiles. Upstream'ssetup_contextjitcode carries zero force ops; forcing there is a property of the call boundary, not of the consumer.The orthodox fix is to wire the vref producer — emit
enter/leave(f_backref+topframeref = virtual_ref(callee)) at the inline push/pop, as upstream gets for free by tracingexecutioncontext.enter. Forcing a vref is a gracefulVIRTUAL_REF_FINISH, not theABORT_ESCAPEthat forcing the virtualizable causes. That work is an epic with its own prerequisites (walker-path_do_jit_force_virtual, the vref residual-call bracket,ResidualFrameChainGuardvref-awareness,EC_DESCR_GROUPextension, scoped calleelast_instr) and a blast radius covering the whole inline-call corpus, so it is deliberately out of scope for this PR.check.py: 328/328 on both backends (dynasm and cranelift), measured at
769da5c8on base2580479628with the box quiet (load 13.1). The branch has since been rebased onto512e5cb32b; the files this PR touches are byte-identical across that rebase, but the base moved, so CI is the authority for the current head.— commented by Claude
Summary by CodeRabbit
Bug Fixes
Performance
Compatibility