diff --git a/.github/workflows/pr-benchmarks.yml b/.github/workflows/pr-benchmarks.yml index 098641916..609ff1ff6 100644 --- a/.github/workflows/pr-benchmarks.yml +++ b/.github/workflows/pr-benchmarks.yml @@ -19,14 +19,29 @@ jobs: benchmark: if: github.event.label.name == 'benchmark' runs-on: ubuntu-24.04 + strategy: + fail-fast: false + matrix: + include: + - name: base + repository: ${{ github.repository }} + ref: ${{ github.event.pull_request.base.sha }} + sha: ${{ github.event.pull_request.base.sha }} + - name: head + repository: ${{ github.event.pull_request.head.repo.full_name }} + ref: ${{ github.event.pull_request.head.sha }} + sha: ${{ github.event.pull_request.head.sha }} permissions: contents: read # checkout only + env: + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + PR_NUMBER: ${{ github.event.pull_request.number }} steps: - - name: Checkout PR head + - name: Checkout ${{ matrix.name }} uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: - repository: ${{ github.event.pull_request.head.repo.full_name }} - ref: ${{ github.event.pull_request.head.sha }} + repository: ${{ matrix.repository }} + ref: ${{ matrix.ref }} persist-credentials: false fetch-depth: 0 @@ -40,22 +55,71 @@ jobs: uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: ~/.m2/repository - key: ${{ runner.os }}-pr-bench-maven-${{ github.event.pull_request.head.sha }} + key: ${{ runner.os }}-pr-bench-maven-${{ matrix.name }}-${{ matrix.sha }} restore-keys: | ${{ runner.os }}-pr-bench-maven- ${{ runner.os }}-maven- - - name: Run JMH benchmarks + - name: Run ${{ matrix.name }} JMH benchmarks run: mise run benchmark:ci-json + - name: Capture ${{ matrix.name }} runner info + run: | + python3 ./.mise/tasks/generate_benchmark_summary.py \ + --write-system-info runner-info.json + + - name: Upload ${{ matrix.name }} raw benchmark results + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: pr-benchmark-raw-${{ matrix.name }}-${{ env.PR_NUMBER }}-${{ env.HEAD_SHA }} + path: | + benchmark-results.json + runner-info.json + retention-days: 5 + + summarize: + if: github.event.label.name == 'benchmark' + runs-on: ubuntu-24.04 + needs: benchmark + permissions: + contents: read # checkout only + env: + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + PR_NUMBER: ${{ github.event.pull_request.number }} + steps: + - name: Checkout PR head + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + repository: ${{ github.event.pull_request.head.repo.full_name }} + ref: ${{ github.event.pull_request.head.sha }} + persist-credentials: false + fetch-depth: 0 + + - name: Download base benchmark results + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: pr-benchmark-raw-base-${{ env.PR_NUMBER }}-${{ env.HEAD_SHA }} + path: /tmp/base-benchmark-results + + - name: Download head benchmark results + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: pr-benchmark-raw-head-${{ env.PR_NUMBER }}-${{ env.HEAD_SHA }} + path: /tmp/head-benchmark-results + - name: Generate benchmark summary run: | - mise run benchmark:generate-summary \ - --input benchmark-results.json \ + python3 ./.mise/tasks/generate_benchmark_summary.py \ + --input /tmp/head-benchmark-results/benchmark-results.json \ + --baseline /tmp/base-benchmark-results/benchmark-results.json \ + --system-info /tmp/head-benchmark-results/runner-info.json \ + --baseline-system-info /tmp/base-benchmark-results/runner-info.json \ --output-dir benchmark-results \ - --commit-sha "${{ github.event.pull_request.head.sha }}" + --commit-sha "${{ github.event.pull_request.head.sha }}" \ + --baseline-sha "${{ github.event.pull_request.base.sha }}" env: GITHUB_REPOSITORY: ${{ github.event.pull_request.head.repo.full_name }} + GITHUB_BASE_REPOSITORY: ${{ github.repository }} - name: Prepare PR comment summary run: | @@ -64,13 +128,13 @@ jobs: - name: Upload PR benchmark results uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: - name: pr-benchmark-results-pr-${{ github.event.pull_request.number }}-${{ github.event.pull_request.head.sha }} + name: pr-benchmark-results-pr-${{ env.PR_NUMBER }}-${{ env.HEAD_SHA }} path: benchmark-results retention-days: 5 - name: Upload PR benchmark comment uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: - name: pr-benchmark-comment-pr-${{ github.event.pull_request.number }}-${{ github.event.pull_request.head.sha }} + name: pr-benchmark-comment-pr-${{ env.PR_NUMBER }}-${{ env.HEAD_SHA }} path: pr-benchmark-comment.md retention-days: 5 diff --git a/.mise/tasks/generate_benchmark_summary.py b/.mise/tasks/generate_benchmark_summary.py index 0b0c4fb01..cd51e8ac0 100644 --- a/.mise/tasks/generate_benchmark_summary.py +++ b/.mise/tasks/generate_benchmark_summary.py @@ -7,7 +7,8 @@ Generate a markdown summary from JMH benchmark JSON results. Usage: - python3 .mise/tasks/generate_benchmark_summary.py [--input results.json] [--output-dir ./benchmark-results] + python3 .mise/tasks/generate_benchmark_summary.py \ + [--input results.json] [--output-dir ./benchmark-results] This script: 1. Reads JMH JSON output @@ -22,7 +23,7 @@ import sys from datetime import datetime, timezone from pathlib import Path -from typing import Dict, List, Optional +from typing import Dict, List, Optional, Tuple def parse_args(): @@ -44,6 +45,36 @@ def parse_args(): default=None, help="Git commit SHA (default: read from git or 'local')", ) + parser.add_argument( + "--baseline", + default=None, + help="Optional JMH JSON results file to compare against", + ) + parser.add_argument( + "--baseline-sha", + default=None, + help="Baseline commit SHA (default: read from git or 'local')", + ) + parser.add_argument( + "--baseline-repo", + default=None, + help="Baseline GitHub repository for commit links", + ) + parser.add_argument( + "--system-info", + default=None, + help="Optional JSON file with system info for the measured run", + ) + parser.add_argument( + "--baseline-system-info", + default=None, + help="Optional JSON file with system info for the baseline run", + ) + parser.add_argument( + "--write-system-info", + default=None, + help="Write current system info to this JSON file and exit", + ) return parser.parse_args() @@ -110,6 +141,39 @@ def get_system_info() -> Dict[str, str]: return info +def read_system_info(path: Optional[str]) -> Dict[str, str]: + """Read system info from JSON, or capture it from the current host.""" + if not path: + return get_system_info() + with open(path, "r") as f: + return json.load(f) + + +def write_system_info(path: str) -> None: + """Write current system info to JSON.""" + output_path = Path(path) + output_path.parent.mkdir(parents=True, exist_ok=True) + with open(output_path, "w") as f: + json.dump(get_system_info(), f, indent=2, sort_keys=True) + f.write("\n") + + +def format_system_info(sysinfo: Optional[Dict[str, str]]) -> str: + """Format captured system info for markdown.""" + if not sysinfo: + return "unknown" + parts = [] + if sysinfo.get("cpu_model"): + parts.append(sysinfo["cpu_model"]) + if sysinfo.get("cpu_cores"): + parts.append(f"{sysinfo['cpu_cores']} cores") + if sysinfo.get("memory_gb"): + parts.append(f"{sysinfo['memory_gb']} GB RAM") + if sysinfo.get("os"): + parts.append(sysinfo["os"]) + return ", ".join(parts) if parts else "unknown" + + def get_commit_sha(provided_sha: Optional[str]) -> str: """Get commit SHA from argument, git, or return 'local'.""" if provided_sha: @@ -134,6 +198,8 @@ def get_commit_sha(provided_sha: Optional[str]) -> str: def format_score(score) -> str: """Format score with appropriate precision.""" + if score is None: + return "" try: val = float(score) if val >= 1_000_000: @@ -160,9 +226,190 @@ def format_error(error) -> str: return "" -def generate_markdown(results: List, commit_sha: str, repo: str) -> str: - """Generate markdown summary from JMH results.""" +def format_commit_link(commit_sha: str, repo: str) -> str: + """Format a commit SHA as a GitHub markdown link.""" commit_short = commit_sha[:7] + if commit_sha != "local": + return f"[`{commit_short}`](https://github.com/{repo}/commit/{commit_sha})" + return f"`{commit_short}` (local run)" + + +def short_benchmark_name(name: str) -> str: + """Remove the common benchmark package prefix.""" + return name.replace("io.prometheus.metrics.benchmarks.", "") + + +def metric_score(result: Dict) -> Optional[float]: + """Extract a benchmark score as a finite float.""" + try: + score = float(result.get("primaryMetric", {}).get("score")) + if score == score: + return score + except (ValueError, TypeError): + pass + return None + + +def score_interval(result: Dict) -> Optional[Tuple[float, float]]: + """Extract the JMH confidence interval for a benchmark result.""" + metric = result.get("primaryMetric", {}) + confidence = metric.get("scoreConfidence") + if isinstance(confidence, list) and len(confidence) == 2: + try: + low = float(confidence[0]) + high = float(confidence[1]) + if low == low and high == high: + return min(low, high), max(low, high) + except (ValueError, TypeError): + pass + + score = metric_score(result) + if score is None: + return None + try: + error = float(metric.get("scoreError")) + if error == error: + return score - error, score + error + except (ValueError, TypeError): + pass + return None + + +def lower_is_better(result: Dict) -> bool: + """Return true for JMH modes where lower score is better.""" + mode = str(result.get("mode", "")) + unit = str(result.get("primaryMetric", {}).get("scoreUnit", "")) + return mode in {"avgt", "sample", "ss"} or unit.endswith("/op") + + +def comparison_status(head: Dict, baseline: Dict) -> str: + """Classify a benchmark comparison using confidence intervals.""" + head_interval = score_interval(head) + baseline_interval = score_interval(baseline) + head_score = metric_score(head) + baseline_score = metric_score(baseline) + if head_score is None or baseline_score is None: + return "" + + is_lower_better = lower_is_better(head) + if head_interval and baseline_interval: + head_low, head_high = head_interval + baseline_low, baseline_high = baseline_interval + if is_lower_better: + if head_high < baseline_low: + return "faster" + if head_low > baseline_high: + return "slower" + else: + if head_low > baseline_high: + return "faster" + if head_high < baseline_low: + return "slower" + return "within noise" + + if is_lower_better: + return "faster" if head_score < baseline_score else "slower" + return "faster" if head_score > baseline_score else "slower" + + +def performance_change(head: Dict, baseline: Dict) -> Optional[float]: + """Return percent performance change, with positive meaning faster.""" + head_score = metric_score(head) + baseline_score = metric_score(baseline) + if head_score is None or baseline_score in (None, 0): + return None + if lower_is_better(head): + return (float(baseline_score) / head_score - 1) * 100 + return (head_score / float(baseline_score) - 1) * 100 + + +def format_change(change: Optional[float]) -> str: + """Format a percent performance change.""" + if change is None: + return "" + return f"{change:+.1f}%" + + +def generate_comparison_section( + results: List, + baseline_results: List, + commit_sha: str, + baseline_sha: str, + repo: str, + baseline_repo: str, + system_info: Optional[Dict[str, str]] = None, + baseline_system_info: Optional[Dict[str, str]] = None, +) -> List[str]: + """Generate a base-vs-head benchmark comparison section.""" + by_name = {b.get("benchmark", ""): b for b in results if b.get("benchmark")} + baseline_by_name = { + b.get("benchmark", ""): b for b in baseline_results if b.get("benchmark") + } + common_names = sorted(set(by_name) & set(baseline_by_name)) + + md = [] + md.append("## Comparison with base") + md.append("") + md.append(f"- **Head:** {format_commit_link(commit_sha, repo)}") + md.append(f"- **Base:** {format_commit_link(baseline_sha, baseline_repo)}") + md.append("- **Change:** positive means the PR is faster than base.") + if baseline_system_info: + md.append(f"- **Head runner:** {format_system_info(system_info)}") + md.append(f"- **Base runner:** {format_system_info(baseline_system_info)}") + md.append( + "- **Note:** base and head run in parallel jobs, so runner " + "hardware can differ and affect results." + ) + md.append("") + + if not common_names: + md.append("_No matching benchmark names were found in the base results._") + md.append("") + return md + + md.append("| Benchmark | PR | Base | Change | Result |") + md.append("|:----------|---:|-----:|-------:|:-------|") + + for name in common_names: + head = by_name[name] + baseline = baseline_by_name[name] + head_score = metric_score(head) + baseline_score = metric_score(baseline) + md.append( + "| " + f"{short_benchmark_name(name)} | " + f"{format_score(head_score)} | " + f"{format_score(baseline_score)} | " + f"{format_change(performance_change(head, baseline))} | " + f"{comparison_status(head, baseline)} |" + ) + + missing_in_base = sorted(set(by_name) - set(baseline_by_name)) + missing_in_head = sorted(set(baseline_by_name) - set(by_name)) + if missing_in_base or missing_in_head: + md.append("") + if missing_in_base: + missing = ", ".join(short_benchmark_name(name) for name in missing_in_base) + md.append(f"- Benchmarks only in PR results: {missing}") + if missing_in_head: + missing = ", ".join(short_benchmark_name(name) for name in missing_in_head) + md.append(f"- Benchmarks only in base results: {missing}") + + md.append("") + return md + + +def generate_markdown( + results: List, + commit_sha: str, + repo: str, + baseline_results: Optional[List] = None, + baseline_sha: Optional[str] = None, + baseline_repo: Optional[str] = None, + system_info: Optional[Dict[str, str]] = None, + baseline_system_info: Optional[Dict[str, str]] = None, +) -> str: + """Generate markdown summary from JMH results.""" datetime_str = datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") # Extract metadata from first result @@ -174,7 +421,7 @@ def generate_markdown(results: List, commit_sha: str, repo: str) -> str: warmup_iters = first.get("warmupIterations", "?") measure_iters = first.get("measurementIterations", "?") - sysinfo = get_system_info() + sysinfo = system_info or get_system_info() md = [] md.append("# Prometheus Java Client Benchmarks") @@ -183,14 +430,12 @@ def generate_markdown(results: List, commit_sha: str, repo: str) -> str: md.append("## Run Information") md.append("") md.append(f"- **Date:** {datetime_str}") - if commit_sha != "local": - md.append( - f"- **Commit:** [`{commit_short}`](https://github.com/{repo}/commit/{commit_sha})" - ) - else: - md.append(f"- **Commit:** `{commit_short}` (local run)") + md.append(f"- **Commit:** {format_commit_link(commit_sha, repo)}") md.append(f"- **JDK:** {jdk_version} ({vm_name})") - bench_cfg = f"{forks} fork(s), {warmup_iters} warmup, {measure_iters} measurement, {threads} threads" + bench_cfg = ( + f"{forks} fork(s), {warmup_iters} warmup, " + f"{measure_iters} measurement, {threads} threads" + ) md.append(f"- **Benchmark config:** {bench_cfg}") hw_parts = [] @@ -207,6 +452,20 @@ def generate_markdown(results: List, commit_sha: str, repo: str) -> str: md.append("") + if baseline_results and baseline_sha and baseline_repo: + md.extend( + generate_comparison_section( + results, + baseline_results, + commit_sha, + baseline_sha, + repo, + baseline_repo, + system_info=sysinfo, + baseline_system_info=baseline_system_info, + ) + ) + # Group by benchmark class benchmarks_by_class: Dict[str, List] = {} for b in results: @@ -219,7 +478,7 @@ def generate_markdown(results: List, commit_sha: str, repo: str) -> str: class_short = "Other" benchmarks_by_class.setdefault(class_short, []).append(b) - md.append("## Results") + md.append("## Results for PR head") md.append("") # Generate table for each class @@ -235,8 +494,8 @@ def generate_markdown(results: List, commit_sha: str, repo: str) -> str: reverse=True, ) - md.append("| Benchmark | Score | Error | Units | |") - md.append("|:----------|------:|------:|:------|:---|") + md.append("| Benchmark | Score | Error | Units | Within run |") + md.append("|:----------|------:|------:|:------|:-----------|") best_score = ( sorted_benchmarks[0].get("primaryMetric", {}).get("score", 1) @@ -280,7 +539,7 @@ def generate_markdown(results: List, commit_sha: str, repo: str) -> str: ) for b in sorted(results, key=lambda x: x.get("benchmark", "")): - name = b.get("benchmark", "").replace("io.prometheus.metrics.benchmarks.", "") + name = short_benchmark_name(b.get("benchmark", "")) mode = b.get("mode", "thrpt") cnt = b.get("measurementIterations", 0) * b.get("forks", 1) score = b.get("primaryMetric", {}).get("score", 0) @@ -312,6 +571,15 @@ def generate_markdown(results: List, commit_sha: str, repo: str) -> str: md.append("") md.append("- **Score** = Throughput in operations per second (higher is better)") md.append("- **Error** = 99.9% confidence interval") + if baseline_results: + md.append( + "- **Comparison with base** uses JMH confidence intervals when " + 'available; overlapping intervals are marked "within noise".' + ) + md.append( + "- **Within run** compares benchmarks in the same result set, not against " + "the base commit." + ) md.append("") md.append("## Benchmark Descriptions") @@ -336,6 +604,11 @@ def generate_markdown(results: List, commit_sha: str, repo: str) -> str: def main(): args = parse_args() + if args.write_system_info: + write_system_info(args.write_system_info) + print(f"Wrote system info to: {args.write_system_info}") + return + input_path = Path(args.input) if not input_path.exists(): print(f"Error: Input file not found: {input_path}") @@ -350,6 +623,27 @@ def main(): commit_sha = get_commit_sha(args.commit_sha) commit_short = commit_sha[:7] repo = os.environ.get("GITHUB_REPOSITORY", "prometheus/client_java") + baseline_results = None + baseline_sha = None + baseline_repo = args.baseline_repo or os.environ.get("GITHUB_BASE_REPOSITORY", repo) + system_info = read_system_info(args.system_info) + baseline_system_info = None + + baseline_path = Path(args.baseline) if args.baseline else None + if baseline_path: + if not baseline_path.exists(): + print(f"Error: Baseline file not found: {baseline_path}") + sys.exit(1) + print(f"Reading baseline results from: {baseline_path}") + with open(baseline_path, "r") as f: + baseline_results = json.load(f) + baseline_sha = get_commit_sha(args.baseline_sha) + baseline_system_info = ( + read_system_info(args.baseline_system_info) + if args.baseline_system_info + else None + ) + print(f"Found {len(baseline_results)} baseline benchmark results") output_dir = Path(args.output_dir) output_dir.mkdir(parents=True, exist_ok=True) @@ -360,12 +654,26 @@ def main(): shutil.copy(input_path, results_json_path) print(f"Copied results to: {results_json_path}") + if baseline_path: + baseline_json_path = output_dir / "baseline-results.json" + shutil.copy(baseline_path, baseline_json_path) + print(f"Copied baseline results to: {baseline_json_path}") + date_str = datetime.now(timezone.utc).strftime("%Y-%m-%d") history_path = history_dir / f"{date_str}-{commit_short}.json" shutil.copy(input_path, history_path) print(f"Saved historical entry: {history_path}") - markdown = generate_markdown(results, commit_sha, repo) + markdown = generate_markdown( + results, + commit_sha, + repo, + baseline_results=baseline_results, + baseline_sha=baseline_sha, + baseline_repo=baseline_repo, + system_info=system_info, + baseline_system_info=baseline_system_info, + ) readme_path = output_dir / "README.md" with open(readme_path, "w") as f: f.write(markdown)