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..1b62c716eae 100644 --- a/pyre/check.py +++ b/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,18 +1224,26 @@ 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) @@ -1237,7 +1251,7 @@ def _jit_stats_change(saved, current): 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,8 +2051,8 @@ 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) @@ -1982,6 +2060,7 @@ def _apply_snapshot_gate( (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,8 +2797,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, @@ -2713,10 +2806,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") @@ -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),