Skip to content

jit: #1096 review follow-ups, and restore the three jit-stats baselines that have main red on every OS - #1099

Merged
youknowone merged 6 commits into
mainfrom
nbody
Aug 7, 2026
Merged

jit: #1096 review follow-ups, and restore the three jit-stats baselines that have main red on every OS#1099
youknowone merged 6 commits into
mainfrom
nbody

Conversation

@youknowone

@youknowone youknowone commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Three independent commits: the two parity defects the #1096 review round found
after it was merged, and a jit-stats baseline restoration that unblocks main.

bench: restore the three jit-stats baselines #1063 replaced with values no host produces

main is currently red on every OS. pyre/check.py has failed for
binary_int_overflow_local_resume, exc_bridge_entry_guard_not_removed and
list_append_write_barrier_gc since 9d2fff9, on ubuntu-24.04,
macos-latest and windows-latest (run
31140634730), and
locally on macOS across all three backends.

Every one of those hosts observes:

fixture bridges_compiled / guard_failures observed everywhere recorded by 9d2fff9
binary_int_overflow_local_resume 5 / 647 6 / 686
exc_bridge_entry_guard_not_removed 4 / 809 5 / 1009
list_append_write_barrier_gc 5 / 1345 6 / 1562

The observed column is exactly what stood on main before 9d2fff9 (last
written by #947 and #1059). The recorded column reproduces nowhere — the
re-record was taken against a base these fixtures no longer had, and the merge
replayed it over the new one. This is a restoration, not a re-record blessing a
regression: no host has ever produced 6/686.

Re-recorded on dynasm, cranelift and wasm; the field_pos_* fields 9d2fff9
added are kept.

jit: keep find_biggest_function's closed-frame result when the recorder is gone

pyjitpl.py:3562 reads self.history.get_trace_position() unconditionally, so
the max_key the closed-frame loop produced always survives to the return.
pyre's recorder is an Option and the port spelled that read as
self.tracing.as_ref()?, which returns None for the whole function
whenever tracing has ended with an unmatched open entry still in
portal_trace_positions. Only the open frame is unmeasurable without a
recorder, so only its measurement is skipped now.

Not reachable from blackhole_trace_too_long_slow (it holds self.tracing as
Some), but find_biggest_function is pub.
find_biggest_function_keeps_a_closed_frame_when_the_recorder_is_gone covers
it and was checked to fail with the ? put back.

Also corrects blackhole_inlined_callee_local_after_escape_declined.py's second
description block, which still called sys._getframe() the residual after the
file's own header states it folds.

jit: emit sys._getframe's mark_as_escaped as a setfield, and carry the sized frame's jitdriver out of find_biggest_function

Two parity points from the Codex review of #1096:

  • vm.py:54 f.mark_as_escaped() is traced as an ordinary setfield_gc. The
    constant-depth fold emitted it as a void CallN into a Rust helper, hiding
    the update from the optimizer and its heap cache. Replaced with the
    read/or/store the tb_frame fold in the same file already uses
    (specialize.rs:2299-2313): getfield_gc_i(flags) + int_or(FLAG_ESCAPED) +
    setfield_gc + heapcache_setfield_cached. jit_frame_mark_as_escaped is
    deleted.
  • pyjitpl.py:3575 returns max_jdsd, max_key and :2821-2824 uses both — the
    disable goes through the owning driver's warmstate, and that driver is
    what aborted_tracing_jitdriver stores. The port dropped the jd_no its own
    log entries already carry and hardcoded driver 0. It now returns
    Option<(usize, u64)>. pyre keeps one WarmEnterState on the MetaInterp
    rather than one per JitDriverStaticData, so disable_noninlinable_function
    still lands on that single state; the comment names that limitation instead of
    faking it.

No recorded counter moves for either change on any backend.

Verification

pyre/check.py --backend dynasm,cranelift,wasmALL PASSED, 3/3
(dynasm 398/398, cranelift 398/398, wasm 394/394), at
b99bf57.

Review items deliberately not in this PR

  • The fold still declines constant nonzero depths and inline sub-walks
    (Codex §2). The blocker is real and cited: an inlined-callee frame carries
    last_instr = -1 with nothing updating it through the body
    (jitcode_dispatch/mod.rs:875-882), so folding there would compile a
    _getframe().f_lineno reporting the def line. Every declined shape falls
    through to the existing residual and answers correctly.
  • sys._getframe is missing upstream's audit(space, "sys._getframe", [f])
    (Codex §3) — pre-existing, and needs the audit mechanism checked first.
  • A pre-existing JIT-only crash, unrelated to these commits and reproducing on
    binaries built before them: 'i' in tb.tb_frame.f_locals inside an except
    handler in a hot loop panics value-stack underflow: depth=3 base=3, dynasm
    and cranelift; PYRE_NO_JIT=1 is green. A declined merge-point flush leaves
    the operand stack unpublished while last_instr is advanced anyway. Fixing it
    is a design decision, not a patch.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of open and inlined frames during JIT tracing, preserving accurate frame information when recorder support is unavailable.
    • Updated sys._getframe() behavior so frame access correctly marks inlined frames as escaped and maintains expected local-variable and traceback behavior.
    • Improved trace-abort handling for more reliable JIT recovery.
  • Tests

    • Expanded coverage for frame escaping and scenarios where recording is unavailable.
    • Refreshed benchmark statistics to reflect updated JIT behavior.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@youknowone, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 17 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 950992f1-5b9f-4798-82b6-e2910b32397a

📥 Commits

Reviewing files that changed from the base of the PR and between de6e235 and 7f3b700.

📒 Files selected for processing (14)
  • majit/majit-metainterp/src/pyjitpl.rs
  • pyre/bench/synth/getframe_caller_resume_coord_two_call_sites.cranelift.jitstats
  • pyre/bench/synth/getframe_caller_resume_coord_two_call_sites.dynasm.jitstats
  • pyre/bench/synth/getframe_caller_resume_coord_two_call_sites.py
  • pyre/bench/synth/getframe_caller_resume_coord_two_call_sites.wasm.jitstats
  • pyre/bench/synth/handler_tb_frame_locals_after_declined_flush.cranelift.jitstats
  • pyre/bench/synth/handler_tb_frame_locals_after_declined_flush.dynasm.jitstats
  • pyre/bench/synth/handler_tb_frame_locals_after_declined_flush.py
  • pyre/bench/synth/handler_tb_frame_locals_after_declined_flush.wasm.jitstats
  • pyre/check.py
  • pyre/pyre-interpreter/src/module/sys/vm.rs
  • pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs
  • pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs
  • pyre/pyre-jit-trace/src/trace.rs

Walkthrough

The PR updates largest-frame selection and trace-abort ownership tracking. It replaces the sys._getframe escape helper with explicit traced flag updates. Synthetic benchmark statistics and related test documentation are updated.

Changes

JIT frame handling

Layer / File(s) Summary
Largest-frame ownership and recorder handling
majit/majit-metainterp/src/pyjitpl.rs
find_biggest_function returns the JIT driver index and green key. Trace-abort handling records the owning driver. Open frames without recorders no longer replace a previously selected closed frame. Tests cover tuple results and recorder absence.
Explicit inline-frame escape lowering
pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs, pyre/pyre-interpreter/src/module/sys/vm.rs, pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.py
sys._getframe specialization updates FLAG_ESCAPED through traced field operations. The exported escape helper is removed. Test comments identify .f_locals as the escape-triggering operation.
Benchmark statistics baselines
pyre/bench/synth/*.jitstats
Benchmark baselines update bridge and guard-failure counts and add zero-valued blackhole-adoption and store-journal rollback counters.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant MetaInterp
  participant Frame
  participant JITRecorder
  participant JITDriver
  MetaInterp->>Frame: inspect frame size and ownership
  Frame->>JITRecorder: measure open frame when recorder exists
  JITRecorder-->>MetaInterp: return frame size
  MetaInterp->>JITDriver: record aborted tracing driver
Loading

Possibly related PRs

Poem

A rabbit hops through frames so wide,
Finds the driver tucked inside.
Flags turn green, escapes take flight,
Counters settle into night.
Tests keep watch with ears upright.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the JIT review follow-ups and the restoration of jit-stats baselines, which are key changes in the pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch nbody

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/pyre-jit-trace/src/jitcode_dispatch/specialize.rs`:
- Around line 7440-7455: After creating new_flags with record_op(OpCode::IntOr,
...), explicitly stamp it with set_opref_concrete using
Value::Int(known_or_computed_flags) before passing it to SetfieldGc and
heapcache_setfield_cached. Update the surrounding specialization flow without
changing the existing flags computation or field-cache behavior.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 1984e350-98ab-4fc5-9da5-592bf647cf63

📥 Commits

Reviewing files that changed from the base of the PR and between 779da08 and de6e235.

📒 Files selected for processing (13)
  • majit/majit-metainterp/src/pyjitpl.rs
  • pyre/bench/synth/binary_int_overflow_local_resume.cranelift.jitstats
  • pyre/bench/synth/binary_int_overflow_local_resume.dynasm.jitstats
  • pyre/bench/synth/binary_int_overflow_local_resume.wasm.jitstats
  • pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.py
  • pyre/bench/synth/exc_bridge_entry_guard_not_removed.cranelift.jitstats
  • pyre/bench/synth/exc_bridge_entry_guard_not_removed.dynasm.jitstats
  • pyre/bench/synth/exc_bridge_entry_guard_not_removed.wasm.jitstats
  • pyre/bench/synth/list_append_write_barrier_gc.cranelift.jitstats
  • pyre/bench/synth/list_append_write_barrier_gc.dynasm.jitstats
  • pyre/bench/synth/list_append_write_barrier_gc.wasm.jitstats
  • pyre/pyre-interpreter/src/module/sys/vm.rs
  • pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs
💤 Files with no reviewable changes (1)
  • pyre/pyre-interpreter/src/module/sys/vm.rs

Comment on lines +7440 to +7455
let flags_descr = crate::descr::pyframe_flags_descr();
let live_flags =
crate::state::opimpl_getfield_gc_i(ctx.trace_ctx, vable_op, flags_descr.clone());
let escaped_bit = ctx
.trace_ctx
.const_int(i64::from(pyre_interpreter::PyFrame::FLAG_ESCAPED));
let new_flags = ctx
.trace_ctx
.record_op(OpCode::IntOr, &[live_flags, escaped_bit]);
ctx.trace_ctx.record_op_with_descr(
OpCode::SetfieldGc,
&[vable_op, new_flags],
flags_descr.clone(),
);
ctx.trace_ctx
.heapcache_setfield_cached(vable_op, flags_descr.index(), new_flags);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check whether IntOr/SetfieldGc call sites elsewhere in the
# walker consistently stamp a concrete value on the computed OpRef, and
# whether TraceCtx::record_op auto-populates concrete values for pure ops.
set -euo pipefail

rg -n -B2 -A2 'OpCode::IntOr' pyre/pyre-jit-trace/src | rg -n -B2 -A2 'set_opref_concrete|record_op'
ast-grep run --pattern 'fn record_op($$$) { $$$ }' --lang rust pyre/pyre-jit-trace/src majit/majit-metainterp/src

Repository: youknowone/pyre

Length of output: 2296


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate trace_ctx files =="
git ls-files | rg 'TraceCtx|trace' | sed -n '1,80p'

echo "== find record_op definition(s) =="
rg -n 'fn record_op|record_op_with_descr|fn set_opref_concrete|box_value|opimpl_getfield_gc_i|heapcache_setfield_cached' -S --glob '*.rs' | sed -n '1,200p'

echo "== inspect specialize area =="
sed -n '7380,7475p' pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs

Repository: youknowone/pyre

Length of output: 3950


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== TraceCtx methods and concrete-value helpers =="
sed -n '1,360p' majit/majit-metainterp/src/trace_ctx.rs

echo "== specialized function context =="
sed -n '7320,7465p' pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs

echo "== similar IntOr/SetfieldGC usage context =="
sed -n '938,972p' pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs
sed -n '1950,1972p' pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs

echo "== IntOr usages nearby/elsewhere =="
rg -n -B12 -A8 'let .*=.*record_op\(OpCode::IntOr|set_opref_concrete|opimpl_getfield_gc_i' pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs | sed -n '1,260p'

Repository: youknowone/pyre

Length of output: 50371


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== concrete_value methods in trace_ctx.rs =="
python3 - <<'PY'
from pathlib import Path
p = Path('majit/majit-metainterp/src/trace_ctx.rs')
text = p.read_text()
needles = ['pub fn set_opref_concrete', 'pub(crate) fn set_opref_concrete', 'pub fn box_value', 'pub(crate) fn box_value', 'fn record_op', 'impl Trace for Trace']
for n in needles:
    idx = text.find(n)
    print(f'\n-- {n} around first occurrence --')
    if idx != -1:
        lines = text[:idx].count('\n')
        for i in range(max(1, lines-5), min(text.count('\n')+1, lines+200)):
            start = text.find('\n', 0 if i==1 else text.find('\n', 0)+1) + 1
    line_no=1
    PY

Repository: youknowone/pyre

Length of output: 452


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
p = Path('majit/majit-metainterp/src/trace_ctx.rs')
text = p.read_text()
lines = text.splitlines()
for needle in [
    'pub fn set_opref_concrete',
    'pub(crate) fn set_opref_concrete',
    'pub fn box_value',
    'pub(crate) fn box_value',
    'fn record_op',
    'impl Trace for Trace',
]:
    print(f'\n-- {needle} around first occurrence --')
    for i, line in enumerate(lines, 1):
        if needle in line:
            for n in range(max(1, i-8), min(len(lines), i+80)+1):
                print(f'{n}: {lines[n-1]}')
            break
PY

echo "== record_op definition file candidates =="
rg -n 'fn record_op|pub fn record_op' majit/majit-metainterp/src/majit/majit-trace/src majit -g '*.rs' | sed -n '1,120p'

echo "== concrete_of_opref implementations =="
rg -n 'fn concrete_of_opref|concrete_of_opref' majit/majit-metainterp/src majit/majit-trace/src pyre/pyre-jit-trace/src -g '*.rs' | sed -n '1,160p'

echo "== nearby concrete operations in trace_history or trace_ctx =="
rg -n -B3 -A25 'fn.*record_(op|call|guard)\b|fn.*binop_int|fn.*unop_int|set_opref_concrete|fn box_value|fn concrete_of_opref' majit/majit-metainterp/src majit/majit-trace/src -g '*.rs' | sed -n '1,260p'

Repository: youknowone/pyre

Length of output: 11351


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== record_op implementations =="
sed -n '2120,2200p' majit/majit-metainterp/src/history.rs
sed -n '2430,2570p' majit/majit-metainterp/src/opencoder.rs
sed -n '260,320p' majit/majit-metainterp/src/recorder.rs

echo "== concrete_at helpers =="
rg -n -B5 -A40 'fn concrete_at|fn set_concrete_at|concrete_at|set_concrete_at|pub struct OpRef|impl OpRef|pub fn const_int' majit/majit-metainterp/src majit/majit-trace/src -g '*.rs' | sed -n '1,260p'

echo "== opimpl_getfield_gc_i and heapcache cache-hit read path =="
rg -n -B8 -A20 'fn opimpl_getfield_gc_i|pub fn opimpl_getfield_gc_i|heapcache_setfield_cached|fn heapcache_setfield_cached|cached.*box_value|opimpl_getfield_gc_i' pyre/pyre-jit-trace/src majit/majit-metainterp/src -g '*.rs' | sed -n '1,280p'

Repository: youknowone/pyre

Length of output: 50371


Stamp the concrete value on new_flags before reusing it.

record_op(OpCode::IntOr, ...) only allocates the result OpRef; it does not set Op.value. Add an explicit set_opref_concrete(new_flags, Value::Int(known_or_computed_flags)) after this IntOr, matching the pattern for resolved computed boxes elsewhere. The related IntOr sites in mod.rs and inline_call.rs already leave that result symbolic, so this is consistent with the current code shape but still leaves new_flags without a trace-time concrete value.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs` around lines 7440 -
7455, After creating new_flags with record_op(OpCode::IntOr, ...), explicitly
stamp it with set_opref_concrete using Value::Int(known_or_computed_flags)
before passing it to SetfieldGc and heapcache_setfield_cached. Update the
surrounding specialization flow without changing the existing flags computation
or field-cache behavior.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

🤖 Codex parity review

Static analysis of this diff vs the local RPython/PyPy sources (commit 7f3b700).
Updated: 2026-08-07T14:40:56.357Z

Files in the reviewed diff
majit/majit-metainterp/src/pyjitpl.rs
pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.py
pyre/bench/synth/getframe_caller_resume_coord_two_call_sites.py
pyre/bench/synth/handler_tb_frame_locals_after_declined_flush.py
pyre/check.py
pyre/pyre-interpreter/src/module/sys/vm.rs
pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs
pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs
pyre/pyre-jit-trace/src/trace.rs

Codex did not produce a report (exit 1). Last log lines:

to the corresponding RPython/PyPy source code. The RPython and PyPy sources
are available locally (under `rpython/` and `pypy/` in this repository).

If anything was ported incorrectly, report every instance in detail. After
collecting all differences, organize the report into separate sections:

1. Cases where our patch regressed PyPy parity compared to main
2. Other mismatches introduced by our patch
3. Mismatches that already existed before this patch
4. Structural adaptations

Exceptions: some differences cannot be ported 1:1 because of Python 3.11 vs
3.14 differences, opcode mismatches caused by using a CPython-compatible
compiler, GIL/free-threading differences, and fundamental implementation-
language differences between RPython and Rust. Mark those separately under
"Structural adaptations".

Scope discipline: before writing the report, run
`git diff upstream/main --name-only -- . ':(exclude)*.jitstats'` and treat that
file list as the authoritative definition of "this patch" (when an authoritative
changed-file list is appended below, use that instead of re-deriving it). The
excluded `*.jitstats` files are `pyre/check.py`'s recorded jit-stats baselines —
generated golden data with no RPython/PyPy counterpart, so no parity finding can
cite one, and a bulk re-record of them is not a change to review. Findings under
sections 1 and 2 MUST cite our-side files from that list; a divergence in any
file NOT in the list is by definition not introduced by this patch — report
it under section 3 instead, or omit it. Verify every section-1/2 citation
against the list before finalizing the report.

---

Output format requirements (so the report can be parsed mechanically and
posted/triaged automatically). Use these four headings VERBATIM, in this
order, and nothing else at heading level 2:

## 1. 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

Under each heading, list every finding as a bullet. For each finding cite the
concrete `our_file.rs:line ↔ rpython_or_pypy_file.py:line` pair and quote the
divergence concisely. If a section has no findings, still emit the heading
followed by a single line `None.` so all four sections are always present.
Do not modify any files; produce the report only.

Authoritative changed-file list for this patch (git diff upstream/main --name-only,
minus 15 generated `*.jitstats` baseline file(s)):
majit/majit-metainterp/src/pyjitpl.rs
pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.py
pyre/bench/synth/getframe_caller_resume_coord_two_call_sites.py
pyre/bench/synth/handler_tb_frame_locals_after_declined_flush.py
pyre/check.py
pyre/pyre-interpreter/src/module/sys/vm.rs
pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs
pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs
pyre/pyre-jit-trace/src/trace.rs
warning: Codex could not find bubblewrap on PATH. Install bubblewrap with your OS package manager. See the sandbox prerequisites: https://developers.openai.com/codex/concepts/sandboxing#prerequisites. Codex will use the bundled bubblewrap in the meantime.
ERROR: You've hit your usage limit. Visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at Aug 8th, 2026 3:32 AM.
ERROR: You've hit your usage limit. Visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at Aug 8th, 2026 3:32 AM.

…er is gone

pyjitpl.py:3562 reads `self.history.get_trace_position()` unconditionally, so
the `max_key` the closed-frame loop above produced always survives to the
return. pyre's recorder is an `Option` and the port spelled that read as
`self.tracing.as_ref()?`, which returns `None` for the whole function whenever
tracing has ended with an unmatched open entry still in
`portal_trace_positions`. Only the open frame is unmeasurable without a
recorder, so only its measurement is skipped now.

Not reachable from `blackhole_trace_too_long_slow`, which holds `self.tracing`
as `Some`; `find_biggest_function` is `pub`.

`find_biggest_function_keeps_a_closed_frame_when_the_recorder_is_gone` covers
it and fails with the `?` put back.

Also corrects blackhole_inlined_callee_local_after_escape_declined.py's second
description block, which still called `sys._getframe()` the residual after the
file's own header states it folds and the added `.f_locals` read is the force.

Assisted-by: Claude
…e sized

frame's jitdriver out of find_biggest_function

vm.py:54 `f.mark_as_escaped()` is traced as an ordinary `setfield_gc` on the
flag. The constant-depth fold emitted it as a void CallN into a Rust helper
instead, which hides the update from the optimizer and its heap cache. Replaced
with the read/or/store the `tb_frame` fold in the same file already uses
(specialize.rs:2299-2313): getfield_gc_i(flags) + int_or(FLAG_ESCAPED) +
setfield_gc + heapcache_setfield_cached. `jit_frame_mark_as_escaped` is deleted.

pyjitpl.py:3575 returns `max_jdsd, max_key`, and pyjitpl.py:2821-2824 uses both
-- the disable goes through the OWNING driver's warmstate and that driver is
what `aborted_tracing_jitdriver` stores. The port dropped the jd_no its own log
entries already carry and hardcoded driver 0. It now returns
`Option<(usize, u64)>` and the caller stores the index it was given. pyre keeps
one WarmEnterState on the MetaInterp rather than one per JitDriverStaticData, so
`disable_noninlinable_function` still lands on that single state; the comment
names it.

No recorded counter moves on dynasm, cranelift or wasm.

Assisted-by: Claude
…es no host

produces

`pyre/check.py` has been red on main for binary_int_overflow_local_resume,
exc_bridge_entry_guard_not_removed and list_append_write_barrier_gc since
9d2fff9, on ubuntu-24.04, macos-latest AND windows-latest (run
31140634730), and locally on macOS across all three backends.

Every one of those hosts observes bridges_compiled/guard_failures 5/647, 4/809
and 5/1345. Those are exactly the values that stood on main before 9d2fff9
(last written by #947 and #1059); 9d2fff9 recorded 6/686, 5/1009 and 6/1562,
which reproduce nowhere. The re-record was taken against a base whose behaviour
these fixtures no longer had, and the merge replayed it.

Re-recorded on dynasm, cranelift and wasm. The counters land back on the
pre-9d2fff92649 values; the `field_pos_*` fields 9d2fff9 added are kept.

Assisted-by: Claude
…lushed

`flush_active_frame_escape`'s force arm has three outcomes. A committed full
flush publishes a resume pc into `COMMITTED_FRAME_ESCAPE_PC`; an all-or-nothing
decline discards the undo capture; the third -- the full flush declines and
`flush_locals_region_to_frame` writes slots `0..nlocals` on their own -- did
neither.

That leg claims no resume pc, so `take_committed_frame_escape_pc` yields
nothing and the walk-end block gated on it is skipped in its entirety,
including the `restore_escape_flush_undo()` in its `else`. The capture stays
armed, `LiveLastInstrGuard::drop` reads an armed capture as a flush owning the
frame and declines to put `last_instr` back, and the legacy replay re-enters
one opcode past the call on an operand stack no flush wrote: `value-stack
underflow: depth=N base=N`, a JIT-only panic with no program output.

`mark_escape_flush_undo_pending()` routes the leg to the walk-end deferred
restore, which is already conditioned on no continuation having claimed the
flushed frame -- so where the walk goes on to adopt a blackhole image the
request is consumed without restoring and the adoption keeps the frame it
claimed.

Restoring earlier is not equivalent: making `LiveLastInstrGuard::drop` test the
commit instead removes the crash and returns a stale caller line, because the
walk goes on after the residual and nothing else advances `last_instr`.

`bench/synth/handler_tb_frame_locals_after_declined_flush.py` reaches the leg:
`'i' in tb.tb_frame.f_locals` forces the frame mid-expression, with the
`seen.add` receiver and its bound method live below the value being computed.

A/B on the cranelift binary that reproduced it: 10/10 panics without the
change, 0/10 with it, output `[True]` matching `PYRE_NO_JIT=1`.

Assisted-by: Claude
A callee reading its caller's frame through `sys._getframe(1)` had no coverage
of the resume coordinate: `bench/synth` holds ten `_getframe(1)` fixtures, one
`f_lineno` fixture (a traceback frame) and no `f_lasti` fixture at all. Both
fields resolve off `last_instr`, which compiled code does not store per opcode,
so the value only reaches the frame if the force publishes it.

Two call sites are what make that observable. One holds the caller's coordinate
constant by construction, so a frozen read is indistinguishable from a live
one. Surveying every iteration into a set rather than sampling the last one is
the other half: the pre-compile iterations are correct, so a miss appears as a
changed row count.

`f_lasti` is a bytecode offset and so is not comparable against the pypy
oracle; only its discrimination is printed. `f_lineno` is compared directly,
relative to `co_firstlineno`.

Measured by putting a defect back in: with the `flushed` test dropped from
`LiveLastInstrGuard::drop`, so the guard restores at the residual's return
instead of at walk end, the fixture reports

    ([(0, 3), (0, 8), (1, 3), (1, 6)], [0, 0, 1, 1], 3)

against its

    ([(0, 8), (1, 6)], [0, 1], 2)

-- the pre-call coordinate appears alongside the call-site one on both legs.
cpython, pypy, `PYRE_NO_JIT=1`, dynasm, cranelift and wasm all print the
latter.

The walk-end epilogue gains the negative result measured while looking for a
counter to gate the same defect: every walk reaching that point on this fixture
reports `armed=false fb=true`, so a leak counter conditioned on the three
adoption flags being false reads 0 whether or not the force arm arms its
deferred restore.

Assisted-by: Claude
`pyre-jit-trace/build.rs` compares each `build/llbc/*.ullbc` against what its
crate's sources hash to now and reports a mismatch as `cargo::warning`, which
cargo replays only when it re-runs the build script -- so a run whose crates
were cached prints nothing at all. Every number check.py produces is read out
of a binary whose field offsets come from those artefacts.

Measured on this tree: four measurement runs -- a three-backend gate, two A/B
arms and a base control -- carried the mismatch, and the string `LLBC STALE`
appears in none of their logs, while `cargo check -p pyrex` on the same tree
printed it for all three crates. check.py only ever tested for the artefacts
being missing.

It now exports `PYRE_LLBC_STRICT=1` before every backend build, the promotion
build.rs documents for callers that want a gate, and names staleness in the
build-failure diagnostics beside the missing-artefact branch. The cost is that
a rebase which moves the LLBC crates stops the next check.py until a
re-extraction; `PYRE_LLBC_SKIP_FINGERPRINT_CHECK=1` still opts out for an A/B
whose only changed crate contributes no field offsets.

First use found one: the wasm jit-stats fall on
`exception_reused_object_tb_not_doubled` that four arms reproduced was an
artefact of the stale artefacts, and the bench passes on all three backends
after a re-extraction with nothing re-recorded.

Assisted-by: Claude

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7f3b700109

ℹ️ 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 thread pyre/check.py
# check.py stop and ask for a multi-minute re-extraction; the
# alternative is a green run that measured the wrong bytes. Set here
# rather than per-command so the wasm build gets it too.
os.environ["PYRE_LLBC_STRICT"] = "1"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Force the fingerprint check to rerun

When check.py has already built once with this environment value and a tracked pyre-jit source outside src/eval.rs is subsequently changed, Cargo can reuse the cached pyre-jit-trace build-script result because the value remains 1. The rerun directives cover the collected pyre-object/pyre-interpreter paths but intentionally collect only pyre-jit/src/eval.rs (pyre-jit-trace/build.rs:446-472,861-889), so warn_if_llbc_stale is never invoked for changes elsewhere in that crate and the supposedly strict benchmark run can silently consume stale pyre-jit.ullbc. Force the build script to run for every check, track the complete fingerprint input set, or perform the fingerprint check directly in check.py.

AGENTS.md reference: AGENTS.md:L46-L52

Useful? React with 👍 / 👎.

@youknowone
youknowone merged commit 0b1dee0 into main Aug 7, 2026
13 of 17 checks passed
@youknowone
youknowone deleted the nbody branch August 7, 2026 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant