check.py: correct the jit-stats docstring, and re-record eight dynasm baselines - #1019
Merged
Conversation
The docstring said the wasm [jit-stats] line reports only the badness fields, so its baselines carry only those. 364 of the 365 committed wasm baselines now record loops_compiled, bridges_compiled and guard_failures. Replace the claim with the hazard the missing-field rule carries: a field absent from both sides compares equal regardless of value, so a backend whose line stops naming a counter disarms that counter on every one of its baselines without failing anything. Assisted-by: Claude
Each moved in a direction _jit_stats_regression_floor did not gate: bridges_compiled sat in the recorded surface but in none of the gated groups, the badness fields and guard_failures gated rises only, and loops_compiled gated falls only. exception_bridge_traceback_head aborted 1->0, bridges 6->4, gf 1203->802 exception_escape_caller_frame_tb_node aborted 1->0, gf 8->5 exception_raise_caught_same_frame_tb bridges 2->4 exception_residual_raise_caught_in_frame bridges 0->1 exception_traceback_frame_lineno bridges 3->5 gc_bug_bridge_flavor_traceback_names aborted 1->0, bridges 10->7, gf 2018->1655 list_length_hint_validate aborted 14->10, bridges 4->0, gf 828->1 slots_class_var_conflict aborted 5->0, loops_compiled 3->5 Assisted-by: Claude
WalkthroughThe change updates eight synthetic JIT statistics baselines. It also clarifies baseline counter comparison documentation in ChangesJIT statistics and gating
Estimated code review effort: 1 (Trivial) | ~5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyre/check.py`:
- Around line 679-684: Update the explanatory comment near _jit_stats_change to
qualify that a missing field in current disables the gate only when the
corresponding field is also absent from saved or saved already has a zero value;
retain the existing behavior for non-zero saved values, which are reported as
changes.
🪄 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: 1ec5599d-e64b-4a1c-ad3b-b0c4c0e5c250
📒 Files selected for processing (9)
pyre/bench/synth/exception_bridge_traceback_head.dynasm.jitstatspyre/bench/synth/exception_escape_caller_frame_tb_node.dynasm.jitstatspyre/bench/synth/exception_raise_caught_same_frame_tb.dynasm.jitstatspyre/bench/synth/exception_residual_raise_caught_in_frame.dynasm.jitstatspyre/bench/synth/exception_traceback_frame_lineno.dynasm.jitstatspyre/bench/synth/gc_bug_bridge_flavor_traceback_names.dynasm.jitstatspyre/bench/synth/list_length_hint_validate.dynasm.jitstatspyre/bench/synth/slots_class_var_conflict.dynasm.jitstatspyre/check.py
Comment on lines
+679
to
+684
| invariant counter costs no re-record. The cost of that convenience is that a | ||
| field absent from BOTH sides compares equal forever: a backend whose | ||
| [jit-stats] line stops naming a counter disarms that counter's gate on every | ||
| one of its baselines, silently. Whenever a backend's line changes shape, | ||
| re-record its whole baseline surface rather than trusting the run that | ||
| follows.""" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Qualify the missing-counter warning.
When current omits a field but saved contains a non-zero value, _jit_stats_change compares that value with "0" and reports a change. The gate is silently disabled only when both snapshots omit the field, or when the saved value is already zero.
Replace “disarms that counter’s gate on every one of its baselines” with wording that reflects this behavior.
Suggested wording
- field absent from BOTH sides compares equal forever: a backend whose
- [jit-stats] line stops naming a counter disarms that counter's gate on every
- one of its baselines, silently.
+ field absent from BOTH sides compares equal forever. If a backend stops
+ naming a counter, only baselines with a saved value of zero are silently
+ disabled; a non-zero saved value still compares against zero and fails.📝 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.
Suggested change
| invariant counter costs no re-record. The cost of that convenience is that a | |
| field absent from BOTH sides compares equal forever: a backend whose | |
| [jit-stats] line stops naming a counter disarms that counter's gate on every | |
| one of its baselines, silently. Whenever a backend's line changes shape, | |
| re-record its whole baseline surface rather than trusting the run that | |
| follows.""" | |
| invariant counter costs no re-record. The cost of that convenience is that a | |
| field absent from BOTH sides compares equal forever. If a backend stops | |
| naming a counter, only baselines with a saved value of zero are silently | |
| disabled; a non-zero saved value still compares against zero and fails. | |
| Whenever a backend's line changes shape, re-record its whole baseline surface | |
| rather than trusting the run that follows.""" |
🤖 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/check.py` around lines 679 - 684, Update the explanatory comment near
_jit_stats_change to qualify that a missing field in current disables the gate
only when the corresponding field is also absent from saved or saved already has
a zero value; retain the existing behavior for non-zero saved values, which are
reported as changes.
![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: c41904681a
ℹ️ 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
+682
to
+684
| one of its baselines, silently. Whenever a backend's line changes shape, | ||
| re-record its whole baseline surface rather than trusting the run that | ||
| follows.""" |
There was a problem hiding this comment.
When a backend stops emitting a counter whose saved value is nonzero, _jit_stats_change does not silently disarm the gate: the missing current value defaults to 0, so the comparison fails. Following this new advice with --snapshot is what actually disarms it, because record mode bypasses the comparison and rewrites the baseline without the field; subsequent runs then compare missing against missing. A disappearing counter should therefore be fixed or explicitly removed from the gated surface, not handled by re-recording.
Useful? React with 👍 / 👎.
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit c419046). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patchNone. 2. Other mismatches introduced by this patchNone. 3. Pre-existing mismatches (already present before this patch)None. 4. Structural adaptationsNone. |
youknowone
added a commit
that referenced
this pull request
Aug 4, 2026
…aise_reg_unbound_jitstress red #950 and #1019 landed under this branch and moved these counters. They are not this branch's doing: `origin/main` built and run on this host observes exactly what the branch observes, and misses its own committed baselines by the same amounts — | bench | committed | origin/main observes | |---|---|---| | exception_metadata_jitstress | loops_compiled 407 | 409 | | global_store_plain_dict_globals | loops_compiled 4 | 6 | | trace_too_long_effect_replay | loops_compiled 1 | 3 | | exception_subclass_attrs | guard_failures 1 | 3 | | raise_reg_unbound_jitstress | loops_aborted 1, loops_compiled 6 | 2, 8 | `raise_reg_unbound_jitstress` is left at its committed value on both backends. Its `loops_aborted` 1 -> 2 is a badness-field rise, and the control above shows it belongs to the base rather than to this branch, so recording it here would put a name to someone else's regression. The other fourteen rows only raise `loops_compiled` or `guard_failures`; no `loops_aborted` rise, no `loops_compiled` fall, and `internal_compile_panics` and `descr_set_*` do not move. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Aug 4, 2026
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
added a commit
that referenced
this pull request
Aug 4, 2026
The wasm re-record two commits back was taken before #950 and #1019 landed under the branch. Each of these nine rows only moves `guard_failures`, by 1 to 3, and the shift is deterministic: `gc_iterator_source_drop` reads 613 on three consecutive runs against a committed 614, so it is the base and not run-to-run wobble. Four wasm rows stay at their committed values. Three are badness-direction and already failed under the previous one-directional gate: `list_append_write_barrier_gc` (`loops_aborted` 0 -> 1), `recursion_memo_branch` (`loops_aborted` 1 -> 2) and `pickle_terminal_raise_resume` (`loops_compiled` 74 -> 73). The fourth, `exception_inline_callee_tb_frames`, rises `guard_failures` 860 -> 975 with no matching `loops_compiled` or `bridges_compiled` rise and has had no origin/main control run against it, so it is left for the gate to keep naming rather than recorded on a guess. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Aug 4, 2026
…sted-break gate (#945) * faulthandler: leave the RUNNING census while contending on the state lock A thread parked in `FAULTHANDLER_STATE_LOCK`'s futex can no longer poll the eval breaker, so it stayed in the set a collector waits to drain. `register` builds its `OSError` while holding that guard and `os_error_syscall2` allocates, so a failed install could request a stop-the-world that the parked waiter was unable to acknowledge. Acquire through `lock_faulthandler_state`, which brackets the blocking path with `before_external_block` after a `try_lock`, the same split `w_list_lock` uses. Also record on the guard that the host install/removal runs inside it, and on `walk_faulthandler_roots` that the walk runs inside the stop-the-world window and must not take the guard. Assisted-by: Claude * jit: publish every pushvalue slot and port space.newbool `push_and_bump!` bumped `valuestackdepth` without emitting the `setarrayitem_vable_r` that `jtransform.py:1898 do_fixed_list_setitem` gives every `pushvalue`. `capture_resumedata` (pyjitpl.py:2611-2622) carries the whole `virtualizable_boxes` list and `virtualizable.py:126-137 write_from_resume_data_partial` writes back every element the published depth describes, so a slot the trace never bound restored as NULL. Define the macro in terms of `emit_pushvalue_ref!` so all 44 sites emit the store. The boxed COMPARE_OP result was an opaque `CallR(jit_bool_value_from_truth)` the optimizer cannot DCE once that store makes it live. `baseobjspace.py:896-900 newbool` returns one of two prebuilt singletons after a jitcode branch, recorded by `pyjitpl.py:511-520 opimpl_goto_if_not` as `guard_true`/`guard_false` and skipped for a `Const` box by `pyjitpl.py:2582-2584 generate_guard`. `walker_newbool` emits that shape at the five `emit_trace_bool_value_from_truth` sites; the helper and `compare_box_provably_dead` become dead and are removed. The commit's third change, consuming `reconstruct_inline_recipe`'s register section before the frame image, is dropped: `origin/main` landed the same fix as `stream_slots` / `stream_covered` / `overlay_stream_ref_slots`, which applies it to the materialized frame arm as well. Assisted-by: Claude * jit: admit SET_ADD and MAP_ADD in the FOR_ITER body scan `pyopcode.py:1515 SET_ADD` is `space.call_method(w_set, 'add', w_value)` and `pyopcode.py:1525 MAP_ADD` is `space.setitem(w_dict, w_key, w_value)` — the `LOAD_ATTR` + `CALL` pair and `STORE_SUBSCR` the scan already admits. The codewriter lowers both to a void `residual_call_r_v` (`bh_set_add_fn` / `bh_map_add_fn`), so neither is folded and both carry the body-effect accounting of the residual they are. Assisted-by: Claude * io, posix: check signals before retrying a read on EINTR `fd_read_into`, `posix.readinto` and the buffered read path retried an `EINTR` read in a bare Rust loop, and `posix.read` did not retry at all. Neither shape runs the pending Python signal handlers between the interruption and the retry, which `error.py:805-808` `wrap_oserror2(..., eintr_retry=True)` does via `space.getexecutioncontext().checksignals()`. Skipping that step hangs a read whose remaining bytes the handler itself writes: `signal.alarm(1)` firing into a blocked `read(6)` never gets its `os.write` executed, so the read waits forever for bytes no one sends. The no-retry shape instead surfaced `InterruptedError`, which PEP 475 removed. `lib-python/3/test/test_io.py` stopped at `CSignalsTest.test_interrupted_read_retry_buffered` and never finished. `eintr_retry_with` is the shared arm: non-`EINTR` errno goes to the call site's own OSError wrapper, `EINTR` runs `checksignals_now()` and returns `Ok(())` for the caller to re-issue. `fd_read` now carries a `PyError` so its loop can run handlers, and the `posix` loops scope the blocking guard to the syscall alone, since `checksignals` runs Python. Assisted-by: Claude * jit: pin the forced-callee kept-stack SIGSEGV and cover stream-covered slots `bench/synth/recursive_forced_frame_kept_stack.py` is a self-recursive function whose frame is forced by `locals()` with a kept operand-stack slot beneath an inlined comprehension. It exited 139 before the reconstructed-callee slot source was fixed. The fix itself is already on `origin/main` as `stream_slots` / `stream_covered` / `overlay_stream_ref_slots`, so this commit's own version of it is dropped. What that version's tests covered and `origin/main`'s do not is the `stream_covered` mask itself: `materialized_inline_frame_slots_skip_stream_covered_slots` asserts a marked slot is left NONE/Void with no `GETARRAYITEM_GC_R` emitted for it. Assisted-by: Claude * deque: take the block-chain lock in snapshot, store and clear_blocks `origin/main` landed the striped `w_deque_lock` on `append_right`, `append_left`, `pop_right` and `pop_left`. `snapshot` walks the whole block chain, `clear_blocks` rewrites both endpoints and the length, and `store` sequences the two — all three read or write the same chain those four mutate and were left unguarded. The lock is reentrant, so `store`'s guard nests with the `clear_blocks` and `append_right` guards it calls. Each site takes the same push_roots/pin_root/shadow_stack_get shape the existing four use, because acquiring under contention enters the GC-aware external-block region and the receiver can move. Assisted-by: Claude * jit: latch the resume-past image before withdrawing the escape commit `origin/main` already dropped `writes_live_heap` and the frame-entry odometer from the single-frame latch gate, but the withdraw (`cancel_committed_frame_escape_pc` + `restore_escape_flush_undo`) still ran unconditionally ahead of it and the latch still required `committed_frame_escape_pc().is_none()`. So an escape whose residual entered a user Python frame reached the build with the pre-force frame already restored underneath an image built for the forced one, and `apply_single_frame_blackhole_crn` writes into that disagreement. Build and latch first; run the withdraw afterwards and only when no image was latched. The committed escape pc no longer gates the latch: `try_adopt_single_frame_blackhole` takes it and discards the flush undo when it adopts, and `run_perfn_walk`'s epilogue restores the undo when it declines, so leaving it standing keeps a declined adopt on the path it has today. Drops the now-unreachable [latch-vs-bh] feasibility probe. Fixes two wrong answers on both backends. A frame-inspecting property with a side effect ran 5 extra times (`20000 20005`); with an irreversible body effect earlier in the same iteration the FOR_ITER item was dropped instead and its body tail lost (`19995 20000`). Both now match the interpreter at `20000 20000`. Assisted-by: Claude * jit: start the nested-break inner-header scan after the enclosing FOR_ITER `nested_break_bridge_resume_hazard` scanned `target..pop_pc` for the inner loop's FOR_ITER, where `target` is the enclosing loop's own header. With no nested loop in the body the scan named that header as the inner one, so any single-level loop whose body ends in a statement-result POP_TOP before the backedge, and whose guard contains a secondary-edge branch, was declined as a nested break. Scan `(target + 1)..pop_pc` instead. Assisted-by: Claude * jit: per-guard deopt census behind MAJIT_GUARD_CENSUS `guard_failures` is a total, so it cannot distinguish a guard that keeps returning to the runtime because no bridge was attached to it from a wide spread of guards each still below `trace_eagerness`. Those two want opposite fixes and the difference is a distribution. `guard_census_record` tallies `(green_key, fail_index)` on the deopt path and `guard_census_summary` renders `distinct` / `total` plus the heaviest guards, printed with the other `[jit-stats]` lines. Both the tally and the print are gated on `MAJIT_GUARD_CENSUS` because the map write sits on the path under study; the env read is behind a `OnceLock`. Assisted-by: Claude * jit: rustfmt the guard census Assisted-by: Claude * gc: per-minor-collection drain census under MAJIT_GC_DRAIN_CENSUS Records, for each minor collection, the nursery bytes it found, the bytes it copied out to the old generation, and the pinned objects left behind, plus a survival-rate decile histogram. Promoted bytes are read as the delta of `bytes_made_old_since_cycle` across the collection, sampled before the major-progress step that resets it. Setting the variable to a positive integer also dumps the running summary every that many collections, so a run that has to be killed rather than waited out still reports. The summary prints from the `MAJIT_STATS` block in pyrex, alongside the guard census. Assisted-by: Claude * interp: gate eintr_retry_with out on wasm32 `libc::EINTR` and `crate::module::signal` (module/mod.rs:100-101, itself `cfg(not(target_arch = "wasm32"))`) do not exist on wasm32, and the function had no cfg of its own while every caller was already excluded there. The Charon/LLBC wasm32 layout extraction failed to compile it. Assisted-by: Claude * jit(intbounds): re-dispatch de-overflowed INT_{ADD,SUB,MUL} from the first pass optimize_int_{add,sub,mul}_ovf returned `OptimizationResult::Emit` for the replaced non-overflowing op. `Emit` goes straight to `emit_operation` and skips every remaining pass; `intbounds.py:254-255/285-286/301-302` call `send_extra_operation(op)` with `opt=None`, which re-dispatches from `first_optimization` — `Restart` in pyre. Consequence: an `IntSub`/`IntAdd`/`IntMul` whose two arguments are both constant never reached `OptPure`'s all-constant fold (`pure.py:121-136`) and survived into the compiled trace. `_binaryop_or_lea` asserts `arglocs[0]` is a `RegLoc` (`x86/assembler.py:1276`), so on x86_64 such an op panicked `compile_loop` (`global_cell_shortpreamble_hot`, `unpack_drain_star_raise`); aarch64 accepted it and emitted the unfolded arithmetic. Assisted-by: Claude * jit: take the compiled-loop meta before the run, drop it from the exit re-lookup `run_compiled_raw_detailed_with_values` and `run_compiled_detailed` held the `&CompiledEntry` from before `execute_token` across the run; the dispatch-key runner instead re-looked it up afterwards with `.unwrap()`. Neither survives re-entry: compiled code makes residual calls into the interpreter, which re-enters the driver by minting a second `&mut JitDriverPair` out of the `JIT_DRIVER` thread-local cell (`pyre/pyre-jit/src/eval.rs:3977`, `:4393`), and that re-entry can retire the green key (`pyre/pyre-jit/src/eval.rs:7887` range-FOR_ITER demotion, `jitdriver.rs:4361` unrecoverable resume). The borrow checker does not see the second `&mut`, so the held reference dangles and the re-lookup panics. `CompileResult`/`RawCompileResult` now own their `meta`, cloned before the run: the exit values being unpacked were produced by the code that metadata describes, mirroring `warmstate.py:398`'s hold on the `loop_token` object. The trace lookup stays after the run — a bridge compiled during the run is only visible there — but is fallible. When that lookup misses, the synthesized fallback layout takes its resume payload from the failing descr through `ResumeStorage::from_fail_descr`, which is where upstream reads it from in the first place (`compile.py:849 get_resumestorage(): return self`, `compile.py:855 ResumeGuardDescr._attrs_`). The green-key index is a second index of the same `rd_numb`/`rd_consts`/ `rd_virtuals`/`rd_pendingfields` already carried on `majit_ir::FailDescr`. Assisted-by: Claude * wasm: drain the guest stderr buffer on a trap, and route the exception-channel reporter through the fd-2 seam The guest has no descriptors, so everything it writes to fd 2 accumulates in linear memory until `pyre_take_stderr` hands it over, and only the success path called that export. A trap therefore discarded the whole channel at the one moment it carries the most: the diagnostics a fatal path emits just before aborting. `take_guest_stderr` factors the drain out and the trap path now runs it before returning the error. `reject_non_exception_channel_value` reported through `eprintln!`, which is a silent sink in the guest, so its two lines never reached the host. Sends them through `pyre_interpreter::host_seam::emit_stderr` instead. The `blackhole_resume_via_rd_numb` failure context gains `guard_exc`, the Python pc for the failing and entry jitcode positions, `deadframe_types`, the raw deadframe, and the ref register bank; its positional arguments were also shifted by one against their labels. Assisted-by: Claude * bench: re-record the dynasm/cranelift jit-stats baselines this branch moves Ten synthetic fixtures whose committed .jitstats predate this branch's tracing changes. Every one of them compiles strictly more than the baseline recorded: set_hash_protocol loops 0->1 bridges 0->1 loops_comprehension loops 4->7 bridges 5->12 list_append_write_barrier_gc loops 1->12 bridges 1->5 exception_try_call_inlined_callee_raise loops 3 bridges 2->3 global_quasiimmut_invalidation loops 2 bridges 2->5 kept_stack_deep_var_shortcircuit loops 6 bridges 4->10 polymorphic_slot_retype loops 1->2 bridges 4->5 recursion_memo_branch loops 2->3 bridges 13->16 imp_lock_rlock_semantics loops 0->1 bridges 0 The guard_failures rise tracks that: a guard fails trace_eagerness (=200, rlib/jit.py:590) times before its bridge attaches, and the deltas are proportional to the newly compiled guards, not to extra guards on unchanged code. The floor gate fails on a fall in loops_compiled and a rise in guard_failures, so it can only report the cost of compiling more. exception_try_call_inlined_callee_raise also drops loops_aborted 1->0. list_append_write_barrier_gc and recursion_memo_branch gain loops_aborted 0->1; both are benches where loops_compiled rose, and mc_diag shows cb_entered == bridges_compiled with the compile_bridge arity giveup at 0, so no compile-side giveup fired — it is a root trace abort with permanent=false, retried. recursive_forced_frame_kept_stack had no committed baseline on either backend; recorded. exception_args_virtual and list_length_hint_validate are left alone: they fail on main as well, so their baselines are not this branch's to rewrite. Verified: pyre/check.py --synthetic-only --backend dynasm,cranelift is 353 passed / 2 failed per backend, the two being those inherited rows. Assisted-by: Claude * jit(wasm): publish the compiled loop's inputarg types on the token `Backend::compile_loop` already builds `input_types` for the pending self-CALL_ASSEMBLER census but never wrote it to the token, so `JitCellToken::inputarg_types()` stayed empty on wasm. Both native backends set it (`majit-backend-dynasm/src/runner.rs:2269`, `majit-backend-cranelift/src/compiler.rs:15613`). `jitdriver.rs:4686 extend_compiled_live_values` sizes the argument list handed to `execute_token` from that length. Reading 0 makes `compiled_inputs <= live_values.len()` always true, so the virtualizable extension is skipped and a trace with more inputargs than the portal's live values is entered with the short list; the slots past the args then read the zero-filled frame. With the types published the same entry either extends correctly or returns None, and every caller turns None into a decline. Measured on the wasm synthetic suite: 346 passed / 8 failed, the same eight fixtures as before the change. It does not close the `recursive_forced_frame_kept_stack` crash. Assisted-by: Claude * jit: name the nested-residual decline arm under PYRE_LB_SITE `fbw_abort_nested_unjournaled_residual` declines on either of two independent conditions — a FOR_ITER body admitted under `CalleeReplaySafety::DeferredCall` whose nested CALL did not inline, or `fbw_inline_callee_hazardous` finding a loop-bearing / self-recursive / duplicate-`w_code` frame — and both raise the same `LoopBearingCalleeInlineUnsupported`. `PYRE_LB_SITE` already prints the raising source location to tell decline reasons apart, but every arm here shares one raise site, so the location alone cannot separate them. Print the two predicates next to it. Measured with it: `recursion_memo_branch`'s single abort is the DeferredCall arm and `list_append_write_barrier_gc`'s is the hazard scan. Assisted-by: Claude * jit, gc: drop the dead compiled-loop unwrap, widen the drain decile scale Two findings from the PR review bots on 945. `run_compiled_raw_with_values` bound `self.compiled_loops.get(&green_key).unwrap()` right after `execute_token_ints` and never read it — the later `.and_then(|compiled| …)` shadows the name inside its own closure. The binding is dead but the `unwrap()` is not: a residual call that re-enters the driver can remove this green key during the run, and the panic then fires ahead of the fresh fallible lookup and its `trace_for_exit_by_rd_loop_token` fallback, so that fallback could never run in the case it was written for. Deleting the binding leaves no other use. `record_drain_sample` scaled the decile with `promoted.min(used_before) * 100` in `usize`. On wasm32 that wraps once the nursery passes ~43 MB, bucketing the sample wrong in release and tripping the overflow check in debug. Widen to u64 before scaling. pyre/check.py --synthetic-only --backend dynasm: 356/356. Assisted-by: Claude * bench, check.py: un-record the platform-split imp_lock baseline, report what a regressed run compiled `imp_lock_rlock_semantics` produces a different `[jit-stats]` line on every runner: linux 0 loops / 0 guard failures, macos 1 / 3, windows 0 / 7. Recording the macos numbers made linux fail the opposite gate (`loops_compiled 1 -> 0`) on a bench where main is green, so restore the committed baseline. No single value passes all three; picking one needs the windows `loops_compiled`, which nothing currently reports. That is the second half. `_jit_stats_regression_floor` names only the fields that moved the wrong way, so a `guard_failures` rise reads as a defect even when the same run compiled more loops — each new guard fails `trace_eagerness` (200) times before its bridge attaches. Locally the answer is one re-run away; on a runner the developer does not have it is not reachable at all, which is why the windows rows on this branch cannot currently be judged. Append the observed `loops_compiled` / `bridges_compiled` to the failure message. jit-stats regression: guard_failures 0 -> 3 (observed loops_compiled=1 bridges_compiled=0) pyre/check.py --synthetic-only --backend dynasm: 355 passed, 1 failed (that bench, on macos). Assisted-by: Claude * jit(wasm): drop the duplicated set_inputarg_types; record the wasm baseline for recursive_forced_frame_kept_stack The rebase brought a second `token.set_inputarg_types` into the same `compile_loop`, so the function published the entry arity twice. The setter is a `OnceLock::set` and ignores the later call, making the two behave alike, but only the surviving one runs after codegen has succeeded — publishing the arity for a loop whose compile then failed is not something to keep a second path for. Keep that one and delete the earlier copy. `recursive_forced_frame_kept_stack` no longer crashes on wasm, so the bench now reaches the "no committed jit-stats baseline" gate instead. Record it. pyre/check.py --synthetic-only --backend wasm: 352 passed, 3 failed before this commit; the recorded baseline closes one of the three. Assisted-by: Claude * _collections: root the deque snapshot entries across the block-chain rebuild `store` published only `self_obj` and left the replacement entries in a plain `Vec<PyObjectRef>`. `append_right` allocates a block chain, so any append can collect and move the objects the vector still names, and every later append then reads a stale address. `self_obj` was already re-read from the shadow stack after each GC-capable step for exactly this reason; the entries need the same treatment. Publish both slices with `publish_roots` and normalize the range once, rather than `pin_root(self_obj)` followed by a second pin: `pin_root` normalizes, that query is a safepoint, and the entries are not yet on the stack when it runs — the window `normalize_roots`' own doc describes. Each append then reads its entry back at the slot. Reported by the PR review bots on 945. collections.deque append/appendleft/remove/rotate/maxlen over 2000 items matches CPython byte for byte on three consecutive runs; pyre/check.py --synthetic-only --backend dynasm unchanged at 355 passed. Assisted-by: Claude * bench/synth: raise guard_failures baselines to the windows-observed count 13 synthetic benches fail the jit-stats floor on the windows runner with a `guard_failures` rise and nothing else. The same runs compile more loops than the baseline records — windows `loops_compiled` is 2 where the baseline holds 0, 3 where it holds 1, and 7 where it holds 5 — and macos reproduces the same direction one step lower (1 / 2 / 6). Each newly compiled loop's guards fail `trace_eagerness` times before its bridge attaches, so the counter moves with the loop count. `guard_failures` is rise-bounded (`cur > base + max(base // 4, 2)`), which is why this reads as windows-only: the macos and linux +1 sits inside the band while the windows +3/+5/+7 does not. The baselines now carry the highest count of the three platforms, so one committed file admits all of them. Only `guard_failures` moves; `loops_compiled` stays at its recorded floor, since raising a fall field to a value one platform does not reach is what broke linux on `imp_lock_rlock_semantics` before. Assisted-by: Claude * jit: seed the sub-walk vstack from the carrier recipe when no ActiveResumeFrame `bridge_subwalk` left `vstack_boxes` empty when the richer coordinate source was absent, though `resume.py rebuild_from_resumedata` seeds the rebuilt MIFrame's operand stack from the frame recipe alone. The first may-force call then invalidated the heapcache, a later CALL read a color-reloaded box with no runtime value, and the effectful sub-walk replayed from the guard, consuming the framed-pickle stream twice. `residual_call` and `vstack_mirror` carry the matching reads. Assisted-by: Claude * jit: stamp const pools with a process-global walk generation instead of a per-walk visit set `MetaInterp::walk_rd_consts_refs` allocated a fresh `IndexSet<usize>` on every GC root walk and hash-inserted each pool's `Arc::as_ptr` identity to avoid visiting a shared pool twice. `SharedConstPool` now carries an `AtomicU64` visited generation; the walk bumps a counter once and each pool swaps it in, visiting only when the previous value differs. The counter is one process-global `AtomicU64`, not a `MetaInterp` field. `register_thread_root_areas` (pyre-jit/src/eval.rs:3839) registers `rd_consts_root_walker_area` per mutator and `JIT_DRIVER` is `thread_local!`, so per-instance counters would each start at 0 and advance about once per collection; two of them reach equal values routinely, and a pool reachable from two instances would then be treated as already visited — its `Const::Ref` slots never reaching the visitor. The `IndexSet` this replaces was local to one walk and could not fail that way. One relaxed `fetch_add` per walk keeps generations distinct regardless of instance or thread; generations start at 1 and pools initialise to 0. Measured, min-of-3 user CPU, both arms built on this base, on N independent compiled loops: | N | per-walk IndexSet | generation stamp | delta | |---|---|---|---| | 100 | 0.73s | 0.72s | -1.4% | | 200 | 1.76s | 1.65s | -6.3% | | 400 | 7.30s | 5.44s | -25.5% | The win grows with the compiled-loop count. `sample` self time on N=400: the hash/insert frames fall from 664 of 4070 active samples to 69 of 2941, inside a run that is 77% GC + root walk. check.py dynasm 372/372, cranelift 372/372. Repeated-run gate on a 96-loop allocation-churning fixture: 600 runs per arm, 0 crashes and 0 wrong answers on both the stamp and the IndexSet build. Assisted-by: Claude * jit: decline the compare-box scan in sub-walks, let it pass over the pushvalue publish `classify_compare_box_use` decodes `compare_pc` against the snapshot root's JitCode. An inlined sub-walk has its own callee JitCode and deliberately shares the root snapshot, so the two pcs name different code and the scan reads the callee's offset out of the caller's bytes. Decline there, as `walker_foriter_green_key` already does for the same mismatch. The scan also counted the `push_and_bump!` publish as a second reader of the box, which took every compare out of the `FeedsBranchOnly` shape and left `walker_newbool_guarded` emitting the `jit_bool_value_from_truth` residual — a `CallR` returning Ref plus its allocation, once per iteration. That store mirrors the box into the operand-stack slot the guard's own resume image describes; substituting a prebuilt singleton for the recorded call result leaves it storing the same value. `VablePublish` says which of the two consumers may pass over it: `compare_box_provably_dead` elides the box outright and still needs one to store, so it keeps counting the publish. `pyre/bench/int_loop.py` returns to 40 ops before optimization and 25 after, with no Ref-returning `CallR` in the trace. Assisted-by: Claude * bench: re-record the 33 baselines this branch moves under the two-directional gate #1007 made every recorded counter gate in both directions, so the changes this branch already made to what the JIT compiles now report instead of passing as unmeasured improvements. Recorded with `check.py --snapshot`; dynasm and cranelift observe the same values on all 33, so none of them is a backend divergence. Audited every recorded line: `internal_compile_panics` and the `descr_set_*` counters do not move at all, no `loops_aborted` rises and no `loops_compiled` falls. What moves is `loops_aborted` 5 -> 0 and 1 -> 0 (three benches), `loops_compiled` up on nineteen, and `bridges_compiled` down with the matching `guard_failures` — 10 -> 4 with 2027 -> 819, 4 -> 0 with 828 -> 1, 6 -> 4 with 1203 -> 802, 10 -> 7 with 2018 -> 1655. Every `guard_failures` rise sits in a run whose `loops_compiled` also rose, which is the new loops' guards failing `trace_eagerness` times each before their bridges attach. Assisted-by: Claude * bench: re-record the 19 wasm baselines whose counters moved in a neutral or better direction Same two-directional gate (#1007) as the dynasm/cranelift re-record, run with `check.py --backend wasm --snapshot`. `loops_aborted` falls 5 -> 0 and 2 -> 1, `loops_compiled` rises on eleven, and every `guard_failures` rise sits in a run whose `loops_compiled` or `bridges_compiled` also rose. Three wasm rows are deliberately left at their committed values so the gate keeps naming them: `list_append_write_barrier_gc` (`loops_aborted` 0 -> 1), `recursion_memo_branch` (`loops_aborted` 1 -> 2) and `pickle_terminal_raise_resume` (`loops_compiled` 74 -> 73). All three already failed on this branch under the previous one-directional gate, so they are not a consequence of this re-record and none of them is blessed by it. Assisted-by: Claude * bench: re-record the fourteen baselines the new base moves, leaving raise_reg_unbound_jitstress red #950 and #1019 landed under this branch and moved these counters. They are not this branch's doing: `origin/main` built and run on this host observes exactly what the branch observes, and misses its own committed baselines by the same amounts — | bench | committed | origin/main observes | |---|---|---| | exception_metadata_jitstress | loops_compiled 407 | 409 | | global_store_plain_dict_globals | loops_compiled 4 | 6 | | trace_too_long_effect_replay | loops_compiled 1 | 3 | | exception_subclass_attrs | guard_failures 1 | 3 | | raise_reg_unbound_jitstress | loops_aborted 1, loops_compiled 6 | 2, 8 | `raise_reg_unbound_jitstress` is left at its committed value on both backends. Its `loops_aborted` 1 -> 2 is a badness-field rise, and the control above shows it belongs to the base rather than to this branch, so recording it here would put a name to someone else's regression. The other fourteen rows only raise `loops_compiled` or `guard_failures`; no `loops_aborted` rise, no `loops_compiled` fall, and `internal_compile_panics` and `descr_set_*` do not move. Assisted-by: Claude * bench: move the nine wasm guard_failures baselines the new base shifts The wasm re-record two commits back was taken before #950 and #1019 landed under the branch. Each of these nine rows only moves `guard_failures`, by 1 to 3, and the shift is deterministic: `gc_iterator_source_drop` reads 613 on three consecutive runs against a committed 614, so it is the base and not run-to-run wobble. Four wasm rows stay at their committed values. Three are badness-direction and already failed under the previous one-directional gate: `list_append_write_barrier_gc` (`loops_aborted` 0 -> 1), `recursion_memo_branch` (`loops_aborted` 1 -> 2) and `pickle_terminal_raise_resume` (`loops_compiled` 74 -> 73). The fourth, `exception_inline_callee_tb_frames`, rises `guard_failures` 860 -> 975 with no matching `loops_compiled` or `bridges_compiled` rise and has had no origin/main control run against it, so it is left for the gate to keep naming rather than recorded on a guess. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Aug 4, 2026
… the test_asyncio hang) (#1016) * jit: env-gated report and decline switches for the raw-color result-slot 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 * jit: keep constructor inlining out of parented guard snapshots 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 * cpython_tests: widen the rust-panic reason cap to 200 chars 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 * check.py: re-record 38 jitstats baselines after the rebase onto main 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 * extra_tests: satisfy the runner's OK convention in seven new parity scripts 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 * builtins: BaseExceptionGroup and ExceptionGroup basicsize 96 per CPython 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
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
1 participant
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.
Follow-up to #1007, which armed
_jit_stats_changeon the whole recordedsurface in both directions.
check.py: drop a stale claim from the_jit_stats_changedocstringThe docstring said the wasm
[jit-stats]line reports only the badness fields,so its baselines carry only those. 364 of the 365 committed wasm baselines now
record
loops_compiled,bridges_compiledandguard_failures.Replaced with the hazard the missing-field rule actually carries: a field absent
from both sides compares equal regardless of value, so a backend whose line
stops naming a counter disarms that counter on every one of its baselines
without failing anything.
bench: re-record eight dynasm baselinesEach of the eight moved in a direction the previous
_jit_stats_regression_floordid not gate —bridges_compiledsat in therecorded surface but in none of the gated groups, the badness fields and
guard_failuresgated rises only, andloops_compiledgated falls only. So thecommitted numbers had stopped describing the tree without anything going red.
exception_bridge_traceback_headexception_escape_caller_frame_tb_nodeexception_raise_caught_same_frame_tbexception_residual_raise_caught_in_frameexception_traceback_frame_linenogc_bug_bridge_flavor_traceback_nameslist_length_hint_validateslots_class_var_conflictNone is branch-attributable: evaluating the previous floor's own predicate over
these deltas scores all eight green, so they are drift the old gate could not
surface.
Verification
LLBC re-extracted after the rebase (
all extractions complete., everybuild/llbc/*.ullbcpost-dating it), then:cranelift and wasm were not run locally; this branch touches no baseline of
either.
— authored by Claude
Summary by CodeRabbit
Tests
Documentation