Skip to content
Merged
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
bridges_compiled=6
bridges_compiled=5
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_rolled_back_with_effects=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=686
guard_failures=647
internal_compile_panics=0
loops_aborted=0
loops_compiled=5
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
bridges_compiled=6
bridges_compiled=5
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_rolled_back_with_effects=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=686
guard_failures=647
internal_compile_panics=0
loops_aborted=0
loops_compiled=5
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
bridges_compiled=6
bridges_compiled=5
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_rolled_back_with_effects=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=686
guard_failures=647
internal_compile_panics=0
loops_aborted=0
loops_compiled=5
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
bridges_compiled=5
bridges_compiled=4
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_rolled_back_with_effects=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=1009
guard_failures=809
internal_compile_panics=0
loops_aborted=0
loops_compiled=4
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
bridges_compiled=5
bridges_compiled=4
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_rolled_back_with_effects=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=1009
guard_failures=809
internal_compile_panics=0
loops_aborted=0
loops_compiled=4
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
bridges_compiled=5
bridges_compiled=4
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_rolled_back_with_effects=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=1009
guard_failures=809
internal_compile_panics=0
loops_aborted=0
loops_compiled=4
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ bridges_compiled=3
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_blackhole_adopted_multi_frame=0
fbw_blackhole_adopted_single_frame=3
fbw_rolled_back_with_effects=0
fbw_store_journal_rollback_failed=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=600
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
bridges_compiled=6
bridges_compiled=5
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_rolled_back_with_effects=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=1562
guard_failures=1345
internal_compile_panics=0
loops_aborted=1
loops_compiled=12
4 changes: 2 additions & 2 deletions pyre/bench/synth/list_append_write_barrier_gc.dynasm.jitstats
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
bridges_compiled=6
bridges_compiled=5
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_rolled_back_with_effects=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=1562
guard_failures=1345
internal_compile_panics=0
loops_aborted=1
loops_compiled=12
4 changes: 2 additions & 2 deletions pyre/bench/synth/list_append_write_barrier_gc.wasm.jitstats
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
bridges_compiled=6
bridges_compiled=5
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_rolled_back_with_effects=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=1562
guard_failures=1345
internal_compile_panics=0
loops_aborted=1
loops_compiled=12
53 changes: 34 additions & 19 deletions pyre/check.py
Original file line number Diff line number Diff line change
Expand Up @@ -1891,21 +1891,36 @@ def _performance_gate_passed(
def failed_bound(measured, baseline_value):
exec_measured = self._exec_time(backend, measured)
exec_baseline = self._exec_time(baseline_key, baseline_value)
# A perf ratio is dominated by timer resolution only when BOTH
# sides are: a baseline pinned to EXEC_TIME_FLOOR_S by startup
# subtraction over-estimates its real (sub-floor) work, so the
# reported ratio measured/floor is a LOWER bound on the true ratio.
# Exempting on a clamped baseline alone would therefore hide a
# provable slowdown -- if measured/floor already clears the gate,
# the true ratio clears it by even more. Require the backend to be
# at the floor too, so the exemption fires only when neither side
# has measurable work; a backend above the floor is a real
# measurement the gate still applies to (this is the actual-clamping
# test the 100ms absolute threshold failed to be).
if (
self._baseline_exec_time_clamped(baseline_key, baseline_value)
and exec_measured <= EXEC_TIME_FLOOR_S
):
# A baseline pinned to EXEC_TIME_FLOOR_S by startup subtraction is
# not a measurement of the baseline, it is the floor constant. The
# ratio built on it is `exec_measured / EXEC_TIME_FLOOR_S`, so the
# recorded ceiling it is compared against is an absolute wall-clock
# budget of `limit * EXEC_TIME_FLOOR_S` seconds -- and that budget
# was fitted on whichever host last wrote the header. Applying it
# elsewhere compares two hosts' wall clocks with no baseline
# standing between them, which is what the printed line already
# says ("ratio not a measurement") while failing the run on it.
# Measured: `class_reassign_hot` read 27.0x and 27.3x on one host
# and 49.2x on a CI runner against the same code, because only the
# numerator moves.
#
# The earlier reading kept the ceiling armed here because
# measured/floor is a LOWER bound on the true ratio, so a failure
# does prove the fixture is at least that many times slower than
# pypy. It is still not a bound this ceiling can judge: the
# recorded number carries the same clamp, so no pypy measurement
# enters the comparison on either side. Three consecutive `main`
# runs failed exactly this way on three different fixtures and two
# runners -- global_cell_shortpreamble_hot 24.1x > 19x,
# class_reassign_hot 49.2x > 47x, reentrant_key_eq_mutation 10.3x >
# 5x -- which is a population, not three regressions.
#
# Only the ceiling relaxes: the floor declines to arm below
# FLOOR_GATE_MIN_BASELINE_S for the neighbouring reason, which a
# clamped baseline is always under. The number stays visible either
# way -- the comparison table prints it with a `~`. A fixture that
# wants a ratio gate has to give pypy enough work to measure.
if self._baseline_exec_time_clamped(baseline_key, baseline_value):
return None
if exec_measured > exec_baseline * limit + compare_buffer:
return "ceiling"
Expand Down Expand Up @@ -1953,7 +1968,6 @@ def _gate_fail_detail(
ratio = "-"
else:
ratio = f"{float(exec_m) / float(exec_b):.1f}x"
clamped = self._baseline_exec_time_clamped(baseline, baseline_time)
if bound == "floor":
detail = (
f"exec {exec_m:.2f}s vs {baseline} {exec_b:.2f}s "
Expand All @@ -1965,8 +1979,6 @@ def _gate_fail_detail(
f"exec {exec_m:.2f}s > {baseline} {exec_b:.2f}s "
f"ratio {ratio} > gate {float(limit):g}x"
)
if clamped:
detail += f" [{baseline} exec clamped to floor; ratio not a measurement]"
return detail

def _run_backend_bench(
Expand Down Expand Up @@ -2420,7 +2432,10 @@ def print_comparison_table(self):
header += "".join(f" {b:>18s}" for b in cols)
print(header)
if any("~" in c[b] for c in self.comparisons for b in cols):
print(" ~ pypy exec clamped to floor; ratio is not a measurement")
print(
" ~ pypy exec clamped to floor; ratio is not a measurement, "
"and no ratio gate is applied to it"
)
print(" " + "─" * (54 + 19 * len(cols)))
for c in self.comparisons:
row = f" {c['name']:<35s} {c['cpython']:>8s} {c['pypy']:>8s}"
Expand Down
9 changes: 7 additions & 2 deletions pyre/pyre-interpreter/src/module/posix/interp_posix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3326,8 +3326,13 @@ pub fn register_module(ns: pyre_object::PyObjectRef) {
None => default_follow,
};
// interp_posix.py:634-644 `do_stat` tests the descriptor first: with one
// in hand neither other argument has anything to apply to, and both
// rejections precede the platform's dir_fd availability.
// in hand neither other argument has anything to apply to. Only the
// `follow_symlinks` rejection precedes the platform's dir_fd
// availability, though — `_DirFD_Unavailable` (`interp_posix.py:285-292`,
// the `!HAVE_FSTATAT` arm above) turns the argument away while
// unwrapping it, a step earlier than this, so where `fstatat` does not
// exist a descriptor passed with `dir_fd` reports the platform rather
// than the conflict.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
if path.as_fd != -1 {
if dir_fd.is_some() {
// 3.14 words this "can't specify dir_fd without matching
Expand Down
44 changes: 30 additions & 14 deletions pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,9 +277,12 @@ fn capture_vstack_mirror_image<Sym: WalkSym>(
}
other => {
latchdbg!(
"origin={origin} mirror-slot {}/{} unresolved opref={opref:?} concrete={other:?}",
"origin={origin} mirror-slot {}/{} unresolved opref={opref:?} \
concrete={other:?} pypc={} boxes={:?}",
slots.len(),
ctx.vstack_boxes.len(),
ctx.vstack_cur_pypc,
ctx.vstack_boxes,
);
return None;
}
Expand Down Expand Up @@ -4746,6 +4749,17 @@ pub(crate) fn dispatch_residual_call_iRd_kind<Sym: WalkSym>(
// Without it the abort lands in the `Generic` catch-all and the
// `abort: force quasi-immut` counter stays at 0.
crate::state::note_force_quasi_immut_abort();
// Stamp the abort coordinate at the raise point, the way the two
// kept-stack branch-guard raises do, so the flush gate cannot observe
// an UNRELATED prior abort. `abort_in_subwalk` is sticky for the whole
// trace attempt (`claim_abort_coordinate` only ever sets it), so an
// earlier inline sub-walk abort the walk RECOVERED from — the attempt
// discarded, the call residualized, the walk continued — leaves it true
// for every later abort. `flush_qmut_abort_state`'s gate then declines
// a root-frame qmut abort as though its pc named a callee jitcode, and
// the legacy replay re-runs the region on top of the residuals the walk
// already executed.
ctx.session.borrow_mut().abort_in_subwalk = ctx.fbw_mode.inline_subwalk;

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 Reset the abort coordinate at the recovery boundary

When an inline subwalk abort is caught and residualized, abort_in_subwalk remains true, as the new comment acknowledges. Overwriting it only in this namespace-qmut path fixes that one follow-up abort, but a later root-frame UnsupportedOpname, OrthodoxSubWalkTraceUnsupported, or another complete-image error still reaches claim_abort_coordinate(false) with the stale value, skips latch_abort_blackhole, and can fall back to entry replay after residual effects have run. Clear or scope the flag where the subwalk is recovered rather than patching one subsequent raise site.

AGENTS.md reference: AGENTS.md:L252-L254

Useful? React with 👍 / 👎.

return Err(DispatchError::ForceQuasiImmutable { pc: op.pc });
}

Expand Down Expand Up @@ -5870,19 +5884,21 @@ pub(crate) fn dispatch_residual_call_iIRd_kind<Sym: WalkSym>(
// `GUARD_NOT_INVALIDATED`, so a rebind or a delete fails the loop instead of
// reaching a NameError, and a successful fold provably cannot raise.
//
// The gate is load-bearing because the fold INSTALLS the module dict's
// `version?` watcher, and that watcher is what makes a later bumping write
// in the same program abandon the walk with `ForceQuasiImmutable`. That
// abort resumes mid-expression off a latched operand mirror, and when one
// latched slot is unbound the flush declines to the legacy replay, which
// then REFUSES to re-deliver an in-flight FOR_ITER item once a body effect
// has committed — the iteration is dropped and its accumulator increment is
// silently lost. Lifting the gate to `DELETE_NAME`/`DELETE_GLOBAL` only
// (the implicit `del e` an `except X as e:` emits) reaches exactly that:
// `bench/synth/pickle_terminal_raise_resume` then prints 214 under the JIT
// against 216 interpreted, off one dropped iteration. Both halves of that
// chain — the unbound mirror slot and the silent drop the decline falls
// back to — have to be closed before the handler shape stops standing in.
// The gate stands on compile behaviour, not on correctness. The fold
// INSTALLS the module dict's `version?` watcher, and that watcher is what
// makes a later bumping write in the same program abandon the walk with
// `ForceQuasiImmutable` — so lifting the gate multiplies those aborts in
// handler-bearing module bodies. The three wrong-code shapes that used to
// ride on that (a dropped FOR_ITER iteration in
// `pickle_terminal_raise_resume`, and the two double-applies in
// `iter57/real_exception` and `exception_reentry_guard_finally_residual`)
// are closed: the first no longer reproduces, and the other two were the
// stale `abort_in_subwalk` this file now stamps at its own qmut raise plus
// the live-NULL mirror slot `reseed_vstack_from_shadow` now accepts. With
// the gate lifted the whole `bench/synth` corpus is output-correct; what
// still fails is `exception_reraise_tb_depth_jitstress` at 13.0x against
// its 4x pypy gate, and four benches' jit-stats move (most visibly
// `exception_reraise_tb_depth_hot`, `loops_aborted 0 -> 63`).
//
// The scan is whole-body, so one `try` anywhere in a module also charges
// every name access in it a live dict lookup (~83ns each, linear in the
Expand Down
32 changes: 30 additions & 2 deletions pyre/pyre-jit-trace/src/jitcode_dispatch/vstack_mirror.rs
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,18 @@ fn reseed_vstack_from_callee_shadow<Sym: WalkSym>(
if *slot != OpRef::NONE {
continue;
}
// The NULL const-ptr rejection here is stricter than the shadow can
// justify: `opref` is a sparse map, so a PRESENT key is already the
// proof that this walk wrote the slot, and a present key holding
// CONST_NULL is a deliberately written NULL — PUSH_NULL's `self_or_null`
// ahead of a call inside the inlined callee. `reseed_vstack_from_shadow`
// needed a per-slot live-NULL side table to draw that same distinction
// only because its source is a dense array, where absent and NULL are
// the same word. Kept as-is regardless: dropping the clause leaves the
// whole dynasm corpus at 386/386 with NO jit-stats movement, so nothing
// measures the difference, and an unwitnessed widening of what counts as
// a resolved mirror slot is the direction that turns a decline into a
// wrong answer.
match shadow.opref.get(&((stack_base + s) as i64)).copied() {
Some(value) if value != OpRef::NONE && !opref_is_null_const_ptr(value) => {
*slot = value;
Expand Down Expand Up @@ -725,6 +737,17 @@ pub(crate) fn reseed_vstack_from_shadow<Sym: WalkSym>(
// portal never wrote through) fails the whole re-seed so the caller
// leaves the slot NONE; `stack_sync` then omits it (resume
// re-materializes).
//
// A NULL const-ptr is rejected because it cannot be told apart from a slot
// the portal never wrote — EXCEPT where the slot carries the live-NULL
// marker, which says the last executed store into it wrote a NULL on
// purpose. That is PUSH_NULL's `self_or_null` sentinel: it stays live
// across the whole callable/args/kwargs build ahead of a CALL, and the
// reorder region reseeds the mirror from the shadow in the middle of that
// build. Rejecting it left slot NONE, `capture_vstack_mirror_image`
// refuses an image with any unresolved slot, and an escape inside the call
// then had no blackhole image at all and fell back to the legacy entry
// replay — which re-runs the residuals the walk already executed.
if ctx.vstack_boxes.len() < new_depth {
ctx.vstack_boxes.resize(new_depth, OpRef::NONE);
}
Expand All @@ -733,8 +756,13 @@ pub(crate) fn reseed_vstack_from_shadow<Sym: WalkSym>(
if ctx.vstack_boxes[s] != OpRef::NONE {
continue;
}
match ctx.trace_ctx.virtualizable_box_at(nvs + nlocals + s) {
Some(b) if b != OpRef::NONE && !opref_is_null_const_ptr(b) => {
let flat = nvs + nlocals + s;
match ctx.trace_ctx.virtualizable_box_at(flat) {
Some(b)
if b != OpRef::NONE
&& (!opref_is_null_const_ptr(b)
|| ctx.trace_ctx.virtualizable_slot_stored_live_null(flat)) =>
{
ctx.vstack_boxes[s] = b;
}
// Fill what we can; an unsourceable hole (NONE / NULL const-ptr —
Expand Down
Loading
Loading