diff --git a/CMakeLists.txt b/CMakeLists.txt index 768e349572..dddbb8fc9f 100644 --- a/CMakeLists.txt +++ b/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() diff --git a/tools/performance_compare.py b/tools/performance_compare.py new file mode 100644 index 0000000000..31131859f9 --- /dev/null +++ b/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("&", "&").replace("<", "<").replace(">", ">").replace("|", "|").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() diff --git a/tools/tests/test_performance_compare.py b/tools/tests/test_performance_compare.py new file mode 100644 index 0000000000..7cba7c6deb --- /dev/null +++ b/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 | \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 | <vendor> 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()