check: gate a schedule-sensitive jit-stats counter with a declared band

pyre/bench/synth/generator_tree_recursion.py

@@ -1,4 +1,13 @@
 # pyre-check: max-pypy-ratio=7.6
+# pyre-check: jitstats-band=guard_failures=8
+# Jitcounter decay is 0.96 every 32 minor collections
+# (majit-trace/src/counter.rs), so guard_failures tracks collection count during
+# each guard's warm-up rather than a compile decision. One host measured
+# 2951..2958 across nursery sizes; PYRE_JIT=decay=0 pinned 2999 everywhere,
+# while loops_compiled=3 and bridges_compiled=26 stayed invariant and remain
+# gated exactly. Width 8 adds one count of margin (0.27%); real regressions this
+# gate caught moved by hundreds to thousands (828 -> 4923, 404 -> 812,
+# 937 -> 7408).
 # Generator-driven accumulation over recursive tree/linear results. The
 # tree_sum recursion once silently miscompiled on cranelift (first checksum
 # already wrong) and recovered a regalloc panic on dynasm. Deterministic;

pyre/check.py

@@ -1202,14 +1202,20 @@ def _parse_jit_stats(snapshot):
 # regressions, so the ambiguous case is the one a human is asked to look at.
 
 
-def _jit_stats_change(saved, current):
-    """Return `(regressions, improvements)`, each a list of "field a -> b".
+def _jit_stats_change(saved, current, bands=None):
+    """Return `(regressions, improvements, banded)` change descriptions.
 
     Both directions gate: a rise means the JIT started aborting traces, hitting
     internal compile panics or failing guards it did not before, and a fall
     means it stopped compiling something it used to compile. Neither may pass
     unrecorded — see the surface comment above.
 
+    A band reclassifies an integer-valued move at or below its symmetric width,
+    whether that move would be a regression or an improvement. The move is
+    reported in `banded` rather than hidden. A band cannot reclassify a
+    non-integer value or a move outside the band, and the fixture parser rejects
+    bands on badness counters whose healthy value must remain exactly zero.
+
     A field missing from either side reads as "0", so a baseline recorded before
     a counter existed still matches a run that reports it as 0, and adding an
     invariant counter costs no re-record. The cost of that convenience is that a
@@ -1218,26 +1224,34 @@ def _jit_stats_change(saved, current):
     one of its baselines, silently. Whenever a backend's line changes shape,
     re-record its whole baseline surface rather than trusting the run that
     follows."""
+    bands = bands or {}
     old_fields = _parse_jit_stats(saved)
     new_fields = _parse_jit_stats(current)
-    regressions, improvements = [], []
+    regressions, improvements, banded = [], [], []
     for field in JITSTATS_SNAPSHOT_FIELDS:
         old, new = old_fields.get(field, "0"), new_fields.get(field, "0")
         if old == new:
             continue
         try:
-            rose = int(new) > int(old)
+            old_int, new_int = int(old), int(new)
         except ValueError:
             # A counter that stopped being an integer is not a gain.
             rose = None
+        else:
+            if field in bands and abs(new_int - old_int) <= bands[field]:
+                banded.append(
+                    f"{field} {old} -> {new} (within band {bands[field]})"
+                )
+                continue
+            rose = new_int > old_int
         if rose is not None and (
             (rose and field in JITSTATS_REGRESSION_ON_FALL)
             or (not rose and field in JITSTATS_REGRESSION_ON_RISE)
         ):
             improvements.append(f"{field} {old} -> {new}")
         else:
             regressions.append(f"{field} {old} -> {new}")
-    return regressions, improvements
+    return regressions, improvements, banded
 
 
 def _jit_stats_vacuous(stderr):
@@ -1471,6 +1485,66 @@ def synth_skip_backends(path):
     return ()
 
 
+def synth_jitstats_bands(path):
+    """Read an optional per-fixture jit-stats band from its header:
+        # pyre-check: jitstats-band=guard_failures=8
+
+    The band is symmetric around the recorded baseline and absorbs moves in
+    both directions: a delta within the band is not signal either way. An
+    absorbed move is reported on that run rather than swallowed, so the
+    allowance is visible whenever it is used and not only in the fixture
+    header. It is only for schedule-sensitive counters; badness counters must
+    remain exactly zero. The directive must be followed by a comment describing
+    the measured variance, so the allowance is reviewable next to the workload.
+    Unknown or duplicate fields, repeated directives, and non-positive or
+    non-integer widths are errors rather than silent no-ops. A directive below
+    the 20-line window is simply not read, so the fixture gates normally in
+    that case.
+    """
+    prefix = "# pyre-check: jitstats-band="
+    bands = None
+    with open(path, encoding="utf-8") as source:
+        for _ in range(20):
+            line = source.readline()
+            if not line:
+                break
+            if not line.startswith(prefix):
+                continue
+            if bands is not None:
+                raise ValueError(f"duplicate jit-stats band directive in {path}: {line.strip()}")
+            bands = {}
+            entries = line[len(prefix):].strip().split(",")
+            for entry in entries:
+                parts = entry.split("=")
+                if len(parts) != 2 or not all(part.strip() for part in parts):
+                    raise ValueError(f"invalid jit-stats band in {path}: {line.strip()}")
+                field, raw_width = (part.strip() for part in parts)
+                if field not in JITSTATS_SNAPSHOT_FIELDS:
+                    raise ValueError(
+                        f"unknown jit-stats band field {field!r} in {path}: {line.strip()}"
+                    )
+                if field in JITSTATS_BADNESS_FIELDS:
+                    raise ValueError(
+                        f"badness counter cannot be banded in {path}: {line.strip()}"
+                    )
+                if field in bands:
+                    raise ValueError(
+                        f"duplicate jit-stats band field {field!r} in {path}: {line.strip()}"
+                    )
+                try:
+                    width = int(raw_width)
+                except ValueError as e:
+                    raise ValueError(
+                        f"invalid jit-stats band width in {path}: {line.strip()}"
+                    ) from e
+                if width <= 0:
+                    raise ValueError(
+                        f"jit-stats band width must be positive in {path}: {line.strip()}"
+                    )
+                bands[field] = width
+    return bands if bands is not None else {}
+
+
 def _synth_header_flag(path, directive, malformed):
     """Read a valueless per-fixture header flag from the first 20 lines.
 
@@ -1690,6 +1764,9 @@ def __init__(self, args):
         # Benches whose counters did not reproduce across repeats in this same
         # invocation. Reported, never failed — see JITSTATS_STABILITY_RUNS.
         self.jitstats_unstable = []
+        # Benches whose gated counters moved only inside a declared per-fixture
+        # band. Reported, never failed.
+        self.jitstats_banded = []
         self.jitstats_missing = []
         # Benches whose run printed no `[jit-stats]` line at all. Tracked apart
         # from `jitstats_missing` because the two say opposite things: a missing
@@ -1927,6 +2004,7 @@ def _apply_snapshot_gate(
         time_path = self._snapshot_path(backend, name, "time")
         jitstats_path = self._jitstats_baseline_path(backend, script)
         jitstats = _jit_stats_snapshot(stderr)
+        jitstats_bands = synth_jitstats_bands(script)
 
         # The jit-stats gate — enforced on EVERY run, so a structural JIT change
         # reddens the default `pyre/check.py` (locally, and in the bare CI
@@ -1973,15 +2051,16 @@ def _apply_snapshot_gate(
             if vacuous:
                 self.jitstats_vacuous.append(f"{backend}/{name}")
                 return "fail", vacuous
-            regressions, improvements = _jit_stats_change(
-                jitstats_path.read_text(encoding="utf-8"), jitstats
+            regressions, improvements, banded = _jit_stats_change(
+                jitstats_path.read_text(encoding="utf-8"), jitstats, jitstats_bands
             )
             if regressions or improvements:
                 repeats = self._jitstats_repeats(backend, script, timeout)
                 drifted = next(
                     (s for s in repeats or () if s != jitstats), None
                 )
                 if drifted is not None:
+                    # Drift compares two samples directly and is deliberately unbanded.
                     moved = _jit_stats_change(jitstats, drifted)
                     self.jitstats_unstable.append(f"{backend}/{name}")
                     return "unstable", (
@@ -1997,6 +2076,8 @@ def _apply_snapshot_gate(
                     parts.append("regressed: " + ", ".join(regressions))
                 if improvements:
                     parts.append("improved: " + ", ".join(improvements))
+                if banded:
+                    parts.append("within band: " + ", ".join(banded))
                 reason = (
                     "jit-stats change — " + "; ".join(parts)
                     + _jit_stats_context(jitstats)
@@ -2008,6 +2089,17 @@ def _apply_snapshot_gate(
                     return "regressed", reason
                 self.jitstats_improvements.append(f"{backend}/{name}")
                 return "improved", reason
+            if banded:
+                # Assigned, not returned: every other verdict above is a
+                # failure, so leaving early costs those nothing. A band means
+                # the run is fine, and returning here would carry the fixture
+                # past the `--snapshot-diff` and `--threshold` gates below on
+                # every run of a host that sits inside its band.
+                self.jitstats_banded.append(f"{backend}/{name}")
+                status, reason = "banded", (
+                    "jit-stats within band — " + ", ".join(banded)
+                    + _jit_stats_context(jitstats)
+                )
 
         if self.args.snapshot_mode == "record":
             out_path.parent.mkdir(parents=True, exist_ok=True)
@@ -2681,10 +2773,11 @@ def _ratio(elapsed_val, pypy_val):
             backend, name, script, output, stderr, elapsed, timeout,
         )
         if snap_status != "ok":
-            if snap_status == "unstable":
-                # Warned, not failed: the counter did not reproduce itself in
-                # this invocation, so there is nothing to gate on.
-                unstable_line = f"{yellow('UNSTABLE')}  {snap_reason}"
+            if snap_status in ("unstable", "banded"):
+                # Warned, not failed: unstable counters cannot be gated, while
+                # banded counters stayed inside their declared allowance.
+                note_label = snap_status.upper()
+                note_line = f"{yellow(note_label)}  {snap_reason}"
             else:
                 # `improved` is still a failure; the label only tells the
                 # reader whether to investigate or just re-record.
@@ -2693,9 +2786,9 @@ def _ratio(elapsed_val, pypy_val):
                 failures.append(
                     (f"{paint(label)}  {snap_reason}", snap_reason, label, None)
                 )
-                unstable_line = None
+                note_line = None
         else:
-            unstable_line = None
+            note_line = None
 
         if failures:
             self._record(
@@ -2704,19 +2797,19 @@ def _ratio(elapsed_val, pypy_val):
             )
             for index, (line, _, _, _) in enumerate(failures):
                 print(f"{' ' * 14 if index else ''}{line}")
-            if unstable_line is not None:
-                print(f"{' ' * 14}{unstable_line}")
+            if note_line is not None:
+                print(f"{' ' * 14}{note_line}")
             _, _, comparison_cell, comparison_note = failures[0]
             self._append_comparison(
                 backend, name, t_cpython, t_pypy,
                 comparison_cell, comparison_note,
             )
             return
 
-        if unstable_line is not None:
+        if note_line is not None:
             self._record(backend, True, name, f"{elapsed:.2f}s")
-            print(unstable_line)
-            self._append_comparison(backend, name, t_cpython, t_pypy, "UNSTABLE")
+            print(note_line)
+            self._append_comparison(backend, name, t_cpython, t_pypy, note_label)
             return
 
         self._record(backend, True, name, f"{elapsed:.2f}s")
@@ -3188,6 +3281,7 @@ def print_summary(self):
                 (red, "jit-stats regressed", self.jitstats_diffs),
                 (yellow, "jit-stats improved (re-record)", self.jitstats_improvements),
                 (yellow, "jit-stats unstable (not gated)", self.jitstats_unstable),
+                (yellow, "jit-stats within band (not gated)", self.jitstats_banded),
                 (red, "jit-stats baseline missing", self.jitstats_missing),
                 (red, "jit-stats line absent", self.jitstats_absent),
                 (red, "jit-stats census vacuous", self.jitstats_vacuous),