diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1695baf9..86d5aef1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -113,8 +113,10 @@ jobs: # The databases depend on the climate-ref version on each side, which the lock files pin. - name: Key the synthetic database cache id: perf-db + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} run: | - key=$(cat uv.lock <(git show "origin/${{ github.base_ref }}:backend/uv.lock") | sha256sum | cut -c1-16) + key=$(cat uv.lock <(git show "$BASE_SHA:backend/uv.lock") | sha256sum | cut -c1-16) echo "key=perf-db-$key" >> "$GITHUB_OUTPUT" - name: Cache the synthetic databases @@ -126,5 +128,6 @@ jobs: - name: Compare query performance against the base branch env: REF_PERF_ROUNDS: "10" + BASE_SHA: ${{ github.event.pull_request.base.sha }} run: | - scripts/perf-compare.sh "origin/${{ github.base_ref }}" + scripts/perf-compare.sh "$BASE_SHA" diff --git a/backend/scripts/perf-compare.sh b/backend/scripts/perf-compare.sh index 94ff3612..a90cd263 100755 --- a/backend/scripts/perf-compare.sh +++ b/backend/scripts/perf-compare.sh @@ -24,11 +24,16 @@ git -C "$BACKEND" worktree add --quiet --detach "$WORK/base" "$BASE" uv sync --locked --quiet uv run --with pytest-benchmark pytest "$BACKEND/tests/perf" \ --rootdir="$BACKEND" --confcutdir="$BACKEND/tests/perf" -p no:cacheprovider -q \ - --benchmark-storage="$STORAGE" --benchmark-save=base -) || echo "::warning::some performance tests failed on $BASE, comparing the rest" + --benchmark-storage="$STORAGE" --benchmark-save=base \ + --benchmark-json="$WORK/base.json" --junitxml="$WORK/base.xml" +) || true cd "$BACKEND" +status=0 uv run pytest tests/perf -q \ --benchmark-storage="$STORAGE" --benchmark-compare \ --benchmark-compare-fail="median:${REF_PERF_THRESHOLD:-30}%" \ - --benchmark-columns=median,iqr,rounds + --benchmark-columns=median,iqr,rounds --benchmark-json="$WORK/head.json" || status=$? + +python3 scripts/perf-report.py "$WORK" "$BASE" "${REF_PERF_THRESHOLD:-30}" +exit "$status" diff --git a/backend/scripts/perf-report.py b/backend/scripts/perf-report.py new file mode 100644 index 00000000..f0b4517f --- /dev/null +++ b/backend/scripts/perf-report.py @@ -0,0 +1,86 @@ +""" +Summarise a `perf-compare.sh` run as a Markdown table of medians, base against this checkout. + +Usage: python3 scripts/perf-report.py WORK_DIR BASE THRESHOLD + +Reads `base.json`, `base.xml` and `head.json` from WORK_DIR. +The table goes to stdout, and to the job summary when run in GitHub Actions. +Each endpoint with no baseline is also named in a warning annotation. +""" + +import json +import os +import re +import sys +import xml.etree.ElementTree as ET +from pathlib import Path + +_PARAM = re.compile(r"\[(.*)\]$") + + +def _endpoint(name: str) -> str: + match = _PARAM.search(name) + return match.group(1) if match else name + + +def _medians(path: Path) -> dict[str, float]: + if not path.exists(): + return {} + benchmarks = json.loads(path.read_text())["benchmarks"] + return {_endpoint(b["name"]): b["stats"]["median"] * 1000 for b in benchmarks} + + +def _not_run(report: Path) -> dict[str, str]: + """Why each endpoint has no baseline, from the base run's JUnit report.""" + reasons = {} + for case in ET.parse(report).iter("testcase"): # noqa: S314, written by our own pytest run + problems = [p for tag in ("failure", "error", "skipped") if (p := case.find(tag)) is not None] + if problems: + message = problems[0].get("message") or problems[0].tag + reasons[_endpoint(case.get("name", ""))] = message.splitlines()[0][:200].replace("|", "\\|") + return reasons + + +def main() -> None: + work, base, threshold = Path(sys.argv[1]), sys.argv[2], float(sys.argv[3]) + if re.fullmatch(r"[0-9a-f]{40}", base): + base = base[:12] + if not (work / "base.xml").exists(): + sys.exit(f"::error::the performance tests did not run on {base}") + + before, after = _medians(work / "base.json"), _medians(work / "head.json") + not_run = _not_run(work / "base.xml") + for endpoint, reason in not_run.items(): + print(f"::warning title=Not compared::{endpoint} on {base}: {reason.replace('%', '%25')}") + + rounds, scale = os.environ.get("REF_PERF_ROUNDS", "5"), os.environ.get("REF_PERF_SCALE", "0.1") + lines = [ + f"### API performance against `{base}`", + "", + f"Median of {rounds} rounds at scale {scale}. More than {threshold:g}% slower fails the job.", + "", + ] + if not after: + lines.append("No endpoint ran on this branch.") + else: + lines += ["| Endpoint | Base (ms) | This branch (ms) | Change |", "| --- | ---: | ---: | ---: |"] + for endpoint in sorted(after): + if endpoint not in before: + reason = not_run.get(endpoint, "no result") + lines.append(f"| `{endpoint}` | not compared | {after[endpoint]:.1f} | {reason} |") + continue + change = round((after[endpoint] / before[endpoint] - 1) * 100) + flag = " **slower**" if change > threshold else "" + lines.append( + f"| `{endpoint}` | {before[endpoint]:.1f} | {after[endpoint]:.1f} | {change:+d}%{flag} |" + ) + + table = "\n".join(lines) + "\n" + print(table) + if summary := os.environ.get("GITHUB_STEP_SUMMARY"): + with open(summary, "a") as f: + f.write(table) + + +if __name__ == "__main__": + main() diff --git a/backend/tests/perf/README.md b/backend/tests/perf/README.md index 13784f1a..2f039b8d 100644 --- a/backend/tests/perf/README.md +++ b/backend/tests/perf/README.md @@ -30,6 +30,9 @@ make perf-compare BASE=v0.8.2 # against any ref This checks out the base in a temporary worktree and runs this branch's tests against both. It fails when an endpoint's median is more than `REF_PERF_THRESHOLD` percent slower than on the base, default 30. +An endpoint that fails or skips on the base, such as one added by this branch, is not compared. +Each one is named in a warning. +CI writes a table of every endpoint's medians and change to the job summary, from `scripts/perf-report.py`. Running the same code twice locally drifts by up to about 20%, so a smaller threshold is noise. CI runs this on every pull request, and caches the databases until either lock file changes. diff --git a/changelog/149.trivial.md b/changelog/149.trivial.md new file mode 100644 index 00000000..2ffcb0d0 --- /dev/null +++ b/changelog/149.trivial.md @@ -0,0 +1 @@ +The CI performance comparison now uses the pull request's base commit, and writes a table of every endpoint's timings to the job summary, naming each endpoint it could not compare.