diff --git a/majit/majit-metainterp/src/pyjitpl.rs b/majit/majit-metainterp/src/pyjitpl.rs index c5026b8547e..9dbb54b6379 100644 --- a/majit/majit-metainterp/src/pyjitpl.rs +++ b/majit/majit-metainterp/src/pyjitpl.rs @@ -5256,7 +5256,9 @@ impl MetaInterp { // Bridge traces start from rebuilt resume state, not a fresh portal // entry, so `initial_inputarg_consts` is not seeded with the // virtualizable inputarg's ConstPtr. The TraceCtx pointer is the - // trace-bound equivalent of `orig_inpargs[idx].getref_base()`. + // trace-bound equivalent of `orig_inpargs[idx].getref_base()` + // (compile.py:510), which reads a value belonging to the trace being + // compiled and never an ambient one. // // Prefer it over MetaInterp's ambient pointer: an inlined residual // callee can temporarily update `self.vable_ptr`, while the residual diff --git a/pyre/bench/synth/getframe_inline_subwalk_multiframe.py b/pyre/bench/synth/getframe_inline_subwalk_multiframe.py index 0f7982073b8..71a1fb5da21 100644 --- a/pyre/bench/synth/getframe_inline_subwalk_multiframe.py +++ b/pyre/bench/synth/getframe_inline_subwalk_multiframe.py @@ -2,32 +2,38 @@ # forces an outer frame while the walk is already inside a residual call. # # The walker executes a residual call concretely, so that level gets a real -# frame from the interpreter's own call sequence; an inline push never runs -# that sequence, so its level has none. A force fired from the inlined body -# therefore builds a frame chain that mixes the two, and the chain's root is -# the intermediate residual frame rather than the walked frame -- which is -# what `try_adopt_multi_frame_blackhole` declines on today (it wants the -# `jit.virtual_ref` emit at the inline push, `executioncontext.py:89`). This -# fixture pins that the declined path still returns byte-identical results. +# frame from the interpreter's own call sequence; an inline push did not run +# that sequence, so its level had none. A force fired from the inlined body +# therefore built a frame chain that mixed the two, rooted at the intermediate +# residual frame rather than the walked frame, and +# `try_adopt_multi_frame_blackhole`'s chain-root identity gate declined it. What +# that decline wanted was the `jit.virtual_ref` emit at the inline push +# (`executioncontext.py:89`); `walker_ec_enter` / `walker_ec_leave` landed it. +# The gate no longer fires here: the shape now adopts once per build and returns +# the same result as before. This fixture pins that result. # # One `sys._getframe(1)` level does NOT reach the build: the chain needs a # residual level under the walked frame and an inlined level under that, so # the force has to reach two frames up. No other fixture in the corpus gets -# here -- swept with `PYRE_FBW_DEBUG_ABORT=1 PYRE_FBW_MULTIFRAME=1`, 0 of 310 -# reach `BUILT multi-frame`, so without this one the path has no repro at all. +# here -- in the historical gate-enabled sweep with `PYRE_FBW_DEBUG_ABORT=1`, +# 0 of 310 reached `BUILT multi-frame`, so without this one the path has no +# repro at all. # -# With the gate at its default the image is not built either, so under a plain -# run this is an output guard; the coverage it adds is for `PYRE_FBW_MULTIFRAME=1` -# (5 builds), which is how the gate's owner exercises the path. +# The multi-frame image is built unconditionally when the latch conditions hold, +# so this is both an output guard and build-path coverage: 5 builds, and now 5 +# adopts with zero chain-root declines (`PYRE_FBW_DEBUG_ABORT=1` prints both +# tallies; the other 5 escapes in the run have `inline_subwalk=false` and take +# the single-frame arm). # -# What the decline is holding back, measured by lifting it: the resumed chain -# shifts every `sys._getframe(n)` up exactly one level, so the read below lands -# on the module frame and raises `KeyError: 'base'`. Variants of this shape that -# cannot raise return a wrong number instead, silently -- `f_locals.get("base", -# -1)` scores -1 for 7, `len(f_locals)` scores the module globals' 12 for this -# frame's 3, `len(f_code.co_name)` scores ``'s 8 for a 9-character -# caller name. So the adopt is wrong for every outcome arm, not only the one -# that carries a resume coordinate. +# What the decline used to hold back, measured by lifting it before the +# execution-context push landed: the resumed chain shifted every +# `sys._getframe(n)` up exactly one level, so the read below landed on the +# module frame and raised `KeyError: 'base'`. Variants of this shape that cannot +# raise returned a wrong number instead, silently -- `f_locals.get("base", -1)` +# scored -1 for 7, `len(f_locals)` scored the module globals' 12 for this +# frame's 3, `len(f_code.co_name)` scored ``'s 8 for a 9-character +# caller name. That is the failure mode this fixture still guards: every outcome +# arm was wrong, not only the one carrying a resume coordinate. # # Deliberately carries no `# pyre-check: max-pypy-ratio=` header: this guards # an output, and the forcing read makes it a poor perf subject. diff --git a/pyre/bench/synth/getframe_while_escaping_read_frame_identity.py b/pyre/bench/synth/getframe_while_escaping_read_frame_identity.py index 3e6f5f6f1d8..2f9052741fe 100644 --- a/pyre/bench/synth/getframe_while_escaping_read_frame_identity.py +++ b/pyre/bench/synth/getframe_while_escaping_read_frame_identity.py @@ -1,29 +1,28 @@ -# The frame-identity read that the multi-frame blackhole adopt gets wrong, and -# the acceptance test for flipping `PYRE_FBW_MULTIFRAME` default-ON. +# The frame-identity read the multi-frame blackhole adopt commits, and the +# regression guard for making that path unconditional. # # The walk executes the forcing residual CONCRETELY, and an inline push never -# runs the interpreter's call sequence, so `ec.topframeref` still names the -# CALLER while the inlined callee body runs. A `sys._getframe` that is itself -# the escaping call therefore reads the caller's frame at walk time, and the -# adopt commits that answer instead of discarding it the way the legacy -# escape/replay path does. +# runs the interpreter's call sequence. Before `walker_ec_enter` / +# `walker_ec_leave` published the callee frame on the execution context, +# `ec.topframeref` still named the CALLER while the inlined callee body ran, so +# a `sys._getframe` that is itself the escaping call read the caller's frame at +# walk time and the adopt committed that answer instead of discarding it the way +# the legacy escape/replay path does. Measured then as one wrong iteration per +# multi-frame adopt, in each part: # -# Measured 2026-07-26 with the gate forced on -- one wrong iteration per -# multi-frame adopt, 5 adopts and 5 wrong in each part: -# -# part_a `_gf()` names `main`, not `leaf` -# part_b `_gf(1)` names ``, not `main` -- one level too far up, which +# part_a `_gf()` named `main`, not `leaf` +# part_b `_gf(1)` named ``, not `main` -- one level too far up, which # is the same error seen through the argument # -# A `_gf(1)` reading `f_locals` on that shape raises `KeyError` for any caller +# A `_gf(1)` reading `f_locals` on that shape raised `KeyError` for any caller # local, for the same reason and not because outer locals go unmaterialized. # -# Both are correct with the gate off, which is the default, so this fixture -# passes today. It exists to fail loudly if the gate is flipped before the -# inlined-call push publishes the callee frame on the execution context. Note -# the read has to be the ESCAPING call: once the escape has happened, a -# `sys._getframe(1)` executed inside the blackhole is correct, because the chain -# publishes each level's frame as it runs. +# Both answers are correct now, with the adopt committing rather than declining +# (`PYRE_FBW_DEBUG_ABORT=1` prints one `adopted multi-frame terminal` per +# iteration that latches, and no `chain rooted at` decline). Note the read has +# to be the ESCAPING call: once the escape has happened, a `sys._getframe(1)` +# executed inside the blackhole was always correct, because the chain publishes +# each level's frame as it runs. import sys _gf = sys._getframe diff --git a/pyre/bench/synth/getframe_while_inlined_callee_subwalk.py b/pyre/bench/synth/getframe_while_inlined_callee_subwalk.py index b5c70f96b1a..985932ef15a 100644 --- a/pyre/bench/synth/getframe_while_inlined_callee_subwalk.py +++ b/pyre/bench/synth/getframe_while_inlined_callee_subwalk.py @@ -1,4 +1,4 @@ -# Coverage guard for the multi-frame blackhole path (PYRE_FBW_MULTIFRAME). +# Coverage guard for the unconditional multi-frame blackhole path. # # A vable escape inside an INLINE sub-walk is what latches a multi-frame # blackhole image. The rest of the corpus never produces one: every other diff --git a/pyre/gate-triage.md b/pyre/gate-triage.md index 21776aee0dc..7a1596152da 100644 --- a/pyre/gate-triage.md +++ b/pyre/gate-triage.md @@ -222,7 +222,7 @@ upstream lines: | gate | orthodox side | outcome | |---|---|---| | PYRE_FBW_VABLE_SCALAR_CA | **OFF** | **RETIRED** — the ON design contradicts upstream | -| PYRE_FBW_MULTIFRAME | **ON** | keep default-OFF; the ON path is the port and the adopt now works, but §1d measures one remaining wrong answer under it — a `sys._getframe` that is itself the escaping residual reads the caller frame | +| `_MULTIFRAME` (retired) | **ON** | **RETIRED** — reader and OFF path deleted after the default-ON verification; the adopt is now unconditional when the multi-frame latch conditions hold. §1d's one remaining wrong answer (a `sys._getframe` that is itself the escaping residual reading the caller frame) was closed by `walker_ec_enter` / `walker_ec_leave` publishing the callee frame at the inlined-call push | | PYRE_FBW_CALLEE_VSTACK | NEITHER | keep OFF; see §5 | The walker's default-ON `PYRE_FBW_*` cluster was retired separately in #757. @@ -337,8 +337,9 @@ committed image". **Measured 2026-07-25: the multi-frame path has no corpus coverage.** The vable-escape latch site was instrumented and all **318** benchmarks -(`pyre/bench` + `pyre/bench/synth`) run under `PYRE_FBW_MULTIFRAME=1`. The site -is reached in **3 benches** (`getframe_escape_flush_writethrough_regression`, +(`pyre/bench` + `pyre/bench/synth`) were historically run with the multi-frame +gate enabled. The site was reached in **3 benches** +(`getframe_escape_flush_writethrough_regression`, `synth/getframe_inlined_callee_own_frame`, `synth/getframe_stored_fback_walk`), 5 events each, and **all 15 have `inline_subwalk=false`** — every one takes the single-frame arm and adopts. `build_multi_frame_miframe` is therefore never @@ -355,8 +356,8 @@ Under that shape `build_multi_frame_miframe` **succeeds at depth 2**, so the build side was never what blocked. It is landed as `synth/getframe_while_inlined_callee_subwalk`; the three shape choices in its header are load-bearing and changing any of them silently stops exercising the -path. With the comparison fixed, that fixture under `PYRE_FBW_MULTIFRAME=1` -reports **5 `BUILT multi-frame depth=2` and 5 `adopted multi-frame terminal`, +path. With the comparison fixed, that fixture historically reported **5 +`BUILT multi-frame depth=2` and 5 `adopted multi-frame terminal`, zero declines** (the other 5 escapes in the run have `inline_subwalk=false` and take the single-frame arm, as before), and prints the same result as CPython and PyPy. @@ -385,12 +386,12 @@ concrete banks at an inline escape — not a change to the capture itself. Both are pinned by `synth/getframe_while_subwalk_decline_shapes` so a decline cannot silently become a wrong answer. -**The flip is blocked, and the blocker is a wrong answer, not a decline.** +**The flip was blocked by a wrong answer, not a decline — resolved 2026-07-30.** Measured 2026-07-26. The walker executes residuals **concretely** while an -inline push never runs the interpreter's call sequence, so `ec.topframeref` -still names the CALLER while an inlined callee body runs. A `sys._getframe` -that is *itself* the escaping residual therefore reads the wrong frame at walk -time, and the adopt commits that answer where legacy escape/replay discards it: +inline push did not run the interpreter's call sequence, so `ec.topframeref` +still named the CALLER while an inlined callee body ran. A `sys._getframe` +that is *itself* the escaping residual therefore read the wrong frame at walk +time, and the adopt committed that answer where legacy escape/replay discards it: ``` _gf().f_code.co_name -> "main", not "leaf" @@ -399,39 +400,50 @@ _gf(1).f_locals["k"] -> KeyError # same cause, seen through ``` **One wrong iteration per multi-frame adopt** — 5 adopts, 5 wrong, in each part -of `synth/getframe_while_escaping_read_frame_identity`, which is the acceptance -test: it passes today (gate off, the default) and fails loudly if the gate is -flipped first. This is *not* outer-locals staleness. A `sys._getframe` -executed **after** the escape, inside the blackhole, is correct — the chain -publishes each level's frame as it runs — and an in-blackhole read of a caller -local mutated earlier in the same iteration was measured correct against CPython -and PyPy. Closing it needs the inlined-call push to publish the callee frame on -the execution context, which is what the open `walker_ec_enter` / `walker_ec_leave` -work does; the `jit.virtual_ref` emit rides along with it. So the original -decline comment was right that an inline-push `enter` is the prerequisite, and -wrong only about which check it gated. +of `synth/getframe_while_escaping_read_frame_identity`, the acceptance test. +This was *not* outer-locals staleness. A `sys._getframe` executed **after** the +escape, inside the blackhole, is correct — the chain publishes each level's +frame as it runs — and an in-blackhole read of a caller local mutated earlier in +the same iteration was measured correct against CPython and PyPy. Closing it +needed the inlined-call push to publish the callee frame on the execution +context, which is what `walker_ec_enter` / `walker_ec_leave` do; the +`jit.virtual_ref` emit rode along with them. So the original decline comment was +right that an inline-push `enter` is the prerequisite, and wrong only about which +check it gated. + +**Resolved 2026-07-30.** With that push landed, the same fixture reports +`30000 30000 0 0` — zero wrong frames — under CPython and under pyre, with 10 +`adopted multi-frame terminal` events and no chain-root decline, so the adopt +commits rather than declining. The gate was flipped default-ON and then retired +outright (§3); the acceptance test became the regression guard for the +unconditional path. `synth/getframe_inline_subwalk_multiframe`, whose header had +recorded that the chain-root identity gate declined its shape, now measures 5 +builds, 5 adopts, zero declines, and the same output as before. One thing the ON path already fixes: with a side-effecting inlined callee under a `while` loop that returns from inside the loop, the OFF path runs the callee's side effect ~5.2k extra times (the recorded trace-abort double-run class) while the adopt gives the exact count. -Everything else that was thought to block the flip has been measured and does -not: the full corpus is **336/336 with the gate on (dynasm) and 336/336 with it -off (cranelift)**, the blast radius is exactly `inline_subwalk = true` at a -vable escape (the latch is an `if`/`else if` whose single-frame arm requires -`!inline_subwalk`, so with the gate off that condition latches nothing and falls -to legacy escape/replay), and the two build-side declines above are correct. - -Note the multi-frame latch is -nested inside `single_frame_blackhole_resume_enabled()`, so it also requires -`_BLACKHOLE_RESUME` to stay ON. The pre-existing `[s2-gate]` eprintln (under +Everything else that was thought to block the flip was measured and did not: the +full corpus was **336/336 with the gate on (dynasm) and 336/336 with it off +(cranelift)** at the time, the blast radius is exactly `inline_subwalk = true` at +a vable escape (the latch is an `if`/`else if` whose single-frame arm requires +`!inline_subwalk`, so the multi-frame arm is the only one that shape can take), +and the two build-side declines above are correct. Re-measured at the flip: +**1 failed / 341 passed on dynasm and on cranelift with the gate forced on**, the +one failure being a cpython/pypy reference mismatch unrelated to this path. + +The multi-frame latch shares the outer `writes_live_heap`, odometer-unchanged, +non-bridge, blackhole-result, and resolvable-snapshot conditions with the +single-frame latch, then takes the `inline_subwalk` arm when its frame-stack +image builds successfully. The pre-existing `[s2-gate]` eprintln (under `PYRE_FBW_DEBUG_ABORT`) already reports `inline_subwalk` at that site. **A second coverage benchmark, 2026-07-27.** `synth/getframe_inline_subwalk_multiframe` (#798) reaches the latch with -`inline_subwalk=true` and drives `build_multi_frame_miframe` — under -`PYRE_FBW_MULTIFRAME=1 PYRE_FBW_DEBUG_ABORT=1` it prints 5 `[s2-gate] +`inline_subwalk=true` and drives `build_multi_frame_miframe`. In the historical +gate-enabled measurement with `PYRE_FBW_DEBUG_ABORT=1`, it printed 5 `[s2-gate] inline_subwalk` lines each followed by `[s2-build-decline] BUILT multi-frame depth=2`. One `sys._getframe(1)` level does not get there; the chain needs a residual level under the walked frame and an inlined level under that, so the @@ -452,7 +464,7 @@ delete, do not count as gates.** - `PYRE_JIT_DISABLED` — a `OnceLock` cache name holding the `PYRE_JIT==0` result (`pyre-jit/src/eval.rs`); the env var is `PYRE_JIT` - `PYRE_STACKTOOBIG` — `pub static PyreStackTooBig` runtime symbol (`stack_check.rs`) -## §3 — Dead (10): no env read site +## §3 — Dead (12): no env read site No source reads these. Comment-only or absent. **Historical measurement notes are preserved in place per N7** (they record why code was deemed dead / what a @@ -470,6 +482,8 @@ census verified); they are not live gates and cost nothing. | PYRE_S8B_HARNESS | retired census; "82/82 agreement" measurement kept | | PYRE_MODULE_LOOP_TRACE | retired switch; historical note kept | | PYRE_FULL_BODY_WALK | retired switch; the full-body walk is the sole tracer, so the OFF path (the deleted trait leg) is gone (#344) | +| `_MULTIFRAME` | retired switch; reader and OFF path deleted once `walker_ec_enter` / `walker_ec_leave` closed the escaping-`sys._getframe` identity answer (§1d). Flipped default-ON and retired 2026-07-30; `_MULTIFRAME_DEPTH` is a separate live depth bound and is not this gate | +| `_BLACKHOLE_RESUME` | retired switch; reader and OFF path deleted after #754 closed, with the multi-frame twin's retirement unblocking removal; it was flipped default-ON on 2026-07-25 | ## §4 — Live default-ON gates KEPT (retire when the epic closes) @@ -478,7 +492,6 @@ OFF path is a needed safety net. Retire at the listed trigger (A7). | var | subsystem | retire when | |---|---|---| -| PYRE_FBW_BLACKHOLE_RESUME | single-frame resume-past-escape (#754) | flipped default-ON 2026-07-25; retirement is conditioned on the multi-frame twin (`_MULTIFRAME`) flipping, and the multi-frame latch is nested inside `single_frame_blackhole_resume_enabled()`, so this gate has to stay ON while that one is in play. Both earlier premises are spent: the root-mismatch decline is resolved (§1d) and the twin's corpus coverage is no longer zero (20 adopts, 10 pinned declines). What blocks the twin now is the escaping `sys._getframe` identity answer, not this gate | | PYRE_TWO_PHASE_RTYPE, PYRE_TUPLE_PER_SHAPE_CLASSDEF | rtyper prepass / per-shape tuple classdef | WS2 / #346 rtyper epic | | PYRE_ORIGINAL_BOXES | greens++reds original_boxes index shape | box-identity #202 / resume F1 | | PYRE_MIR_FRAMESTATE | framestate-threaded MIR lowering | MIR front-end #176/#181/#346 | @@ -503,15 +516,17 @@ Kept as-is; listed for completeness. `_GIN`, `_INLINE_RECOG`, `PYRE_WASM_DUMP_ALL_TRACES`, `_DUMP_BAD_TRACE`, `_EXEC_TRACE`, `_JIT_STATS`, `PYRE_INTERP_RETURN_LOG`, `PYRE_NBODY_DEBUG`, `PYRE_DEBUG_CALL`, `PYRE_DEBUG_CLASS`. -- **Default-OFF experiments (3 remaining)** — triaged in §1b/§1c (4 retired +- **Default-OFF experiments (2 remaining)** — triaged in §1b/§1c (4 retired in the 2026-07-05 pass, 8 retired since then; `PYRE_P2_DRAIN` retired with the framestack-walk deletion; `_VABLE_SCALAR_CA` retired 2026-07-25, see - §1d). Kept: `_MULTIFRAME` (multi-frame blackhole image — the ON path IS the - upstream structure, see §1d), `_CALLEE_VSTACK` (callee-local operand-stack - mirror), and `PYRE_CARRIER_EXC_RESUME`. For these the *ON* path is the - unattested one, so they are adoption targets rather than retirement targets. - `_BLACKHOLE_RESUME` graduated out of this bucket on 2026-07-25 (flipped - default-ON, now in §4). + §1d). Kept: `_CALLEE_VSTACK` (callee-local operand-stack mirror) and + `PYRE_CARRIER_EXC_RESUME`. For these the *ON* path is the unattested one, so + they are adoption targets rather than retirement targets. + The single-frame resume-past-escape switch graduated out of this bucket on + 2026-07-25 when it flipped default-ON. It is now retired alongside the + multi-frame switch, with both readers and OFF paths deleted after + `walker_ec_enter` / `walker_ec_leave` closed the escaping-`sys._getframe` + identity answer. `_CALLEE_VSTACK` was evaluated for a flip on 2026-07-25 and **declined — the ON path is a half-finished port with no consumer**. Parity first: diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs index 8dd41afb906..6036d0fef15 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs @@ -238,42 +238,6 @@ pub(crate) fn latch_trace_too_long_blackhole( } } -/// `PYRE_FBW_BLACKHOLE_RESUME` (default ON) — a top-level one-frame walk whose -/// residual forced the vable and writes live heap resumes PAST the escaping -/// opcode through the blackhole instead of falling back to escape/replay. Both -/// the latch (`writes_live_heap`, odometer unchanged, non-bridge, empty -/// framestack, no committed escape pc, resolvable snapshot sym) and the adopt -/// (`try_adopt_single_frame_blackhole` → `apply_single_frame_blackhole_crn`, -/// which validates every mapped color and every live operand-stack slot before -/// writing anything) decline to the pre-existing path on any unmet condition, -/// so the flip only ever replaces a replay that would have produced the same -/// state. `=0`/`false` opts back out. -fn single_frame_blackhole_resume_enabled() -> bool { - static ENABLED: std::sync::OnceLock = std::sync::OnceLock::new(); - *ENABLED.get_or_init(|| match std::env::var_os("PYRE_FBW_BLACKHOLE_RESUME") { - Some(v) => { - let v = v.to_string_lossy(); - v != "0" && !v.eq_ignore_ascii_case("false") - } - None => true, - }) -} - -/// Opt-in for adopting the MULTI-frame (inlined sub-walk) blackhole image. -/// The build side (`build_multi_frame_miframe`, the input-arg `_resref` seed, -/// and the getfield-chain `recover_ref_value`) reconstructs the frame stack -/// correctly, but the resume side (`drive_multi_frame_blackhole` → -/// `convert_and_run_from_pyjitpl`) does not yet materialize an OUTER frame's -/// locals into its live frame before the innermost frame re-reads them (an -/// inner `sys._getframe(1).f_locals[...]` runs first in the blackhole chain and -/// sees the not-yet-restored caller frame). Until that materialization lands, -/// keep the multi-frame image from being latched so an inline-sub-walk force -/// declines to the legacy escape/replay path (correct output) instead of -/// resuming into an incomplete caller frame. -fn multi_frame_blackhole_resume_enabled() -> bool { - std::env::var_os("PYRE_FBW_MULTIFRAME").as_deref() == Some(std::ffi::OsStr::new("1")) -} - fn build_single_frame_miframe( ctx: &WalkContext<'_, '_, Sym>, jitcode: std::sync::Arc, @@ -2460,10 +2424,9 @@ pub(crate) fn try_execute_residual_call_via_executor( } if fbw_debug_abort_enabled() && ctx.fbw_mode.inline_subwalk { eprintln!( - "[s2-gate] inline_subwalk fs={} flag={} writes_live={} odo_unchanged={} \ + "[s2-gate] inline_subwalk fs={} writes_live={} odo_unchanged={} \ committed_none={} not_bridge={} bh_result_some={} sym_nonnull={}", ctx.session.borrow().framestack.len(), - single_frame_blackhole_resume_enabled(), writes_live_heap, odometer_unchanged, committed_frame_escape_pc().is_none(), @@ -2472,8 +2435,18 @@ pub(crate) fn try_execute_residual_call_via_executor( !ctx.fbw_mode.snapshot_sym.is_null(), ); } - if single_frame_blackhole_resume_enabled() - && writes_live_heap + // A top-level one-frame walk whose residual forced the vable and + // writes live heap resumes PAST the escaping opcode through the + // blackhole instead of falling back to escape/replay. Both the + // latch (`writes_live_heap`, odometer unchanged, non-bridge, empty + // framestack, no committed escape pc, resolvable snapshot sym) and + // the adopt (`try_adopt_single_frame_blackhole` → + // `apply_single_frame_blackhole_crn`, which validates every mapped + // color and every live operand-stack slot before writing anything) + // decline to the pre-existing path on any unmet condition, so this + // only ever replaces a replay that would have produced the same + // state. + if writes_live_heap && odometer_unchanged && !ctx.trace_ctx.is_bridge_trace && let Some((resume_pc, result_bank, result_color)) = blackhole_result @@ -2507,8 +2480,31 @@ pub(crate) fn try_execute_residual_call_via_executor( }); }); } + // An inlined sub-walk adopts the multi-frame blackhole image. + // The build side (`build_multi_frame_miframe`, the input-arg + // `_resref` seed, and the getfield-chain `recover_ref_value`) + // reconstructs the frame stack; the resume side + // (`drive_multi_frame_blackhole` → + // `convert_and_run_from_pyjitpl`) publishes each level as the + // chain reaches it. The blast radius is exactly + // `inline_subwalk` at a vable escape: this is an `if`/`else + // if`, and the single-frame arm requires + // `framestack.is_empty() && !inline_subwalk`. + // + // This path was once gated because the walker executes + // residuals concretely while an inline push does not run the + // interpreter's call sequence. `ec.topframeref` therefore + // named the CALLER while an inlined callee body ran, so a + // `sys._getframe` that was itself the escaping residual read + // the wrong frame at walk time. Adopting committed that answer + // where legacy escape/replay discarded it. A `sys._getframe` + // executed later, inside the blackhole, was always correct. + // `walker_ec_enter` / `walker_ec_leave` (the port of + // `executioncontext.py:85-107`) publish the callee frame at + // the inlined-call push, which closed the gap. + // `synth/getframe_while_escaping_read_frame_identity` is the + // regression guard. } else if ctx.fbw_mode.inline_subwalk - && multi_frame_blackhole_resume_enabled() && let Some(framestack) = build_multi_frame_miframe( ctx, resume_pc, diff --git a/pyre/pyre-jit-trace/src/trace.rs b/pyre/pyre-jit-trace/src/trace.rs index 86f36c9eac5..b393080c7e1 100644 --- a/pyre/pyre-jit-trace/src/trace.rs +++ b/pyre/pyre-jit-trace/src/trace.rs @@ -2481,18 +2481,17 @@ fn try_adopt_multi_frame_blackhole( // unconditionally, and its `frames[0]` is always the portal frame, so this // shape cannot arise there. // - // Passing this gate is NOT sufficient for the adopt to be right, and the - // remaining gap is why `PYRE_FBW_MULTIFRAME` is still opt-in. The walker - // executes residuals CONCRETELY while an inline push never runs the - // interpreter's call sequence, so `ec.topframeref` still names the CALLER - // while an inlined callee body runs. A `sys._getframe` that is itself the - // escaping residual therefore already read the wrong frame at walk time, - // and adopting commits that answer where the legacy escape/replay path - // discards it — measured as one wrong iteration per adopt - // (`synth/getframe_while_escaping_read_frame_identity`). Closing it needs - // the inlined-call push to publish the callee frame on the execution - // context; a `sys._getframe` executed later, inside the blackhole, is + // Passing this gate is not by itself sufficient for the adopt to be right. + // The walker executes residuals CONCRETELY while an inline push never runs + // the interpreter's call sequence, so `ec.topframeref` used to still name + // the CALLER while an inlined callee body ran: a `sys._getframe` that is + // itself the escaping residual read the wrong frame at walk time, and + // adopting committed that answer where the legacy escape/replay path + // discarded it. `walker_ec_enter` / `walker_ec_leave` publish the callee + // frame on the execution context at the inlined-call push, which closes + // that hole; a `sys._getframe` executed later, inside the blackhole, was // already correct because each level is published as it runs. + // `synth/getframe_while_escaping_read_frame_identity` guards both readings. mfdbg!( "chain root={root_addr:#x} cf_addr={cf_addr:#x} levels=[{}]", per_frame