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

majit/majit-metainterp/src/pyjitpl.rs

@@ -5204,18 +5204,22 @@ impl<M: Clone> MetaInterp<M> {
         let outermost_merge_key = ctx.current_merge_points_first_greenkey();
         // pyjitpl.py:2793: find_biggest_function — if an inlined function
         // caused the bloat, disable just that function.
-        let huge_fn_key = self.find_biggest_function();
+        let huge_fn = self.find_biggest_function();
         // pyjitpl.py:2795: `self.portal_trace_positions = None` marks the
         // abort boundary so post-abort consumers (e.g. test inspections
         // at pyjitpl.py:3547) can detect a terminated trace session.
         self.portal_trace_positions = None;
-        if let Some(huge_fn_key) = huge_fn_key {
+        if let Some((huge_fn_jd_no, huge_fn_key)) = huge_fn {
+            // pyjitpl.py:2822 disables through `jd_sd.warmstate`, the warmstate
+            // of the driver that owns the oversized frame.  Pyre keeps one
+            // `WarmEnterState` on the MetaInterp rather than one per
+            // `JitDriverStaticData`, so the disable lands on that single state;
+            // the owning index is still carried through below.
             self.warm_state.disable_noninlinable_function(huge_fn_key);
             // pyjitpl.py:2799-2800: stash the aborted jd_sd + greenkey so
             // `aborted_tracing(reason)` can fire `on_trace_too_long` when
-            // the hook is ported.  Pyre only registers a single jitdriver
-            // so jd_sd.index is always 0.
-            self.aborted_tracing_jitdriver = Some(0);
+            // the hook is ported.
+            self.aborted_tracing_jitdriver = Some(huge_fn_jd_no);
             self.aborted_tracing_greenkey = Some(huge_fn_key);
             // pyjitpl.py:2801-2804: only boost retrace for the outermost
             // loop (when `current_merge_points` is non-empty).  Bridge
@@ -14208,41 +14212,50 @@ impl<M: Clone> MetaInterp<M> {
     /// `size` is the distance between two `TracePosition::_pos` cursors, the
     /// `pos[0]` upstream subtracts (opencoder.py:475).
     ///
-    /// Returns the green key of the largest frame, or `None` when the log
+    /// Returns the owning jitdriver's index and the green key of the largest
+    /// frame — `max_jdsd, max_key` upstream, which the caller needs both of
+    /// (`pyjitpl.py:2821-2824` disables through the frame's OWN driver and
+    /// stashes that driver in `aborted_tracing_jitdriver`). `None` when the log
     /// holds no closed or open portal frame — the root frame is created by
     /// `initialize_state_from_start` without a greenkey and never enters the
     /// log, so a trace that inlined nothing answers `None` and the caller
     /// falls through to `prepare_trace_segmenting`.
-    pub fn find_biggest_function(&self) -> Option<u64> {
+    pub fn find_biggest_function(&self) -> Option<(usize, u64)> {
         let positions = self.portal_trace_positions.as_ref()?;
-        let mut start_stack: Vec<(u64, usize)> = Vec::new();
+        let mut start_stack: Vec<(usize, u64, usize)> = Vec::new();
         let mut max_size = 0usize;
         let mut max_key = None;
-        for &(_jd_no, key, pos) in positions {
+        for &(jd_no, key, pos) in positions {
             match key {
                 // pyjitpl.py:3547-3548 `if key is not None: start_stack.append`.
-                Some(key) => start_stack.push((key, pos._pos)),
+                Some(key) => start_stack.push((jd_no, key, pos._pos)),
                 // pyjitpl.py:3549-3559 the closing entry sizes the frame it
                 // closes. An unmatched close cannot happen while `newframe` /
                 // `popframe` are the only writers, so it is left to `pop`'s
                 // `None` rather than given a recovery path.
                 None => {
-                    if let Some((green_key, start_pos)) = start_stack.pop() {
+                    if let Some((jd_no, green_key, start_pos)) = start_stack.pop() {
                         let size = pos._pos.saturating_sub(start_pos);
                         if size > max_size {
                             max_size = size;
-                            max_key = Some(green_key);
+                            max_key = Some((jd_no, green_key));
                         }
                     }
                 }
             }
         }
         // pyjitpl.py:3560-3570 `if start_stack:` — one frame, the outermost,
-        // measured against where the trace stopped.
-        if let Some(&(green_key, start_pos)) = start_stack.first() {
-            let current = self.tracing.as_ref()?.get_trace_position()._pos;
+        // measured against where the trace stopped.  Upstream reads
+        // `self.history` there unconditionally; pyre's recorder is an `Option`,
+        // and a `?` on it would return `None` for the whole function and throw
+        // away a `max_key` the closed frames above already produced.  Only the
+        // open frame is unmeasurable without a recorder, so only it is skipped.
+        if let Some(&(jd_no, green_key, start_pos)) = start_stack.first()
+            && let Some(tracing) = self.tracing.as_ref()
+        {
+            let current = tracing.get_trace_position()._pos;
             if current.saturating_sub(start_pos) > max_size {
-                max_key = Some(green_key);
+                max_key = Some((jd_no, green_key));
             }
         }
         max_key
@@ -20478,7 +20491,7 @@ mod metainterp_static_data_tests {
 
         assert_eq!(
             meta.find_biggest_function(),
-            Some(0xa11),
+            Some((0, 0xa11)),
             "the larger frame wins even though both have returned"
         );
     }
@@ -20498,7 +20511,32 @@ mod metainterp_static_data_tests {
         meta.perform_call(jc, &[], Some(0xb22)).unwrap_err();
         record_ops(&mut meta, 5);
 
-        assert_eq!(meta.find_biggest_function(), Some(0xb22));
+        assert_eq!(meta.find_biggest_function(), Some((0, 0xb22)));
+    }
+
+    #[test]
+    fn find_biggest_function_keeps_a_closed_frame_when_the_recorder_is_gone() {
+        // pyjitpl.py:3560-3570 reads `self.history` unconditionally, so a
+        // closed frame's size always survives to the return. pyre's recorder is
+        // an `Option`: an unmatched open entry plus `tracing = None` must skip
+        // only the open frame's measurement, not discard `max_key`.
+        let (mut meta, jc) = meta_with_recursive_portal();
+        start_tracing(&mut meta);
+
+        meta.perform_call(jc.clone(), &[], Some(0xa11)).unwrap_err();
+        record_ops(&mut meta, 5);
+        meta.popframe(true);
+
+        meta.perform_call(jc, &[], Some(0xb22)).unwrap_err();
+        record_ops(&mut meta, 1);
+        // Left open, and the recorder retired under it.
+        meta.tracing = None;
+
+        assert_eq!(
+            meta.find_biggest_function(),
+            Some((0, 0xa11)),
+            "the closed frame's size survives a missing recorder"
+        );
     }
 
     #[test]

pyre/bench/synth/binary_int_overflow_local_resume.cranelift.jitstats

@@ -2,7 +2,10 @@ bridges_compiled=5
 descr_set_absent=0
 descr_set_ambiguous=0
 descr_set_stale_absent=0
+fbw_blackhole_adopted_multi_frame=0
+fbw_blackhole_adopted_single_frame=0
 fbw_rolled_back_with_effects=0
+fbw_store_journal_rollback_failed=0
 field_pos_attached_misplaced=0
 field_pos_spec_misplaced=0
 guard_failures=647

pyre/bench/synth/binary_int_overflow_local_resume.dynasm.jitstats

@@ -2,7 +2,10 @@ bridges_compiled=5
 descr_set_absent=0
 descr_set_ambiguous=0
 descr_set_stale_absent=0
+fbw_blackhole_adopted_multi_frame=0
+fbw_blackhole_adopted_single_frame=0
 fbw_rolled_back_with_effects=0
+fbw_store_journal_rollback_failed=0
 field_pos_attached_misplaced=0
 field_pos_spec_misplaced=0
 guard_failures=647

pyre/bench/synth/binary_int_overflow_local_resume.wasm.jitstats

@@ -2,7 +2,10 @@ bridges_compiled=5
 descr_set_absent=0
 descr_set_ambiguous=0
 descr_set_stale_absent=0
+fbw_blackhole_adopted_multi_frame=0
+fbw_blackhole_adopted_single_frame=0
 fbw_rolled_back_with_effects=0
+fbw_store_journal_rollback_failed=0
 field_pos_attached_misplaced=0
 field_pos_spec_misplaced=0
 guard_failures=647

pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.py

@@ -16,9 +16,10 @@
 #
 # Guard for what an adopted multi-frame blackhole chain owes its inner levels.
 #
-# An inlined callee assigns a local, the frame then escapes through a residual
-# `sys._getframe()`, and an attribute read POSITIONED AFTER that escape reads the
-# local back.  The read is executed by the blackhole, not by the walk, so the
+# An inlined callee assigns a local, the frame then escapes through the
+# `.f_locals` read added above -- `sys._getframe()` itself folds here -- and an
+# attribute read POSITIONED AFTER that escape reads the local back.  The read is
+# executed by the blackhole, not by the walk, so the
 # shape holds the adopt to two separate obligations and fails differently on
 # each:
 #

pyre/bench/synth/exc_bridge_entry_guard_not_removed.cranelift.jitstats

@@ -2,7 +2,10 @@ bridges_compiled=4
 descr_set_absent=0
 descr_set_ambiguous=0
 descr_set_stale_absent=0
+fbw_blackhole_adopted_multi_frame=0
+fbw_blackhole_adopted_single_frame=0
 fbw_rolled_back_with_effects=0
+fbw_store_journal_rollback_failed=0
 field_pos_attached_misplaced=0
 field_pos_spec_misplaced=0
 guard_failures=809

pyre/bench/synth/exc_bridge_entry_guard_not_removed.dynasm.jitstats

@@ -2,7 +2,10 @@ bridges_compiled=4
 descr_set_absent=0
 descr_set_ambiguous=0
 descr_set_stale_absent=0
+fbw_blackhole_adopted_multi_frame=0
+fbw_blackhole_adopted_single_frame=0
 fbw_rolled_back_with_effects=0
+fbw_store_journal_rollback_failed=0
 field_pos_attached_misplaced=0
 field_pos_spec_misplaced=0
 guard_failures=809

pyre/bench/synth/exc_bridge_entry_guard_not_removed.wasm.jitstats

@@ -2,7 +2,10 @@ bridges_compiled=4
 descr_set_absent=0
 descr_set_ambiguous=0
 descr_set_stale_absent=0
+fbw_blackhole_adopted_multi_frame=0
+fbw_blackhole_adopted_single_frame=0
 fbw_rolled_back_with_effects=0
+fbw_store_journal_rollback_failed=0
 field_pos_attached_misplaced=0
 field_pos_spec_misplaced=0
 guard_failures=809

pyre/bench/synth/getframe_caller_resume_coord_two_call_sites.cranelift.jitstats

@@ -0,0 +1,14 @@
+bridges_compiled=0
+descr_set_absent=0
+descr_set_ambiguous=0
+descr_set_stale_absent=0
+fbw_blackhole_adopted_multi_frame=5
+fbw_blackhole_adopted_single_frame=5
+fbw_rolled_back_with_effects=0
+fbw_store_journal_rollback_failed=0
+field_pos_attached_misplaced=0
+field_pos_spec_misplaced=0
+guard_failures=0
+internal_compile_panics=0
+loops_aborted=10
+loops_compiled=0

pyre/bench/synth/getframe_caller_resume_coord_two_call_sites.dynasm.jitstats

@@ -0,0 +1,14 @@
+bridges_compiled=0
+descr_set_absent=0
+descr_set_ambiguous=0
+descr_set_stale_absent=0
+fbw_blackhole_adopted_multi_frame=5
+fbw_blackhole_adopted_single_frame=5
+fbw_rolled_back_with_effects=0
+fbw_store_journal_rollback_failed=0
+field_pos_attached_misplaced=0
+field_pos_spec_misplaced=0
+guard_failures=0
+internal_compile_panics=0
+loops_aborted=10
+loops_compiled=0

pyre/bench/synth/getframe_caller_resume_coord_two_call_sites.py

@@ -0,0 +1,66 @@
+# A callee reads its CALLER's resume coordinate -- `f_lineno` and `f_lasti` --
+# from two DIFFERENT call sites in the same hot loop.
+#
+# `f_lineno` and `f_lasti` both resolve off the frame's `last_instr`, which
+# compiled code does not store per opcode, so the value only reaches the frame
+# if the force publishes it.  A single call site makes that unobservable: the
+# caller's coordinate is then constant by construction and a frozen read is
+# indistinguishable from a live one.  Calling from two lines makes the correct
+# answer ALTERNATE, so a frame left holding one leg's coordinate collapses the
+# two rows into one.
+#
+# The staleness this pins runs in the other direction too: a publish that is
+# withdrawn too early -- put back at the residual's return instead of at walk
+# end -- leaves the caller answering for the coordinate it held BEFORE the
+# call, and the shape set gains an extra row rather than losing one.  Surveying
+# every iteration into a set is what catches both, because the pre-compile
+# iterations are correct and a miss appears as a changed row count.
+#
+# Measured, by putting that defect back in: dropping the `flushed` test from
+# `LiveLastInstrGuard::drop`, so the guard always restores at the residual's
+# return, prints
+#     ([(0, 3), (0, 8), (1, 3), (1, 6)], [0, 0, 1, 1], 3)
+# against this fixture's
+#     ([(0, 8), (1, 6)], [0, 1], 2)
+# -- the pre-call coordinate appears alongside the call-site one on BOTH legs,
+# and the offsets' discrimination count rises with it.  Nothing else in
+# bench/synth reads `f_lasti` at all, and only the traceback-frame fixture
+# reads an `f_lineno`.
+#
+# `f_lasti` is a bytecode offset, so its absolute value is a property of the
+# compiler and cannot be compared against the pypy oracle.  Only its
+# DISCRIMINATION is portable: the two call sites must report two distinct
+# offsets, one per leg.  `f_lineno` is a source coordinate and is compared
+# directly, relative to `co_firstlineno` so edits above these functions do not
+# move the expected values.
+#
+# Neither read goes through `.f_locals`: that getset forces the frame on its
+# own, and a forcing read would mask exactly the staleness under test.
+import sys
+
+N = 30000
+
+
+def inner():
+    f = sys._getframe(1)
+    return (f.f_lineno - f.f_code.co_firstlineno, f.f_lasti)
+
+
+def outer(n):
+    lines = set()
+    offsets = set()
+    i = 0
+    while i < n:
+        if i & 1:
+            line, off = inner()
+        else:
+            line, off = inner()
+        lines.add((i & 1, line))
+        offsets.add((i & 1, off))
+        i = i + 1
+    legs = sorted(leg for leg, _ in offsets)
+    distinct = len({off for _, off in offsets})
+    return (sorted(lines), legs, distinct)
+
+
+print(outer(N))

pyre/bench/synth/getframe_caller_resume_coord_two_call_sites.wasm.jitstats

@@ -0,0 +1,14 @@
+bridges_compiled=0
+descr_set_absent=0
+descr_set_ambiguous=0
+descr_set_stale_absent=0
+fbw_blackhole_adopted_multi_frame=5
+fbw_blackhole_adopted_single_frame=5
+fbw_rolled_back_with_effects=0
+fbw_store_journal_rollback_failed=0
+field_pos_attached_misplaced=0
+field_pos_spec_misplaced=0
+guard_failures=0
+internal_compile_panics=0
+loops_aborted=10
+loops_compiled=0

pyre/bench/synth/handler_tb_frame_locals_after_declined_flush.cranelift.jitstats

@@ -0,0 +1,14 @@
+bridges_compiled=0
+descr_set_absent=0
+descr_set_ambiguous=0
+descr_set_stale_absent=0
+fbw_blackhole_adopted_multi_frame=0
+fbw_blackhole_adopted_single_frame=5
+fbw_rolled_back_with_effects=0
+fbw_store_journal_rollback_failed=0
+field_pos_attached_misplaced=0
+field_pos_spec_misplaced=0
+guard_failures=0
+internal_compile_panics=0
+loops_aborted=5
+loops_compiled=0

pyre/bench/synth/handler_tb_frame_locals_after_declined_flush.dynasm.jitstats

@@ -0,0 +1,14 @@
+bridges_compiled=0
+descr_set_absent=0
+descr_set_ambiguous=0
+descr_set_stale_absent=0
+fbw_blackhole_adopted_multi_frame=0
+fbw_blackhole_adopted_single_frame=5
+fbw_rolled_back_with_effects=0
+fbw_store_journal_rollback_failed=0
+field_pos_attached_misplaced=0
+field_pos_spec_misplaced=0
+guard_failures=0
+internal_compile_panics=0
+loops_aborted=5
+loops_compiled=0

pyre/bench/synth/handler_tb_frame_locals_after_declined_flush.py

@@ -0,0 +1,40 @@
+# A forced frame read from INSIDE a Python expression, so the merge-point
+# escape flush declines and only the locals region is written.
+#
+# `'i' in tb.tb_frame.f_locals` forces the frame while the operand stack holds
+# `[seen, add, <bool being computed>]`.  A mid-expression stack slot reads NULL
+# from the virtualizable shadow, so `flush_walk_end_state_to_frame_inner`
+# declines the whole write and `flush_locals_region_to_frame` writes slots
+# `0..nlocals` on their own -- an unforced array would render `f_locals` as an
+# EMPTY mapping, a wrong answer rather than a stale one.
+#
+# That leg claims no resume pc, so nothing sets `COMMITTED_FRAME_ESCAPE_PC` and
+# the walk-end block gated on it never runs.  Its deferred undo restore has to
+# be armed at the residual instead: `LiveLastInstrGuard` declines to put
+# `last_instr` back while an undo capture is live, so without the arming the
+# frame keeps the EXECUTING pc over an operand stack no flush ever wrote, and
+# the replay re-enters one opcode late on an empty stack -- `value-stack
+# underflow: depth=N base=N`, a JIT-only panic with no output at all.
+#
+# The handler is what puts the force inside an expression whose stack is deep
+# enough to notice: the `seen.add(...)` receiver and its bound method are both
+# live below the value being computed.
+import sys
+
+N = 30000
+
+
+def run():
+    seen = set()
+    i = 0
+    while i < N:
+        try:
+            raise ValueError(i)
+        except ValueError:
+            tb = sys.exc_info()[2]
+            seen.add('i' in tb.tb_frame.f_locals)
+        i = i + 1
+    return sorted(seen)
+
+
+print(run())