feat(tools): compare performance capture summaries

CMakeLists.txt

@@ -148,6 +148,9 @@ if(BUILD_TESTING)
             ${CMAKE_CURRENT_SOURCE_DIR}/tools/tests/test_check_conventions_agent_files.py)
         add_test(NAME check_compile COMMAND ${Python3_EXECUTABLE}
             ${CMAKE_CURRENT_SOURCE_DIR}/tools/tests/test_check_compile.py)
+        add_test(NAME performance_compare COMMAND ${Python3_EXECUTABLE}
+            ${CMAKE_CURRENT_SOURCE_DIR}/tools/tests/test_performance_compare.py)
+        set_tests_properties(performance_compare PROPERTIES TIMEOUT 20)
     endif()
 endif()
 

tools/performance_compare.py

@@ -0,0 +1,178 @@
+import argparse
+import json
+import math
+import sys
+from pathlib import Path
+
+
+CONTEXT = ("gpu", "driver", "os", "resolution")
+PERCENTILES = ("p50", "p90", "p99", "max")
+
+
+def number(value, name):
+    try:
+        valid = type(value) in (int, float) and math.isfinite(value) and value >= 0
+    except OverflowError:
+        valid = False
+    if not valid:
+        raise ValueError(f"{name} must be a finite, non-negative number")
+    return value
+
+
+def validate(summary):
+    for key in ("commit", "trace", *CONTEXT):
+        if not isinstance(summary.get(key), str):
+            raise ValueError(f"{key} must be a string")
+    if type(summary.get("trace_dropped")) is not bool:
+        raise ValueError("trace_dropped must be a boolean")
+    if "dirty" not in summary or (summary["dirty"] is not None and type(summary["dirty"]) is not bool):
+        raise ValueError("dirty must be a boolean or null")
+    env = summary.get("env")
+    if not isinstance(env, dict) or any(not isinstance(v, str) for v in env.values()):
+        raise ValueError("env must map names to strings")
+    window = summary.get("window")
+    if not isinstance(window, dict) or not isinstance(window.get("spec"), str):
+        raise ValueError("window must contain a string spec")
+    if number(window.get("ms"), "window.ms") == 0:
+        raise ValueError("window.ms must be positive")
+    if type(window.get("first_present")) is not int or window["first_present"] < 0:
+        raise ValueError("window.first_present must be a non-negative integer")
+    if type(summary.get("presents")) is not int or summary["presents"] <= 0:
+        raise ValueError("presents must be a positive integer")
+    number(summary["presents"], "presents")
+    frames = summary.get("frame_ms")
+    if not isinstance(frames, dict):
+        raise ValueError("frame_ms must be an object")
+    values = [number(frames.get(key), f"frame_ms.{key}") for key in PERCENTILES]
+    if values != sorted(values):
+        raise ValueError("frame_ms must satisfy p50 <= p90 <= p99 <= max")
+    number(summary.get("gpu_busy_ms_per_present"), "gpu_busy_ms_per_present")
+    phases = summary.get("top_phases_ms_per_frame")
+    if not isinstance(phases, dict):
+        raise ValueError("top_phases_ms_per_frame must be an object")
+    for name, value in phases.items():
+        number(value, f"top_phases_ms_per_frame.{name}")
+    return summary
+
+
+def load_summary(path, trace=None):
+    records = []
+    with Path(path).open(encoding="utf-8") as source:
+        for line_number, line in enumerate(source, 1):
+            if not line.strip():
+                continue
+            try:
+                record = json.loads(line)
+                if not isinstance(record, dict):
+                    raise ValueError("summary must be an object")
+            except ValueError as error:
+                raise ValueError(f"{path}:{line_number}: {error}") from error
+            if trace is None or record.get("trace") == trace:
+                records.append((line_number, record))
+    if len(records) != 1:
+        selection = f" matching trace {trace!r}" if trace is not None else ""
+        raise ValueError(f"{path}: expected one summary{selection}, found {len(records)}; "
+                         "select a unique trace with --baseline-trace/--candidate-trace "
+                         "or provide a file containing one summary line")
+    line_number, record = records[0]
+    try:
+        return validate(record)
+    except ValueError as error:
+        raise ValueError(f"{path}:{line_number}: {error}") from error
+
+
+def delta(baseline, candidate):
+    change = candidate - baseline if baseline is not None and candidate is not None else None
+    percent = change / baseline * 100 if change is not None and baseline else None
+    if change is not None and not math.isfinite(change):
+        raise ValueError("metric difference is outside the finite numeric range")
+    if percent is not None and not math.isfinite(percent):
+        raise ValueError("metric percentage change is outside the finite numeric range")
+    return {"baseline": baseline, "candidate": candidate, "change": change, "change_pct": percent}
+
+
+def compare(baseline, candidate):
+    metrics = {f"frame_ms.{key}": delta(baseline["frame_ms"][key], candidate["frame_ms"][key])
+               for key in PERCENTILES}
+    metrics["mean_frame_ms"] = delta(baseline["window"]["ms"] / baseline["presents"],
+                                      candidate["window"]["ms"] / candidate["presents"])
+    metrics["gpu_busy_ms_per_present"] = delta(baseline["gpu_busy_ms_per_present"], candidate["gpu_busy_ms_per_present"])
+    before_phases = baseline["top_phases_ms_per_frame"]
+    after_phases = candidate["top_phases_ms_per_frame"]
+    phases = {name: delta(before_phases.get(name), after_phases.get(name))
+              for name in sorted(before_phases.keys() | after_phases.keys())}
+    contexts = []
+    for summary in (baseline, candidate):
+        context = {key: summary[key] for key in CONTEXT}
+        context.update({f"window.{key}": summary["window"][key] for key in ("spec", "first_present")})
+        context.update({f"env.{key}": value for key, value in summary["env"].items()})
+        contexts.append(context)
+    differences = {key: {"baseline": contexts[0].get(key), "candidate": contexts[1].get(key)}
+                   for key in sorted(contexts[0].keys() | contexts[1].keys())
+                   if contexts[0].get(key) != contexts[1].get(key)}
+    notes = ["Compare captures from the same machine, title and scene; summaries do not record the CPU, title or scene.",
+             "Changes are candidate minus baseline. A negative frame-time change means a shorter frame interval.",
+             "A missing top phase is unreported, not zero; its change is unavailable."]
+    if differences:
+        notes.append("Recorded configuration differs; these measurements do not isolate the code change.")
+    for label, summary in (("baseline", baseline), ("candidate", candidate)):
+        if summary["trace_dropped"]:
+            notes.append(f"{label}: trace events were dropped; the event trace omits some of the work included in phase totals.")
+        if summary["dirty"] is not False:
+            notes.append(f"{label}: source tree is dirty or its state is unknown; the commit alone does not identify the build.")
+        missing = [key for key in CONTEXT if not summary[key]]
+        if missing:
+            notes.append(f"{label}: missing configuration: {', '.join(missing)}.")
+    return {"baseline": baseline, "candidate": candidate, "configuration_differences": differences,
+            "metrics_ms": metrics, "top_phases_ms_per_frame": phases, "notes": notes}
+
+
+def cell(value):
+    text = json.dumps(value, ensure_ascii=False) if not isinstance(value, str) else value
+    return text.replace("&", "&amp;").replace("<", "&lt;").replace(">", "&gt;").replace("|", "&#124;").replace("\r", " ").replace("\n", " ")
+
+
+def markdown(report):
+    lines = ["# Performance comparison", "", "| Capture | Commit | Trace | Presents | Window (ms) |",
+             "| --- | --- | --- | ---: | ---: |"]
+    for label in ("baseline", "candidate"):
+        summary = report[label]
+        lines.append(f"| {label} | {cell(summary['commit'])} | {cell(summary['trace'])} | "
+                     f"{summary['presents']} | {summary['window']['ms']:.3f} |")
+    lines.extend(["", *[f"- {note}" for note in report["notes"]]])
+    if report["configuration_differences"]:
+        lines.extend(["", "## Configuration differences", "", "| Setting | Baseline | Candidate |", "| --- | --- | --- |"])
+        for key, values in report["configuration_differences"].items():
+            lines.append(f"| {cell(key)} | {cell(values['baseline'])} | {cell(values['candidate'])} |")
+    for title, key in (("Frame and GPU times (ms)", "metrics_ms"), ("Top phases (ms per frame)", "top_phases_ms_per_frame")):
+        lines.extend(["", f"## {title}", "", "| Metric | Baseline | Candidate | Change | Change (%) |",
+                      "| --- | ---: | ---: | ---: | ---: |"])
+        for name, values in report[key].items():
+            columns = ["n/a" if values[field] is None else format(values[field], "+.3f" if field.startswith("change") else ".3f")
+                       for field in ("baseline", "candidate", "change", "change_pct")]
+            lines.append(f"| {cell(name)} | {' | '.join(columns)} |")
+    return "\n".join(lines) + "\n"
+
+
+def main(argv=None):
+    for stream in (sys.stdout, sys.stderr):
+        if hasattr(stream, "reconfigure"):
+            stream.reconfigure(encoding="utf-8")
+    parser = argparse.ArgumentParser(description="Compare two aps5-summary.jsonl captures. "
+                                     "Reports numerical differences, not statistical significance or an automatic pass/fail verdict.")
+    parser.add_argument("baseline", type=Path, help="Baseline summary JSONL file")
+    parser.add_argument("candidate", type=Path, help="Candidate summary JSONL file")
+    parser.add_argument("--baseline-trace", help="Exact trace field to select in the baseline file")
+    parser.add_argument("--candidate-trace", help="Exact trace field to select in the candidate file")
+    parser.add_argument("--json", action="store_true", help="Write JSON instead of Markdown")
+    args = parser.parse_args(argv)
+    try:
+        report = compare(load_summary(args.baseline, args.baseline_trace), load_summary(args.candidate, args.candidate_trace))
+        output = json.dumps(report, indent=2, ensure_ascii=False, allow_nan=False) + "\n" if args.json else markdown(report)
+    except (OSError, ValueError, OverflowError) as error:
+        parser.exit(2, f"{parser.prog}: {error}\n")
+    sys.stdout.write(output)
+
+
+if __name__ == "__main__":
+    main()

tools/tests/test_performance_compare.py

@@ -0,0 +1,198 @@
+import copy
+import json
+import os
+import subprocess
+import sys
+import tempfile
+import unittest
+from pathlib import Path
+
+sys.path.insert(0, str(Path(__file__).resolve().parents[1]))
+import performance_compare
+
+
+SUMMARY = {
+    "commit": "abc123", "dirty": False, "env": {"APS5_TRACE_FRAMES": "600:4"},
+    "gpu": "GPU", "driver": "Driver 1", "os": "OS 1", "resolution": "1920x1080",
+    "window": {"spec": "frames 600:4", "first_present": 600, "ms": 80.0}, "presents": 4,
+    "frame_ms": {"p50": 20.0, "p90": 40.0, "p99": 40.0, "max": 40.0},
+    "gpu_busy_ms_per_present": 5.0, "worker_busy_pct": {"queue worker 0x0": 50.0},
+    "top_phases_ms_per_frame": {"Driver.Packet": 12.0, "Driver.Present": 2.0},
+    "trace_dropped": False, "trace": "aps5-trace-abc123-7.json.gz",
+}
+
+
+class PerformanceCompareTests(unittest.TestCase):
+    def setUp(self):
+        self.directory = tempfile.TemporaryDirectory()
+        self.addCleanup(self.directory.cleanup)
+        self.baseline = copy.deepcopy(SUMMARY)
+        self.candidate = copy.deepcopy(SUMMARY)
+        self.candidate.update(commit="def456", trace="aps5-trace-def456-8.json.gz")
+        self.before = Path(self.directory.name) / "before.jsonl"
+        self.after = Path(self.directory.name) / "after.jsonl"
+
+    def write(self, path, *records):
+        path.write_text("\n" + "\n\n".join(json.dumps(record) for record in records) + "\n", encoding="utf-8")
+
+    def run_cli(self, *args):
+        return subprocess.run([sys.executable, str(Path(performance_compare.__file__)), str(self.before), str(self.after), *args],
+                              capture_output=True, text=True, encoding="utf-8", timeout=5,
+                              env=dict(os.environ, PYTHONIOENCODING="ascii"))
+
+    def test_timings_and_mean_are_compared_without_inverting_percentiles(self):
+        self.candidate["window"]["ms"] = 40.0
+        self.candidate["frame_ms"] = {"p50": 10.0, "p90": 20.0, "p99": 20.0, "max": 20.0}
+        self.candidate["gpu_busy_ms_per_present"] = 3.0
+        self.write(self.before, self.baseline)
+        self.write(self.after, self.candidate)
+        result = self.run_cli("--json")
+        self.assertEqual(result.returncode, 0, result.stderr)
+        self.assertEqual(result.stderr, "")
+        report = json.loads(result.stdout)
+        self.assertEqual(report["metrics_ms"]["frame_ms.p99"],
+                         {"baseline": 40.0, "candidate": 20.0, "change": -20.0, "change_pct": -50.0})
+        self.assertEqual(report["metrics_ms"]["mean_frame_ms"]["candidate"], 10.0)
+        self.assertEqual(report["metrics_ms"]["gpu_busy_ms_per_present"]["change_pct"], -40.0)
+        self.assertEqual(report["configuration_differences"], {})
+        self.assertEqual(report["baseline"], self.baseline)
+        self.assertEqual(report["candidate"], self.candidate)
+
+    def test_mean_uses_each_captures_own_present_count(self):
+        self.candidate["presents"] = 8
+        report = performance_compare.compare(self.baseline, self.candidate)
+        self.assertEqual(report["metrics_ms"]["mean_frame_ms"]["change_pct"], -50.0)
+
+    def test_zero_baseline_and_missing_top_phases_are_not_fake_improvements(self):
+        self.baseline["gpu_busy_ms_per_present"] = 0.0
+        self.candidate["top_phases_ms_per_frame"] = {"Driver.Packet": 18.0, "Driver.NewPhase": 1.0}
+        report = performance_compare.compare(self.baseline, self.candidate)
+        self.assertIsNone(report["metrics_ms"]["gpu_busy_ms_per_present"]["change_pct"])
+        self.assertEqual(report["metrics_ms"]["gpu_busy_ms_per_present"]["change"], 5.0)
+        self.assertEqual(report["top_phases_ms_per_frame"]["Driver.Packet"]["change_pct"], 50.0)
+        for phase in ("Driver.Present", "Driver.NewPhase"):
+            self.assertIsNone(report["top_phases_ms_per_frame"][phase]["change"])
+            self.assertIsNone(report["top_phases_ms_per_frame"][phase]["change_pct"])
+        self.assertIn("n/a", performance_compare.markdown(report))
+
+    def test_exact_trace_selection_does_not_pick_the_last_session(self):
+        self.write(self.before, self.baseline, self.candidate)
+        self.write(self.after, self.candidate, self.baseline)
+        original = (self.before.read_bytes(), self.after.read_bytes())
+        result = self.run_cli("--baseline-trace", self.baseline["trace"], "--candidate-trace", self.candidate["trace"], "--json")
+        self.assertEqual(result.returncode, 0, result.stderr)
+        report = json.loads(result.stdout)
+        self.assertEqual(report["baseline"]["commit"], "abc123")
+        self.assertEqual(report["candidate"]["commit"], "def456")
+        self.assertEqual((self.before.read_bytes(), self.after.read_bytes()), original)
+
+    def test_empty_ambiguous_missing_and_duplicate_selections_fail(self):
+        self.write(self.after, self.candidate)
+        for records, selector, count in (([], [], 0), ([self.baseline, self.candidate], [], 2),
+                                         ([self.baseline], ["--baseline-trace", "absent"], 0),
+                                         ([self.baseline, self.baseline], ["--baseline-trace", self.baseline["trace"]], 2)):
+            with self.subTest(records=len(records), selector=selector):
+                self.write(self.before, *records)
+                result = self.run_cli(*selector)
+                self.assertEqual(result.returncode, 2)
+                self.assertEqual(result.stdout, "")
+                self.assertIn(f"found {count}", result.stderr)
+                self.assertNotIn("Traceback", result.stderr)
+
+    def test_configuration_differences_and_incomplete_evidence_are_visible(self):
+        self.candidate.update(gpu="Other GPU", driver="Driver 2", os="OS 2", resolution="1280x720",
+                              dirty=True, trace_dropped=True)
+        self.candidate["window"].update(spec="frames 800:4", first_present=800)
+        self.candidate["env"] = {"APS5_TRACE_FRAMES": "800:4", "APS5_WORKER_AFFINITY": "1"}
+        report = performance_compare.compare(self.baseline, self.candidate)
+        self.assertEqual(set(report["configuration_differences"]),
+                         {"gpu", "driver", "os", "resolution", "window.spec", "window.first_present",
+                          "env.APS5_TRACE_FRAMES", "env.APS5_WORKER_AFFINITY"})
+        self.assertIsNone(report["configuration_differences"]["env.APS5_WORKER_AFFINITY"]["baseline"])
+        rendered = performance_compare.markdown(report)
+        for text in ("Other GPU", "Driver 2", "source tree is dirty", "trace events were dropped", "do not isolate"):
+            self.assertIn(text, rendered)
+
+    def test_unknown_metadata_is_not_a_claim_of_equivalent_machines(self):
+        self.baseline.update(dirty=None, gpu="", driver="")
+        report = performance_compare.compare(self.baseline, self.candidate)
+        notes = "\n".join(report["notes"])
+        self.assertIn("state is unknown", notes)
+        self.assertIn("missing configuration: gpu, driver", notes)
+        self.assertIn("do not record the CPU, title or scene", notes)
+
+    def test_invalid_metric_and_metadata_shapes_are_rejected_with_location(self):
+        cases = [(("presents",), value) for value in (0, -1, True, 1.5, 10 ** 400)]
+        cases += [(("window", "ms"), value) for value in (0, -1, True, "10", None, float("nan"), float("inf"))]
+        cases += [(("frame_ms", "p50"), -1), (("frame_ms", "p50"), 50), (("frame_ms",), []),
+                  (("gpu_busy_ms_per_present",), float("-inf")), (("window",), None),
+                  (("window", "first_present"), True), (("env",), []), (("env",), {"APS5_TRACE_FRAMES": 4}),
+                  (("trace_dropped",), "false"), (("dirty",), 1), (("trace",), None),
+                  (("top_phases_ms_per_frame",), []), (("top_phases_ms_per_frame",), {"phase": -1})]
+        for path, value in cases:
+            with self.subTest(path=path, value=value):
+                invalid = copy.deepcopy(self.baseline)
+                target = invalid
+                for key in path[:-1]:
+                    target = target[key]
+                target[path[-1]] = value
+                self.write(self.before, invalid)
+                with self.assertRaisesRegex(ValueError, r"before.jsonl:2:"):
+                    performance_compare.load_summary(self.before)
+
+    def test_bad_json_and_missing_file_produce_only_a_cli_error(self):
+        self.write(self.after, self.candidate)
+        for content in ("{bad json", "[]", "{}", '{"frame_ms": Infinity}'):
+            with self.subTest(content=content):
+                self.before.write_text(content, encoding="utf-8")
+                result = self.run_cli("--json")
+                self.assertEqual(result.returncode, 2)
+                self.assertEqual(result.stdout, "")
+                self.assertIn("before.jsonl:1:", result.stderr)
+                self.assertNotIn("Traceback", result.stderr)
+        self.before.unlink()
+        result = self.run_cli()
+        self.assertEqual(result.returncode, 2)
+        self.assertEqual(result.stdout, "")
+        self.assertNotIn("Traceback", result.stderr)
+
+    def test_percentage_overflow_is_an_error(self):
+        self.baseline["gpu_busy_ms_per_present"] = 1e-308
+        self.candidate["gpu_busy_ms_per_present"] = 1e308
+        self.write(self.before, self.baseline)
+        self.write(self.after, self.candidate)
+        result = self.run_cli("--json")
+        self.assertEqual(result.returncode, 2)
+        self.assertEqual(result.stdout, "")
+        self.assertIn("finite numeric range", result.stderr)
+
+    def test_markdown_reports_deltas_and_escapes_table_values(self):
+        self.candidate["frame_ms"]["max"] = 60.0
+        self.candidate["gpu"] = "GPU | <vendor>\nline"
+        self.write(self.before, self.baseline)
+        self.write(self.after, self.candidate)
+        result = self.run_cli()
+        self.assertEqual(result.returncode, 0, result.stderr)
+        self.assertIn("| frame_ms.max | 40.000 | 60.000 | +20.000 | +50.000 |", result.stdout)
+        self.assertIn("GPU &#124; &lt;vendor&gt; line", result.stdout)
+        self.assertIn("abc123", result.stdout)
+        self.assertIn("def456", result.stdout)
+
+    def test_utf8_paths_and_metadata_work_with_a_non_utf8_output_default(self):
+        self.before = Path(self.directory.name) / "avant-\u00e9.jsonl"
+        self.candidate["gpu"] = "GPU \u03a9"
+        self.write(self.before, self.baseline)
+        self.write(self.after, self.candidate)
+        for args in ((), ("--json",)):
+            with self.subTest(args=args):
+                result = self.run_cli(*args)
+                self.assertEqual(result.returncode, 0, result.stderr)
+                self.assertIn("GPU \u03a9", result.stdout)
+        self.before.write_text("bad", encoding="utf-8")
+        result = self.run_cli()
+        self.assertEqual(result.returncode, 2)
+        self.assertIn("avant-\u00e9.jsonl:1:", result.stderr)
+
+
+if __name__ == "__main__":
+    unittest.main()