Skip to content

Commit 1d977c3

Browse files
committed
Clean up and add tests to perf-smoke POC
Finish env-fingerprint bucketing and per-sample provenance for the rolling-window history, add unit tests for the orchestrator, rebaseline tool, and fingerprint/provenance helpers, and document the architecture in DESIGN.md.
1 parent 587409b commit 1d977c3

9 files changed

Lines changed: 840 additions & 23 deletions

File tree

.github/workflows/perf-gate.yml

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,18 +76,26 @@ jobs:
7676
echo "Matrix tasks: $tasks_json"
7777
7878
# ---------------------------------------------------------------------------
79-
# Comparator logic tests. Pure stdlib unittest -- no GPU, no Isaac Sim, and
80-
# arch-independent -- so this validates the PASS/WARN/BLOCK verdict logic on
81-
# every trigger and gives fast signal before the GPU job is scheduled.
79+
# Pure-logic tests for the comparator, orchestrator, rebaseline tool, and the
80+
# history-bucketing helpers. Pure stdlib unittest -- no GPU, no Isaac Sim, and
81+
# arch-independent -- so this validates the PASS/WARN/BLOCK verdict logic, the
82+
# launch-config plumbing, and the rolling-window writer on every trigger and
83+
# gives fast signal before the GPU job is scheduled.
8284
# ---------------------------------------------------------------------------
8385
comparator-unit-tests:
8486
name: Comparator Unit Tests
8587
runs-on: ubuntu-latest
8688
steps:
8789
- name: Checkout
8890
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
89-
- name: Run comparator unittest suite
90-
run: python3 tools/perf_smoke/test_check_perf_regression.py
91+
- name: Run unittest suite
92+
run: |
93+
set -e
94+
for t in test_check_perf_regression test_history_fingerprint test_run_perf_gate test_rebaseline; do
95+
echo "::group::$t"
96+
python3 "tools/perf_smoke/$t.py"
97+
echo "::endgroup::"
98+
done
9199
92100
# ---------------------------------------------------------------------------
93101
# The GPU gate. One job PER TASK (matrix from baseline.json): each task is its

tools/perf_smoke/DESIGN.md

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
# Perf Smoke Gate — Architecture
2+
3+
This document explains *how* the gate is put together: the data flow, the
4+
baseline/history model, and the verdict logic. For a plain-English file-by-file
5+
tour, see [`README.md`](README.md).
6+
7+
## Design goals
8+
9+
1. **Cheap and per-PR.** A handful of short, stable tasks on a fixed GPU — fast
10+
enough to run on every pull request.
11+
2. **Robust to noise.** Small run-to-run wobble must not flake the gate; only a
12+
real, sustained drop blocks.
13+
3. **In-tree and reviewed.** Baselines and history live in the repo and only
14+
change through a normal, reviewed PR — never a silent side-write from CI. This
15+
keeps the gate auditable and lets `git blame` answer "why did the bar move?".
16+
4. **No new dependencies.** Everything is standard-library Python so the
17+
comparator and its tests run on any runner without Isaac Sim or a GPU.
18+
19+
## Components
20+
21+
| Layer | Module | Responsibility |
22+
|---|---|---|
23+
| Orchestration | `run_perf_gate.py` | Per task: resolve launch config from `baseline.json`, launch the benchmark as its own Isaac Sim subprocess (retry once), hand the result to the comparator, aggregate verdicts (worst wins). |
24+
| Decision (pure logic) | `check_perf_regression.py` | Read one benchmark result + baseline + rolling window, compute the KPI, and return PASS / WARN / BLOCK. No GPU, no Isaac Sim. |
25+
| Stored state | `baseline.json`, `perf_history/`, `baseline_overrides.json` | The launch config + static fallback, the rolling window of recent samples, and manual threshold overrides. |
26+
| Maintenance | `rebaseline.py`, `seed_history.py` | Produce/refresh the stored state from fresh or existing runs (always via a reviewed PR). |
27+
28+
## Data flow (one PR)
29+
30+
```
31+
baseline.json ─┐
32+
├─► run_perf_gate ─► benchmark_non_rl.py (subprocess) ─► result.json
33+
perf_history/ ─┤ │
34+
overrides ─────┘ ▼
35+
check_perf_regression ◄─────────────────┘
36+
37+
RESULT=PASS|WARN|BLOCK + $GITHUB_STEP_SUMMARY table
38+
```
39+
40+
The orchestrator only builds commands and aggregates; **all** of the regression
41+
judgement lives in the comparator, which is why the comparator is independently
42+
unit-testable without hardware.
43+
44+
## The KPI
45+
46+
The gating metric is the **post-warm-up steady FPS** (`steady_fps`): the
47+
benchmark's per-frame effective-FPS array with the first `warmup_frames` dropped.
48+
Using the same statistic the backend already reports — just windowed — keeps the
49+
measured value directly comparable to the stored history. Wall-clock seconds are
50+
carried as a secondary, advisory signal only.
51+
52+
## Baseline & history model
53+
54+
There are two stores, deliberately layered:
55+
56+
- **Rolling window (`perf_history/`, primary).** Per `(task, GPU)`, the last
57+
*N* known-good samples. The comparator computes its threshold *at test time*
58+
from this window with a robust **median + MAD** estimator:
59+
60+
```
61+
center = median(window)
62+
spread = max(1.4826 * MAD(window), min_spread_pct/100 * center)
63+
WARN when measured < center - k_warn * spread
64+
BLOCK when measured < center - k_block * spread
65+
```
66+
67+
A `min_spread_pct` floor stops a very low-variance task from blocking on
68+
trivial dips.
69+
70+
- **Static fallback (`baseline.json`, secondary).** When the window is too small
71+
to trust (`< MIN_WINDOW` samples), the comparator falls back to a static
72+
`baseline_fps` + percentage bands calibrated for that task/GPU. This keeps a
73+
fresh store from silently passing everything before it has accumulated history.
74+
75+
**Overrides** (`baseline_overrides.json`) are a manual escape hatch keyed by
76+
*stable* test identity (`task` + GPU), applied on top of either source — used for
77+
one-off threshold relaxations or `skip` that ride along in the PR.
78+
79+
### Environment fingerprint buckets
80+
81+
Performance is only comparable within the same software stack: a Warp bump or an
82+
Isaac Sim upgrade can legitimately shift FPS, and mixing those samples into one
83+
window would corrupt the baseline. So history is **bucketed by an environment
84+
fingerprint**:
85+
86+
```
87+
perf_history/
88+
<task>__<gpu>.json # flat "default" bucket (legacy / no provenance)
89+
env-<hash>/<task>__<gpu>.json # one bucket per (warp, isaaclab, cuda) stack
90+
```
91+
92+
- `env_fingerprint(result)` hashes the environment-defining provenance
93+
(`warp`, `isaaclab`, `cuda`) into a short, stable `env-<hash>` key. GPU is
94+
*not* in the hash because it is already in the file name.
95+
- The comparator derives the fingerprint from the run under test and reads the
96+
matching bucket, **falling back to the flat file** when no bucket exists yet —
97+
so the change is backward-compatible with already-seeded flat history.
98+
- A consistent filename (`history_basename`) is shared by the reader and every
99+
writer so a written bucket is always found again.
100+
101+
### Per-sample provenance
102+
103+
Every stored sample carries the context needed to audit or re-bucket it without
104+
re-running the benchmark: `commit`, `warp`, `isaaclab`, `cuda`, plus the
105+
`fingerprint` recorded at the window level. This makes the in-tree history
106+
self-describing — a reviewer reading a `perf_history/` diff can see exactly which
107+
commit and stack produced each number.
108+
109+
## Re-baselining lifecycle
110+
111+
`rebaseline.py` is the only writer of the stored state and serves two jobs from
112+
one measurement path:
113+
114+
1. **Variance study (default).** Run each task `--repeat` times and report robust
115+
stats (median / CV / MAD / min / max) so thresholds can be justified to
116+
reviewers.
117+
2. **Rolling re-baseline (`--apply`).** Append the new samples to the window
118+
(pruned to a cap, stamped with provenance, written into the env bucket) and
119+
refresh the static fallback in `baseline.json`.
120+
121+
A **boiling-frog guard** keeps a rolling baseline from quietly absorbing a real
122+
regression: a task whose new median drops the baseline by more than
123+
`--soft-drop-pct` is *flagged for review*; a drop beyond `--hard-drop-pct` is
124+
*refused* (old value kept) unless `--force`. Both stores then change only through
125+
the PR that `perf-rebaseline.yml` opens.
126+
127+
## CI wiring
128+
129+
- `perf-gate.yml` — on a PR it runs a fast, GPU-free **unit-test job** (comparator,
130+
orchestrator, rebaseline, fingerprint helpers) for early signal, then a
131+
per-task GPU matrix on the L40S fleet that posts the verdict back. Advisory
132+
(`continue-on-error`) until cross-runner variance is confirmed.
133+
- `perf-rebaseline.yml` — manual workflow that runs `rebaseline.py --apply` on the
134+
fleet and opens a PR with the `baseline.json` + `perf_history/` diff.
135+
136+
## Why these boundaries
137+
138+
- **Pure-logic comparator** ⇒ the regression rules are fully testable on any
139+
runner; the GPU job only *produces* numbers, it never *decides*.
140+
- **In-tree, reviewed state** ⇒ no opaque external baseline service; every change
141+
to the bar is a diff someone approved.
142+
- **Layered window → static → override** ⇒ robust thresholds when history exists,
143+
a safe floor when it doesn't, and a human escape hatch when neither fits.

tools/perf_smoke/README.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,15 @@ mild dip is an advisory **WARN** and only a real, sustained drop is a **BLOCK**.
4242
|---|---|
4343
| `test_check_perf_regression.py` | Unit tests for the comparator logic (no GPU). |
4444
| `test_stress_check_perf_regression.py` | Heavier stress/edge-case tests for the comparator. |
45+
| `test_history_fingerprint.py` | Unit tests for env-fingerprint bucketing + per-sample provenance. |
46+
| `test_run_perf_gate.py` | Unit tests for the orchestrator (config, command building, aggregation). |
47+
| `test_rebaseline.py` | Unit tests for the rebaseline tool (window stats, store writer, boiling-frog guard). |
4548
| `test_perf_gate.py` | The pytest entry point CI uses to drive a single task end-to-end. |
4649
| `pytest.ini` | Local pytest config for this directory. |
4750

51+
For the architecture (data flow, the baseline/history model, and the verdict
52+
logic), see [`DESIGN.md`](DESIGN.md).
53+
4854
### CI wiring (in `.github/`)
4955

5056
| File | Plain-English purpose |

tools/perf_smoke/check_perf_regression.py

Lines changed: 79 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555

5656
import argparse
5757
import glob
58+
import hashlib
5859
import json
5960
import os
6061
import sys
@@ -366,6 +367,66 @@ def _extract_provenance(result: dict) -> dict[str, object]:
366367
return out
367368

368369

370+
def _extract_commit(result: dict) -> str | None:
371+
"""Pull the source commit the run was built from (best-effort).
372+
373+
Reads ``version_info.dev.commit_hash`` (the benchmark backend's dev block)
374+
with a couple of common fallbacks. Returns ``None`` when unavailable.
375+
"""
376+
version = result.get("version_info")
377+
if not isinstance(version, dict):
378+
return None
379+
dev = version.get("dev")
380+
if isinstance(dev, dict):
381+
for key in ("commit_hash", "commit_hash_short", "commit"):
382+
val = dev.get(key)
383+
if isinstance(val, str) and val:
384+
return val
385+
for key in ("commit_hash", "commit"):
386+
val = version.get(key)
387+
if isinstance(val, str) and val:
388+
return val
389+
return None
390+
391+
392+
# Provenance keys that define the *environment* (perf regime), and thus the
393+
# history bucket. GPU is already encoded in the file name, so it is excluded.
394+
_FINGERPRINT_KEYS = ("warp", "isaaclab", "cuda")
395+
396+
397+
def env_fingerprint(result: dict) -> str | None:
398+
"""Compute a short, stable bucket key from a run's environment provenance.
399+
400+
The fingerprint partitions the rolling-window history so that samples from
401+
incomparable software stacks (e.g. a Warp bump that shifts the perf regime)
402+
never pollute one another's baseline. Returns ``None`` when no provenance is
403+
available, which makes callers fall back to the flat ("default") bucket.
404+
"""
405+
prov = _extract_provenance(result)
406+
parts = {key: prov[key] for key in _FINGERPRINT_KEYS if prov.get(key)}
407+
if not parts:
408+
return None
409+
canonical = json.dumps(parts, sort_keys=True, separators=(",", ":"))
410+
return "env-" + hashlib.sha256(canonical.encode("utf-8")).hexdigest()[:12]
411+
412+
413+
def sample_provenance(result: dict) -> dict[str, object]:
414+
"""Return the per-sample provenance stamped into each rolling-window record.
415+
416+
Bundles the environment versions (warp / isaaclab / cuda), the source commit,
417+
and the derived :func:`env_fingerprint` so every stored sample is auditable
418+
and re-bucketable without re-running the benchmark.
419+
"""
420+
prov = _extract_provenance(result)
421+
commit = _extract_commit(result)
422+
if commit:
423+
prov["commit"] = commit
424+
fingerprint = env_fingerprint(result)
425+
if fingerprint:
426+
prov["fingerprint"] = fingerprint
427+
return prov
428+
429+
369430
def _extract_gpu_name(result: dict) -> str | None:
370431
"""Read the runner's GPU model name from the result's hardware metadata."""
371432
hw = result.get("hardware_info")
@@ -440,6 +501,15 @@ def _resolve_baseline(
440501
return matched_key, task_entry, entry
441502

442503

504+
def history_basename(task: str, gpu_key: str) -> str:
505+
"""Filesystem-safe ``<task>__<gpu>`` stem shared by the reader and writers.
506+
507+
Centralising this keeps the comparator (reader) and rebaseline/seed scripts
508+
(writers) byte-for-byte consistent so bucketed history is always found.
509+
"""
510+
return f"{task}__{gpu_key}".replace("/", "_").replace(" ", "_")
511+
512+
443513
def _history_window(history_dir: str | None, fingerprint: str | None, task: str, gpu_key: str) -> dict:
444514
"""Load the rolling-window samples for ``(task, gpu)`` from the history store.
445515
@@ -449,7 +519,7 @@ def _history_window(history_dir: str | None, fingerprint: str | None, task: str,
449519
"""
450520
if not history_dir:
451521
return {}
452-
safe = f"{task}__{gpu_key}".replace("/", "_").replace(" ", "_")
522+
safe = history_basename(task, gpu_key)
453523
candidates = []
454524
if fingerprint:
455525
candidates.append(Path(history_dir) / fingerprint / f"{safe}.json")
@@ -544,7 +614,11 @@ def main(argv: list[str] | None = None) -> int:
544614
parser.add_argument("--baseline", required=True, help="Path to baseline.json (run config + static fallback).")
545615
parser.add_argument("--history-dir", default=None, help="Rolling-window store (orphan-branch checkout).")
546616
parser.add_argument("--overrides", default=None, help="Path to baseline_overrides.json (committed with the PR).")
547-
parser.add_argument("--fingerprint", default=None, help="History bucket key (git-subtree+deps hash).")
617+
parser.add_argument(
618+
"--fingerprint",
619+
default=None,
620+
help="History bucket key; overrides the env fingerprint auto-derived from the result.",
621+
)
548622
parser.add_argument("--measured-wall-s", type=float, default=None, help="Wall-clock seconds of the run.")
549623
parser.add_argument("--results-glob", default=None, help=f"Result glob (defaults to {DEFAULT_GLOB_TEMPLATE!r}).")
550624
parser.add_argument("--gpu-override", default=None, help="Override the GPU name read from the result JSON.")
@@ -581,7 +655,8 @@ def main(argv: list[str] | None = None) -> int:
581655
_emit("PASS", task=args.task, gpu=gpu_key, note="skipped_by_override")
582656
return EXIT_PASS
583657

584-
window = _history_window(args.history_dir, args.fingerprint, args.task, gpu_key)
658+
fingerprint = args.fingerprint or env_fingerprint(result)
659+
window = _history_window(args.history_dir, fingerprint, args.task, gpu_key)
585660
center, spread, k_warn, k_block, source = _thresholds(window, entry, ov)
586661
delta_pct = (measured_fps - center) / center * 100.0
587662
warn_floor = center - k_warn * spread
@@ -590,6 +665,7 @@ def main(argv: list[str] | None = None) -> int:
590665
common: dict[str, object] = {
591666
"task": args.task,
592667
"gpu": gpu_key,
668+
"bucket": fingerprint or "flat",
593669
"thresholds": source,
594670
"center_fps": f"{center:.0f}",
595671
"measured_fps": f"{measured_fps:.0f}",

0 commit comments

Comments
 (0)