From 6db7a15118c8a7799b941ad8464c3fb8ec60083b Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sun, 16 Aug 2026 23:51:21 +0900 Subject: [PATCH 1/3] check: add a per-fixture jit-stats band directive and band generator_tree_recursion's guard_failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `# pyre-check: jitstats-band==` suppresses an integer-valued jit-stats move whose absolute delta is at or below the width, in both directions. Unknown fields, duplicates, non-integer and non-positive widths are errors rather than silent no-ops, and a badness counter cannot be banded because its healthy value is exactly zero. Read from the first 20 lines like the other `# pyre-check:` headers; below that window it is not read and the fixture gates normally. `_apply_snapshot_gate` passes the fixture's bands to both `_jit_stats_change` calls. The second one selects its `drifted` snapshot by comparing text, so a repeat that moved only inside a band would report "jit-stats unstable — re-running the same binary moved" with nothing after it; the unstable path is now taken only when the banded comparison is non-empty. `generator_tree_recursion` gets `guard_failures=8`. Measured on one host, one binary, one arch: the jitcounter decays by 0.96 every 32 minor collections (majit-trace/src/counter.rs), so the counter follows the minor-collection count during each guard's warm-up. Sweeping the nursery from 512KB to 8MB reads 2951..2958, and `PYRE_JIT=decay=0` reads 2999 at every one of those sizes. `loops_compiled=3` and `bridges_compiled=26` hold throughout and stay gated exactly. Assisted-by: Claude --- pyre/bench/synth/generator_tree_recursion.py | 9 ++ pyre/check.py | 95 +++++++++++++++++--- 2 files changed, 91 insertions(+), 13 deletions(-) diff --git a/pyre/bench/synth/generator_tree_recursion.py b/pyre/bench/synth/generator_tree_recursion.py index 3d11045f879..efee4007f1a 100644 --- a/pyre/bench/synth/generator_tree_recursion.py +++ b/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; diff --git a/pyre/check.py b/pyre/check.py index 8fe5a00c721..b753929267c 100644 --- a/pyre/check.py +++ b/pyre/check.py @@ -1202,7 +1202,7 @@ 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): +def _jit_stats_change(saved, current, bands=None): """Return `(regressions, improvements)`, each a list of "field a -> b". Both directions gate: a rise means the JIT started aborting traces, hitting @@ -1210,6 +1210,11 @@ def _jit_stats_change(saved, current): means it stopped compiling something it used to compile. Neither may pass unrecorded — see the surface comment above. + A band suppresses an integer-valued move at or below its symmetric width, + whether that move is a regression or an improvement. It cannot suppress 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,6 +1223,7 @@ 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 = [], [] @@ -1226,10 +1232,14 @@ def _jit_stats_change(saved, current): 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]: + 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) @@ -1471,6 +1481,61 @@ 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 smaller than the band is not signal either way. + 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 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=" + 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 + 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 + return {} + + def _synth_header_flag(path, directive, malformed): """Read a valueless per-fixture header flag from the first 20 lines. @@ -1927,6 +1992,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 @@ -1974,7 +2040,7 @@ def _apply_snapshot_gate( self.jitstats_vacuous.append(f"{backend}/{name}") return "fail", vacuous regressions, improvements = _jit_stats_change( - jitstats_path.read_text(encoding="utf-8"), jitstats + jitstats_path.read_text(encoding="utf-8"), jitstats, jitstats_bands ) if regressions or improvements: repeats = self._jitstats_repeats(backend, script, timeout) @@ -1982,16 +2048,19 @@ def _apply_snapshot_gate( (s for s in repeats or () if s != jitstats), None ) if drifted is not None: - moved = _jit_stats_change(jitstats, drifted) - self.jitstats_unstable.append(f"{backend}/{name}") - return "unstable", ( - "jit-stats unstable — re-running the same binary moved " - + ", ".join(moved[0] + moved[1]) - + ", so this run's counters are not a property of the " - "tree and the baseline comparison (" - + ", ".join(regressions + improvements) - + ") is not gated" - ) + moved = _jit_stats_change(jitstats, drifted, jitstats_bands) + # Snapshot text can drift inside a band without making any + # gated counter unstable. + if moved[0] or moved[1]: + self.jitstats_unstable.append(f"{backend}/{name}") + return "unstable", ( + "jit-stats unstable — re-running the same binary moved " + + ", ".join(moved[0] + moved[1]) + + ", so this run's counters are not a property of the " + "tree and the baseline comparison (" + + ", ".join(regressions + improvements) + + ") is not gated" + ) parts = [] if regressions: parts.append("regressed: " + ", ".join(regressions)) From bff816847e878292a2e0a80b903b52e0379bd98b Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Mon, 17 Aug 2026 07:17:18 +0900 Subject: [PATCH 2/3] check: report a jit-stats move a band absorbed instead of swallowing it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_jit_stats_change` returns `(regressions, improvements, banded)`. A field the band suppresses is listed in `banded` as `field old -> new (within band N)` and in neither of the other two, so the move is reclassified rather than hidden. A run whose only movement was absorbed returns the new non-failing `banded` status, which renders like `unstable` — a yellow line, the bench still passes — and is tallied in the end-of-run summary as "jit-stats within band (not gated)". When the row is already failing for another counter, the absorbed move is appended to the same reason as a `within band:` part. `unstable_line` becomes `note_line` at the call site, since it now carries either note, and the label is derived from the status. Assisted-by: Claude --- pyre/check.py | 61 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 40 insertions(+), 21 deletions(-) diff --git a/pyre/check.py b/pyre/check.py index b753929267c..e8a87ce52b5 100644 --- a/pyre/check.py +++ b/pyre/check.py @@ -1203,15 +1203,16 @@ def _parse_jit_stats(snapshot): def _jit_stats_change(saved, current, bands=None): - """Return `(regressions, improvements)`, each a list of "field a -> b". + """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 suppresses an integer-valued move at or below its symmetric width, - whether that move is a regression or an improvement. It cannot suppress a + 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. @@ -1226,7 +1227,7 @@ def _jit_stats_change(saved, current, bands=None): 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: @@ -1238,6 +1239,9 @@ def _jit_stats_change(saved, current, bands=None): 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 ( @@ -1247,7 +1251,7 @@ def _jit_stats_change(saved, current, bands=None): 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): @@ -1486,10 +1490,12 @@ def synth_jitstats_bands(path): # pyre-check: jitstats-band=guard_failures=8 The band is symmetric around the recorded baseline and absorbs moves in - both directions: a delta smaller than the band is not signal either way. - 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. + 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 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. @@ -1755,6 +1761,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 @@ -2039,7 +2048,7 @@ def _apply_snapshot_gate( if vacuous: self.jitstats_vacuous.append(f"{backend}/{name}") return "fail", vacuous - regressions, improvements = _jit_stats_change( + regressions, improvements, banded = _jit_stats_change( jitstats_path.read_text(encoding="utf-8"), jitstats, jitstats_bands ) if regressions or improvements: @@ -2066,6 +2075,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) @@ -2077,6 +2088,12 @@ def _apply_snapshot_gate( return "regressed", reason self.jitstats_improvements.append(f"{backend}/{name}") return "improved", reason + if banded: + self.jitstats_banded.append(f"{backend}/{name}") + return "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) @@ -2750,10 +2767,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. @@ -2762,9 +2780,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( @@ -2773,8 +2791,8 @@ 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, @@ -2782,10 +2800,10 @@ def _ratio(elapsed_val, pypy_val): ) 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") @@ -3257,6 +3275,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), From 2648e46d5d662794dbcb1833b57501b4116db153 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Mon, 17 Aug 2026 07:46:00 +0900 Subject: [PATCH 3/3] check: three fixes to the jit-stats band from review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A banded verdict is assigned rather than returned. Every other verdict reached before it is a failure, so leaving the function early costs those nothing; a band means the run is fine, and returning there carried the fixture past the `--snapshot-diff` output comparison and the `--threshold` time comparison — on a host that sits inside its band, on every run. The repeat-drift comparison no longer takes the bands. It compares two samples of the same invocation, so a band applied there measures from whichever sample ran first instead of from the recorded baseline: with baseline 2951 and band 8, a first run of 2960 was gated while a repeat of 2958 was absorbed as a difference of 2, and swapping the samples changed the verdict. Unbanded, two snapshots that differ textually must differ in a `JITSTATS_SNAPSHOT_FIELDS` entry, so the guard added with the bands is unreachable and the original unstable path is restored. `synth_jitstats_bands` scans the whole 20-line window and raises on a second directive instead of returning at the first. It already rejects a duplicate field inside one line, so dropping a duplicate line silently would let one of two per-field directives do nothing. Assisted-by: Claude --- pyre/check.py | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/pyre/check.py b/pyre/check.py index e8a87ce52b5..1b62c716eae 100644 --- a/pyre/check.py +++ b/pyre/check.py @@ -1496,11 +1496,13 @@ def synth_jitstats_bands(path): 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 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. + 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() @@ -1508,6 +1510,8 @@ def synth_jitstats_bands(path): 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: @@ -1538,8 +1542,7 @@ def synth_jitstats_bands(path): f"jit-stats band width must be positive in {path}: {line.strip()}" ) bands[field] = width - return bands - return {} + return bands if bands is not None else {} def _synth_header_flag(path, directive, malformed): @@ -2057,19 +2060,17 @@ def _apply_snapshot_gate( (s for s in repeats or () if s != jitstats), None ) if drifted is not None: - moved = _jit_stats_change(jitstats, drifted, jitstats_bands) - # Snapshot text can drift inside a band without making any - # gated counter unstable. - if moved[0] or moved[1]: - self.jitstats_unstable.append(f"{backend}/{name}") - return "unstable", ( - "jit-stats unstable — re-running the same binary moved " - + ", ".join(moved[0] + moved[1]) - + ", so this run's counters are not a property of the " - "tree and the baseline comparison (" - + ", ".join(regressions + improvements) - + ") is not gated" - ) + # Drift compares two samples directly and is deliberately unbanded. + moved = _jit_stats_change(jitstats, drifted) + self.jitstats_unstable.append(f"{backend}/{name}") + return "unstable", ( + "jit-stats unstable — re-running the same binary moved " + + ", ".join(moved[0] + moved[1]) + + ", so this run's counters are not a property of the " + "tree and the baseline comparison (" + + ", ".join(regressions + improvements) + + ") is not gated" + ) parts = [] if regressions: parts.append("regressed: " + ", ".join(regressions)) @@ -2089,8 +2090,13 @@ def _apply_snapshot_gate( 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}") - return "banded", ( + status, reason = "banded", ( "jit-stats within band — " + ", ".join(banded) + _jit_stats_context(jitstats) )