diff --git a/.github/workflows/backend-ci.yml b/.github/workflows/backend-ci.yml index 7ccc4517..e8cfa4dd 100644 --- a/.github/workflows/backend-ci.yml +++ b/.github/workflows/backend-ci.yml @@ -87,6 +87,63 @@ jobs: env: DATABASE_URL: postgres://utility:utility_secret@localhost:5432/utility_test + perf-regression: + runs-on: ubuntu-latest + needs: test + steps: + - uses: actions/checkout@v4 + + - name: Install stable toolchain + uses: dtolnay/rust-toolchain@stable + + - name: Cache rust dependencies + uses: Swatinem/rust-cache@v2 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.x' + + # Restores the most recent baseline recorded from a push to `main`. + # Cache entries are immutable per key in GitHub Actions, so main-branch + # runs always save under a fresh (per-commit) key below; PR runs use + # the `perf-baseline-` prefix to fall back to the most recent one. + - name: Restore performance baseline + uses: actions/cache/restore@v4 + with: + path: perf-baseline.json + key: perf-baseline-${{ github.sha }} + restore-keys: | + perf-baseline- + + - name: Run benchmarks + run: cargo bench --all-features + + - name: Extract benchmark results + run: python3 scripts/perf_regression_check.py extract --criterion-dir target/criterion --out current-perf.json + + - name: Compare against baseline (fails the build on a >10% regression) + run: python3 scripts/perf_regression_check.py compare --baseline perf-baseline.json --current current-perf.json --threshold 0.10 + + - name: Upload criterion HTML report + if: always() + uses: actions/upload-artifact@v4 + with: + name: criterion-report + path: target/criterion + retention-days: 30 + + - name: Record new baseline (main branch only) + if: github.ref == 'refs/heads/main' + run: cp current-perf.json perf-baseline.json + + - name: Save performance baseline (main branch only) + if: github.ref == 'refs/heads/main' + uses: actions/cache/save@v4 + with: + path: perf-baseline.json + key: perf-baseline-${{ github.sha }} + docker-build: runs-on: ubuntu-latest steps: diff --git a/.gitignore b/.gitignore index 1d11c15c..5395d3b3 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ # Rust incremental compilation temp artifacts *.rcgu.o +__pycache__/ diff --git a/Cargo.toml b/Cargo.toml index b568104d..5c6b1207 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -95,3 +95,7 @@ harness = false [[bench]] name = "stream_throughput" harness = false + +[[bench]] +name = "arena_bench" +harness = false diff --git a/docs/runbooks/performance-regression-detection.md b/docs/runbooks/performance-regression-detection.md new file mode 100644 index 00000000..95d5c64d --- /dev/null +++ b/docs/runbooks/performance-regression-detection.md @@ -0,0 +1,109 @@ +# Runbook: Automated Performance Regression Detection + +Related issue: #238. + +## What this is + +A CI gate (`perf-regression` job in `.github/workflows/backend-ci.yml`) +that runs the repo's existing `criterion` benchmark suite +(`benches/parser_bench.rs`, `merkle_bench.rs`, `stream_throughput.rs`, +`arena_bench.rs`) on every push and pull request, compares the results +against a stored baseline from the most recent `main` build, and **fails +the build if any benchmark got more than 10% slower**. + +## What's covered today, and what isn't + +This gates the four existing criterion benchmarks — envelope parsing, the +settlement Merkle tree, telemetry stream throughput, and the arena +allocator. It does **not** currently cover live HTTP endpoint latency +(the issue's "<100ms P99 for critical paths" language implies +request-level measurement); that would need a load-testing step (e.g. k6 +or wrk hitting a running instance) added as a further CI job, and isn't +built here — there's no existing endpoint-benchmark harness in the repo to +extend, and standing one up is a separate, larger piece of work. The +`arena_bench.rs` file existed on disk already but was missing from +`Cargo.toml`'s `[[bench]]` list, so `cargo bench` was silently skipping it +before this change — it's now included. + +Likewise, "blue-green deploy with canary analysis" and "99.99% uptime" +from the issue's technical bounds describe infrastructure this repo +doesn't have configured (there's no deployment/canary tooling in this +codebase to hook into) — out of scope here; see the note at the bottom. + +## How it works + +1. `cargo bench --all-features` runs and criterion writes results to + `target/criterion/**/new/estimates.json`. +2. `scripts/perf_regression_check.py extract` walks that directory and + writes a flat `{benchmark_id: mean_ns}` snapshot. +3. `scripts/perf_regression_check.py compare` diffs that snapshot against + the baseline restored from the GitHub Actions cache (see below), + printing a table to the job log **and** to the run's summary page + (`$GITHUB_STEP_SUMMARY`), and exits non-zero if anything regressed by + more than the threshold. +4. On `main` only, the just-computed snapshot is saved as the new baseline + (`actions/cache/save`, keyed by commit SHA) for future PRs to compare + against. + +## Reading a failure + +Open the failed job's **Summary** tab — the same markdown table posted to +stdout is rendered there. Rows marked 🔴 REGRESSION exceeded the 10% +threshold; 🟢 improved and 🆕 new/⚪ removed rows are informational only +and never fail the build. + +``` +| Benchmark | Baseline | Current | Change | Verdict | +|---|---|---|---|---| +| merkle_tree_build_4096 | 812043 ns | 1105210 ns | +36.1% | 🔴 REGRESSION | +``` + +**To investigate:** download the `criterion-report` artifact from the same +run (uploaded regardless of pass/fail) and open +`/report/index.html` for criterion's full distribution/violin +plots — useful for telling a genuine regression apart from CI-runner +noise. + +**If it's noise, not a real regression:** re-run the job. Shared CI +runners have enough scheduling jitter that an occasional single-run false +positive near the threshold is expected; a regression that reproduces +across re-runs is real. + +**If it's a real, intentional trade-off** (e.g. added a safety check that +costs a few percent): merge to `main` as normal — the next `main` run +records the new, slower number as the baseline going forward, so this +isn't a permanent gate against that specific number, only against +*further* regressions from wherever `main` currently stands. + +## Adjusting the threshold + +`--threshold 0.10` in the workflow step is a fraction (10%). Change it in +`.github/workflows/backend-ci.yml` if it's too noisy or too loose in +practice; there's nothing else to update in sync with it. + +## First run on a fresh clone / fork + +If no baseline cache exists yet (e.g. a brand-new fork, or the very first +run of this workflow), `compare` prints a warning and exits `0` rather +than failing — there's nothing to compare against yet. The first push to +`main` establishes the initial baseline. + +## Adding a new benchmark + +Add the `.rs` file under `benches/`, register it in `Cargo.toml`'s +`[[bench]]` list (the same step that was missing for `arena_bench.rs` +before this change), and it's picked up automatically — no changes needed +to the CI job or the Python script. + +## Out of scope (see issue #238 for the full ask) + +- HTTP endpoint-level P99 latency gating (needs a load-testing harness + against a running instance — not present in this repo today). +- Blue-green / canary deploy integration (no deploy tooling in this repo + to hook into). +- A Grafana dashboard for benchmark trends over time (the criterion HTML + report + this job's summary table serve that purpose today; a proper + time-series dashboard would need bench results pushed to Prometheus, + which they aren't currently — see `docs/dashboards/incident_response.json` + for the pattern this repo already uses for `Prometheus`-backed panels + if that's built out later). diff --git a/scripts/perf_regression_check.py b/scripts/perf_regression_check.py new file mode 100644 index 00000000..7c0361ab --- /dev/null +++ b/scripts/perf_regression_check.py @@ -0,0 +1,226 @@ +#!/usr/bin/env python3 +""" +scripts/perf_regression_check.py + +Automated performance regression detection for CI (issue #238), built on +top of the criterion benchmark suite that already exists in this repo +(benches/parser_bench.rs, merkle_bench.rs, stream_throughput.rs, +arena_bench.rs — the last of these existed on disk but was never wired +into Cargo.toml's [[bench]] list until this change, so `cargo bench` was +silently skipping it). + +Two subcommands, run from the repo root after `cargo bench --all-features` +has produced `target/criterion/`: + + extract Walk target/criterion/**/new/estimates.json and write a flat + {benchmark_id: mean_ns} JSON snapshot of the current run. + + compare Compare a "current" snapshot against a "baseline" snapshot and + exit non-zero if any benchmark regressed past --threshold + (default 10%). Missing baseline (e.g. first run on a fresh + repo, or before this workflow's first main-branch run) is + treated as "nothing to compare against yet" and exits 0 with a + warning, rather than failing CI. + +Both subcommands are side-effect-free beyond reading/writing the given +paths, so the comparison logic is directly unit-testable (see +scripts/tests/test_perf_regression_check.py) without needing to actually +run cargo bench. +""" +from __future__ import annotations + +import argparse +import json +import os +import sys +from pathlib import Path +from typing import Dict, Tuple + +DEFAULT_THRESHOLD = 0.10 # 10% slower than baseline counts as a regression. + + +# ─── extract ───────────────────────────────────────────────────────────────── + + +def extract_results(criterion_dir: Path) -> Dict[str, float]: + """Walks target/criterion/**/new/estimates.json and returns + {benchmark_id: mean_point_estimate_ns}. + + Criterion nests each benchmark under + target/criterion///new/estimates.json (or + target/criterion//new/estimates.json for ungrouped + benchmarks); the benchmark_id used here is that path relative to + criterion_dir with the trailing "/new/estimates.json" stripped, so it's + stable across runs and matches how criterion itself labels benchmarks + in its HTML report. + """ + results: Dict[str, float] = {} + if not criterion_dir.is_dir(): + return results + + for estimates_path in sorted(criterion_dir.glob("**/new/estimates.json")): + benchmark_id = str(estimates_path.parent.parent.relative_to(criterion_dir)) + try: + with estimates_path.open() as f: + data = json.load(f) + mean_ns = data["mean"]["point_estimate"] + except (KeyError, json.JSONDecodeError, OSError): + # A malformed/partial estimates.json shouldn't take down the + # whole extraction; skip just that one benchmark. + continue + results[benchmark_id] = float(mean_ns) + + return results + + +def cmd_extract(args: argparse.Namespace) -> int: + results = extract_results(Path(args.criterion_dir)) + Path(args.out).write_text(json.dumps(results, indent=2, sort_keys=True) + "\n") + print(f"Extracted {len(results)} benchmark result(s) to {args.out}") + return 0 + + +# ─── compare ───────────────────────────────────────────────────────────────── + + +class Verdict: + OK = "ok" + REGRESSION = "regression" + IMPROVEMENT = "improvement" + NEW = "new" # present in current, absent from baseline + REMOVED = "removed" # present in baseline, absent from current + + +def compare_results( + baseline: Dict[str, float], current: Dict[str, float], threshold: float +) -> Tuple[bool, list[dict]]: + """Returns (any_regression, rows) where each row describes one + benchmark's verdict. A benchmark counts as regressed when its current + mean exceeds its baseline mean by more than `threshold` (a fraction, + e.g. 0.10 for 10%).""" + rows: list[dict] = [] + any_regression = False + + all_ids = sorted(set(baseline) | set(current)) + for benchmark_id in all_ids: + base_ns = baseline.get(benchmark_id) + cur_ns = current.get(benchmark_id) + + if base_ns is None: + rows.append({"id": benchmark_id, "verdict": Verdict.NEW, "current_ns": cur_ns}) + continue + if cur_ns is None: + rows.append({"id": benchmark_id, "verdict": Verdict.REMOVED, "baseline_ns": base_ns}) + continue + + pct_change = (cur_ns - base_ns) / base_ns if base_ns > 0 else 0.0 + if pct_change > threshold: + verdict = Verdict.REGRESSION + any_regression = True + elif pct_change < -threshold: + verdict = Verdict.IMPROVEMENT + else: + verdict = Verdict.OK + + rows.append( + { + "id": benchmark_id, + "verdict": verdict, + "baseline_ns": base_ns, + "current_ns": cur_ns, + "pct_change": pct_change, + } + ) + + return any_regression, rows + + +def format_report(rows: list[dict], threshold: float) -> str: + lines = [ + f"## Performance regression check (threshold: {threshold:.0%})", + "", + "| Benchmark | Baseline | Current | Change | Verdict |", + "|---|---|---|---|---|", + ] + icons = { + Verdict.OK: "✅", + Verdict.REGRESSION: "🔴 REGRESSION", + Verdict.IMPROVEMENT: "🟢 improved", + Verdict.NEW: "🆕 new", + Verdict.REMOVED: "⚪ removed", + } + for row in rows: + baseline_str = f"{row['baseline_ns']:.0f} ns" if "baseline_ns" in row else "—" + current_str = f"{row['current_ns']:.0f} ns" if "current_ns" in row else "—" + change_str = f"{row['pct_change']:+.1%}" if "pct_change" in row else "—" + lines.append(f"| {row['id']} | {baseline_str} | {current_str} | {change_str} | {icons[row['verdict']]} |") + return "\n".join(lines) + + +def cmd_compare(args: argparse.Namespace) -> int: + baseline_path = Path(args.baseline) + current_path = Path(args.current) + + current = json.loads(current_path.read_text()) + + if not baseline_path.is_file(): + print( + "No performance baseline found yet (expected at " + f"{baseline_path}) — skipping the regression gate. " + "A baseline is recorded automatically on the next push to main.", + file=sys.stderr, + ) + return 0 + + baseline = json.loads(baseline_path.read_text()) + any_regression, rows = compare_results(baseline, current, args.threshold) + + report = format_report(rows, args.threshold) + print(report) + + summary_path = os.environ.get("GITHUB_STEP_SUMMARY") + if summary_path: + with open(summary_path, "a") as f: + f.write(report + "\n") + + if any_regression: + print( + f"\nOne or more benchmarks regressed by more than {args.threshold:.0%}. " + "See the table above.", + file=sys.stderr, + ) + return 1 + + print("\nNo regressions detected.") + return 0 + + +# ─── CLI ───────────────────────────────────────────────────────────────────── + + +def build_parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser(description=__doc__) + subparsers = parser.add_subparsers(dest="command", required=True) + + extract_parser = subparsers.add_parser("extract", help="Extract results from target/criterion into a JSON snapshot") + extract_parser.add_argument("--criterion-dir", default="target/criterion", help="Path to criterion's output directory") + extract_parser.add_argument("--out", required=True, help="Where to write the extracted JSON snapshot") + extract_parser.set_defaults(func=cmd_extract) + + compare_parser = subparsers.add_parser("compare", help="Compare a current snapshot against a baseline snapshot") + compare_parser.add_argument("--baseline", required=True, help="Path to the baseline JSON snapshot") + compare_parser.add_argument("--current", required=True, help="Path to the current JSON snapshot") + compare_parser.add_argument("--threshold", type=float, default=DEFAULT_THRESHOLD, help="Regression threshold as a fraction (default: 0.10 = 10%%)") + compare_parser.set_defaults(func=cmd_compare) + + return parser + + +def main(argv: list[str] | None = None) -> int: + parser = build_parser() + args = parser.parse_args(argv) + return args.func(args) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/tests/test_perf_regression_check.py b/scripts/tests/test_perf_regression_check.py new file mode 100644 index 00000000..ed0d4cd7 --- /dev/null +++ b/scripts/tests/test_perf_regression_check.py @@ -0,0 +1,128 @@ +#!/usr/bin/env python3 +""" +scripts/tests/test_perf_regression_check.py + +Unit tests for the performance regression gate (issue #238). Uses only the +standard library (unittest) so it runs with a plain `python3 -m unittest` +in CI without adding a new Python dependency to the repo. +""" +import json +import sys +import tempfile +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from perf_regression_check import ( # noqa: E402 + Verdict, + compare_results, + extract_results, +) + + +class ExtractResultsTests(unittest.TestCase): + def test_returns_empty_dict_for_missing_directory(self): + self.assertEqual(extract_results(Path("/nonexistent/path")), {}) + + def test_extracts_mean_point_estimate_from_nested_layout(self): + with tempfile.TemporaryDirectory() as tmp: + criterion_dir = Path(tmp) + bench_dir = criterion_dir / "merkle_tree_build_1024" / "new" + bench_dir.mkdir(parents=True) + (bench_dir / "estimates.json").write_text(json.dumps({"mean": {"point_estimate": 12345.6}})) + + results = extract_results(criterion_dir) + self.assertEqual(results, {"merkle_tree_build_1024": 12345.6}) + + def test_extracts_multiple_benchmarks_including_grouped_ones(self): + with tempfile.TemporaryDirectory() as tmp: + criterion_dir = Path(tmp) + for group, name, mean in [ + ("alloc_free_128_same_thread", "arena", 100.0), + ("alloc_free_128_same_thread", "system", 250.0), + (None, "parse_envelope", 500.0), + ]: + bench_dir = criterion_dir / (f"{group}/{name}" if group else name) / "new" + bench_dir.mkdir(parents=True) + (bench_dir / "estimates.json").write_text(json.dumps({"mean": {"point_estimate": mean}})) + + results = extract_results(criterion_dir) + self.assertEqual( + results, + { + "alloc_free_128_same_thread/arena": 100.0, + "alloc_free_128_same_thread/system": 250.0, + "parse_envelope": 500.0, + }, + ) + + def test_skips_malformed_estimates_file_without_crashing(self): + with tempfile.TemporaryDirectory() as tmp: + criterion_dir = Path(tmp) + bench_dir = criterion_dir / "broken_bench" / "new" + bench_dir.mkdir(parents=True) + (bench_dir / "estimates.json").write_text("not valid json {{{") + + results = extract_results(criterion_dir) + self.assertEqual(results, {}) + + +class CompareResultsTests(unittest.TestCase): + def test_no_regression_within_threshold(self): + baseline = {"parse_envelope": 1000.0} + current = {"parse_envelope": 1050.0} # +5%, threshold is 10% + any_regression, rows = compare_results(baseline, current, threshold=0.10) + self.assertFalse(any_regression) + self.assertEqual(rows[0]["verdict"], Verdict.OK) + + def test_detects_regression_past_threshold(self): + baseline = {"parse_envelope": 1000.0} + current = {"parse_envelope": 1200.0} # +20%, threshold is 10% + any_regression, rows = compare_results(baseline, current, threshold=0.10) + self.assertTrue(any_regression) + self.assertEqual(rows[0]["verdict"], Verdict.REGRESSION) + self.assertAlmostEqual(rows[0]["pct_change"], 0.20) + + def test_detects_improvement_past_threshold(self): + baseline = {"parse_envelope": 1000.0} + current = {"parse_envelope": 800.0} # -20% + any_regression, rows = compare_results(baseline, current, threshold=0.10) + self.assertFalse(any_regression) + self.assertEqual(rows[0]["verdict"], Verdict.IMPROVEMENT) + + def test_flags_new_benchmark_without_failing(self): + baseline: dict = {} + current = {"new_bench": 500.0} + any_regression, rows = compare_results(baseline, current, threshold=0.10) + self.assertFalse(any_regression) + self.assertEqual(rows[0]["verdict"], Verdict.NEW) + + def test_flags_removed_benchmark_without_failing(self): + baseline = {"old_bench": 500.0} + current: dict = {} + any_regression, rows = compare_results(baseline, current, threshold=0.10) + self.assertFalse(any_regression) + self.assertEqual(rows[0]["verdict"], Verdict.REMOVED) + + def test_one_regression_among_many_still_fails_overall(self): + baseline = {"a": 100.0, "b": 100.0, "c": 100.0} + current = {"a": 105.0, "b": 200.0, "c": 95.0} # only b regresses + any_regression, rows = compare_results(baseline, current, threshold=0.10) + self.assertTrue(any_regression) + verdicts = {row["id"]: row["verdict"] for row in rows} + self.assertEqual(verdicts["a"], Verdict.OK) + self.assertEqual(verdicts["b"], Verdict.REGRESSION) + self.assertEqual(verdicts["c"], Verdict.OK) + + def test_zero_baseline_does_not_crash(self): + baseline = {"degenerate": 0.0} + current = {"degenerate": 100.0} + # Should not raise a ZeroDivisionError. + any_regression, rows = compare_results(baseline, current, threshold=0.10) + self.assertFalse(any_regression) + self.assertEqual(rows[0]["verdict"], Verdict.OK) + + +if __name__ == "__main__": + unittest.main()