diff --git a/scripts/release_performance/_helpers/report.py b/scripts/release_performance/_helpers/report.py index 43eed207c9..350c0c3d50 100644 --- a/scripts/release_performance/_helpers/report.py +++ b/scripts/release_performance/_helpers/report.py @@ -19,7 +19,7 @@ def comparison_markdown(*, comparison: ReleaseComparison) -> str: - """Summarize per-command medians, ratios and the overall verdict as Markdown.""" + """Summarize per-command medians, worst-run peak RSS, ratios and the verdict as Markdown.""" lines: list[str] = [ f"### Release performance: {comparison.candidate_version} (candidate) vs " @@ -28,7 +28,8 @@ def comparison_markdown(*, comparison: ReleaseComparison) -> str: f"Same runner: {comparison.runner.cpu_model}, {comparison.runner.cpu_count} CPUs; " f"load average {_load(comparison.runner.load_average_before)} before and " f"{_load(comparison.runner.load_average_after)} after. " - f"{comparison.runs} interleaved runs per version; medians shown.", + f"{comparison.runs} interleaved runs per version; wall and CPU are medians, " + "peak RSS is the worst run.", "", f"Limits: wall {_percent(MAX_WALL_RATIO)} (ignored under " f"{MIN_WALL_REGRESSION_SECONDS:g} s), CPU {_percent(MAX_CPU_RATIO)} (ignored under " diff --git a/scripts/release_performance/_helpers/verdict.py b/scripts/release_performance/_helpers/verdict.py index d688ac2451..23a6fb0777 100644 --- a/scripts/release_performance/_helpers/verdict.py +++ b/scripts/release_performance/_helpers/verdict.py @@ -21,10 +21,10 @@ def metric_verdicts(*, comparison: CommandComparison) -> tuple[MetricVerdict, ...]: - """Compare median wall time, CPU time and peak RSS of one command.""" + """Compare median wall and CPU time and worst-run peak RSS of one command.""" - baseline: CommandSample = median_sample(samples=comparison.baseline) - candidate: CommandSample = median_sample(samples=comparison.candidate) + baseline: CommandSample = summary_sample(samples=comparison.baseline) + candidate: CommandSample = summary_sample(samples=comparison.candidate) return ( _verdict( command=comparison.name, @@ -72,13 +72,13 @@ def all_verdicts(*, commands: tuple[CommandComparison, ...]) -> tuple[MetricVerd return tuple(verdicts) -def median_sample(*, samples: tuple[CommandSample, ...]) -> CommandSample: - """Return the per-metric median of samples.""" +def summary_sample(*, samples: tuple[CommandSample, ...]) -> CommandSample: + """Return median wall and CPU time and the worst-run peak RSS of samples.""" return CommandSample( wall_seconds=statistics.median(sample.wall_seconds for sample in samples), cpu_seconds=statistics.median(sample.cpu_seconds for sample in samples), - peak_rss_bytes=int(statistics.median(sample.peak_rss_bytes for sample in samples)), + peak_rss_bytes=max(sample.peak_rss_bytes for sample in samples), ) diff --git a/scripts/release_performance/constants.py b/scripts/release_performance/constants.py index 4880c25fb7..7ecb9cd08d 100644 --- a/scripts/release_performance/constants.py +++ b/scripts/release_performance/constants.py @@ -15,7 +15,7 @@ MIN_CPU_REGRESSION_SECONDS: float = 0.5 MIN_RSS_REGRESSION_BYTES: int = 32 * 1024 * 1024 -DEFAULT_RUNS: int = 3 +DEFAULT_RUNS: int = 5 DEFAULT_PYTHON: str = "3.12" DEFAULT_INSPECTION_MODELS: int = 3_000 DEFAULT_BUILD_MODELS: int = 1_000 diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/COMPILE_BENCHMARKS.md b/tests/e2e/src/sqlbuild/cli/commands/main/compile/COMPILE_BENCHMARKS.md index 7df260b12a..e94d8bf957 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/COMPILE_BENCHMARKS.md +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/COMPILE_BENCHMARKS.md @@ -92,8 +92,10 @@ pull request also compares the candidate with the previous published release on `scripts/compare_release_performance.py` installs the candidate wheel and the baseline from PyPI into separate Python 3.12 environments, generates the inspection and build benchmarks above once, and gives each version its own copy with compile, lineage and scope caches warmed by that version. -It then runs each command alternately for baseline and candidate, three times each, and compares -the medians of wall time, CPU time (user+sys) and peak RSS: +It then runs each command alternately for baseline and candidate, five times each, and compares +the medians of wall time and CPU time (user+sys) and the worst-run peak RSS. Peak RSS depends on +whether concurrent phases overlap, so it lands on one of a few levels from run to run; a median +flips between those levels, while the worst run is stable: - warm and uncached `sqb compile --json`, `sqb plan --json`, `sqb dag --json` and `sqb scope --json` on the 3,000-model inspection benchmark; @@ -101,7 +103,7 @@ the medians of wall time, CPU time (user+sys) and peak RSS: `shared_orders_rollup.amount`; - `sqb build` of the 1,000-model build benchmark from an empty warehouse, restored before each run. -A command fails when its candidate median exceeds the baseline by more than 25% and by more than +A command fails when a candidate value exceeds the baseline by more than 25% and by more than 0.5 s (wall and CPU) or 32 MiB (peak RSS); the limits live in `scripts/release_performance/constants.py`. The baseline defaults to the highest version below the candidate that is installable from PyPI, or release-tagged but not yet on PyPI; yanked releases diff --git a/tests/unit/scripts/release_performance/_helpers/test_verdict.py b/tests/unit/scripts/release_performance/_helpers/test_verdict.py index 85a0d051ca..a46d3a46ea 100644 --- a/tests/unit/scripts/release_performance/_helpers/test_verdict.py +++ b/tests/unit/scripts/release_performance/_helpers/test_verdict.py @@ -64,14 +64,53 @@ expected_regressed=(False, True, False), ), MetricVerdictTestCase( - description="one slow outlier does not move the median", + description="one slow outlier does not move the median time", comparison=comparison( name="build (empty warehouse)", baseline=((60.0, 60.0, 700),) * 3, - candidate=((60.0, 60.0, 700), (95.0, 95.0, 1_400), (61.0, 61.0, 710)), + candidate=((60.0, 60.0, 700), (95.0, 95.0, 705), (61.0, 61.0, 710)), ), expected_regressed=(False, False, False), ), + MetricVerdictTestCase( + description="peak RSS landing on the high level more often passes on the worst run", + comparison=comparison( + name="compile (no cache)", + baseline=( + (10.0, 19.0, 640), + (10.0, 19.0, 860), + (10.0, 19.0, 640), + (10.0, 19.0, 640), + (10.0, 19.0, 890), + ), + candidate=( + (10.0, 19.0, 850), + (10.0, 19.0, 860), + (10.0, 19.0, 630), + (10.0, 19.0, 855), + (10.0, 19.0, 868), + ), + ), + expected_regressed=(False, False, False), + ), + MetricVerdictTestCase( + description="one high candidate run fails where the median would pass", + comparison=comparison( + name="compile (no cache)", + baseline=((10.0, 19.0, 640), (10.0, 19.0, 860), (10.0, 19.0, 640)), + candidate=((10.0, 19.0, 640), (10.0, 19.0, 640), (10.0, 19.0, 1_120)), + ), + expected_regressed=(False, False, True), + ), + MetricVerdictTestCase( + description="a larger worst-run peak RSS fails", + comparison=comparison( + name="compile (no cache)", + baseline=((10.0, 19.0, 640), (10.0, 19.0, 860), (10.0, 19.0, 640)), + candidate=((10.0, 19.0, 900), (10.0, 19.0, 1_120), (10.0, 19.0, 910)), + ), + expected_regressed=(False, False, True), + ), ), ids=lambda case: case.description, )