Repository navigation
jit: keep constructor inlining out of parented guard snapshots (fixes the test_asyncio hang) - #1016
Merged
Merged
Conversation
|
Warning Review limit reached
Next review available in: 53 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (54)
WalkthroughThe PR centralizes JitCode-to-Python coordinate resolution in ChangesJIT coordinate migration
Estimated code review effort: 4 (Complex) | ~45 minutes 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 |
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit 7652b1e). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patch
2. Other mismatches introduced by this patch
3. Pre-existing mismatches (already present before this patch)
4. Structural adaptations
|
![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
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d5a17abb3
ℹ️ 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".
| && inline_depth < fbw_max_multiframe_depth() | ||
| && callee_code.cellvars.is_empty(); | ||
| && callee_code.cellvars.is_empty() | ||
| && constructor_result.is_none(); |
There was a problem hiding this comment.
When a straight-line __init__ performs an observable mutation before a later guard (for example, updating an argument's field and then reading an attribute from another argument whose class changes after warmup), this condition disables strict_seed, so parent_frame becomes None and the guard resumes at the caller's CALL boundary. The compiled prefix has already performed the mutation, and re-executing the entire constructor in the interpreter performs it again. Keep these constructors residual or preserve a callee frame while explicitly reconstructing the constructor's instance result.
AGENTS.md reference: AGENTS.md:L32-L34
Useful? React with 👍 / 👎.
…lot fallback `inject_root_call_result`'s raw-color fallback is documented as load-bearing and simultaneously wrong. `PYRE_INJECT_RCA` reports each engagement with its coordinate; `PYRE_INJECT_FALLBACK_DECLINE` declines the bridge compile instead of engaging it, as the A/B control for attributing a wrong-slot injection. Both are off in production; the gated branches are the only added code. Assisted-by: Claude
A guard emitted inside an inlined `__init__` resumed through a parented
snapshot chain ([caller paused after the CALL, `__init__` frame]), so the
blackhole delivered `__init__`'s return value — None — into the caller's CALL
dst register. `typeobject.py descr_call` discards `__init__`'s result and
returns the instance; the flattened constructor inline has no frame that can
reconstruct that discard at a mid-`__init__` pause.
Two changes in `try_walker_inline_resolved_user_call`:
* a constructor callee that is not strict-straight-line returns `Ok(None)`
(residual instantiation) instead of entering the forward-branch multiframe
path — and before the arm that would decline the enclosing key to
interpretation;
* `strict_seed` excludes constructors, so a straight-line `__init__` inlines
through the ordinary single-frame path whose guards collapse to the caller
boundary (deopt re-executes the whole instantiation call).
This was the cause of the test.test_asyncio hang/error cascade: a warmed
`BaseEventLoop._call_soon` trace inlined `events.Handle(...)` including its
branchy `__init__`; flipping the loop debug flag failed the `get_debug()` bool
guard inside the inlined body and every subsequent `call_soon` produced
`handle = None` (`AttributeError: 'NoneType' object has no attribute
'_source_traceback'`, lost wakeups, event loop parked in kevent forever).
New fixture `synth/type_call_inline_init_branch_deopt` reproduces the shape
deterministically (verified failing on the unfixed build) with converged
jitstats baselines on all three backends.
Verified: check.py dynasm 372/372, cranelift 372/372, wasm 368/368;
cpython_tests gate 47 PASS / 0 regressions (test.test_asyncio included);
full test_asyncio suite completes in ~2min (one intermittent
`_run_once` `self`-slot corruption class remains, tracked separately).
Assisted-by: Claude
The 80-char cap cut a CI panic reason at "...blackhole.rs:6", dropping the line number. The sibling signal/abort branch already keeps 200 chars. Assisted-by: Claude
The rebase onto main (through #1015 and #1019) moved structural jit-stats on 26 synthetic fixtures. #1019 re-recorded only the eight dynasm baselines; the cranelift/wasm baselines of the same fixtures and the remaining drifted fixtures are re-recorded here from a 3-backend measurement. No correctness or output change; the recorded values reproduced exactly across the record run and a full verification run (dynasm 373/373, cranelift 373/373, wasm 369/369 green). Assisted-by: Claude
youknowone
force-pushed
the
rewrite-tracer
branch
from
August 4, 2026 06:57
b74e5b6 to
9742df5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9742df5030
ℹ️ 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".
Comment on lines
+2945
to
+2946
| if constructor_result.is_some() && !strict_inlinable { | ||
| return Ok(None); |
There was a problem hiding this comment.
When __init__ contains any branch, loop, or vable shape that makes strict_inlinable false, this early return makes try_walker_inline_type_call discard the emitted allocation and execute the entire class call residually on every compiled-loop iteration. Upstream instead enters each inlined call through its own MIFrame (rpython/jit/metainterp/pyjitpl.py:2445-2475), which lets descr_call receive and discard the initializer result without disabling constructor optimization. Please preserve the constructor continuation and its per-frame red frame rather than fixing the resume mismatch by removing all non-strict constructors from the JIT.
AGENTS.md reference: AGENTS.md:L32-L42
Useful? React with 👍 / 👎.
…cripts The parity runner passes a script only when it exits 0 AND its last stdout line is exactly "OK". Five of #950's scripts printed nothing, one printed lowercase "ok", and newmemoryview_omitted_geometry imported __pypy__ unconditionally, failing under the CPython reference runner. These never ran in CI before because the check.py step ahead of the parity step was red on main. Assisted-by: Claude
…hon 3.14.6 CPython 3.14.6 grew PyBaseExceptionGroupObject by one pointer slot, so __basicsize__ moved 88 -> 96 for BaseExceptionGroup and ExceptionGroup. Update cpython314_builtin_abi and the type_members_python314 expected table to the 3.14.6 values (measured; the two groups are the only changed entries, flags unchanged). CI's reference CPython is 3.14.6 while local homebrew is 3.14.5, which is why this only failed in CI. Assisted-by: Claude
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7652b1eab8
ℹ️ 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".
Comment on lines
+10
to
+12
| except ImportError: # the CPython reference runner has no __pypy__ | ||
| print("OK") | ||
| raise SystemExit |
There was a problem hiding this comment.
If a pyre backend regresses its __pypy__ module registration or import path, this broad ImportError handler prints the runner's success sentinel and exits before exercising newmemoryview. Because parity_tests/run.py considers exit code 0 plus a final OK sufficient, losing the entire module would now pass this test; gate the skip on the known CPython runner instead so import failures remain failures under pyre.
Useful? React with 👍 / 👎.
This was referenced Aug 4, 2026
youknowone
added a commit
that referenced
this pull request
Aug 4, 2026
Recorded with `--snapshot` on dynasm and cranelift. The counters that move are bridges_compiled, guard_failures, loops_compiled and loops_aborted; internal_compile_panics and the three descr_set_* invariants stay 0 in every file. No loops_compiled recorded here is below its value before #1016. Where one reads lower than main's, main's own value rose further: pickle_ctor_args cranelift goes 4 -> 6 (main) -> 5 (here) with guard_failures 201 -> 437 -> 202 and bridges_compiled 1 -> 2 -> 1, and pickle_terminal_raise_resume cranelift goes 34 -> 37 -> 36 with guard_failures 652 -> 664 -> 652. The two foriter_* fixtures drop bridges_compiled 2 -> 1 with guard_failures 496 -> 296 and 401 -> 201 while loops_compiled holds. The wasm baselines of existing fixtures are left untouched: only ubuntu-24.04 runs that leg and a macOS recording disagrees with it. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Aug 5, 2026
Recorded with `--snapshot` on dynasm and cranelift. The counters that move are bridges_compiled, guard_failures, loops_compiled and loops_aborted; internal_compile_panics and the three descr_set_* invariants stay 0 in every file. No loops_compiled recorded here is below its value before #1016. Where one reads lower than main's, main's own value rose further: pickle_ctor_args cranelift goes 4 -> 6 (main) -> 5 (here) with guard_failures 201 -> 437 -> 202 and bridges_compiled 1 -> 2 -> 1, and pickle_terminal_raise_resume cranelift goes 34 -> 37 -> 36 with guard_failures 652 -> 664 -> 652. The two foriter_* fixtures drop bridges_compiled 2 -> 1 with guard_failures 496 -> 296 and 401 -> 201 while loops_compiled holds. The wasm baselines of existing fixtures are left untouched: only ubuntu-24.04 runs that leg and a macOS recording disagrees with it. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Aug 5, 2026
Recorded with `--snapshot` on dynasm and cranelift. The counters that move are bridges_compiled, guard_failures, loops_compiled and loops_aborted; internal_compile_panics and the three descr_set_* invariants stay 0 in every file. No loops_compiled recorded here is below its value before #1016. Where one reads lower than main's, main's own value rose further: pickle_ctor_args cranelift goes 4 -> 6 (main) -> 5 (here) with guard_failures 201 -> 437 -> 202 and bridges_compiled 1 -> 2 -> 1, and pickle_terminal_raise_resume cranelift goes 34 -> 37 -> 36 with guard_failures 652 -> 664 -> 652. The two foriter_* fixtures drop bridges_compiled 2 -> 1 with guard_failures 496 -> 296 and 401 -> 201 while loops_compiled holds. The wasm baselines of existing fixtures are left untouched: only ubuntu-24.04 runs that leg and a macOS recording disagrees with it. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Aug 5, 2026
Recorded with `--snapshot` on dynasm and cranelift. The counters that move are bridges_compiled, guard_failures, loops_compiled and loops_aborted; internal_compile_panics and the three descr_set_* invariants stay 0 in every file. No loops_compiled recorded here is below its value before #1016. Where one reads lower than main's, main's own value rose further: pickle_ctor_args cranelift goes 4 -> 6 (main) -> 5 (here) with guard_failures 201 -> 437 -> 202 and bridges_compiled 1 -> 2 -> 1, and pickle_terminal_raise_resume cranelift goes 34 -> 37 -> 36 with guard_failures 652 -> 664 -> 652. The two foriter_* fixtures drop bridges_compiled 2 -> 1 with guard_failures 496 -> 296 and 401 -> 201 while loops_compiled holds. The wasm baselines of existing fixtures are left untouched: only ubuntu-24.04 runs that leg and a macOS recording disagrees with it. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Aug 5, 2026
…rgs_w (#1033) * exc: store args_w as an object-strategy list `W_BaseException.args_w` is an RPython `list of W_Root` (interp_exceptions.py:114 `args_w = []`). List strategies belong to the app-level `W_ListObject` (objspace/std/listobject.py) and have no counterpart in an RPython list, but pyre built the slot with `w_list_new`, which picks a representation from the element types: `ValueError(7)` got Integer, `ValueError(1.5)` Float, `ValueError()` Empty. Add `w_exception_args_new` and route every `args_w` construction through it, including `coerce_to_list_for_args` (descr_setargs) and the message helpers. The `args` load fold (`exception_attr_slot_fold` consumer in specialize.rs) decomposes the object items block and declines any other strategy, so those shapes left `e.args` as a residual getattr call. With the slot pinned, the fold applies at every arity. The trace-side mirror had to move with it: virtualized exception construction picked its emit shape from `list_strategy_for`, so it emitted an Integer-strategy list while the interpreter now builds an Object one, and the disagreement tripped the `_opimpl_getfield_gc_any_pureornot` strategy-field sanity check on `raise ValueError(i)` in a loop. Emit the one object shape instead, dropping the Int/Float/Empty arms and their unboxing guards, and route the bare-class raise through the same helper. `emit_object_list_inline` now allocates `len.max(1)` capacity, matching `alloc_list_items_block_gc`, so the zero-argument emit agrees with the interpreter's block. Measured on dynasm release, `(o.args is not None)` in a hot loop (ns/iter, pypy3 0.5 for all): `ValueError(7)` 463.7 -> 11.7, `ValueError()` 289.5 -> 12.2, caught `e.args` 269.0 -> 12.1. `ValueError(7, 8)` stays residual on the pre-existing arity-2 `Cls_ii` / `Cls_ff` specialised-tuple decline in the load fold. An args oracle over element types, the setargs round-trip, str/repr, __reduce__/pickle, OSError flattening, StopIteration.value and SystemExit.code is byte-identical to pypy3 before and after. check.py: dynasm 371/371, cranelift 371/371, wasm 367/367. Assisted-by: Claude * jit: trace SET_ADD / MAP_ADD comprehension bodies for_iter_bodies_all_jit_safe admitted LIST_APPEND but not SET_ADD or MAP_ADD, so a set or dict comprehension anywhere in a function marked the whole code object CurrentFrameOnly: no jitcode was built and every loop in that function, including ones lexically before the comprehension, stayed interpreted (mc_entered=0). Admit all three accumulators under the existing no-CALL-in-body rule, and add an enclosing-loop clause for SET_ADD / MAP_ADD: a comprehension nested in a call-bearing FOR_ITER body starts its own FOR_ITER with the enclosing expression's operands live on the value stack, and re-entering compiled code through a bridge from that shape segfaults today. BUILD_SET / BUILD_MAP now write the accumulator into its value-stack slot instead of only bumping the symbolic depth, matching BUILD_LIST: SET_ADD and MAP_ADD read the container back through getarrayitem_vable_r, and the blackhole replays BUILD -> accumulate from the jitcode on a mid-frame resume. The reload itself is factored out as emit_accumulator_reload and shared with LIST_APPEND. loops_comprehension 25.6x -> 10.6x pypy on dynasm (26.0x -> 14.1x cranelift), comprehension_accumulators 8.5x -> 3.8x; both jitstats baselines re-snapshotted (loops_compiled 4 -> 7 and 0 -> 1). check.py dynasm 371/371, cranelift 371/371, wasm 367/367. Assisted-by: Claude * jit: inline a user __getitem__ at BINARY_SUBSCR `try_walker_specialize_subscr` recognizes only the canonical tuple and exact-`list[int]` storage shapes; every other receiver kept the generic `binary_op` residual, so a `__getitem__` written in Python ran as a fresh interpreter frame behind a CALL_MAY_FORCE on every iteration. Add `getitem_fast_path` (baseobjspace.rs), which resolves the method `getitem` would dispatch for the two receiver shapes whose subscript the type's own MRO owns: a builtin sequence layout overriding `__getitem__` (the `subclass_special_override` arm) and a user instance (the `getitem_instance` arm). Both resolve from `ob_type` and `w_class` alone. `try_walker_inline_subscr_getitem` enters the resolved-callee inline with that method as `self`-bound callee, guarded by the receiver class + type version tag. A callee owning its own loop header is declined: that is the one arm of `try_walker_inline_resolved_user_call` that reads the residual's operand list as a CALL's, which this route does not carry. The verdict rides `InlineBodyFacts` as `owns_loop_header`. Measured dynasm release, `o[3]` on a plain class, one case per process: 248.1 -> 1.8 ns/it (pypy3 0.7); with a branch and an untaken raise in the body, 3.1 ns/it. New bench `synth/subscr_user_getitem_inline` covers both receiver shapes plus a second class at the site, an `__getitem__` rebound mid-loop, and an instance-dict `__getitem__`. check.py dynasm 371/371, cranelift 371/371, wasm 367/367; cargo test --all --features dynasm green. Assisted-by: Claude * jit: fold co_name and co_firstlineno off the code object Extend the traceback-walk field fold past `f_code`: `co_name` reads the realized `w_name` slot and `co_firstlineno` reads `co_firstlineno_raw`, each behind the receiver's type/version guard plus a non-null guard on `code_ptr` — the `require_code` check `code_get_field` runs before every field. A null `w_name` (not yet realized) declines to the residual. Three standalone `PyreFieldDescr`s carry the layout (`code_ptr`, `w_name`, `co_firstlineno_raw`) rather than a positional group: a `PyCode` is never allocated from a trace, so a group would publish a second, partial layout answer under the live `W_CODE_GC_TYPE_ID`. `co_firstlineno_raw` is the first signed 4-byte field descr; a code object built through the `CodeType` constructor with a negative first line reads back correctly on dynasm, cranelift and wasm. The remaining code fields are left residual: they read the host `CodeObject` behind `code_ptr` rather than a slot on the `PyCode`. Assisted-by: Claude * jit: inline __getitem__ at a generic sequence FOR_ITER `for x in obj` over an object that defines only `__getitem__` builds a `W_SeqIterObject` whose `next` reaches the generic `space.getitem` arm, so every item cost a full interpreter frame through the opaque `ForIterNext` residual. Specialize that step: guard the iterator class, read `seq` and `index` off the cursor, box the index, and route the subscript through the same inline lever `BINARY_SUBSCR` uses. The cursor advance is emitted and applied only after the callee returns an item, so a guard inside the body resumes at this FOR_ITER with the same index. That is also how exhaustion works — the terminating `__getitem__` raise is never traced; its branch guard fails, the interpreter re-executes the FOR_ITER residually, and the existing `IndexError`-to-exhaustion path runs unchanged. `try_walker_inline_resolved_user_call` therefore keeps the callee frame unseeded for a `ForIterNext` descr rather than taking the multiframe seed. Admission is `Clean` or `DeferredCall` replay safety. `DeferredCall` is needed because the terminating `raise` classifies as one; the shape the shared FOR_ITER gate withholds it from — a `BINARY_OP` dunder entry with an `arg_class_guard` — cannot reach a route whose only operand is the iterator. Builtin `seq` payloads (list/tuple/str/bytes-like/array/generic-alias) take their own arms in `next` and keep the residual. A declined inline cuts the speculative ops together with the snapshots that named them. Marginal per-item cost on a 45-element cursor: 400ns -> 5ns. Gate ratios: seqiter_tuple_error_parity 31.0x -> 19.3x, seqiter_getitem_lazy 15.3x -> 12.2x on dynasm; 31.2x -> 17.1x and 22.6x -> 19.4x on wasm. Assisted-by: Claude * jit: add W_Range.stop to the range descr census `W_Range` has four pointer slots but `RANGE_DESCR_GROUP` listed three — `stop` had no offset const and no descriptor at all. Nothing allocates a `W_Range` from a trace today, so the omission is inert; it stops being inert the moment one does, because `clear_gc_fields` walks exactly this group's `gc_fielddescrs` to emit a fresh object's delayed NULL stores and an unlisted slot would keep recycled nursery bytes for the collector to follow as a child reference. This is the census-completeness argument `FUNCTION_DESCR_GROUP` already states. Adding `stop` in byte-offset order shifts `step` and `length`, so the three accessors resolve by offset through a `range_field_descr` helper rather than by list position, the way `function_field_descr` does. Behaviour-neutral: the range GET_ITER/FOR_ITER virtualization measures identically before and after (a hoisted `range(5)` inner loop stays at 7.6 ns per outer iteration, a 45-element one at 33.5). Assisted-by: Claude * jit: virtualize range() with trace-constant bounds `range(stop)` / `range(start, stop)` / `range(start, stop, step)` called on the exact `range` type with exact canonical plain-int arguments now lowers to `NewWithVtable` plus four `SetfieldGc` instead of the opaque constructor residual, so the existing GET_ITER specialization can consume it without forcing either allocation. The route guards the callable by value when it is not already a trace constant, guards each argument's class and exact `w_class`, then reads the unboxed `intval`. The authentic `W_Range` is built first under `force_plain_eval`, and its four fields are what the emitted boxes are pinned to. Bounds must be trace constants. That is what makes the emitted `length` a record-time constant; a bound varying per iteration would pair a stale length with fresh start/stop/step. Emitting `compute_range_length` instead regressed two gate shapes -- an alternating empty/non-empty bound side-exits a guard every other call, and the division chain aborts the wasm trace for a `range` inside a self-recursive callee inlined per level. The test needs the unboxed value, so it runs after the guards and rewinds the trace with `cut_trace_with_snapshots` plus a heapcache reset on decline. Supporting descriptors: `range_stop_descr()` (`W_Range.stop` joined the census in the preceding commit) and `w_range_size_descr()`. New bench `range_ctor_in_loop` covers the fast shapes plus a `range` subclass, an `__index__` argument, a zero-step `ValueError`, and 20000 escaping ranges stored in a list. dynasm 373/373, cranelift 373/373, wasm 369/369, `cargo test --all --features dynasm` green. Assisted-by: Claude * jit: inline a method-form callee whose body raises `widened_method_form && body_facts.contains_raise` declined the inline, so an unbound method-form callee that both reads `self.attr` and contains a `raise` stayed on the residual call -- whether or not the raise ever executed. The decline was there because a guard whose resume coordinate landed on the `Reraise` needed ref registers the recorded path never wrote (`collect_callee_active_boxes`), and it arrived mid-recording on a non-effect-free opcode with no mid-body carrier, discarding the whole enclosing loop. e755f4f9aa keeps the raise operand in the filtered resume liveness, so the sub-walk now records the handler region with the registers that coordinate names. Measured, dynasm release, 800k calls, min-of-5 (scratchpad harness crossing call form against body content). `o.m()` form, ns/call: if self.i < 0: raise; self.i += 1; return 1 1705 -> 15.3 if self.i >= self.n: raise; self.i += 1; ... 1207 -> 15.9 ... same but `return self.i` 1006 -> 17.5 The same bodies reached as `FN(o)` were already 14-29, and replacing the `raise` with a `return` was already 17.7 -- so the raise token was the whole difference, not the branch, the compare or the store. Two jit-stats baselines move, both downward and both deterministic (5/5 identical runs): pickle_ctor_args loops 5->4, bridges 2->1, guard_failures 436->201 (cranelift) pickle_terminal_raise_resume loops 35->34, guard_failures 664->652 (cranelift) `loops_aborted` stays 0 on both. The lost loop is a callee that no longer compiles separately because it is inlined; a pairwise-median A/B against a 1.005 self-control puts `pickle_ctor_args` at 0.929 of the declining arm. dynasm 373/373, cranelift 373/373, wasm 369/369, `cargo test --all --features dynasm` green. Assisted-by: Claude * jit: keep a foreign innermost pc out of the live frame's last_instr `decode_and_restore_guard_failure` returns the innermost resumed section's Python pc whenever the resume spans more than one frame, so for an inlined callee that pc belongs to the callee's own code object. The paired valuestackdepth correction is already gated on that section belonging to the physical frame's code; the `last_instr` write next to it was not. Every arm after the write can still return `ResumeBlackhole`, and the blackhole refuses a jitcode carrying an abort opcode or a pc whose liveness it cannot decode, so the interpreter can resume the live frame at a pc that indexes a foreign code object. Gate the write with the same condition and keep the vable-derived position otherwise; a `Resume::ForeignInnermostLastInstr` census key marks the suppressed override. Single-frame resumes are unchanged: there the returned pc is already the vable-derived one. The key records 0 times across the 358 synthetic benches, so this is a latent path, not a measured failure. Assisted-by: Claude * jit: census the branchy-handler inline decline by replay class The decline for a raising callee that carries its own exception handler emitted nothing, so it had no denominator and no way to say which of its four terms was binding. Bind the replay-safety class instead of testing it inline — the short-circuit order is unchanged, the three cheaper terms still gate the call — and record `InlineCallee::BranchyHandlerDeferredCall` or `...Dirty` on the decline. Measured: a callee shaped `try: ... raise ... except:` declines as DeferredCall. The key records 0 times across the exception-bearing synthetic benches, so no existing bench covers the shape. Assisted-by: Claude * check.py: re-record 15 jitstats baselines on both backends Recorded with `--snapshot` on dynasm and cranelift. The counters that move are bridges_compiled, guard_failures, loops_compiled and loops_aborted; internal_compile_panics and the three descr_set_* invariants stay 0 in every file. No loops_compiled recorded here is below its value before #1016. Where one reads lower than main's, main's own value rose further: pickle_ctor_args cranelift goes 4 -> 6 (main) -> 5 (here) with guard_failures 201 -> 437 -> 202 and bridges_compiled 1 -> 2 -> 1, and pickle_terminal_raise_resume cranelift goes 34 -> 37 -> 36 with guard_failures 652 -> 664 -> 652. The two foriter_* fixtures drop bridges_compiled 2 -> 1 with guard_failures 496 -> 296 and 401 -> 201 while loops_compiled holds. The wasm baselines of existing fixtures are left untouched: only ubuntu-24.04 runs that leg and a macOS recording disagrees with it. Assisted-by: Claude * display: route dunder dispatch through the method cache The five dunder-dispatch helpers in display.rs called `lookup_where_pair`, which walks the MRO twice — once through `lookup_in_type_where_uncached` for the value half and once through `lookup_where_class_uncached` for the class half. `typeobject.py:453-455` defines `lookup_where` as `lookup_where_with_method_cache`, with the raw `_lookup_where` walk reached only on a cache miss. The same file already routed its two `__repr__` lookups through that front door. `str(e)` on an exception reaches `exc_user_dunder_obj` twice — once from `py_str_obj` and once from the `py_str_wtf8` -> `exception_descr_str_wtf8` fall-through — so an exception with no `__str__` override paid four uncached MRO walks per call and discarded all four. Measured on dynasm release, interleaved arms, statistic = median of pairwise ratios of child CPU time (self-A/B floor 0.998-1.003x): str(ValueError()) 0.516x str(e) 0.580x str(e) with an arg 0.591x repr(e) 0.709x str(int) 0.992x control e.__class__.__name__ 1.018x control raise/catch 0.981x control check.py ALL PASSED 379/379 on dynasm and cranelift with no jit-stats change. An oracle over every branch of the five helpers — including re-dispatch after adding, overriding and deleting a dunder on the class, on a base class, and after a warmed loop — is byte-identical between the two binaries. Assisted-by: Claude * display: dispatch a metaclass __str__ for str(cls) `type_metaclass_dunder_obj` is generic over the dunder name, but all three of its call sites passed "__repr__", so `str(cls)` never consulted the metaclass and produced the native `<class '...'>` text instead. `descroperation.py:900-925` generates `str` and `repr` from one source template over [('str', '__str__'), ('repr', '__repr__')]; both call `space.lookup(w_obj, specialname)`, which walks `type(w_obj)` — the metaclass for a class object. So the two are the same dispatch and honouring one without the other is the divergence. Add the probe to the three `str` functions that mirror the three `repr` functions already carrying it: `py_str` alongside `py_repr`, `py_str_wtf8` alongside `py_repr_wtf8`, and `py_str_obj` alongside `py_repr_obj`. `type` defines no `__str__`, so a metaclass without an override resolves to `object`'s, the helper declines, and the native text stands. Observable changes, all toward CPython 3.14 and pypy3: str(cls) / format / f-string <class '...'> -> the override metaclass with __str__+__repr__ used __repr__ -> uses __str__ inherited metaclass override ignored -> applied raising override ignored -> propagates non-string return ignored -> TypeError str subclass result flattened -> type preserved New parity test `type_metaclass_str_dispatch` pins the eight contract points; it reads OK on CPython 3.14, pypy3, dynasm and cranelift, and fails on the binary built before this change. check.py ALL PASSED 379/379 on dynasm and cranelift with no output or jit-stats change. Assisted-by: Claude * gc: add a blackhole store probe over the frame field names Records the fields a blackhole materialization stores into and checks the minor collection sees no young refs left behind, keyed by the PyFrame GC type id. Assisted-by: Claude * jit: keep a compiled-frame exit from re-recording its own traceback node A compiled run records its frame's traceback node at the raise. The exit that hands the exception back to the interpreter for the exception-table search left `attach_tb` set, so `handle_operation_error` attached a second node at `frame.last_instr`, which by then names the handler dispatch rather than the raise. `screen_frame_already_recorded` clears the flag when the chain head already names the frame; an exit carrying no node of its own is unaffected. Fixture `exception_escape_hot_callee_tb_node_once` covers the non-matching `except`, the second of two clauses, a bare reraise and a raise from a frame below the try, with a try-free callee and a `finally`-only callee as controls and `raise caught_object` as the guard on the screen. Assisted-by: Claude * gc: rustfmt the blackhole store probe `cargo fmt --check` is red on the five sites the probe added: two `bh_probe`-gated blocks in the collector, `BhProbePhase::enter`, and the two long entries in the frame field-name table. Assisted-by: Claude * check.py: record the wasm jit-stats this branch moved, from CI The dynasm and cranelift halves of these were re-recorded earlier on this branch; the wasm halves were not, because no wasm binary builds on the host that recorded them, so the only truthful source is a CI run. Values are taken from the ubuntu-24.04 leg, which reports each moved counter and the observed loops_compiled / bridges_compiled alongside it. `foriter_call_body` and `foriter_loadglobal_body` drop a bridge and 200 guard failures with it; the two +2 guard_failures on the traceback fixtures are the same moves the dynasm baselines already carry. `range_ctor_in_loop` and `subscr_user_getitem_inline` were missing loops_compiled, bridges_compiled and guard_failures outright rather than holding a stale value. A counter absent from a baseline reads as 0, which is the healthy end for two of the three and the worst end for loops_compiled, so neither gated anything. Their wasm values match what the other two backends already record. `recursive_call_frame_relocation` reads guard_failures 637 on ubuntu-24.04 and 638 on macos-latest and on this host, across two runs each, with every other counter identical. That is what the per-platform overlay is for, so the shared file keeps 638 and a `.dynasm.linux.jitstats` records the Linux value, leaving both hosts gated exactly. dynasm 381/381, cranelift 381/381. Assisted-by: Claude * check: fail a pypy perf gate that finishes far under its ceiling `max-pypy-ratio` only bounded a fixture from above, so a fixture that got faster kept passing against a ceiling nobody re-recorded. Add the other side. The floor is the ceiling divided by PERF_GATE_FLOOR_DIVISOR unless a fixture overrides it with # pyre-check: min-pypy-ratio=N read by synth_min_perf_gate from the same 20-line header window. A floor failure prints FASTER and names the ceiling that needs re-recording. The cpython gate stays one-sided. The floor half only applies when the pypy baseline exec time reaches FLOOR_GATE_MIN_BASELINE_S. Exec time is a difference between two independently measured quantities, so a baseline just above EXEC_TIME_FLOOR_S carries a relative error large enough that whether it clamps at all moves between runs: the same fixture set produced 248 clamped baselines in one run and 127 in the next, and 56 fixtures failed the floor gate on that flip alone. The comparison table prints its ratio from the same startup-subtracted times but showed raw times in its columns, so a clamped denominator was invisible there. Mark those ratios with `~` and explain the mark under the header. Assisted-by: Claude * bench: re-record max-pypy-ratio on the fixtures whose pypy term is measurable 46 fixtures held a ceiling well above anything they measure; the widest sat at 362x against a 30.9x reading. Each ceiling here is twice the largest ratio observed for that fixture across dynasm and cranelift, and each gets an explicit min-pypy-ratio at a third of the smallest, because the band the two-sided gate derives on its own is too narrow to hold the spread between hosts. Only fixtures whose pypy exec time clears the execution floor by a margin are re-recorded. A baseline pinned to the floor divides by that floor instead of by its own time, which understates the ratio, so a ceiling fitted to one can sit below the fixture's true ratio elsewhere. Of 363 fixtures 71 clear the margin. Assisted-by: Claude * bench: run exception_escape_hot_callee_tb_node_once at N=4000 Every iteration of every driver in this fixture raises and unwinds, so N=20000 spent its time on exception throughput rather than on the duplicate traceback node it exists to pin. Checked against a binary without the screen in eval.rs: the duplicate first appears at N=2500 and never at N=1500. N=4000 reproduces all five shapes with the structure N=20000 gave -- loops_compiled 15, bridges_compiled 4, guard_failures 816 to 814 on both backends. Assisted-by: Claude * gc: gate the blackhole store probe on wasm behind a feature bh_probe_enabled() returned true unconditionally for wasm32 because the guest has no process environment to read MAJIT_GC_BH_PROBE from. The probe panics with its violation report once it reaches CLASS_BUDGET or REPORT_AT_MINOR, so 43 synthetic benchmarks aborted in the wasm leg with "recovered panic: BH PROBE: post-minor nursery references". Give the guest a compile-time gate instead: the new bh_probe feature, off by default. The probe, its violation recording and its panic are unchanged, and `--features bh_probe` restores the wasm behaviour. The gate also removes the probe's cost from ordinary wasm builds -- bh_probe_enabled() guards work inside finish_alloc_in_oldgen and the post-minor scan, so every old-generation allocation carried extra bookkeeping and every minor collection a full scan. Assisted-by: Claude * bench: run range_ctor_in_loop at N=400000 pypy ran the body in 0.01s at N=20000, which put its startup-subtracted time under EXEC_TIME_FLOOR_S, so the ratio divided by the floor instead of by pypy's own time. That reading understates, and the two hosts disagreed across the ceiling: macos read 38.7x against ubuntu's 59.0x on a gate of 50. At N=400000 pypy spends 0.10s and the denominator is its own. The ratio that exposes is 83x as a median of interleaved pairwise runs, so the ceiling is re-recorded at 190 with a floor beside it. The number rose because the basis changed, not because the fixture got slower. Assisted-by: Claude * bench: size eight fixtures so pypy's term is its own time Each of these ran its body fast enough that pypy's startup-subtracted time fell to EXEC_TIME_FLOOR_S, so the ratio divided by that floor. The reading that produces understates, and it moved across the gate between hosts and between runs. Raising the loop count until pypy spends 0.06-0.10s changes the answer in both directions, which is why each bound here is re-recorded from the enlarged fixture rather than scaled: wasm_ca_trampoline_decline 5.7x -> 39.0x floored denominator inline_chain_depth_typeflip ceiling 718 -> 24 at 5.9-11.5x recursion_memo_branch 25.3x -> 12.6x warmup no longer dominates Constants derived from the loop count move with it -- FLIP_AT stays two thirds through inline_chain_depth_typeflip, and recursion_memo_branch keeps its branch threshold and print interval at the same share of the loop. dynasm and cranelift jit-stats are re-recorded here; the wasm baselines for these eight can only come from CI. imp_lock_rlock_semantics has no loop at all and its own header says "Correctness-only fixture: no perf gate header, the work here is trivial", so its ratio gate is dropped rather than given one. 35 other synthetic fixtures already carry no ceiling. Assisted-by: Claude * bench: re-record max-pypy-ratio from four accumulated suite runs Deriving a bound from a single run makes that run's noise the bound. At the sizes these fixtures run, the ratio moves enough between runs that one reading is not a measurement of the fixture: attr_instance_shadows_class spans 22.7x to 58.5x across four runs while break_except_live_local holds 10.6-10.7x, and a single-run pass would have treated the two alike. Each ceiling is twice the largest ratio any of the four runs produced and each floor a third of the smallest, with ratios printed against a floored pypy denominator left out of the floor entirely. A ceiling is only ever lowered. 225 fixtures move; their ceilings total 20138 before and 7632 after. Assisted-by: Claude * jit: call an exact fixed-arity bound builtin through its gateway A materialized bound Method reached the generic callable dispatcher in bh_call_fn_impl, which unwrapped it, allocated a receiver-prefixed argument vector, and dispatched the underlying Function a second time. Unwrap the Method in bh_call_fn_impl instead and take the positional gateway call directly when the target is a BuiltinCode whose natural fast arity equals the exact positional count and that count is at most 4. Optional, variadic, null-receiver, non-builtin and mismatched-arity shapes stay on the generic path. The new code and receiver are pinned on the same shadow stack the entry guard already covers, and their values are re-read from their slots after pinning. Measured on dynasm release, N=3000000, best-of-three child user CPU, net of the empty loop: s.add(7) 0.30s to 0.15s, s.__contains__(7) 0.30s to 0.14s. exception_traceback_loop_forms 0.72s to 0.59s. check.py: dynasm 382/382, cranelift 382/382. Assisted-by: Claude
2 of 75 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the
test.test_asyncioCI red (PASS -> TIMEOUT/CRASH since #1003 turned the gate on).Root cause (deterministic repro included)
try_walker_inline_type_callflattens class instantiation (allocate virtual + inline the Python__init__body). On the recorded path the caller's CALL dst receives the instance, but a guard failing INSIDE the inlined__init__resumed through a parented snapshot chain[caller paused after the CALL, __init__ frame], and the blackhole delivered__init__'s return value — None — into the caller's CALL dst.typeobject.py descr_calldiscards__init__'s result and returns the instance; the flattened inline has no frame that can reconstruct that discard at a mid-__init__pause.In asyncio: a warmed
BaseEventLoop._call_soontrace inlinedevents.Handle(...)including its branchy__init__(if self._loop.get_debug():); tests toggling debug mode failed that bool guard, and every subsequentcall_soonproducedhandle = None→AttributeError: 'NoneType' object has no attribute '_source_traceback', lost wakeups, and the event loop parked inkeventforever (the CI TIMEOUT). #1003's removal of the has_branch pre-call gates made branchy__init__inlinable, exposing this latent hole (the strict-seed two-frame path had the same hole for straight-line__init__).Fix
In
try_walker_inline_resolved_user_call:Ok(None)→ residual instantiation (correct result, enclosing loop still compiles) — placed before the arm that would decline the enclosing key to interpretation;strict_seedexcludes constructors, so straight-line__init__keeps inlining via the single-frame path whose guards collapse to the caller boundary (deopt re-executes the whole instantiation call).Plus (separate commit) env-gated
PYRE_INJECT_RCA/PYRE_INJECT_FALLBACK_DECLINEswitches oninject_root_call_result's documented known-wrong fallback — used to refute that hypothesis during this RCA; off in production.Verification
synth/type_call_inline_init_branch_deopt(deterministic warm-then-flip repro; verified FAILING on the unfixed build) with converged jitstats baselines on all three backends (2 loops / 1 bridge / 201 guard_failures each).test.test_asyncioincluded._run_onceself-slot corruption class (3 errors,'code' object has no attribute '_current_handle') — a separate pre-existing defect, tracked for its own RCA.Note: the first 3 commits on this branch are content-duplicates of the already-squash-merged #1012 (the branch forked before that merge landed); the effective diff of this PR is the last 2 commits — 6 files, +115/−3 vs current main.
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Diagnostics