From 1fcb6bcbeac4872c07c2c79262dc8ae9c0c61980 Mon Sep 17 00:00:00 2001 From: xangcastle Date: Thu, 16 Jul 2026 12:16:01 -0600 Subject: [PATCH 01/11] perf(benchmark): gate on Bazel analysis phase and report per-function Starlark cost Rewrite the benchmark to gate on Bazel's analysis phase (runAnalysisPhase extracted from --profile) instead of process wall time, excluding JVM/IO noise. The two separate workspaces are merged into a single parameterized one where --packages drive both the analysis load and a fan-in py_binary whose venv scales with it, and the PR comment now surfaces a per-function Starlark CPU breakdown (via --starlark_cpu_profile) so that when something regresses it tells you which functions got slower not just that it did. --- .github/workflows/performance-benchmark.yml | 139 +++---- benchmark/analysis/.bazelrc | 2 - benchmark/analysis/MODULE.bazel.template | 27 -- benchmark/analysis/compare.py | 217 ----------- benchmark/analysis/workspace/.gitignore | 4 - benchmark/compare.py | 358 ++++++++++++++++++ benchmark/pprof_decode.py | 161 ++++++++ benchmark/profile_benchmark.py | 294 ++++++++++++++ benchmark/startup/BUILD.bazel | 19 - benchmark/startup/MODULE.bazel.template | 26 -- benchmark/startup/compare.py | 232 ------------ benchmark/startup/generate_module.py | 74 ---- benchmark/startup/main.py | 5 - benchmark/startup/requirements.txt | 4 - benchmark/startup/syspath_probe.py | 35 -- benchmark/workspace/.bazelrc | 2 + benchmark/workspace/.gitignore | 8 + .../MODULE.bazel.template} | 19 +- .../generate_module.py | 4 +- .../workspace/generate_workspace.py | 69 +++- .../{analysis => }/workspace/pyproject.toml | 0 benchmark/{analysis => }/workspace/uv.lock | 0 22 files changed, 951 insertions(+), 748 deletions(-) delete mode 100644 benchmark/analysis/.bazelrc delete mode 100644 benchmark/analysis/MODULE.bazel.template delete mode 100644 benchmark/analysis/compare.py delete mode 100644 benchmark/analysis/workspace/.gitignore create mode 100644 benchmark/compare.py create mode 100644 benchmark/pprof_decode.py create mode 100644 benchmark/profile_benchmark.py delete mode 100644 benchmark/startup/BUILD.bazel delete mode 100644 benchmark/startup/MODULE.bazel.template delete mode 100644 benchmark/startup/compare.py delete mode 100644 benchmark/startup/generate_module.py delete mode 100644 benchmark/startup/main.py delete mode 100644 benchmark/startup/requirements.txt delete mode 100644 benchmark/startup/syspath_probe.py create mode 100644 benchmark/workspace/.bazelrc create mode 100644 benchmark/workspace/.gitignore rename benchmark/{analysis/MODULE.bazel => workspace/MODULE.bazel.template} (56%) rename benchmark/{analysis => workspace}/generate_module.py (94%) rename benchmark/{analysis => }/workspace/generate_workspace.py (74%) rename benchmark/{analysis => }/workspace/pyproject.toml (100%) rename benchmark/{analysis => }/workspace/uv.lock (100%) diff --git a/.github/workflows/performance-benchmark.yml b/.github/workflows/performance-benchmark.yml index 8e6e1cf39..e99b837b9 100644 --- a/.github/workflows/performance-benchmark.yml +++ b/.github/workflows/performance-benchmark.yml @@ -6,9 +6,12 @@ on: branches: [main] workflow_dispatch: +permissions: + pull-requests: write + env: - BCR_VERSION: "1.11.7" - ANALYSIS_BCR_VERSION: "2.0.0-alpha.4" + BCR_VERSION: "2.0.0-alpha.4" + PACKAGES: "50" jobs: startup: @@ -39,77 +42,59 @@ jobs: rm -f "$DEB" hyperfine --version + - name: Generate workspace + run: | + set -euo pipefail + cd rules_py_pr/benchmark/workspace + python3 generate_workspace.py --root . --packages "${PACKAGES}" + # ── BCR baseline ──────────────────────────────────────────────────────── - name: Benchmark BCR ${{ env.BCR_VERSION }} run: | set -euo pipefail - cd rules_py_pr/benchmark/startup + cd rules_py_pr/benchmark/workspace python3 generate_module.py bcr --version "${BCR_VERSION}" OUT_BASE="/tmp/bazel-bcr" rm -rf "$OUT_BASE" - - START=$(date +%s%N) - bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc build --disk_cache= //:bench //:bench_syspath - END=$(date +%s%N) - BUILD_MS=$(( (END - START) / 1000000 )) - echo "{\"build_ms\": $BUILD_MS}" > "${GITHUB_WORKSPACE}/bcr-build.json" + bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc build --disk_cache= //:bench BIN=$(bazel --output_base="$OUT_BASE" cquery //:bench --disk_cache= --output=starlark --starlark:expr='target.files_to_run.executable.path' | tail -n1) - test -x "$BIN" || { echo "ERROR: benchmark binary not executable: $BIN"; exit 1; } + test -x "$BIN" || { echo "ERROR: bench binary not executable: $BIN"; exit 1; } RUNFILES_DIR="$PWD/$BIN.runfiles" hyperfine --warmup 5 --runs 50 --export-json "${GITHUB_WORKSPACE}/bcr.json" "$BIN" - - BIN_SP=$(bazel --output_base="$OUT_BASE" cquery //:bench_syspath --disk_cache= --output=starlark --starlark:expr='target.files_to_run.executable.path' | tail -n1) - test -x "$BIN_SP" || { echo "ERROR: bench_syspath binary not executable: $BIN_SP"; exit 1; } - RUNFILES_DIR="$PWD/$BIN_SP.runfiles" "$BIN_SP" "$GITHUB_WORKSPACE/bcr-syspath.json" + RUNFILES_DIR="$PWD/$BIN.runfiles" "$BIN" "${GITHUB_WORKSPACE}/bcr-syspath.json" # ── HEAD main ─────────────────────────────────────────────────────────── - name: Benchmark HEAD main run: | set -euo pipefail - cd rules_py_pr/benchmark/startup + cd rules_py_pr/benchmark/workspace python3 generate_module.py local --path "$GITHUB_WORKSPACE/rules_py_main" OUT_BASE="/tmp/bazel-main" rm -rf "$OUT_BASE" - - START=$(date +%s%N) - bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc build --disk_cache= //:bench //:bench_syspath - END=$(date +%s%N) - BUILD_MS=$(( (END - START) / 1000000 )) - echo "{\"build_ms\": $BUILD_MS}" > "${GITHUB_WORKSPACE}/main-build.json" + bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc build --disk_cache= //:bench BIN=$(bazel --output_base="$OUT_BASE" cquery //:bench --disk_cache= --output=starlark --starlark:expr='target.files_to_run.executable.path' | tail -n1) - test -x "$BIN" || { echo "ERROR: benchmark binary not executable: $BIN"; exit 1; } + test -x "$BIN" || { echo "ERROR: bench binary not executable: $BIN"; exit 1; } RUNFILES_DIR="$PWD/$BIN.runfiles" hyperfine --warmup 5 --runs 50 --export-json "${GITHUB_WORKSPACE}/main.json" "$BIN" - - BIN_SP=$(bazel --output_base="$OUT_BASE" cquery //:bench_syspath --disk_cache= --output=starlark --starlark:expr='target.files_to_run.executable.path' | tail -n1) - test -x "$BIN_SP" || { echo "ERROR: bench_syspath binary not executable: $BIN_SP"; exit 1; } - RUNFILES_DIR="$PWD/$BIN_SP.runfiles" "$BIN_SP" "$GITHUB_WORKSPACE/main-syspath.json" + RUNFILES_DIR="$PWD/$BIN.runfiles" "$BIN" "${GITHUB_WORKSPACE}/main-syspath.json" # ── Current commit (PR) ───────────────────────────────────────────────── - name: Benchmark current PR run: | set -euo pipefail - cd rules_py_pr/benchmark/startup + cd rules_py_pr/benchmark/workspace python3 generate_module.py local --path "$GITHUB_WORKSPACE/rules_py_pr" OUT_BASE="/tmp/bazel-pr" rm -rf "$OUT_BASE" - - START=$(date +%s%N) - bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc build --disk_cache= //:bench //:bench_syspath - END=$(date +%s%N) - BUILD_MS=$(( (END - START) / 1000000 )) - echo "{\"build_ms\": $BUILD_MS}" > "${GITHUB_WORKSPACE}/pr-build.json" + bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc build --disk_cache= //:bench BIN=$(bazel --output_base="$OUT_BASE" cquery //:bench --disk_cache= --output=starlark --starlark:expr='target.files_to_run.executable.path' | tail -n1) - test -x "$BIN" || { echo "ERROR: benchmark binary not executable: $BIN"; exit 1; } + test -x "$BIN" || { echo "ERROR: bench binary not executable: $BIN"; exit 1; } RUNFILES_DIR="$PWD/$BIN.runfiles" hyperfine --warmup 5 --runs 50 --export-json "${GITHUB_WORKSPACE}/pr.json" "$BIN" - - BIN_SP=$(bazel --output_base="$OUT_BASE" cquery //:bench_syspath --disk_cache= --output=starlark --starlark:expr='target.files_to_run.executable.path' | tail -n1) - test -x "$BIN_SP" || { echo "ERROR: bench_syspath binary not executable: $BIN_SP"; exit 1; } - RUNFILES_DIR="$PWD/$BIN_SP.runfiles" "$BIN_SP" "$GITHUB_WORKSPACE/pr-syspath.json" + RUNFILES_DIR="$PWD/$BIN.runfiles" "$BIN" "${GITHUB_WORKSPACE}/pr-syspath.json" # ── Compare ───────────────────────────────────────────────────────────── - name: Compare startup results @@ -117,7 +102,7 @@ jobs: continue-on-error: true run: | set -euo pipefail - python3 rules_py_pr/benchmark/startup/compare.py \ + python3 rules_py_pr/benchmark/compare.py startup \ --output-table startup-table.txt \ bcr.json main.json pr.json @@ -156,84 +141,79 @@ jobs: ref: main path: rules_py_main - - name: Install hyperfine - run: | - set -euo pipefail - HYPERFINE_VERSION="1.18.0" - DEB="hyperfine_${HYPERFINE_VERSION}_amd64.deb" - URL="https://github.com/sharkdp/hyperfine/releases/download/v${HYPERFINE_VERSION}/${DEB}" - wget -qO "$DEB" "$URL" - sudo dpkg -i "$DEB" - rm -f "$DEB" - hyperfine --version - - name: Generate workspace run: | set -euo pipefail - cd rules_py_pr/benchmark/analysis - python3 workspace/generate_workspace.py --root workspace --packages 50 + cd rules_py_pr/benchmark/workspace + python3 generate_workspace.py --root . --packages "${PACKAGES}" # ── BCR baseline ──────────────────────────────────────────────────────── - - name: Benchmark BCR ${{ env.ANALYSIS_BCR_VERSION }} + - name: Benchmark BCR ${{ env.BCR_VERSION }} run: | set -euo pipefail - cd rules_py_pr/benchmark/analysis - python3 generate_module.py bcr --version "${ANALYSIS_BCR_VERSION}" + cd rules_py_pr/benchmark/workspace + python3 generate_module.py bcr --version "${BCR_VERSION}" OUT_BASE="/tmp/bazel-bcr" rm -rf "$OUT_BASE" + bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc fetch //... - bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc fetch //workspace/... - - hyperfine --warmup 1 --runs 10 \ + python3 ../profile_benchmark.py \ + --runs 10 --warmup 1 \ --prepare 'rm -rf /tmp/bazel-bcr-analysis' \ - --export-json "${GITHUB_WORKSPACE}/bcr.json" \ - 'bazel --output_base=/tmp/bazel-bcr-analysis --bazelrc=../../.github/workflows/ci.bazelrc build --disk_cache= --nobuild //workspace/...' + --output "${GITHUB_WORKSPACE}/bcr.json" \ + --save-profile "${GITHUB_WORKSPACE}/profiles/bcr-profile.gz" \ + --save-starlark "${GITHUB_WORKSPACE}/profiles/bcr-starlark.pprof.gz" \ + -- bazel --output_base=/tmp/bazel-bcr-analysis --bazelrc=../../.github/workflows/ci.bazelrc build --disk_cache= --nobuild --profile={PROFILE} --starlark_cpu_profile={STARPROFILE} //... - bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc query //workspace/... --output=package > /tmp/bcr-packages.txt - bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc query //workspace/... > /tmp/bcr-targets.txt + bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc query //... --output=package > /tmp/bcr-packages.txt + bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc query //... > /tmp/bcr-targets.txt echo "{\"packages\": $(wc -l < /tmp/bcr-packages.txt | tr -d ' '), \"targets\": $(wc -l < /tmp/bcr-targets.txt | tr -d ' ')}" > "${GITHUB_WORKSPACE}/bcr-aux.json" # ── HEAD main ─────────────────────────────────────────────────────────── - name: Benchmark HEAD main run: | set -euo pipefail - cd rules_py_pr/benchmark/analysis + cd rules_py_pr/benchmark/workspace python3 generate_module.py local --path "$GITHUB_WORKSPACE/rules_py_main" OUT_BASE="/tmp/bazel-main" rm -rf "$OUT_BASE" + bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc fetch //... - bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc fetch //workspace/... - - hyperfine --warmup 1 --runs 10 \ + python3 ../profile_benchmark.py \ + --runs 10 --warmup 1 \ --prepare 'rm -rf /tmp/bazel-main-analysis' \ - --export-json "${GITHUB_WORKSPACE}/main.json" \ - 'bazel --output_base=/tmp/bazel-main-analysis --bazelrc=../../.github/workflows/ci.bazelrc build --disk_cache= --nobuild //workspace/...' + --output "${GITHUB_WORKSPACE}/main.json" \ + --save-profile "${GITHUB_WORKSPACE}/profiles/main-profile.gz" \ + --save-starlark "${GITHUB_WORKSPACE}/profiles/main-starlark.pprof.gz" \ + -- bazel --output_base=/tmp/bazel-main-analysis --bazelrc=../../.github/workflows/ci.bazelrc build --disk_cache= --nobuild --profile={PROFILE} --starlark_cpu_profile={STARPROFILE} //... - bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc query //workspace/... --output=package > /tmp/main-packages.txt - bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc query //workspace/... > /tmp/main-targets.txt + bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc query //... --output=package > /tmp/main-packages.txt + bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc query //... > /tmp/main-targets.txt echo "{\"packages\": $(wc -l < /tmp/main-packages.txt | tr -d ' '), \"targets\": $(wc -l < /tmp/main-targets.txt | tr -d ' ')}" > "${GITHUB_WORKSPACE}/main-aux.json" # ── Current commit (PR) ───────────────────────────────────────────────── - name: Benchmark current PR run: | set -euo pipefail - cd rules_py_pr/benchmark/analysis + cd rules_py_pr/benchmark/workspace python3 generate_module.py local --path "$GITHUB_WORKSPACE/rules_py_pr" OUT_BASE="/tmp/bazel-pr" rm -rf "$OUT_BASE" + bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc fetch //... - bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc fetch //workspace/... - - hyperfine --warmup 1 --runs 10 \ + python3 ../profile_benchmark.py \ + --runs 10 --warmup 1 \ --prepare 'rm -rf /tmp/bazel-pr-analysis' \ - --export-json "${GITHUB_WORKSPACE}/pr.json" \ - 'bazel --output_base=/tmp/bazel-pr-analysis --bazelrc=../../.github/workflows/ci.bazelrc build --disk_cache= --nobuild //workspace/...' + --output "${GITHUB_WORKSPACE}/pr.json" \ + --save-profile "${GITHUB_WORKSPACE}/profiles/pr-profile.gz" \ + --save-starlark "${GITHUB_WORKSPACE}/profiles/pr-starlark.pprof.gz" \ + -- bazel --output_base=/tmp/bazel-pr-analysis --bazelrc=../../.github/workflows/ci.bazelrc build --disk_cache= --nobuild --profile={PROFILE} --starlark_cpu_profile={STARPROFILE} //... - bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc query //workspace/... --output=package > /tmp/pr-packages.txt - bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc query //workspace/... > /tmp/pr-targets.txt + bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc query //... --output=package > /tmp/pr-packages.txt + bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc query //... > /tmp/pr-targets.txt echo "{\"packages\": $(wc -l < /tmp/pr-packages.txt | tr -d ' '), \"targets\": $(wc -l < /tmp/pr-targets.txt | tr -d ' ')}" > "${GITHUB_WORKSPACE}/pr-aux.json" # ── Compare ───────────────────────────────────────────────────────────── @@ -242,7 +222,7 @@ jobs: continue-on-error: true run: | set -euo pipefail - python3 rules_py_pr/benchmark/analysis/compare.py \ + python3 rules_py_pr/benchmark/compare.py analysis \ --output-table analysis-table.txt \ bcr.json main.json pr.json @@ -253,6 +233,7 @@ jobs: name: analysis-benchmark-results path: | analysis-table.txt + profiles/ if-no-files-found: warn - name: Fail on regression diff --git a/benchmark/analysis/.bazelrc b/benchmark/analysis/.bazelrc deleted file mode 100644 index 1efb76ffc..000000000 --- a/benchmark/analysis/.bazelrc +++ /dev/null @@ -1,2 +0,0 @@ -# Default dependency group for the analysis benchmark workload. -common --@pypi//dep_group=default diff --git a/benchmark/analysis/MODULE.bazel.template b/benchmark/analysis/MODULE.bazel.template deleted file mode 100644 index 0ec46287c..000000000 --- a/benchmark/analysis/MODULE.bazel.template +++ /dev/null @@ -1,27 +0,0 @@ -"Benchmark workspace for Bazel analysis phase performance" - -module(name = "analysis_benchmark") - -{{RULES_PY_DECLARATION}} - -bazel_dep(name = "bazel_skylib", version = "1.9.0") -bazel_dep(name = "rules_python", version = "1.9.0") - -# Python interpreters provisioned from python-build-standalone via aspect_rules_py -interpreters = use_extension("@aspect_rules_py//py:extensions.bzl", "python_interpreters") -interpreters.toolchain( - python_version = "3.11", -) -use_repo(interpreters, "python_interpreters") - -register_toolchains("@python_interpreters//:all") - -# UV-based PyPI hub used to stress the rules_py analysis extension. -uv = use_extension("@aspect_rules_py//uv:extensions.bzl", "uv") -uv.declare_hub(hub_name = "pypi") -uv.project( - hub_name = "pypi", - lock = "//workspace:uv.lock", - pyproject = "//workspace:pyproject.toml", -) -use_repo(uv, "pypi") diff --git a/benchmark/analysis/compare.py b/benchmark/analysis/compare.py deleted file mode 100644 index 6760b9e39..000000000 --- a/benchmark/analysis/compare.py +++ /dev/null @@ -1,217 +0,0 @@ -#!/usr/bin/env python3 -"""Parse hyperfine JSON output for `bazel build --nobuild //...`, build a markdown - table, and exit 1 on regression. - -The regression gate compares PR against HEAD main (not BCR). -BCR is kept as a historical baseline for context, but gating against it is -misleading because transitive dependency versions drift between releases. -""" - -from __future__ import annotations - -import argparse -import json -import os -import sys -from pathlib import Path -from typing import Any - -THRESHOLD_REGRESSION_PCT = 10 # fail CI if PR is >10% slower than HEAD main - - -def write_gh_output(text: str) -> None: - """Write to GITHUB_OUTPUT if available, so sticky PR comment always has content.""" - gh_output = os.environ.get("GITHUB_OUTPUT") - if gh_output: - with open(gh_output, "a") as f: - f.write("table\u003c\u003cEOF\n") - f.write(text) - f.write("EOF\n") - - -def load_runtime(path: str) -> dict[str, Any]: - """Load a single hyperfine JSON result.""" - p = Path(path) - if not p.exists(): - msg = f"ERROR: result file not found: {path}" - print(msg, file=sys.stderr) - write_gh_output(f"❌ {msg}") - sys.exit(2) - - with p.open() as f: - data = json.load(f) - - if "results" not in data or not data["results"]: - msg = f"ERROR: no results in {path}" - print(msg, file=sys.stderr) - write_gh_output(f"❌ {msg}") - sys.exit(2) - - r = data["results"][0] - for key in ("mean", "stddev", "min", "max", "median"): - if key not in r: - msg = f"ERROR: missing '{key}' in {path}" - print(msg, file=sys.stderr) - write_gh_output(f"❌ {msg}") - sys.exit(2) - - return { - "mean_ms": r["mean"] * 1000, - "stddev_ms": r["stddev"] * 1000, - "min_ms": r["min"] * 1000, - "max_ms": r["max"] * 1000, - "median_ms": r["median"] * 1000, - } - - -def load_auxiliary(path: str) -> dict[str, Any] | None: - """Load optional auxiliary metrics JSON emitted by the benchmark harness.""" - p = Path(path) - if not p.exists(): - return None - with p.open() as f: - return json.load(f) - - -def pct(a: float, b: float) -> float: - """Percentage delta from a to b.""" - if a == 0: - return 0.0 - return (b - a) / a * 100 - - -def fmt(val: float) -> str: - """Format milliseconds with sensible precision.""" - return f"{val:.3f}" - - -def warn(delta: float) -> str: - """Return warning emoji if delta exceeds threshold.""" - return "⚠️" if delta > THRESHOLD_REGRESSION_PCT else "" - - -def main() -> None: - parser = argparse.ArgumentParser(description="Compare analysis benchmark results") - parser.add_argument("bcr", help="BCR hyperfine JSON") - parser.add_argument("main", help="HEAD main hyperfine JSON") - parser.add_argument("pr", help="PR hyperfine JSON") - parser.add_argument( - "--output-table", - help="Write only the markdown table to this file instead of stdout", - ) - args = parser.parse_args() - - bcr_path, main_path, pr_path = args.bcr, args.main, args.pr - - bcr = load_runtime(bcr_path) - main = load_runtime(main_path) - pr = load_runtime(pr_path) - - bcr_aux = load_auxiliary(bcr_path.replace(".json", "-aux.json")) - main_aux = load_auxiliary(main_path.replace(".json", "-aux.json")) - pr_aux = load_auxiliary(pr_path.replace(".json", "-aux.json")) - - main_vs_bcr = pct(bcr["mean_ms"], main["mean_ms"]) - pr_vs_bcr = pct(bcr["mean_ms"], pr["mean_ms"]) - pr_vs_main = pct(main["mean_ms"], pr["mean_ms"]) - - has_aux = bcr_aux is not None or main_aux is not None or pr_aux is not None - - table = "## Bazel analysis benchmark\n\n" - if has_aux: - table += "| Version | Mean (ms) | Median (ms) | ± stddev | vs BCR | vs main | Packages | Targets |\n" - table += "|---------|-----------|-------------|----------|--------|---------|----------|----------|\n" - else: - table += "| Version | Mean (ms) | Median (ms) | ± stddev | vs BCR | vs main |\n" - table += "|---------|-----------|-------------|----------|--------|---------|\n" - - def aux_cell(aux: dict[str, Any] | None) -> str: - if aux is None: - return "— | —" - packages = aux.get("packages", "—") - targets = aux.get("targets", "—") - return f"{packages} | {targets}" - - def row( - label: str, - d: dict[str, Any], - vs_bcr: str, - vs_main: str, - aux: dict[str, Any] | None, - ) -> str: - line = ( - f"| {label} | {fmt(d['mean_ms'])} | {fmt(d['median_ms'])} | " - f"±{fmt(d['stddev_ms'])} | {vs_bcr} | {vs_main}" - ) - if has_aux: - line += f" | {aux_cell(aux)}" - line += " |\n" - return line - - table += row( - "BCR 2.0.0-alpha.4 (baseline)", bcr, "—", "—", bcr_aux - ) - table += row( - "HEAD main", - main, - f"{main_vs_bcr:+.1f}% {warn(main_vs_bcr)}", - "—", - main_aux, - ) - table += row( - "This PR", - pr, - f"{pr_vs_bcr:+.1f}% {warn(pr_vs_bcr)}", - f"{pr_vs_main:+.1f}% {warn(pr_vs_main)}", - pr_aux, - ) - - table += ( - f"\n> Measured with `hyperfine --warmup 1 --runs 10` on " - f"`{os.environ.get('RUNNER_OS', 'local')}`\n" - ) - table += ( - f"> **Gate**: PR vs HEAD main (threshold: {THRESHOLD_REGRESSION_PCT}%). " - f"BCR is shown only as a historical baseline.\n" - ) - table += ( - "> **Command**: cold `bazel build --nobuild //workspace/...` with isolated output base, " - "no disk cache.\n" - ) - - if has_aux: - table += ( - "\n### Auxiliary metrics\n\n" - "| Version | Loaded packages | Configured targets |\n" - "|---------|-----------------|---------------------|\n" - ) - - def aux_row(label: str, aux: dict[str, Any] | None) -> str: - if aux is None: - return f"| {label} | — | — |\n" - return f"| {label} | {aux.get('packages', '—')} | {aux.get('targets', '—')} |\n" - - table += aux_row("BCR 2.0.0-alpha.4 (baseline)", bcr_aux) - table += aux_row("HEAD main", main_aux) - table += aux_row("This PR", pr_aux) - - write_gh_output(table) - - if args.output_table: - Path(args.output_table).write_text(table) - else: - print(table) - - if pr_vs_main > THRESHOLD_REGRESSION_PCT: - print( - f"\n❌ REGRESSION: PR is {pr_vs_main:.1f}% slower than HEAD main " - f"(threshold: {THRESHOLD_REGRESSION_PCT}%)" - ) - sys.exit(1) - - print(f"\n✅ No regression detected (PR is {pr_vs_main:+.1f}% vs HEAD main)") - sys.exit(0) - - -if __name__ == "__main__": - main() diff --git a/benchmark/analysis/workspace/.gitignore b/benchmark/analysis/workspace/.gitignore deleted file mode 100644 index a6d217b72..000000000 --- a/benchmark/analysis/workspace/.gitignore +++ /dev/null @@ -1,4 +0,0 @@ -# Generated synthetic workload; do not edit manually. -src/ -tests/ -BUILD.bazel diff --git a/benchmark/compare.py b/benchmark/compare.py new file mode 100644 index 000000000..1ae88b337 --- /dev/null +++ b/benchmark/compare.py @@ -0,0 +1,358 @@ +#!/usr/bin/env python3 +"""Unified benchmark results comparator. + +Two subcommands, each gating PR vs HEAD main at THRESHOLD_REGRESSION_PCT: + - analysis : profile_benchmark JSON (analysis_ms from runAnalysisPhase). + Also renders the per-function Starlark CPU diagnostic section. + - startup : hyperfine runtime JSON for the fan-in //:bench py_binary. + +BCR is shown as a historical baseline only (transitive dep versions drift +between releases, so gating against it is misleading). See tdr.md and +docs/superpowers/specs/*-design.md. +""" +from __future__ import annotations + +import argparse +import json +import os +import sys +from pathlib import Path +from typing import Any + +THRESHOLD_REGRESSION_PCT = 10 +EM = "\u2014" # em dash; module constant so it can appear inside f-string expressions + + +def write_gh_output(text: str) -> None: + """Write to GITHUB_OUTPUT if available, so the sticky PR comment always has content.""" + gh_output = os.environ.get("GITHUB_OUTPUT") + if gh_output: + with open(gh_output, "a") as f: + f.write("table< float: + """Percentage delta from a to b.""" + if a == 0: + return 0.0 + return (b - a) / a * 100 + + +def fmt(val: float) -> str: + return f"{val:.3f}" + + +def warn(delta: float) -> str: + return "\u26a0\ufe0f" if delta > THRESHOLD_REGRESSION_PCT else "" + + +# --------------------------------------------------------------------------- # +# analysis subcommand +# --------------------------------------------------------------------------- # + +def load_result(path: str) -> dict[str, Any]: + """Load a profile_benchmark JSON result and validate the analysis_ms metric.""" + p = Path(path) + if not p.exists(): + _fail(f"result file not found: {path}") + with p.open() as f: + data = json.load(f) + if "analysis_ms" not in data: + _fail(f"missing 'analysis_ms' in {path}") + for key in ("mean", "median", "stddev"): + if key not in data["analysis_ms"]: + _fail(f"missing 'analysis_ms.{key}' in {path}") + return data + + +def load_auxiliary(path: str) -> dict[str, Any] | None: + """Load optional auxiliary metrics JSON emitted by the benchmark harness.""" + p = Path(path) + if not p.exists(): + return None + with p.open() as f: + return json.load(f) + + +def is_regression(main_result: dict[str, Any], pr_result: dict[str, Any], threshold: float) -> bool: + """True iff PR analysis mean is more than `threshold`% slower than main.""" + return pct(main_result["analysis_ms"]["mean"], pr_result["analysis_ms"]["mean"]) > threshold + + +def _short(name: str, limit: int = 48) -> str: + """Shorten verbose pprof function names (e.g. MODULE.bazel URLs).""" + if "MODULE.bazel" in name and "/" in name: + name = name.rsplit("/", 1)[-1] + if len(name) > limit: + name = name[: limit - 1] + "\u2026" + return name + + +def _starlark_section(main_result: dict[str, Any], pr_result: dict[str, Any]) -> str: + """Diagnostic 'where is the problem' diff of per-function Starlark CPU. + + Informational only -- does not affect the gate. Requires both main and PR to + carry a starlark_fn breakdown. + """ + main_fn = main_result.get("starlark_fn") or [] + pr_fn = pr_result.get("starlark_fn") or [] + if not main_fn or not pr_fn: + return "" + + main_map = {r["name"]: r["mean_ms"] for r in main_fn} + pr_map = {r["name"]: r["mean_ms"] for r in pr_fn} + pr_pct = {r["name"]: r.get("pct", 0.0) for r in pr_fn} + + movers = [] + for name, pr_ms in pr_map.items(): + m_ms = main_map.get(name, 0.0) + delta = pr_ms - m_ms + if delta > 0: + movers.append((name, m_ms, pr_ms, delta)) + movers.sort(key=lambda x: x[3], reverse=True) + + out = "\n### \U0001f50d Starlark CPU \u2014 where the problem is (PR vs main)\n\n" + if movers: + out += "**Top movers (biggest regression \u0394ms):**\n\n" + out += "| Function | main ms | PR ms | \u0394 ms | \u0394 % |\n|---|---|---|---|---|\n" + for name, m_ms, pr_ms, delta in movers[:10]: + if m_ms > 0: + dpct = f"+{delta / m_ms * 100:.0f}%" + else: + dpct = "new" + flag = " \u26a0\ufe0f" if (m_ms > 0 and delta / m_ms * 100 > THRESHOLD_REGRESSION_PCT) else "" + out += ( + f"| `{_short(name)}` | {m_ms:.1f} | {pr_ms:.1f} | " + f"+{delta:.1f} | {dpct}{flag} |\n" + ) + else: + out += "_No movers (no function got slower in PR)._ \n" + + abs_top = sorted(pr_map.items(), key=lambda kv: kv[1], reverse=True)[:10] + out += "\n**Top by absolute time (PR):**\n\n" + out += "| Function | PR ms | % of total |\n|---|---|---|\n" + for name, ms in abs_top: + out += f"| `{_short(name)}` | {ms:.1f} | {pr_pct.get(name, 0.0):.1f}% |\n" + + out += ( + "\n> Sample-based attribution (Starlark CPU across the whole `--nobuild` run: " + "bzlmod loading + analysis). Noisy for small functions; builtins are " + "attributed to the caller.\n" + ) + return out + + +def run_analysis(args: argparse.Namespace) -> int: + bcr_path, main_path, pr_path = args.bcr, args.main, args.pr + + bcr = load_result(bcr_path) + main = load_result(main_path) + pr = load_result(pr_path) + + bcr_aux = load_auxiliary(bcr_path.replace(".json", "-aux.json")) + main_aux = load_auxiliary(main_path.replace(".json", "-aux.json")) + pr_aux = load_auxiliary(pr_path.replace(".json", "-aux.json")) + + main_vs_bcr = pct(bcr["analysis_ms"]["mean"], main["analysis_ms"]["mean"]) + pr_vs_bcr = pct(bcr["analysis_ms"]["mean"], pr["analysis_ms"]["mean"]) + pr_vs_main = pct(main["analysis_ms"]["mean"], pr["analysis_ms"]["mean"]) + + has_aux = bcr_aux is not None or main_aux is not None or pr_aux is not None + + table = "## Bazel analysis benchmark\n\n" + table += "| Version | Analysis (ms) | Median (ms) | \u00b1 stddev | Wall (ms) | vs BCR | vs main | Packages | Targets |\n" + table += "|---------|--------------|-------------|----------|----------|--------|---------|----------|----------|\n" + + def aux_cell(aux: dict[str, Any] | None) -> str: + if aux is None: + return f"{EM} | {EM}" + return f"{aux.get('packages', EM)} | {aux.get('targets', EM)}" + + def row(label: str, d: dict[str, Any], vs_bcr: str, vs_main: str, aux: dict[str, Any] | None) -> str: + a = d["analysis_ms"] + wall = d.get("wall_ms", {}).get("mean") + wall_str = fmt(wall) if wall is not None else EM + return ( + f"| {label} | {fmt(a['mean'])} | {fmt(a['median'])} | " + f"\u00b1{fmt(a['stddev'])} | {wall_str} | {vs_bcr} | {vs_main} | {aux_cell(aux)} |\n" + ) + + table += row("BCR 2.0.0-alpha.4 (baseline)", bcr, EM, EM, bcr_aux) + table += row("HEAD main", main, f"{main_vs_bcr:+.1f}% {warn(main_vs_bcr)}", EM, main_aux) + table += row("This PR", pr, f"{pr_vs_bcr:+.1f}% {warn(pr_vs_bcr)}", + f"{pr_vs_main:+.1f}% {warn(pr_vs_main)}", pr_aux) + + table += ( + f"\n> Analysis phase (`runAnalysisPhase`) extracted from `--profile` " + f"on `{os.environ.get('RUNNER_OS', 'local')}`\n" + ) + table += ( + f"> **Gate**: analysis_ms, PR vs HEAD main (threshold: {THRESHOLD_REGRESSION_PCT}%). " + f"Wall time is informational (JVM/IO overhead). BCR is a historical baseline only.\n" + ) + table += _starlark_section(main, pr) + + _emit(table, args.output_table) + + if is_regression(main, pr, THRESHOLD_REGRESSION_PCT): + print(f"\n\u274c REGRESSION: PR analysis is {pr_vs_main:.1f}% slower than HEAD main " + f"(threshold: {THRESHOLD_REGRESSION_PCT}%)") + return 1 + print(f"\n\u2705 No regression detected (PR analysis is {pr_vs_main:+.1f}% vs HEAD main)") + return 0 + + +# --------------------------------------------------------------------------- # +# startup subcommand +# --------------------------------------------------------------------------- # + +def load_runtime(path: str) -> dict[str, Any]: + """Load a single hyperfine runtime JSON result.""" + p = Path(path) + if not p.exists(): + _fail(f"result file not found: {path}") + with p.open() as f: + data = json.load(f) + if "results" not in data or not data["results"]: + _fail(f"no results in {path}") + r = data["results"][0] + for key in ("mean", "stddev", "min", "max", "median"): + if key not in r: + _fail(f"missing '{key}' in {path}") + return { + "mean_ms": r["mean"] * 1000, + "stddev_ms": r["stddev"] * 1000, + "min_ms": r["min"] * 1000, + "max_ms": r["max"] * 1000, + "median_ms": r["median"] * 1000, + } + + +def load_syspath(path: str) -> dict[str, int] | None: + p = Path(path) + if not p.exists(): + return None + with p.open() as f: + data = json.load(f) + return { + "total_entries": data.get("total_entries", 0), + "distinct_sp_roots": data.get("distinct_sp_roots", 0), + "dupe_realpaths": data.get("dupe_realpaths", 0), + } + + +def run_startup(args: argparse.Namespace) -> int: + bcr_path, main_path, pr_path = args.bcr, args.main, args.pr + + bcr = load_runtime(bcr_path) + main = load_runtime(main_path) + pr = load_runtime(pr_path) + + bcr_syspath = load_syspath(bcr_path.replace(".json", "-syspath.json")) + main_syspath = load_syspath(main_path.replace(".json", "-syspath.json")) + pr_syspath = load_syspath(pr_path.replace(".json", "-syspath.json")) + + rt_main_vs_bcr = pct(bcr["mean_ms"], main["mean_ms"]) + rt_pr_vs_bcr = pct(bcr["mean_ms"], pr["mean_ms"]) + rt_pr_vs_main = pct(main["mean_ms"], pr["mean_ms"]) + + has_syspath = bcr_syspath is not None or main_syspath is not None or pr_syspath is not None + + table = "## py_binary startup benchmark\n\n" + table += "| Version | Startup (ms) | Median (ms) | \u00b1 stddev | vs BCR | vs main |\n" + table += "|---------|-------------|-------------|----------|--------|---------|\n" + + def row(label: str, d: dict[str, Any], vs_bcr: str, vs_main: str) -> str: + return ( + f"| {label} | {fmt(d['mean_ms'])} | {fmt(d['median_ms'])} | " + f"\u00b1{fmt(d['stddev_ms'])} | {vs_bcr} | {vs_main} |\n" + ) + + table += row("BCR 2.0.0-alpha.4 (baseline)", bcr, EM, EM) + table += row("HEAD main", main, f"{rt_main_vs_bcr:+.1f}% {warn(rt_main_vs_bcr)}", EM) + table += row("This PR", pr, f"{rt_pr_vs_bcr:+.1f}% {warn(rt_pr_vs_bcr)}", + f"{rt_pr_vs_main:+.1f}% {warn(rt_pr_vs_main)}") + + table += ( + f"\n> Startup measured with `hyperfine --warmup 5 --runs 50` on " + f"`{os.environ.get('RUNNER_OS', 'local')}`. Target: fan-in `//:bench` " + f"(venv scales with --packages).\n" + ) + table += ( + f"> **Gate** (threshold {THRESHOLD_REGRESSION_PCT}%, PR vs HEAD main): " + "startup mean. BCR is a historical baseline only.\n" + ) + + if has_syspath: + table += "\n### sys.path quality\n\n" + table += "| Version | sys.path entries | distinct site-packages roots | duplicate realpaths |\n" + table += "|---------|-----------------|------------------------------|---------------------|\n" + + def syspath_row(label: str, sp: dict[str, int] | None) -> str: + if sp is None: + return f"| {label} | {EM} | {EM} | {EM} |\n" + dupe_flag = " \u26a0\ufe0f" if sp["dupe_realpaths"] > 0 else "" + return ( + f"| {label} | {sp['total_entries']} | {sp['distinct_sp_roots']} " + f"| {sp['dupe_realpaths']}{dupe_flag} |\n" + ) + + table += syspath_row("BCR 2.0.0-alpha.4 (baseline)", bcr_syspath) + table += syspath_row("HEAD main", main_syspath) + table += syspath_row("This PR", pr_syspath) + table += ( + "\n> **sys.path quality** measured by `bench_main.py` inside the assembled venv. " + "Duplicate realpaths indicate symlink redundancy; many distinct site-packages roots " + "suggest an inefficient venv layout.\n" + ) + + _emit(table, args.output_table) + + if rt_pr_vs_main > THRESHOLD_REGRESSION_PCT: + print(f"\n\u274c REGRESSION: startup {rt_pr_vs_main:.1f}% slower than HEAD main " + f"(threshold: {THRESHOLD_REGRESSION_PCT}%)") + return 1 + print(f"\n\u2705 No regression (startup {rt_pr_vs_main:+.1f}% vs HEAD main)") + return 0 + + +# --------------------------------------------------------------------------- # +# shared plumbing +# --------------------------------------------------------------------------- # + +def _fail(msg: str) -> None: + print(f"ERROR: {msg}", file=sys.stderr) + write_gh_output(f"\u274c ERROR: {msg}") + sys.exit(2) + + +def _emit(table: str, output_table: str | None) -> None: + write_gh_output(table) + if output_table: + Path(output_table).write_text(table) + else: + print(table) + + +def main() -> None: + parser = argparse.ArgumentParser(description="Compare benchmark results (analysis | startup)") + sub = parser.add_subparsers(dest="kind", required=True) + + def add_common(p: argparse.ArgumentParser) -> None: + p.add_argument("bcr", help="BCR result JSON") + p.add_argument("main", help="HEAD main result JSON") + p.add_argument("pr", help="PR result JSON") + p.add_argument("--output-table", help="write only the markdown table to this file") + + add_common(sub.add_parser("analysis", help="analysis_ms gate + Starlark CPU diagnostic")) + add_common(sub.add_parser("startup", help="runtime gate + sys.path quality")) + args = parser.parse_args() + + rc = run_analysis(args) if args.kind == "analysis" else run_startup(args) + sys.exit(rc) + + +if __name__ == "__main__": + main() diff --git a/benchmark/pprof_decode.py b/benchmark/pprof_decode.py new file mode 100644 index 000000000..b4dcf2851 --- /dev/null +++ b/benchmark/pprof_decode.py @@ -0,0 +1,161 @@ +"""Minimal stdlib pprof decoder for Bazel `--starlark_cpu_profile` output. + +Parses the protobuf-wire pprof Profile message without third-party deps and +returns CPU time aggregated by leaf Starlark function. No protobuf library -- +just varint + length-delimited wire types, packed and non-packed repeated fields. + +The pprof is gzip-compressed; we fall back to raw bytes if not. If the format +changes and nothing decodes, callers get an empty dict (fail-open, no crash). +""" +from __future__ import annotations + +import gzip +from collections import defaultdict +from typing import Iterator + + +def _read_varint(buf: bytes, i: int) -> tuple[int, int]: + result = 0 + shift = 0 + while True: + byte = buf[i] + i += 1 + result |= (byte & 0x7F) << shift + if not (byte & 0x80): + return result, i + shift += 7 + + +def _scan(buf: bytes) -> Iterator[tuple[int, int, object]]: + """Yield (field_number, wire_type, value) for a message's top-level fields. + + value is an int for varint (wt 0/0b), raw bytes for length-delimited (wt 2), + or raw bytes for fixed (wt 1=64bit, wt 5=32bit). + """ + i = 0 + n = len(buf) + while i < n: + tag, i = _read_varint(buf, i) + field = tag >> 3 + wt = tag & 7 + if wt == 0: + val, i = _read_varint(buf, i) + yield field, wt, val + elif wt == 2: + ln, i = _read_varint(buf, i) + yield field, wt, buf[i:i + ln] + i += ln + elif wt == 1: + yield field, wt, buf[i:i + 8] + i += 8 + elif wt == 5: + yield field, wt, buf[i:i + 4] + i += 4 + else: + return + + +def _fields(buf: bytes) -> dict[int, list]: + """Parse a message into {field_number: [values]} (wt0->int, wt2->bytes).""" + out: dict[int, list] = defaultdict(list) + for field, wt, val in _scan(buf): + out[field].append(val) + return out + + +def _parse_packed_or_singles(buf: bytes, target_field: int) -> list[int]: + """Read a repeated varint field that may be packed (wt2) or unpacked (wt0).""" + vals: list[int] = [] + for field, wt, val in _scan(buf): + if field != target_field: + continue + if wt == 0: + vals.append(val) # type: ignore[arg-type] + elif wt == 2: + sub = val # type: ignore[assignment] + j = 0 + while j < len(sub): + v, j = _read_varint(sub, j) + vals.append(v) + return vals + + +def _read_payload(path: str) -> bytes: + try: + return gzip.open(path, "rb").read() + except OSError: + return open(path, "rb").read() + + +def decode_starlark_pprof(path: str) -> dict[str, float]: + """Return {function_name: cpu_ms} aggregated by leaf Starlark function. + + Returns {} if the profile is missing or unparseable (fail-open). + """ + data = _read_payload(path) + top = _fields(data) + + strings_raw = top.get(6, []) + strings = [s.decode("utf-8", "replace") if isinstance(s, bytes) else "" for s in strings_raw] + + def s(idx: object | int) -> str: + if isinstance(idx, int) and 0 <= idx < len(strings): + return strings[idx] + return "" + + # sample_type: repeated ValueType{1:type, 2:unit} (string_table indices) + sample_types: list[tuple[str, str]] = [] + for chunk in top.get(1, []): + f = _fields(chunk) + t = f.get(1, [0]) + u = f.get(2, [0]) + sample_types.append((s(t[0]), s(u[0]))) + + # Pick the CPU value index and its unit->ms divisor. + val_idx = 0 + divisor = 1000.0 + for i, (_t, unit) in enumerate(sample_types): + if unit == "microseconds": + val_idx, divisor = i, 1000.0 + elif unit == "nanoseconds": + val_idx, divisor = i, 1000000.0 + + # function: id -> name index + fn_name: dict[int, int] = {} + for chunk in top.get(5, []): + f = _fields(chunk) + fid = f.get(1, [0])[0] + name_idx = f.get(2, [0])[0] + fn_name[fid] = name_idx + + # location: id -> leaf function id (first line) + loc_to_fn: dict[int, int] = {} + for chunk in top.get(4, []): + f = _fields(chunk) + lid = f.get(1, [0])[0] + lines = f.get(4, []) + if lines: + line = _fields(lines[0]) + loc_to_fn[lid] = line.get(1, [0])[0] + + totals: dict[str, float] = defaultdict(float) + for chunk in top.get(2, []): + loc_ids = _parse_packed_or_singles(chunk, 1) + values = _parse_packed_or_singles(chunk, 2) + if not loc_ids or val_idx >= len(values): + continue + leaf = loc_ids[0] + fid = loc_to_fn.get(leaf) + if fid is None: + continue + name_idx = fn_name.get(fid) + totals[s(name_idx)] += values[val_idx] / divisor + + return dict(totals) + + +if __name__ == "__main__": + import sys + rows = sorted(decode_starlark_pprof(sys.argv[1]).items(), key=lambda kv: -kv[1]) + for name, ms in rows[:25]: + print(f"{ms:9.2f} ms {name}") diff --git a/benchmark/profile_benchmark.py b/benchmark/profile_benchmark.py new file mode 100644 index 000000000..b734d6679 --- /dev/null +++ b/benchmark/profile_benchmark.py @@ -0,0 +1,294 @@ +#!/usr/bin/env python3 +"""Run a bazel command N times with --profile and report the analysis-phase +duration extracted from each profile, replacing wall-time benchmark gating. + +The relevant metric for Starlark-rule performance is Bazel's analysis phase +(runAnalysisPhase event), not process wall time (~99% JVM/IO overhead unrelated +to rules_py). See tdr.md. + +Generic mode -- pass the bazel command with a {PROFILE} placeholder: + + python3 profile_benchmark.py --runs 10 --warmup 1 \\ + --prepare 'rm -rf /tmp/baz-analysis' --output pr.json \\ + -- bazel --output_base=/tmp/baz-analysis --bazelrc=... \\ + build --disk_cache= --nobuild --profile={PROFILE} //... + +Analysis convenience mode (--packages) -- clean, generate the synthetic +workspace, generate MODULE.bazel (local checkout) and run the analysis +benchmark in one shot. Coupled to benchmark/workspace by design: + + python3 profile_benchmark.py --packages 50 + +Output JSON (consumed by compare.py): analysis_ms and wall_ms stats in ms, +plus an optional starlark_fn breakdown (per-function CPU) when the command +also carries {STARPROFILE} for --starlark_cpu_profile. + + python3 profile_benchmark.py --packages 50 + python3 profile_benchmark.py --no-starlark-profile --packages 50 # fast, no fn breakdown +""" +from __future__ import annotations + +import argparse +import gzip +import json +import os +import shutil +import statistics +import subprocess +import sys +import tempfile +import time +from pathlib import Path +from typing import Any + +from pprof_decode import decode_starlark_pprof + +PROFILE_PLACEHOLDER = "{PROFILE}" +STAR_PLACEHOLDER = "{STARPROFILE}" +DEFAULT_OUTPUT_BASE = "/tmp/bazel_pbench" + + +def extract_analysis_us(profile_path: str) -> int | None: + """Return the runAnalysisPhase duration (microseconds) from a Bazel profile.""" + with gzip.open(profile_path, "rt") as f: + data = json.load(f) + for event in data.get("traceEvents", []): + if event.get("name") == "runAnalysisPhase" and "dur" in event: + return int(event["dur"]) + return None + + +def stats_ms(values_ms: list[float]) -> dict[str, float]: + """Aggregate millisecond durations into statistics (min/mean/median/stddev).""" + count = len(values_ms) + return { + "mean": statistics.mean(values_ms), + "median": statistics.median(values_ms), + "stddev": statistics.stdev(values_ms) if count > 1 else 0.0, + "min": min(values_ms), + "max": max(values_ms), + "runs": count, + } + + +def aggregate_starlark(star_runs: list[dict[str, float]]) -> list[dict[str, Any]]: + """Aggregate per-run {fn: ms} dicts into the top functions by mean CPU ms.""" + names: set[str] = set() + for d in star_runs: + names.update(d.keys()) + rows: list[dict[str, Any]] = [] + for name in names: + series = [d.get(name, 0.0) for d in star_runs] + rows.append({"name": name, "mean_ms": statistics.mean(series)}) + total = sum(r["mean_ms"] for r in rows) or 1.0 + for r in rows: + r["pct"] = r["mean_ms"] / total * 100.0 + r["runs"] = len(star_runs) + rows.sort(key=lambda r: r["mean_ms"], reverse=True) + return rows[:20] + + +def substitute(command: list[str], replacements: dict[str, str]) -> list[str]: + """Inject placeholder paths into the command. {PROFILE} is required.""" + if not any(PROFILE_PLACEHOLDER in tok for tok in command): + raise SystemExit( + f"ERROR: command must contain a '{PROFILE_PLACEHOLDER}' token for --profile" + ) + full = command + for placeholder, path in replacements.items(): + full = [tok.replace(placeholder, path) for tok in full] + return full + + +def _copy_profile(src: str, dst: str) -> None: + """Copy a captured profile to an archive path, creating parent dirs.""" + if not os.path.exists(src): + return + dst_path = Path(dst) + dst_path.parent.mkdir(parents=True, exist_ok=True) + shutil.copyfile(src, dst) + print(f"saved profile -> {dst}", file=sys.stderr) + + +def run_once( + command: list[str], + prepare: str | None, + profile_path: str, + star_path: str | None, + cwd: str | None = None, +) -> tuple[int, float, dict[str, float] | None]: + """Run a single measured invocation. Returns (analysis_us, wall_ms, starlark_fn|None).""" + if prepare: + subprocess.run(prepare, shell=True, check=True) + replacements = {PROFILE_PLACEHOLDER: profile_path} + if star_path is not None: + replacements[STAR_PLACEHOLDER] = star_path + full = substitute(command, replacements) + start = time.perf_counter() + result = subprocess.run(full, cwd=cwd) + wall_ms = (time.perf_counter() - start) * 1000.0 + if result.returncode != 0: + raise SystemExit(f"ERROR: command failed (exit {result.returncode}): {' '.join(full)}") + analysis_us = extract_analysis_us(profile_path) + if analysis_us is None: + raise SystemExit(f"ERROR: runAnalysisPhase not found in {profile_path}") + starlark = decode_starlark_pprof(star_path) if star_path is not None else None + return analysis_us, wall_ms, starlark + + +def _workspace_dir() -> Path: + return Path(__file__).resolve().parent / "workspace" + + +def prepare_workspace(packages: int) -> Path: + """Clean and regenerate the synthetic workspace + MODULE.bazel. + + Runs the generators with cwd=benchmark/workspace so relative paths in them + (e.g. generate_module's default --path ../..) resolve against the repo root. + """ + workspace = _workspace_dir() + ws_gen = workspace / "generate_workspace.py" + mod_gen = workspace / "generate_module.py" + for script in (ws_gen, mod_gen): + if not script.exists(): + raise SystemExit(f"ERROR: generator not found: {script}") + print(f"[setup] cleaning + generating {packages} packages in {workspace}", file=sys.stderr) + subprocess.run( + [sys.executable, str(ws_gen), "--root", ".", "--packages", str(packages)], + cwd=str(workspace), + check=True, + ) + subprocess.run( + [sys.executable, str(mod_gen), "local"], + cwd=str(workspace), + check=True, + ) + return workspace + + +def default_analysis_command() -> list[str]: + """Construct the analysis bazel command (//..., ci.bazelrc).""" + repo_root = _workspace_dir().parent.parent + bazelrc = repo_root / ".github" / "workflows" / "ci.bazelrc" + return [ + "bazel", + f"--output_base={DEFAULT_OUTPUT_BASE}", + f"--bazelrc={bazelrc}", + "build", + "--disk_cache=", + "--nobuild", + f"--profile={PROFILE_PLACEHOLDER}", + f"--starlark_cpu_profile={STAR_PLACEHOLDER}", + "//...", + ] + + +def main() -> None: + parser = argparse.ArgumentParser( + description="Run a bazel command N times with --profile and report analysis-phase stats.", + formatter_class=argparse.RawDescriptionHelpFormatter, + ) + parser.add_argument("--runs", type=int, default=10, help="measured runs (default 10)") + parser.add_argument("--warmup", type=int, default=1, help="warmup runs, discarded (default 1)") + parser.add_argument("--prepare", default=None, help="shell command run before each invocation") + parser.add_argument("--output", default=None, help="path to write aggregate JSON (optional)") + parser.add_argument( + "--packages", + type=int, + default=None, + help="analysis convenience mode: clean + generate N-package workspace + MODULE.bazel, " + "then run the analysis benchmark", + ) + parser.add_argument( + "--no-starlark-profile", + action="store_true", + help="disable --starlark_cpu_profile capture (skip per-function breakdown)", + ) + parser.add_argument( + "--save-profile", + default=None, + help="copy the last run's --profile (chrome trace .gz) here for archival", + ) + parser.add_argument( + "--save-starlark", + default=None, + help="copy the last run's --starlark_cpu_profile (pprof .gz) here for archival", + ) + parser.add_argument( + "command", + nargs=argparse.REMAINDER, + help="bazel command; separate from options with '--'. Must contain {PROFILE}. " + "Omit when using --packages.", + ) + args = parser.parse_args() + + command = args.command + if command and command[0] == "--": + command = command[1:] + + run_cwd: str | None = None + if args.packages is not None: + run_cwd = str(prepare_workspace(args.packages)) + if not command: + command = default_analysis_command() + if args.prepare is None: + args.prepare = f"rm -rf {DEFAULT_OUTPUT_BASE}" + elif not command: + parser.error("a bazel command containing {PROFILE} is required (or use --packages)") + + fd, profile_path = tempfile.mkstemp(suffix=".gz", prefix="profile_benchmark_") + os.close(fd) + star_enabled = (not args.no_starlark_profile) and any( + STAR_PLACEHOLDER in tok for tok in command + ) + fd2, star_path = tempfile.mkstemp(suffix=".gz", prefix="starlark_profile_") + os.close(fd2) + try: + for _ in range(args.warmup): + run_once(command, args.prepare, profile_path, + star_path if star_enabled else None, cwd=run_cwd) + analysis_us: list[float] = [] + wall_ms: list[float] = [] + star_runs: list[dict[str, float]] = [] + for i in range(args.runs): + a, w, star = run_once(command, args.prepare, profile_path, + star_path if star_enabled else None, cwd=run_cwd) + analysis_us.append(a) + wall_ms.append(w) + if star: + star_runs.append(star) + print( + f"run {i + 1}/{args.runs}: analysis={a / 1000.0:.1f}ms wall={w:.0f}ms", + file=sys.stderr, + ) + if args.save_profile: + _copy_profile(profile_path, args.save_profile) + if args.save_starlark and star_enabled: + _copy_profile(star_path, args.save_starlark) + finally: + Path(profile_path).unlink(missing_ok=True) + Path(star_path).unlink(missing_ok=True) + + output: dict[str, Any] = { + "analysis_ms": stats_ms([us / 1000.0 for us in analysis_us]), + "wall_ms": stats_ms(wall_ms), + } + + if star_runs: + output["starlark_fn"] = aggregate_starlark(star_runs) + + if args.output: + Path(args.output).write_text(json.dumps(output, indent=2)) + + a = output["analysis_ms"] + print( + f"\nanalysis_ms: mean={a['mean']:.1f} median={a['median']:.1f} " + f"stddev={a['stddev']:.1f} (n={a['runs']})", + file=sys.stderr, + ) + if args.output: + print(f"wrote {args.output}", file=sys.stderr) + + +if __name__ == "__main__": + main() diff --git a/benchmark/startup/BUILD.bazel b/benchmark/startup/BUILD.bazel deleted file mode 100644 index fd00a442f..000000000 --- a/benchmark/startup/BUILD.bazel +++ /dev/null @@ -1,19 +0,0 @@ -load("@aspect_rules_py//py:defs.bzl", "py_binary") - -py_binary( - name = "bench", - srcs = ["main.py"], - main = "main.py", - deps = [ - "@pypi//cowsay", - ], -) - -py_binary( - name = "bench_syspath", - srcs = ["syspath_probe.py"], - main = "syspath_probe.py", - deps = [ - "@pypi//cowsay", - ], -) diff --git a/benchmark/startup/MODULE.bazel.template b/benchmark/startup/MODULE.bazel.template deleted file mode 100644 index ce975076d..000000000 --- a/benchmark/startup/MODULE.bazel.template +++ /dev/null @@ -1,26 +0,0 @@ -"Benchmark workspace for py_binary startup performance" - -module(name = "startup_benchmark") - -{{RULES_PY_DECLARATION}} - -bazel_dep(name = "bazel_features", version = "1.38.0") -bazel_dep(name = "bazel_skylib", version = "1.4.2") -bazel_dep(name = "bazel_lib", version = "3.0.0") -bazel_dep(name = "platforms", version = "1.0.0") -bazel_dep(name = "rules_python", version = "1.9.0") - -# Python interpreters provisioned from python-build-standalone via aspect_rules_py -interpreters = use_extension("@aspect_rules_py//py:extensions.bzl", "python_interpreters") -interpreters.toolchain( - python_version = "3.11", -) - -# Pip dependencies for the benchmark target -pip = use_extension("@rules_python//python/extensions:pip.bzl", "pip") -pip.parse( - hub_name = "pypi", - python_version = "3.11", - requirements_lock = "//:requirements.txt", -) -use_repo(pip, "pypi") diff --git a/benchmark/startup/compare.py b/benchmark/startup/compare.py deleted file mode 100644 index 59fd2fde3..000000000 --- a/benchmark/startup/compare.py +++ /dev/null @@ -1,232 +0,0 @@ -#!/usr/bin/env python3 -"""Parse hyperfine JSON output, build a markdown table, exit 1 on regression. - -The regression gate compares PR against HEAD main (not BCR). -BCR is kept as a historical baseline for context, but gating against it is -misleading because transitive dependency versions drift between releases. -""" - -from __future__ import annotations - -import argparse -import json -import os -import sys -from pathlib import Path -from typing import Any - -THRESHOLD_REGRESSION_PCT = 10 # fail CI if PR is >10% slower than HEAD main - - -def write_gh_output(text: str) -> None: - """Write to GITHUB_OUTPUT if available, so sticky PR comment always has content.""" - gh_output = os.environ.get("GITHUB_OUTPUT") - if gh_output: - with open(gh_output, "a") as f: - f.write("table< dict[str, Any]: - """Load a single hyperfine JSON result.""" - p = Path(path) - if not p.exists(): - msg = f"ERROR: result file not found: {path}" - print(msg, file=sys.stderr) - write_gh_output(f"❌ {msg}") - sys.exit(2) - - with p.open() as f: - data = json.load(f) - - if "results" not in data or not data["results"]: - msg = f"ERROR: no results in {path}" - print(msg, file=sys.stderr) - write_gh_output(f"❌ {msg}") - sys.exit(2) - - r = data["results"][0] - for key in ("mean", "stddev", "min", "max", "median"): - if key not in r: - msg = f"ERROR: missing '{key}' in {path}" - print(msg, file=sys.stderr) - write_gh_output(f"❌ {msg}") - sys.exit(2) - - return { - "mean_ms": r["mean"] * 1000, - "stddev_ms": r["stddev"] * 1000, - "min_ms": r["min"] * 1000, - "max_ms": r["max"] * 1000, - "median_ms": r["median"] * 1000, - } - - -def load_build(path: str) -> dict[str, float] | None: - """Load an optional build-time JSON ({build_ms: int}).""" - p = Path(path) - if not p.exists(): - return None - with p.open() as f: - data = json.load(f) - ms = data.get("build_ms", 0) - return {"build_s": ms / 1000.0} - - -def load_syspath(path: str) -> dict[str, int] | None: - """Load an optional sys.path quality JSON from syspath_probe.py.""" - p = Path(path) - if not p.exists(): - return None - with p.open() as f: - data = json.load(f) - return { - "total_entries": data.get("total_entries", 0), - "distinct_sp_roots": data.get("distinct_sp_roots", 0), - "dupe_realpaths": data.get("dupe_realpaths", 0), - } - - -def pct(a: float, b: float) -> float: - """Percentage delta from a to b.""" - if a == 0: - return 0.0 - return (b - a) / a * 100 - - -def fmt(val: float) -> str: - """Format milliseconds with sensible precision.""" - return f"{val:.3f}" - - -def fmt_s(val: float) -> str: - """Format seconds with sensible precision.""" - return f"{val:.2f}" - - -def warn(delta: float) -> str: - """Return warning emoji if delta exceeds threshold.""" - return "⚠️" if delta > THRESHOLD_REGRESSION_PCT else "" - - -def main() -> None: - parser = argparse.ArgumentParser(description="Compare startup benchmark results") - parser.add_argument("bcr", help="BCR hyperfine JSON") - parser.add_argument("main", help="HEAD main hyperfine JSON") - parser.add_argument("pr", help="PR hyperfine JSON") - parser.add_argument( - "--output-table", - help="Write only the markdown table to this file instead of stdout", - ) - args = parser.parse_args() - - bcr_path, main_path, pr_path = args.bcr, args.main, args.pr - - bcr = load_runtime(bcr_path) - main = load_runtime(main_path) - pr = load_runtime(pr_path) - - bcr_build = load_build(bcr_path.replace(".json", "-build.json")) - main_build = load_build(main_path.replace(".json", "-build.json")) - pr_build = load_build(pr_path.replace(".json", "-build.json")) - - bcr_syspath = load_syspath(bcr_path.replace(".json", "-syspath.json")) - main_syspath = load_syspath(main_path.replace(".json", "-syspath.json")) - pr_syspath = load_syspath(pr_path.replace(".json", "-syspath.json")) - - main_vs_bcr = pct(bcr["mean_ms"], main["mean_ms"]) - pr_vs_bcr = pct(bcr["mean_ms"], pr["mean_ms"]) - pr_vs_main = pct(main["mean_ms"], pr["mean_ms"]) - - has_build = bcr_build is not None or main_build is not None or pr_build is not None - has_syspath = bcr_syspath is not None or main_syspath is not None or pr_syspath is not None - - table = "## py_binary startup benchmark\n\n" - if has_build: - table += "| Version | Mean (ms) | Median (ms) | ± stddev | vs BCR | vs main | Build (s) |\n" - table += "|---------|-----------|-------------|----------|--------|---------|-----------|\n" - else: - table += "| Version | Mean (ms) | Median (ms) | ± stddev | vs BCR | vs main |\n" - table += "|---------|-----------|-------------|----------|--------|---------|\n" - - def row(label: str, d: dict[str, Any], d_build: dict[str, float] | None, vs_bcr: str, vs_main: str) -> str: - line = ( - f"| {label} | {fmt(d['mean_ms'])} | {fmt(d['median_ms'])} | " - f"±{fmt(d['stddev_ms'])} | {vs_bcr} | {vs_main}" - ) - if has_build: - b = fmt_s(d_build["build_s"]) if d_build else "—" - line += f" | {b}" - line += " |\n" - return line - - table += row( - "BCR 1.11.7 (baseline)", bcr, bcr_build, "—", "—" - ) - table += row( - "HEAD main", main, main_build, - f"{main_vs_bcr:+.1f}% {warn(main_vs_bcr)}", "—" - ) - table += row( - "This PR", pr, pr_build, - f"{pr_vs_bcr:+.1f}% {warn(pr_vs_bcr)}", - f"{pr_vs_main:+.1f}% {warn(pr_vs_main)}" - ) - - table += ( - f"\n> Measured with `hyperfine --warmup 5 --runs 50` on " - f"`{os.environ.get('RUNNER_OS', 'local')}`\n" - ) - table += ( - f"> **Gate**: PR vs HEAD main (threshold: {THRESHOLD_REGRESSION_PCT}%). " - f"BCR is shown only as a historical baseline.\n" - ) - if has_build: - table += ( - "> **Build time**: cold `bazel build //:bench` with isolated output base, no disk cache.\n" - ) - - if has_syspath: - table += "\n### sys.path quality\n\n" - table += "| Version | sys.path entries | distinct site-packages roots | duplicate realpaths |\n" - table += "|---------|-----------------|------------------------------|---------------------|\n" - - def syspath_row(label: str, sp: dict[str, int] | None) -> str: - if sp is None: - return f"| {label} | — | — | — |\n" - dupe_flag = " ⚠️" if sp["dupe_realpaths"] > 0 else "" - return ( - f"| {label} | {sp['total_entries']} | {sp['distinct_sp_roots']} " - f"| {sp['dupe_realpaths']}{dupe_flag} |\n" - ) - - table += syspath_row("BCR 1.11.7 (baseline)", bcr_syspath) - table += syspath_row("HEAD main", main_syspath) - table += syspath_row("This PR", pr_syspath) - table += ( - "\n> **sys.path quality** measured by `bench_syspath` inside the assembled venv. " - "Duplicate realpaths indicate symlink redundancy; many distinct site-packages roots " - "suggest an inefficient venv layout.\n" - ) - - write_gh_output(table) - - if args.output_table: - Path(args.output_table).write_text(table) - else: - print(table) - - if pr_vs_main > THRESHOLD_REGRESSION_PCT: - print( - f"\n❌ REGRESSION: PR is {pr_vs_main:.1f}% slower than HEAD main " - f"(threshold: {THRESHOLD_REGRESSION_PCT}%)" - ) - sys.exit(1) - - print(f"\n✅ No regression detected (PR is {pr_vs_main:+.1f}% vs HEAD main)") - sys.exit(0) - - -if __name__ == "__main__": - main() diff --git a/benchmark/startup/generate_module.py b/benchmark/startup/generate_module.py deleted file mode 100644 index ed5615005..000000000 --- a/benchmark/startup/generate_module.py +++ /dev/null @@ -1,74 +0,0 @@ -#!/usr/bin/env python3 -"""Generate MODULE.bazel for the benchmark workspace from a template. - -Replaces fragile sed-based mutation with explicit, validated generation. -""" - -import argparse -import sys -from pathlib import Path - -TEMPLATE = Path(__file__).with_name("MODULE.bazel.template") -OUTPUT = Path(__file__).with_name("MODULE.bazel") - - -def generate(declaration: str) -> str: - """Substitute {{RULES_PY_DECLARATION}} in the template.""" - if not TEMPLATE.exists(): - print(f"ERROR: template not found: {TEMPLATE}", file=sys.stderr) - sys.exit(1) - - content = TEMPLATE.read_text() - if "{{RULES_PY_DECLARATION}}" not in content: - print("ERROR: template missing {{RULES_PY_DECLARATION}} placeholder", file=sys.stderr) - sys.exit(1) - - return content.replace("{{RULES_PY_DECLARATION}}", declaration) - - -def main() -> None: - parser = argparse.ArgumentParser(description="Generate MODULE.bazel for benchmark") - parser.add_argument( - "mode", - choices=["bcr", "local"], - help="'bcr' pins to a BCR release; 'local' uses local_path_override", - ) - parser.add_argument( - "--version", - default="1.11.7", - help="BCR version to pin when mode=bcr (default: 1.11.7)", - ) - parser.add_argument( - "--path", - default="../..", - help="Local path for local_path_override when mode=local (default: ../..)", - ) - parser.add_argument( - "--dry-run", - action="store_true", - help="Print to stdout instead of writing MODULE.bazel", - ) - args = parser.parse_args() - - if args.mode == "bcr": - declaration = f'bazel_dep(name = "aspect_rules_py", version = "{args.version}")' - else: - declaration = ( - f'bazel_dep(name = "aspect_rules_py")\n' - f'local_path_override(\n' - f' module_name = "aspect_rules_py",\n' - f' path = "{args.path}",\n' - f')' - ) - - result = generate(declaration) - - if args.dry_run: - print(result) - else: - OUTPUT.write_text(result) - print(f"Wrote {OUTPUT}") - - -if __name__ == "__main__": - main() diff --git a/benchmark/startup/main.py b/benchmark/startup/main.py deleted file mode 100644 index 01a8181a0..000000000 --- a/benchmark/startup/main.py +++ /dev/null @@ -1,5 +0,0 @@ -#!/usr/bin/env python3 - -import cowsay - -cowsay.cow("bench") diff --git a/benchmark/startup/requirements.txt b/benchmark/startup/requirements.txt deleted file mode 100644 index 5179ddf01..000000000 --- a/benchmark/startup/requirements.txt +++ /dev/null @@ -1,4 +0,0 @@ ---index-url https://pypi.org/simple - -cowsay==6.1 \ - --hash=sha256:274b1e6fc1b966d53976333eb90ac94cb07a450a700b455af9fbdf882244b30a diff --git a/benchmark/startup/syspath_probe.py b/benchmark/startup/syspath_probe.py deleted file mode 100644 index 9008c28b9..000000000 --- a/benchmark/startup/syspath_probe.py +++ /dev/null @@ -1,35 +0,0 @@ -#!/usr/bin/env python3 -"""Output sys.path quality metrics as JSON. - -Adapted from e2e-perf/venv_build_test.py: measures structural venv efficiency -rather than wall-clock timing. A high dupe_realpaths count or many distinct -site-packages roots indicates unnecessary overhead in the assembled venv. -""" - -import json -import os -import sys -from pathlib import Path - - -def main() -> None: - entries = [p for p in sys.path if p] - sp_roots = {p for p in entries if "site-packages" in p} - realpaths = [os.path.realpath(p) for p in entries] - dupe_realpaths = len(realpaths) - len(set(realpaths)) - - metrics = { - "total_entries": len(entries), - "distinct_sp_roots": len(sp_roots), - "dupe_realpaths": dupe_realpaths, - } - - out = sys.argv[1] if len(sys.argv) > 1 else None - if out: - Path(out).write_text(json.dumps(metrics)) - else: - print(json.dumps(metrics)) - - -if __name__ == "__main__": - main() \ No newline at end of file diff --git a/benchmark/workspace/.bazelrc b/benchmark/workspace/.bazelrc new file mode 100644 index 000000000..6c9b9a3a2 --- /dev/null +++ b/benchmark/workspace/.bazelrc @@ -0,0 +1,2 @@ +# Default dependency group for the benchmark workload. +common --@pypi//dep_group=default diff --git a/benchmark/workspace/.gitignore b/benchmark/workspace/.gitignore new file mode 100644 index 000000000..18e506e5e --- /dev/null +++ b/benchmark/workspace/.gitignore @@ -0,0 +1,8 @@ +# Generated synthetic workload + generated module; do not edit manually. +src/ +tests/ +BUILD.bazel +bench_main.py +MODULE.bazel +MODULE.bazel.lock +__pycache__/ diff --git a/benchmark/analysis/MODULE.bazel b/benchmark/workspace/MODULE.bazel.template similarity index 56% rename from benchmark/analysis/MODULE.bazel rename to benchmark/workspace/MODULE.bazel.template index 3873c5ccb..fccda1b61 100644 --- a/benchmark/analysis/MODULE.bazel +++ b/benchmark/workspace/MODULE.bazel.template @@ -1,29 +1,22 @@ -"Benchmark workspace for Bazel analysis phase performance" +"Benchmark workspace for rules_py analysis + startup performance" -module(name = "analysis_benchmark") +module(name = "benchmark_workspace") -bazel_dep(name = "aspect_rules_py") -local_path_override( - module_name = "aspect_rules_py", - path = "../..", -) +{{RULES_PY_DECLARATION}} bazel_dep(name = "bazel_skylib", version = "1.9.0") bazel_dep(name = "rules_python", version = "1.9.0") interpreters = use_extension("@aspect_rules_py//py:extensions.bzl", "python_interpreters") -interpreters.toolchain( - python_version = "3.11", -) +interpreters.toolchain(python_version = "3.11") use_repo(interpreters, "python_interpreters") - register_toolchains("@python_interpreters//:all") uv = use_extension("@aspect_rules_py//uv:extensions.bzl", "uv") uv.declare_hub(hub_name = "pypi") uv.project( hub_name = "pypi", - lock = "//workspace:uv.lock", - pyproject = "//workspace:pyproject.toml", + lock = "//:uv.lock", + pyproject = "//:pyproject.toml", ) use_repo(uv, "pypi") diff --git a/benchmark/analysis/generate_module.py b/benchmark/workspace/generate_module.py similarity index 94% rename from benchmark/analysis/generate_module.py rename to benchmark/workspace/generate_module.py index a1dacb2c9..a3e109c30 100644 --- a/benchmark/analysis/generate_module.py +++ b/benchmark/workspace/generate_module.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -"""Generate MODULE.bazel for the analysis benchmark workspace from a template.""" +"""Generate MODULE.bazel for the unified benchmark workspace from a template.""" import argparse import sys @@ -24,7 +24,7 @@ def generate(declaration: str) -> str: def main() -> None: - parser = argparse.ArgumentParser(description="Generate MODULE.bazel for analysis benchmark") + parser = argparse.ArgumentParser(description="Generate MODULE.bazel for benchmark workspace") parser.add_argument( "mode", choices=["bcr", "local"], diff --git a/benchmark/analysis/workspace/generate_workspace.py b/benchmark/workspace/generate_workspace.py similarity index 74% rename from benchmark/analysis/workspace/generate_workspace.py rename to benchmark/workspace/generate_workspace.py index d46f67f2d..a76522d7c 100644 --- a/benchmark/analysis/workspace/generate_workspace.py +++ b/benchmark/workspace/generate_workspace.py @@ -70,7 +70,7 @@ name = "{name}_test", srcs = ["test.py"], main = "test.py", - deps = ["//workspace/src/{name}:{name}"], + deps = ["//src/{name}:{name}"], ) ''' @@ -119,6 +119,42 @@ def test_compute(): assert compute(1) == {multiplier} + {offset} ''' +BENCH_MAIN_TEMPLATE = '''#!/usr/bin/env python3 +"""Fan-in benchmark binary main: stdlib-only sys.path probe. + +Deliberately imports none of the local libraries or third-party deps, so the +runtime metric reflects rules_py venv assembly/activation (which scales with the +fan-in dep count), not Python import cost. +""" +import json +import os +import sys +from pathlib import Path + + +def main() -> None: + entries = [p for p in sys.path if p] + sp_roots = {p for p in entries if "site-packages" in p} + realpaths = [os.path.realpath(p) for p in entries] + dupe_realpaths = len(realpaths) - len(set(realpaths)) + + metrics = { + "total_entries": len(entries), + "distinct_sp_roots": len(sp_roots), + "dupe_realpaths": dupe_realpaths, + } + + out = sys.argv[1] if len(sys.argv) > 1 else None + if out: + Path(out).write_text(json.dumps(metrics)) + else: + print(json.dumps(metrics)) + + +if __name__ == "__main__": + main() +''' + def generate_package(pkg_dir: Path, name: str, deps: list[str], seed: int) -> None: """Generate source and BUILD files for one local package.""" @@ -172,14 +208,29 @@ def generate_package(pkg_dir: Path, name: str, deps: list[str], seed: int) -> No def generate_root_build(root: Path, package_count: int) -> None: - """Generate a root BUILD that groups all binaries.""" - lines = ['load("@bazel_skylib//rules:build_test.bzl", "build_test")\n\n'] - lines.append('build_test(\n') - lines.append(' name = "all_bins",\n') - targets = [f"//workspace/src/pkg_{i}:pkg_{i}_bin" for i in range(package_count)] - lines.append(f" targets = {targets},\n") - lines.append(')\n') + """Generate root BUILD: build_test grouping all binaries + fan-in //:bench. + + //:bench depends on every local py_library so its venv scales with + package_count; bench_main.py is import-free (see BENCH_MAIN_TEMPLATE). + """ + pkg_libs = [f"//src/pkg_{i}:pkg_{i}" for i in range(package_count)] + pkg_bins = [f"//src/pkg_{i}:pkg_{i}_bin" for i in range(package_count)] + lines = [ + 'load("@aspect_rules_py//py:defs.bzl", "py_binary")\n', + 'load("@bazel_skylib//rules:build_test.bzl", "build_test")\n\n', + 'build_test(\n', + ' name = "all_bins",\n', + f' targets = {pkg_bins},\n', + ')\n\n', + 'py_binary(\n', + ' name = "bench",\n', + ' srcs = ["bench_main.py"],\n', + ' main = "bench_main.py",\n', + f' deps = {pkg_libs},\n', + ')\n', + ] (root / "BUILD.bazel").write_text("".join(lines)) + (root / "bench_main.py").write_text(BENCH_MAIN_TEMPLATE) def clean_generated(root: Path) -> None: @@ -228,7 +279,7 @@ def main() -> int: if i > 0: local_count = rng.randint(1, min(3, i)) local_deps = [ - f"//workspace/src/pkg_{j}:pkg_{j}" + f"//src/pkg_{j}:pkg_{j}" for j in sorted(rng.sample(range(i), local_count)) ] diff --git a/benchmark/analysis/workspace/pyproject.toml b/benchmark/workspace/pyproject.toml similarity index 100% rename from benchmark/analysis/workspace/pyproject.toml rename to benchmark/workspace/pyproject.toml diff --git a/benchmark/analysis/workspace/uv.lock b/benchmark/workspace/uv.lock similarity index 100% rename from benchmark/analysis/workspace/uv.lock rename to benchmark/workspace/uv.lock From 5f5eafcb8f9d8352286162320d74bb6a38247268 Mon Sep 17 00:00:00 2001 From: xangcastle Date: Thu, 16 Jul 2026 16:32:07 -0600 Subject: [PATCH 02/11] keeping all aggregated rows --- benchmark/profile_benchmark.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/benchmark/profile_benchmark.py b/benchmark/profile_benchmark.py index b734d6679..5faa79341 100644 --- a/benchmark/profile_benchmark.py +++ b/benchmark/profile_benchmark.py @@ -72,7 +72,12 @@ def stats_ms(values_ms: list[float]) -> dict[str, float]: def aggregate_starlark(star_runs: list[dict[str, float]]) -> list[dict[str, Any]]: - """Aggregate per-run {fn: ms} dicts into the top functions by mean CPU ms.""" + """Aggregate per-run {fn: ms} dicts into per-function mean CPU ms. + + Returns ALL functions (sorted desc), not a top-N: truncation happens only at + render time in compare.py. Truncating here would make the comparator treat a + function below one side's cutoff as absent (0.0) and misreport it as new. + """ names: set[str] = set() for d in star_runs: names.update(d.keys()) @@ -85,7 +90,7 @@ def aggregate_starlark(star_runs: list[dict[str, float]]) -> list[dict[str, Any] r["pct"] = r["mean_ms"] / total * 100.0 r["runs"] = len(star_runs) rows.sort(key=lambda r: r["mean_ms"], reverse=True) - return rows[:20] + return rows def substitute(command: list[str], replacements: dict[str, str]) -> list[str]: From 5debb8fdcff0c32269eedb2e244404876bf76be5 Mon Sep 17 00:00:00 2001 From: xangcastle Date: Thu, 16 Jul 2026 16:55:25 -0600 Subject: [PATCH 03/11] isolating the noise from the top movers --- MODULE.bazel.lock | 191 +++++++++++++++++++++++++++++++++ benchmark/compare.py | 114 +++++++++++++------- benchmark/profile_benchmark.py | 37 ++++--- 3 files changed, 293 insertions(+), 49 deletions(-) diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index de05f67a0..8b18d3e61 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -301,6 +301,197 @@ ] } }, + "@@llvm+//3rd_party/gcc/extension:gcc.bzl%gcc": { + "general": { + "bzlTransitiveDigest": "iHNvzUWsnksXsSsXCqerZjXSoULm/+xfo+aIUcYmRdc=", + "usagesDigest": "9IP/e++b81Ga1zICxzXu+yqfe5i4DX5F/SsrgV5a568=", + "recordedFileInputs": {}, + "recordedDirentsInputs": {}, + "envVariables": {}, + "generatedRepoSpecs": { + "gcc": { + "repoRuleId": "@@llvm+//:http_bsdtar_archive.bzl%http_bsdtar_archive", + "attributes": { + "build_file": "@@llvm+//3rd_party/gcc:gcc.BUILD.bazel", + "includes": [ + "gcc/BASE-VER", + "gcc/DATESTAMP", + "gcc/ginclude/unwind-arm-common.h", + "config/acx.m4", + "config/cet.m4", + "config/futex.m4", + "config/gc++filt.m4", + "config/gthr.m4", + "config/hwcaps.m4", + "config/iconv.m4", + "config/lthostflags.m4", + "config/multi.m4", + "config/no-executables.m4", + "config/tls.m4", + "config/toolexeclibdir.m4", + "config/unwind_ipinfo.m4", + "include/ansidecl.h", + "include/demangle.h", + "include/dyn-string.h", + "include/getopt.h", + "include/libiberty.h", + "libgcc/gthr-posix.h", + "libgcc/gthr-single.h", + "libgcc/gthr.h", + "libgcc/config/arm/unwind-arm.h", + "libgcc/unwind-generic.h", + "libgcc/unwind-pe.h", + "libiberty/cp-demangle.c", + "libiberty/cp-demangle.h", + "libstdc++-v3/acinclude.m4", + "libstdc++-v3/config/**", + "libstdc++-v3/configure.ac", + "libstdc++-v3/configure.host", + "libstdc++-v3/crossconfig.m4", + "libstdc++-v3/linkage.m4", + "libstdc++-v3/include/**", + "libstdc++-v3/libsupc++/**", + "libstdc++-v3/src/**" + ], + "sha256": "dc033fdfd79caf199113446af6d082004534437b6ebd276f9732815d86cbe723", + "strip_prefix": "gcc-2bfd402f8569511901ec8fe7628f57471e6d240a", + "urls": [ + "https://github.com/gcc-mirror/gcc/archive/2bfd402f8569511901ec8fe7628f57471e6d240a.tar.gz" + ] + } + } + }, + "recordedRepoMappingEntries": [ + [ + "llvm+", + "bazel_lib", + "bazel_lib+" + ], + [ + "llvm+", + "bazel_tools", + "bazel_tools" + ] + ] + } + }, + "@@pybind11_bazel+//:python_configure.bzl%extension": { + "general": { + "bzlTransitiveDigest": "c9ZWWeXeu6bctL4/SsY2otFWyeFN0JJ20+ymGyJZtWk=", + "usagesDigest": "fycyB39YnXIJkfWCIXLUKJMZzANcuLy9ZE73hRucjFk=", + "recordedFileInputs": { + "@@pybind11_bazel+//MODULE.bazel": "88af1c246226d87e65be78ed49ecd1e6f5e98648558c14ce99176da041dc378e" + }, + "recordedDirentsInputs": {}, + "envVariables": {}, + "generatedRepoSpecs": { + "local_config_python": { + "repoRuleId": "@@pybind11_bazel+//:python_configure.bzl%python_configure", + "attributes": {} + }, + "pybind11": { + "repoRuleId": "@@bazel_tools//tools/build_defs/repo:http.bzl%http_archive", + "attributes": { + "build_file": "@@pybind11_bazel+//:pybind11.BUILD", + "strip_prefix": "pybind11-2.11.1", + "urls": [ + "https://github.com/pybind/pybind11/archive/v2.11.1.zip" + ] + } + } + }, + "recordedRepoMappingEntries": [ + [ + "pybind11_bazel+", + "bazel_tools", + "bazel_tools" + ] + ] + } + }, + "@@rules_fuzzing+//fuzzing/private:extensions.bzl%non_module_dependencies": { + "general": { + "bzlTransitiveDigest": "WHRlQQnxW7e7XMRBhq7SARkDarLDOAbg6iLaJpk5QYM=", + "usagesDigest": "wy6ISK6UOcBEjj/mvJ/S3WeXoO67X+1llb9yPyFtPgc=", + "recordedFileInputs": {}, + "recordedDirentsInputs": {}, + "envVariables": {}, + "generatedRepoSpecs": { + "platforms": { + "repoRuleId": "@@bazel_tools//tools/build_defs/repo:http.bzl%http_archive", + "attributes": { + "urls": [ + "https://mirror.bazel.build/github.com/bazelbuild/platforms/releases/download/0.0.8/platforms-0.0.8.tar.gz", + "https://github.com/bazelbuild/platforms/releases/download/0.0.8/platforms-0.0.8.tar.gz" + ], + "sha256": "8150406605389ececb6da07cbcb509d5637a3ab9a24bc69b1101531367d89d74" + } + }, + "rules_python": { + "repoRuleId": "@@bazel_tools//tools/build_defs/repo:http.bzl%http_archive", + "attributes": { + "sha256": "d70cd72a7a4880f0000a6346253414825c19cdd40a28289bdf67b8e6480edff8", + "strip_prefix": "rules_python-0.28.0", + "url": "https://github.com/bazelbuild/rules_python/releases/download/0.28.0/rules_python-0.28.0.tar.gz" + } + }, + "bazel_skylib": { + "repoRuleId": "@@bazel_tools//tools/build_defs/repo:http.bzl%http_archive", + "attributes": { + "sha256": "cd55a062e763b9349921f0f5db8c3933288dc8ba4f76dd9416aac68acee3cb94", + "urls": [ + "https://mirror.bazel.build/github.com/bazelbuild/bazel-skylib/releases/download/1.5.0/bazel-skylib-1.5.0.tar.gz", + "https://github.com/bazelbuild/bazel-skylib/releases/download/1.5.0/bazel-skylib-1.5.0.tar.gz" + ] + } + }, + "com_google_absl": { + "repoRuleId": "@@bazel_tools//tools/build_defs/repo:http.bzl%http_archive", + "attributes": { + "urls": [ + "https://github.com/abseil/abseil-cpp/archive/refs/tags/20240116.1.zip" + ], + "strip_prefix": "abseil-cpp-20240116.1", + "integrity": "sha256-7capMWOvWyoYbUaHF/b+I2U6XLMaHmky8KugWvfXYuk=" + } + }, + "rules_fuzzing_oss_fuzz": { + "repoRuleId": "@@rules_fuzzing+//fuzzing/private/oss_fuzz:repository.bzl%oss_fuzz_repository", + "attributes": {} + }, + "honggfuzz": { + "repoRuleId": "@@bazel_tools//tools/build_defs/repo:http.bzl%http_archive", + "attributes": { + "build_file": "@@rules_fuzzing+//:honggfuzz.BUILD", + "sha256": "6b18ba13bc1f36b7b950c72d80f19ea67fbadc0ac0bb297ec89ad91f2eaa423e", + "url": "https://github.com/google/honggfuzz/archive/2.5.zip", + "strip_prefix": "honggfuzz-2.5" + } + }, + "rules_fuzzing_jazzer": { + "repoRuleId": "@@bazel_tools//tools/build_defs/repo:http.bzl%http_jar", + "attributes": { + "sha256": "ee6feb569d88962d59cb59e8a31eb9d007c82683f3ebc64955fd5b96f277eec2", + "url": "https://repo1.maven.org/maven2/com/code-intelligence/jazzer/0.20.1/jazzer-0.20.1.jar" + } + }, + "rules_fuzzing_jazzer_api": { + "repoRuleId": "@@bazel_tools//tools/build_defs/repo:http.bzl%http_jar", + "attributes": { + "sha256": "f5a60242bc408f7fa20fccf10d6c5c5ea1fcb3c6f44642fec5af88373ae7aa1b", + "url": "https://repo1.maven.org/maven2/com/code-intelligence/jazzer-api/0.20.1/jazzer-api-0.20.1.jar" + } + } + }, + "recordedRepoMappingEntries": [ + [ + "rules_fuzzing+", + "bazel_tools", + "bazel_tools" + ] + ] + } + }, "@@rules_kotlin+//src/main/starlark/core/repositories:bzlmod_setup.bzl%rules_kotlin_extensions": { "general": { "bzlTransitiveDigest": "rL/34P1aFDq2GqVC2zCFgQ8nTuOC6ziogocpvG50Qz8=", diff --git a/benchmark/compare.py b/benchmark/compare.py index 1ae88b337..f4f3a3349 100644 --- a/benchmark/compare.py +++ b/benchmark/compare.py @@ -14,6 +14,7 @@ import argparse import json +import math import os import sys from pathlib import Path @@ -21,6 +22,9 @@ THRESHOLD_REGRESSION_PCT = 10 EM = "\u2014" # em dash; module constant so it can appear inside f-string expressions +TOTAL_SIGMA = 2.0 +FN_SIGMA = 3.0 +HOTSPOT_MIN_PCT = 1.0 def write_gh_output(text: str) -> None: @@ -91,55 +95,93 @@ def _short(name: str, limit: int = 48) -> str: def _starlark_section(main_result: dict[str, Any], pr_result: dict[str, Any]) -> str: - """Diagnostic 'where is the problem' diff of per-function Starlark CPU. - - Informational only -- does not affect the gate. Requires both main and PR to - carry a starlark_fn breakdown. + """Diagnostic Starlark CPU diff (PR vs main). Informational, not a gate. + + Two layers of noise control so the section stays quiet on a no-op PR: + - Total-level gate: only render the movers table when the PR's total + Starlark CPU is significantly above main's (TOTAL_SIGMA). The total + aggregates every sample, so its variance is small and the test is + robust without the multiple-comparisons problem. + - Per-function flagging: a mover is only marked significant when its delta + exceeds FN_SIGMA * combined stderr, surviving ~100 comparisons. + Hotspots (big, stable functions) are always shown. """ - main_fn = main_result.get("starlark_fn") or [] - pr_fn = pr_result.get("starlark_fn") or [] - if not main_fn or not pr_fn: - return "" - - main_map = {r["name"]: r["mean_ms"] for r in main_fn} - pr_map = {r["name"]: r["mean_ms"] for r in pr_fn} - pr_pct = {r["name"]: r.get("pct", 0.0) for r in pr_fn} - - movers = [] - for name, pr_ms in pr_map.items(): - m_ms = main_map.get(name, 0.0) - delta = pr_ms - m_ms - if delta > 0: - movers.append((name, m_ms, pr_ms, delta)) - movers.sort(key=lambda x: x[3], reverse=True) + main_sf = main_result.get("starlark_fn") + pr_sf = pr_result.get("starlark_fn") + if not isinstance(main_sf, dict) or not isinstance(pr_sf, dict): + return "" # missing or old (pre-stddev) schema -- can't do significance + + main_total = main_sf.get("total", {}) + pr_total = pr_sf.get("total", {}) + main_fns = {r["name"]: r for r in main_sf.get("functions", [])} + pr_fns = {r["name"]: r for r in pr_sf.get("functions", [])} + + runs = main_total.get("runs") or pr_total.get("runs") or 1 + n = max(runs, 1) + + def combined_se(m_std: float, p_std: float) -> float: + # Standard error of the difference of two independent means: each side + # contributes stddev/sqrt(n). Buggy /(n) would shrink the noise band + # ~sqrt(n)x and flag sampling jitter as significant. + sn = math.sqrt(n) + return math.sqrt((m_std / sn) ** 2 + (p_std / sn) ** 2) + + total_se = combined_se(main_total.get("stddev_ms", 0.0), pr_total.get("stddev_ms", 0.0)) + main_total_ms = main_total.get("mean_ms", 0.0) + pr_total_ms = pr_total.get("mean_ms", 0.0) + delta_total = pr_total_ms - main_total_ms + pct_total = pct(main_total_ms, pr_total_ms) if main_total_ms else 0.0 + total_significant = total_se > 0 and delta_total > TOTAL_SIGMA * total_se out = "\n### \U0001f50d Starlark CPU \u2014 where the problem is (PR vs main)\n\n" - if movers: - out += "**Top movers (biggest regression \u0394ms):**\n\n" - out += "| Function | main ms | PR ms | \u0394 ms | \u0394 % |\n|---|---|---|---|---|\n" - for name, m_ms, pr_ms, delta in movers[:10]: + out += ( + f"**Total Starlark CPU:** main {main_total_ms:.0f} ms, PR {pr_total_ms:.0f} ms " + f"(\u0394 {delta_total:+.0f} ms, {pct_total:+.1f}%, " + f"\u00b1{total_se:.0f} ms stderr over {runs} runs).\n\n" + ) + + if not total_significant: + out += ( + "\u2705 **No significant Starlark CPU change** \u2014 the total is within " + "run-to-run noise, so per-function deltas are hidden (they are sampling " + "jitter, not signal).\n" + ) + else: + movers = [] + for name, pr_r in pr_fns.items(): + m_r = main_fns.get(name) + m_ms = m_r["mean_ms"] if m_r else 0.0 + d = pr_r["mean_ms"] - m_ms + if d <= 0: + continue + se = combined_se(m_r.get("stddev_ms", 0.0) if m_r else 0.0, pr_r.get("stddev_ms", 0.0)) + movers.append((name, m_ms, pr_r["mean_ms"], d, se)) + movers.sort(key=lambda x: x[3], reverse=True) + + out += "**Top movers (candidates, \u0394 > 3\u03c3 marked):**\n\n" + out += "| Function | main ms | PR ms | \u0394 ms | \u00b1 stderr | \u0394 % |\n|---|---|---|---|---|---|\n" + for name, m_ms, pr_ms, d, se in movers[:10]: if m_ms > 0: - dpct = f"+{delta / m_ms * 100:.0f}%" + dpct = f"+{d / m_ms * 100:.0f}%" else: dpct = "new" - flag = " \u26a0\ufe0f" if (m_ms > 0 and delta / m_ms * 100 > THRESHOLD_REGRESSION_PCT) else "" + flag = " \u26a0\ufe0f" if (se > 0 and d > FN_SIGMA * se) else "" out += ( f"| `{_short(name)}` | {m_ms:.1f} | {pr_ms:.1f} | " - f"+{delta:.1f} | {dpct}{flag} |\n" + f"+{d:.1f} | \u00b1{se:.1f} | {dpct}{flag} |\n" ) - else: - out += "_No movers (no function got slower in PR)._ \n" - abs_top = sorted(pr_map.items(), key=lambda kv: kv[1], reverse=True)[:10] - out += "\n**Top by absolute time (PR):**\n\n" - out += "| Function | PR ms | % of total |\n|---|---|---|\n" - for name, ms in abs_top: - out += f"| `{_short(name)}` | {ms:.1f} | {pr_pct.get(name, 0.0):.1f}% |\n" + hot = [r for r in pr_fns.values() if r.get("pct", 0.0) >= HOTSPOT_MIN_PCT][:10] + if hot: + out += "\n**Top by absolute time (PR):**\n\n" + out += "| Function | PR ms | % of total |\n|---|---|---|\n" + for r in hot: + out += f"| `{_short(r['name'])}` | {r['mean_ms']:.1f} | {r.get('pct', 0.0):.1f}% |\n" out += ( "\n> Sample-based attribution (Starlark CPU across the whole `--nobuild` run: " - "bzlmod loading + analysis). Noisy for small functions; builtins are " - "attributed to the caller.\n" + "bzlmod loading + analysis). Significance is run-to-run stderr \u00d7 sigma; " + "builtins are attributed to the caller.\n" ) return out diff --git a/benchmark/profile_benchmark.py b/benchmark/profile_benchmark.py index 5faa79341..7eb238443 100644 --- a/benchmark/profile_benchmark.py +++ b/benchmark/profile_benchmark.py @@ -71,26 +71,37 @@ def stats_ms(values_ms: list[float]) -> dict[str, float]: } -def aggregate_starlark(star_runs: list[dict[str, float]]) -> list[dict[str, Any]]: - """Aggregate per-run {fn: ms} dicts into per-function mean CPU ms. +def aggregate_starlark(star_runs: list[dict[str, float]]) -> dict[str, Any]: + """Aggregate per-run {fn: ms} dicts into per-function stats + a run-level total. - Returns ALL functions (sorted desc), not a top-N: truncation happens only at - render time in compare.py. Truncating here would make the comparator treat a - function below one side's cutoff as absent (0.0) and misreport it as new. + Returns {'total': {mean_ms, stddev_ms, runs}, 'functions': [...]}. ALL + functions are kept (truncation is render-time in compare.py) and each carries + its stddev across runs so the comparator can flag only statistically + significant deltas instead of run-to-run sampling noise. """ + runs = len(star_runs) names: set[str] = set() for d in star_runs: names.update(d.keys()) - rows: list[dict[str, Any]] = [] + functions: list[dict[str, Any]] = [] for name in names: series = [d.get(name, 0.0) for d in star_runs] - rows.append({"name": name, "mean_ms": statistics.mean(series)}) - total = sum(r["mean_ms"] for r in rows) or 1.0 - for r in rows: - r["pct"] = r["mean_ms"] / total * 100.0 - r["runs"] = len(star_runs) - rows.sort(key=lambda r: r["mean_ms"], reverse=True) - return rows + functions.append({ + "name": name, + "mean_ms": statistics.mean(series), + "stddev_ms": statistics.stdev(series) if len(series) > 1 else 0.0, + }) + totals = [sum(d.values()) for d in star_runs] + total_mean = statistics.mean(totals) + total_std = statistics.stdev(totals) if len(totals) > 1 else 0.0 + for r in functions: + r["pct"] = (r["mean_ms"] / total_mean * 100.0) if total_mean else 0.0 + r["runs"] = runs + functions.sort(key=lambda r: r["mean_ms"], reverse=True) + return { + "total": {"mean_ms": total_mean, "stddev_ms": total_std, "runs": runs}, + "functions": functions, + } def substitute(command: list[str], replacements: dict[str, str]) -> list[str]: From 750747b3ba0f1674ddf6a48749895dce59ba4860 Mon Sep 17 00:00:00 2001 From: xangcastle Date: Thu, 16 Jul 2026 17:07:29 -0600 Subject: [PATCH 04/11] getting relative paths of the files --- MODULE.bazel.lock | 191 --------------------------------- benchmark/compare.py | 45 +++++++- benchmark/pprof_decode.py | 29 ++--- benchmark/profile_benchmark.py | 14 +-- 4 files changed, 65 insertions(+), 214 deletions(-) diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index 8b18d3e61..de05f67a0 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -301,197 +301,6 @@ ] } }, - "@@llvm+//3rd_party/gcc/extension:gcc.bzl%gcc": { - "general": { - "bzlTransitiveDigest": "iHNvzUWsnksXsSsXCqerZjXSoULm/+xfo+aIUcYmRdc=", - "usagesDigest": "9IP/e++b81Ga1zICxzXu+yqfe5i4DX5F/SsrgV5a568=", - "recordedFileInputs": {}, - "recordedDirentsInputs": {}, - "envVariables": {}, - "generatedRepoSpecs": { - "gcc": { - "repoRuleId": "@@llvm+//:http_bsdtar_archive.bzl%http_bsdtar_archive", - "attributes": { - "build_file": "@@llvm+//3rd_party/gcc:gcc.BUILD.bazel", - "includes": [ - "gcc/BASE-VER", - "gcc/DATESTAMP", - "gcc/ginclude/unwind-arm-common.h", - "config/acx.m4", - "config/cet.m4", - "config/futex.m4", - "config/gc++filt.m4", - "config/gthr.m4", - "config/hwcaps.m4", - "config/iconv.m4", - "config/lthostflags.m4", - "config/multi.m4", - "config/no-executables.m4", - "config/tls.m4", - "config/toolexeclibdir.m4", - "config/unwind_ipinfo.m4", - "include/ansidecl.h", - "include/demangle.h", - "include/dyn-string.h", - "include/getopt.h", - "include/libiberty.h", - "libgcc/gthr-posix.h", - "libgcc/gthr-single.h", - "libgcc/gthr.h", - "libgcc/config/arm/unwind-arm.h", - "libgcc/unwind-generic.h", - "libgcc/unwind-pe.h", - "libiberty/cp-demangle.c", - "libiberty/cp-demangle.h", - "libstdc++-v3/acinclude.m4", - "libstdc++-v3/config/**", - "libstdc++-v3/configure.ac", - "libstdc++-v3/configure.host", - "libstdc++-v3/crossconfig.m4", - "libstdc++-v3/linkage.m4", - "libstdc++-v3/include/**", - "libstdc++-v3/libsupc++/**", - "libstdc++-v3/src/**" - ], - "sha256": "dc033fdfd79caf199113446af6d082004534437b6ebd276f9732815d86cbe723", - "strip_prefix": "gcc-2bfd402f8569511901ec8fe7628f57471e6d240a", - "urls": [ - "https://github.com/gcc-mirror/gcc/archive/2bfd402f8569511901ec8fe7628f57471e6d240a.tar.gz" - ] - } - } - }, - "recordedRepoMappingEntries": [ - [ - "llvm+", - "bazel_lib", - "bazel_lib+" - ], - [ - "llvm+", - "bazel_tools", - "bazel_tools" - ] - ] - } - }, - "@@pybind11_bazel+//:python_configure.bzl%extension": { - "general": { - "bzlTransitiveDigest": "c9ZWWeXeu6bctL4/SsY2otFWyeFN0JJ20+ymGyJZtWk=", - "usagesDigest": "fycyB39YnXIJkfWCIXLUKJMZzANcuLy9ZE73hRucjFk=", - "recordedFileInputs": { - "@@pybind11_bazel+//MODULE.bazel": "88af1c246226d87e65be78ed49ecd1e6f5e98648558c14ce99176da041dc378e" - }, - "recordedDirentsInputs": {}, - "envVariables": {}, - "generatedRepoSpecs": { - "local_config_python": { - "repoRuleId": "@@pybind11_bazel+//:python_configure.bzl%python_configure", - "attributes": {} - }, - "pybind11": { - "repoRuleId": "@@bazel_tools//tools/build_defs/repo:http.bzl%http_archive", - "attributes": { - "build_file": "@@pybind11_bazel+//:pybind11.BUILD", - "strip_prefix": "pybind11-2.11.1", - "urls": [ - "https://github.com/pybind/pybind11/archive/v2.11.1.zip" - ] - } - } - }, - "recordedRepoMappingEntries": [ - [ - "pybind11_bazel+", - "bazel_tools", - "bazel_tools" - ] - ] - } - }, - "@@rules_fuzzing+//fuzzing/private:extensions.bzl%non_module_dependencies": { - "general": { - "bzlTransitiveDigest": "WHRlQQnxW7e7XMRBhq7SARkDarLDOAbg6iLaJpk5QYM=", - "usagesDigest": "wy6ISK6UOcBEjj/mvJ/S3WeXoO67X+1llb9yPyFtPgc=", - "recordedFileInputs": {}, - "recordedDirentsInputs": {}, - "envVariables": {}, - "generatedRepoSpecs": { - "platforms": { - "repoRuleId": "@@bazel_tools//tools/build_defs/repo:http.bzl%http_archive", - "attributes": { - "urls": [ - "https://mirror.bazel.build/github.com/bazelbuild/platforms/releases/download/0.0.8/platforms-0.0.8.tar.gz", - "https://github.com/bazelbuild/platforms/releases/download/0.0.8/platforms-0.0.8.tar.gz" - ], - "sha256": "8150406605389ececb6da07cbcb509d5637a3ab9a24bc69b1101531367d89d74" - } - }, - "rules_python": { - "repoRuleId": "@@bazel_tools//tools/build_defs/repo:http.bzl%http_archive", - "attributes": { - "sha256": "d70cd72a7a4880f0000a6346253414825c19cdd40a28289bdf67b8e6480edff8", - "strip_prefix": "rules_python-0.28.0", - "url": "https://github.com/bazelbuild/rules_python/releases/download/0.28.0/rules_python-0.28.0.tar.gz" - } - }, - "bazel_skylib": { - "repoRuleId": "@@bazel_tools//tools/build_defs/repo:http.bzl%http_archive", - "attributes": { - "sha256": "cd55a062e763b9349921f0f5db8c3933288dc8ba4f76dd9416aac68acee3cb94", - "urls": [ - "https://mirror.bazel.build/github.com/bazelbuild/bazel-skylib/releases/download/1.5.0/bazel-skylib-1.5.0.tar.gz", - "https://github.com/bazelbuild/bazel-skylib/releases/download/1.5.0/bazel-skylib-1.5.0.tar.gz" - ] - } - }, - "com_google_absl": { - "repoRuleId": "@@bazel_tools//tools/build_defs/repo:http.bzl%http_archive", - "attributes": { - "urls": [ - "https://github.com/abseil/abseil-cpp/archive/refs/tags/20240116.1.zip" - ], - "strip_prefix": "abseil-cpp-20240116.1", - "integrity": "sha256-7capMWOvWyoYbUaHF/b+I2U6XLMaHmky8KugWvfXYuk=" - } - }, - "rules_fuzzing_oss_fuzz": { - "repoRuleId": "@@rules_fuzzing+//fuzzing/private/oss_fuzz:repository.bzl%oss_fuzz_repository", - "attributes": {} - }, - "honggfuzz": { - "repoRuleId": "@@bazel_tools//tools/build_defs/repo:http.bzl%http_archive", - "attributes": { - "build_file": "@@rules_fuzzing+//:honggfuzz.BUILD", - "sha256": "6b18ba13bc1f36b7b950c72d80f19ea67fbadc0ac0bb297ec89ad91f2eaa423e", - "url": "https://github.com/google/honggfuzz/archive/2.5.zip", - "strip_prefix": "honggfuzz-2.5" - } - }, - "rules_fuzzing_jazzer": { - "repoRuleId": "@@bazel_tools//tools/build_defs/repo:http.bzl%http_jar", - "attributes": { - "sha256": "ee6feb569d88962d59cb59e8a31eb9d007c82683f3ebc64955fd5b96f277eec2", - "url": "https://repo1.maven.org/maven2/com/code-intelligence/jazzer/0.20.1/jazzer-0.20.1.jar" - } - }, - "rules_fuzzing_jazzer_api": { - "repoRuleId": "@@bazel_tools//tools/build_defs/repo:http.bzl%http_jar", - "attributes": { - "sha256": "f5a60242bc408f7fa20fccf10d6c5c5ea1fcb3c6f44642fec5af88373ae7aa1b", - "url": "https://repo1.maven.org/maven2/com/code-intelligence/jazzer-api/0.20.1/jazzer-api-0.20.1.jar" - } - } - }, - "recordedRepoMappingEntries": [ - [ - "rules_fuzzing+", - "bazel_tools", - "bazel_tools" - ] - ] - } - }, "@@rules_kotlin+//src/main/starlark/core/repositories:bzlmod_setup.bzl%rules_kotlin_extensions": { "general": { "bzlTransitiveDigest": "rL/34P1aFDq2GqVC2zCFgQ8nTuOC6ziogocpvG50Qz8=", diff --git a/benchmark/compare.py b/benchmark/compare.py index f4f3a3349..c0748978b 100644 --- a/benchmark/compare.py +++ b/benchmark/compare.py @@ -94,6 +94,38 @@ def _short(name: str, limit: int = 48) -> str: return name +# Top-level dirs in the rules_py repo; used to reduce noisy pprof file paths +# (external repo or absolute checkout paths) to a repo-relative, clickable path. +_RULES_PY_ROOTS = ("py/", "uv/", "docs/", "e2e/", "examples/", "tools/") + + +def _relpath(file: str | None, limit: int = 56) -> str: + """Reduce a pprof source file to something locatable. + + -> "builtin"; a bzlmod MODULE.bazel URL -> "bzlmod"; otherwise try + to find a rules_py top-level dir and return from there (works for both the + bzlmod external path and an absolute local-checkout path). + """ + if not file or file in ("", ""): + return "" + if file == "": + return "builtin" + if file.startswith("http"): + return "bzlmod" + for root in _RULES_PY_ROOTS: + idx = file.find("/" + root) + if idx != -1: + rel = file[idx + 1:] + return rel if len(rel) <= limit else rel[: limit - 1] + "\u2026" + if "/external/" in file: + after = file.split("/external/", 1)[1] + parts = after.split("/", 1) + rel = parts[1] if len(parts) > 1 else after + return rel if len(rel) <= limit else rel[: limit - 1] + "\u2026" + base = os.path.basename(file) + return base if len(base) <= limit else base[: limit - 1] + "\u2026" + + def _starlark_section(main_result: dict[str, Any], pr_result: dict[str, Any]) -> str: """Diagnostic Starlark CPU diff (PR vs main). Informational, not a gate. @@ -159,7 +191,7 @@ def combined_se(m_std: float, p_std: float) -> float: movers.sort(key=lambda x: x[3], reverse=True) out += "**Top movers (candidates, \u0394 > 3\u03c3 marked):**\n\n" - out += "| Function | main ms | PR ms | \u0394 ms | \u00b1 stderr | \u0394 % |\n|---|---|---|---|---|---|\n" + out += "| Function | File | main ms | PR ms | \u0394 ms | \u00b1 stderr | \u0394 % |\n|---|---|---|---|---|---|---|\n" for name, m_ms, pr_ms, d, se in movers[:10]: if m_ms > 0: dpct = f"+{d / m_ms * 100:.0f}%" @@ -167,16 +199,19 @@ def combined_se(m_std: float, p_std: float) -> float: dpct = "new" flag = " \u26a0\ufe0f" if (se > 0 and d > FN_SIGMA * se) else "" out += ( - f"| `{_short(name)}` | {m_ms:.1f} | {pr_ms:.1f} | " - f"+{d:.1f} | \u00b1{se:.1f} | {dpct}{flag} |\n" + f"| `{_short(name)}` | `{_relpath(pr_fns[name].get('file'))}` | {m_ms:.1f} | " + f"{pr_ms:.1f} | +{d:.1f} | \u00b1{se:.1f} | {dpct}{flag} |\n" ) hot = [r for r in pr_fns.values() if r.get("pct", 0.0) >= HOTSPOT_MIN_PCT][:10] if hot: out += "\n**Top by absolute time (PR):**\n\n" - out += "| Function | PR ms | % of total |\n|---|---|---|\n" + out += "| Function | File | PR ms | % of total |\n|---|---|---|---|\n" for r in hot: - out += f"| `{_short(r['name'])}` | {r['mean_ms']:.1f} | {r.get('pct', 0.0):.1f}% |\n" + out += ( + f"| `{_short(r['name'])}` | `{_relpath(r.get('file'))}` | " + f"{r['mean_ms']:.1f} | {r.get('pct', 0.0):.1f}% |\n" + ) out += ( "\n> Sample-based attribution (Starlark CPU across the whole `--nobuild` run: " diff --git a/benchmark/pprof_decode.py b/benchmark/pprof_decode.py index b4dcf2851..b3c0a58d8 100644 --- a/benchmark/pprof_decode.py +++ b/benchmark/pprof_decode.py @@ -87,10 +87,12 @@ def _read_payload(path: str) -> bytes: return open(path, "rb").read() -def decode_starlark_pprof(path: str) -> dict[str, float]: - """Return {function_name: cpu_ms} aggregated by leaf Starlark function. +def decode_starlark_pprof(path: str) -> dict[str, tuple[float, str]]: + """Return {function_name: (cpu_ms, source_file)} aggregated by leaf function. - Returns {} if the profile is missing or unparseable (fail-open). + source_file is the pprof Function.filename (e.g. an external repo path, or + ```` for native rules/methods). Returns {} on missing/unparseable + profile (fail-open). """ data = _read_payload(path) top = _fields(data) @@ -120,13 +122,14 @@ def s(idx: object | int) -> str: elif unit == "nanoseconds": val_idx, divisor = i, 1000000.0 - # function: id -> name index + # function: id -> (name index, filename index) fn_name: dict[int, int] = {} + fn_file: dict[int, int] = {} for chunk in top.get(5, []): f = _fields(chunk) fid = f.get(1, [0])[0] - name_idx = f.get(2, [0])[0] - fn_name[fid] = name_idx + fn_name[fid] = f.get(2, [0])[0] + fn_file[fid] = f.get(4, [0])[0] # location: id -> leaf function id (first line) loc_to_fn: dict[int, int] = {} @@ -139,6 +142,7 @@ def s(idx: object | int) -> str: loc_to_fn[lid] = line.get(1, [0])[0] totals: dict[str, float] = defaultdict(float) + files: dict[str, str] = {} for chunk in top.get(2, []): loc_ids = _parse_packed_or_singles(chunk, 1) values = _parse_packed_or_singles(chunk, 2) @@ -148,14 +152,15 @@ def s(idx: object | int) -> str: fid = loc_to_fn.get(leaf) if fid is None: continue - name_idx = fn_name.get(fid) - totals[s(name_idx)] += values[val_idx] / divisor + name = s(fn_name.get(fid)) + totals[name] += values[val_idx] / divisor + files.setdefault(name, s(fn_file.get(fid))) - return dict(totals) + return {name: (ms, files.get(name, "")) for name, ms in totals.items()} if __name__ == "__main__": import sys - rows = sorted(decode_starlark_pprof(sys.argv[1]).items(), key=lambda kv: -kv[1]) - for name, ms in rows[:25]: - print(f"{ms:9.2f} ms {name}") + rows = sorted(decode_starlark_pprof(sys.argv[1]).items(), key=lambda kv: -kv[1][0]) + for name, (ms, _file) in rows[:25]: + print(f"{ms:9.2f} ms {name} [{_file}]") diff --git a/benchmark/profile_benchmark.py b/benchmark/profile_benchmark.py index 7eb238443..f90f31881 100644 --- a/benchmark/profile_benchmark.py +++ b/benchmark/profile_benchmark.py @@ -71,13 +71,13 @@ def stats_ms(values_ms: list[float]) -> dict[str, float]: } -def aggregate_starlark(star_runs: list[dict[str, float]]) -> dict[str, Any]: - """Aggregate per-run {fn: ms} dicts into per-function stats + a run-level total. +def aggregate_starlark(star_runs: list[dict[str, tuple[float, str]]]) -> dict[str, Any]: + """Aggregate per-run {fn: (ms, file)} dicts into per-function stats + total. Returns {'total': {mean_ms, stddev_ms, runs}, 'functions': [...]}. ALL functions are kept (truncation is render-time in compare.py) and each carries - its stddev across runs so the comparator can flag only statistically - significant deltas instead of run-to-run sampling noise. + its stddev across runs and source file so the comparator can flag only + statistically significant deltas and point at where to read the code. """ runs = len(star_runs) names: set[str] = set() @@ -85,13 +85,15 @@ def aggregate_starlark(star_runs: list[dict[str, float]]) -> dict[str, Any]: names.update(d.keys()) functions: list[dict[str, Any]] = [] for name in names: - series = [d.get(name, 0.0) for d in star_runs] + series = [d[name][0] for d in star_runs if name in d] + file = next((d[name][1] for d in star_runs if name in d and d[name][1]), "") functions.append({ "name": name, + "file": file, "mean_ms": statistics.mean(series), "stddev_ms": statistics.stdev(series) if len(series) > 1 else 0.0, }) - totals = [sum(d.values()) for d in star_runs] + totals = [sum(ms for (ms, _f) in d.values()) for d in star_runs] total_mean = statistics.mean(totals) total_std = statistics.stdev(totals) if len(totals) > 1 else 0.0 for r in functions: From 384b0e36749d94083fa30e38452fbe6a00033cb4 Mon Sep 17 00:00:00 2001 From: xangcastle Date: Thu, 16 Jul 2026 18:09:14 -0600 Subject: [PATCH 05/11] Total-gate hiding a cancelled regression se>0 treating deterministic regressions as no-signal Conditional mean / dropped runs Single n for both sides --- benchmark/compare.py | 98 ++++++++++++++++++---------------- benchmark/profile_benchmark.py | 6 ++- 2 files changed, 57 insertions(+), 47 deletions(-) diff --git a/benchmark/compare.py b/benchmark/compare.py index c0748978b..964950f01 100644 --- a/benchmark/compare.py +++ b/benchmark/compare.py @@ -22,7 +22,6 @@ THRESHOLD_REGRESSION_PCT = 10 EM = "\u2014" # em dash; module constant so it can appear inside f-string expressions -TOTAL_SIGMA = 2.0 FN_SIGMA = 3.0 HOTSPOT_MIN_PCT = 1.0 @@ -52,6 +51,24 @@ def warn(delta: float) -> str: return "\u26a0\ufe0f" if delta > THRESHOLD_REGRESSION_PCT else "" +def _combined_se(m_std: float, m_n: int, p_std: float, p_n: int) -> float: + """Std error of (PR_mean - main_mean) for independent means with possibly + unequal run counts: sqrt(m_std^2/m_n + p_std^2/p_n). Per-side n matters -- + profile_benchmark only appends non-empty decoded profiles, so main and PR can + have different usable Starlark run counts. + """ + return math.sqrt((m_std ** 2) / max(m_n, 1) + (p_std ** 2) / max(p_n, 1)) + + +def _is_significant(delta: float, se: float) -> bool: + """A positive delta that exceeds the noise band. + + stderr == 0 with delta > 0 is significant: a deterministic regression is + real signal, not 'unknown' (the earlier `se > 0 and ...` guard hid it). + """ + return delta > 0 and (se == 0 or delta > FN_SIGMA * se) + + # --------------------------------------------------------------------------- # # analysis subcommand # --------------------------------------------------------------------------- # @@ -129,13 +146,11 @@ def _relpath(file: str | None, limit: int = 56) -> str: def _starlark_section(main_result: dict[str, Any], pr_result: dict[str, Any]) -> str: """Diagnostic Starlark CPU diff (PR vs main). Informational, not a gate. - Two layers of noise control so the section stays quiet on a no-op PR: - - Total-level gate: only render the movers table when the PR's total - Starlark CPU is significantly above main's (TOTAL_SIGMA). The total - aggregates every sample, so its variance is small and the test is - robust without the multiple-comparisons problem. - - Per-function flagging: a mover is only marked significant when its delta - exceeds FN_SIGMA * combined stderr, surviving ~100 comparisons. + Movers are computed PER FUNCTION and rendered if individually significant + (delta > FN_SIGMA * combined stderr, with stderr==0 && delta>0 counting as + significant). They are NOT gated on the signed run-level total: a real + per-function regression must still surface when an unrelated speedup cancels + it at the total level (which is exactly what this section exists to explain). Hotspots (big, stable functions) are always shown. """ main_sf = main_result.get("starlark_fn") @@ -148,60 +163,51 @@ def _starlark_section(main_result: dict[str, Any], pr_result: dict[str, Any]) -> main_fns = {r["name"]: r for r in main_sf.get("functions", [])} pr_fns = {r["name"]: r for r in pr_sf.get("functions", [])} - runs = main_total.get("runs") or pr_total.get("runs") or 1 - n = max(runs, 1) + main_n = max(main_total.get("runs", 1) or 1, 1) + pr_n = max(pr_total.get("runs", 1) or 1, 1) - def combined_se(m_std: float, p_std: float) -> float: - # Standard error of the difference of two independent means: each side - # contributes stddev/sqrt(n). Buggy /(n) would shrink the noise band - # ~sqrt(n)x and flag sampling jitter as significant. - sn = math.sqrt(n) - return math.sqrt((m_std / sn) ** 2 + (p_std / sn) ** 2) - - total_se = combined_se(main_total.get("stddev_ms", 0.0), pr_total.get("stddev_ms", 0.0)) main_total_ms = main_total.get("mean_ms", 0.0) pr_total_ms = pr_total.get("mean_ms", 0.0) delta_total = pr_total_ms - main_total_ms pct_total = pct(main_total_ms, pr_total_ms) if main_total_ms else 0.0 - total_significant = total_se > 0 and delta_total > TOTAL_SIGMA * total_se + total_se = _combined_se(main_total.get("stddev_ms", 0.0), main_n, + pr_total.get("stddev_ms", 0.0), pr_n) + + movers = [] + for name, pr_r in pr_fns.items(): + m_r = main_fns.get(name) + m_ms = m_r["mean_ms"] if m_r else 0.0 + delta = pr_r["mean_ms"] - m_ms + if delta <= 0: + continue + se = _combined_se(m_r.get("stddev_ms", 0.0) if m_r else 0.0, main_n, + pr_r.get("stddev_ms", 0.0), pr_n) + if not _is_significant(delta, se): + continue + movers.append((name, m_ms, pr_r["mean_ms"], delta, se)) + movers.sort(key=lambda x: x[3], reverse=True) out = "\n### \U0001f50d Starlark CPU \u2014 where the problem is (PR vs main)\n\n" out += ( f"**Total Starlark CPU:** main {main_total_ms:.0f} ms, PR {pr_total_ms:.0f} ms " - f"(\u0394 {delta_total:+.0f} ms, {pct_total:+.1f}%, " - f"\u00b1{total_se:.0f} ms stderr over {runs} runs).\n\n" + f"(\u0394 {delta_total:+.0f} ms, {pct_total:+.1f}%; main {main_n} runs, PR {pr_n} runs)." + " Total is context only \u2014 movers below are per-function.\n\n" ) - if not total_significant: - out += ( - "\u2705 **No significant Starlark CPU change** \u2014 the total is within " - "run-to-run noise, so per-function deltas are hidden (they are sampling " - "jitter, not signal).\n" - ) - else: - movers = [] - for name, pr_r in pr_fns.items(): - m_r = main_fns.get(name) - m_ms = m_r["mean_ms"] if m_r else 0.0 - d = pr_r["mean_ms"] - m_ms - if d <= 0: - continue - se = combined_se(m_r.get("stddev_ms", 0.0) if m_r else 0.0, pr_r.get("stddev_ms", 0.0)) - movers.append((name, m_ms, pr_r["mean_ms"], d, se)) - movers.sort(key=lambda x: x[3], reverse=True) - - out += "**Top movers (candidates, \u0394 > 3\u03c3 marked):**\n\n" + if movers: + out += "**Significant movers (\u0394 > 3\u03c3, independent of the total):**\n\n" out += "| Function | File | main ms | PR ms | \u0394 ms | \u00b1 stderr | \u0394 % |\n|---|---|---|---|---|---|---|\n" for name, m_ms, pr_ms, d, se in movers[:10]: - if m_ms > 0: - dpct = f"+{d / m_ms * 100:.0f}%" - else: - dpct = "new" - flag = " \u26a0\ufe0f" if (se > 0 and d > FN_SIGMA * se) else "" + dpct = f"+{d / m_ms * 100:.0f}%" if m_ms > 0 else "new" out += ( f"| `{_short(name)}` | `{_relpath(pr_fns[name].get('file'))}` | {m_ms:.1f} | " - f"{pr_ms:.1f} | +{d:.1f} | \u00b1{se:.1f} | {dpct}{flag} |\n" + f"{pr_ms:.1f} | +{d:.1f} | \u00b1{se:.1f} | {dpct} |\n" ) + else: + out += ( + "\u2705 **No significant per-function regressions** \u2014 all positive deltas are " + "within run-to-run noise.\n" + ) hot = [r for r in pr_fns.values() if r.get("pct", 0.0) >= HOTSPOT_MIN_PCT][:10] if hot: diff --git a/benchmark/profile_benchmark.py b/benchmark/profile_benchmark.py index f90f31881..4e2765bab 100644 --- a/benchmark/profile_benchmark.py +++ b/benchmark/profile_benchmark.py @@ -85,7 +85,11 @@ def aggregate_starlark(star_runs: list[dict[str, tuple[float, str]]]) -> dict[st names.update(d.keys()) functions: list[dict[str, Any]] = [] for name in names: - series = [d[name][0] for d in star_runs if name in d] + # A function missing from a run is a zero-time observation for that run, + # NOT a reason to drop the run: filtering would compute a conditional + # mean over only the runs where it appeared, understating variance and + # inflating intermittently-sampled functions. + series = [d[name][0] if name in d else 0.0 for d in star_runs] file = next((d[name][1] for d in star_runs if name in d and d[name][1]), "") functions.append({ "name": name, From 2f7f9dc701d3f10c09a3ab405cbfd253786747f0 Mon Sep 17 00:00:00 2001 From: xangcastle Date: Thu, 16 Jul 2026 18:35:50 -0600 Subject: [PATCH 06/11] Run summary --- .github/workflows/performance-benchmark.yml | 1 + benchmark/compare.py | 61 ++++++++++++++++++++- 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/.github/workflows/performance-benchmark.yml b/.github/workflows/performance-benchmark.yml index e99b837b9..e43cb8185 100644 --- a/.github/workflows/performance-benchmark.yml +++ b/.github/workflows/performance-benchmark.yml @@ -224,6 +224,7 @@ jobs: set -euo pipefail python3 rules_py_pr/benchmark/compare.py analysis \ --output-table analysis-table.txt \ + --step-summary "$GITHUB_STEP_SUMMARY" \ bcr.json main.json pr.json - name: Upload analysis artifacts diff --git a/benchmark/compare.py b/benchmark/compare.py index 964950f01..3949f43fe 100644 --- a/benchmark/compare.py +++ b/benchmark/compare.py @@ -227,6 +227,52 @@ def _starlark_section(main_result: dict[str, Any], pr_result: dict[str, Any]) -> return out +def _full_starlark_table(main_result: dict[str, Any], pr_result: dict[str, Any]) -> str: + """Full per-function main-vs-PR table (ALL functions, not truncated). + + Intended for the workflow run summary ($GITHUB_STEP_SUMMARY), where someone + wants to scan every function -- as opposed to the PR comment, which stays + terse (top movers/hotspots only). Sorted by PR time desc; a significant + regression (delta > FN_SIGMA * stderr) is marked. + """ + main_sf = main_result.get("starlark_fn") + pr_sf = pr_result.get("starlark_fn") + if not isinstance(main_sf, dict) or not isinstance(pr_sf, dict): + return "" + + main_total = main_sf.get("total", {}) + pr_total = pr_sf.get("total", {}) + main_fns = {r["name"]: r for r in main_sf.get("functions", [])} + pr_fns = {r["name"]: r for r in pr_sf.get("functions", [])} + main_n = max(main_total.get("runs", 1) or 1, 1) + pr_n = max(pr_total.get("runs", 1) or 1, 1) + + rows = [] + for name, pr_r in pr_fns.items(): + m_r = main_fns.get(name) + m_ms = m_r["mean_ms"] if m_r else 0.0 + delta = pr_r["mean_ms"] - m_ms + se = _combined_se(m_r.get("stddev_ms", 0.0) if m_r else 0.0, main_n, + pr_r.get("stddev_ms", 0.0), pr_n) + file = (pr_r.get("file") or (m_r.get("file") if m_r else "") or "") + rows.append((name, file, m_ms, pr_r["mean_ms"], delta, se)) + rows.sort(key=lambda x: x[3], reverse=True) + + out = "## Starlark CPU \u2014 full per-function (main vs PR)\n\n" + out += ( + f"_All {len(rows)} functions, sorted by PR time. " + f"\u26a0\ufe0f = significant regression (\u0394 > {FN_SIGMA:g}\u03c3)._\n\n" + ) + out += "| Function | File | main ms | PR ms | \u0394 ms | \u00b1 stderr |\n|---|---|---|---|---|---|\n" + for name, file, m_ms, pr_ms, delta, se in rows: + flag = " \u26a0\ufe0f" if _is_significant(delta, se) else "" + out += ( + f"| `{_short(name)}` | `{_relpath(file)}` | {m_ms:.1f} | {pr_ms:.1f} | " + f"{delta:+.1f}{flag} | \u00b1{se:.1f} |\n" + ) + return out + + def run_analysis(args: argparse.Namespace) -> int: bcr_path, main_path, pr_path = args.bcr, args.main, args.pr @@ -279,6 +325,12 @@ def row(label: str, d: dict[str, Any], vs_bcr: str, vs_main: str, aux: dict[str, _emit(table, args.output_table) + if args.step_summary: + full = _full_starlark_table(main, pr) + if full: + with open(args.step_summary, "a") as f: + f.write(full) + if is_regression(main, pr, THRESHOLD_REGRESSION_PCT): print(f"\n\u274c REGRESSION: PR analysis is {pr_vs_main:.1f}% slower than HEAD main " f"(threshold: {THRESHOLD_REGRESSION_PCT}%)") @@ -429,7 +481,14 @@ def add_common(p: argparse.ArgumentParser) -> None: p.add_argument("pr", help="PR result JSON") p.add_argument("--output-table", help="write only the markdown table to this file") - add_common(sub.add_parser("analysis", help="analysis_ms gate + Starlark CPU diagnostic")) + analysis_parser = sub.add_parser("analysis", help="analysis_ms gate + Starlark CPU diagnostic") + add_common(analysis_parser) + analysis_parser.add_argument( + "--step-summary", + default=None, + help="append the full per-function main-vs-PR table to this file " + "(e.g. $GITHUB_STEP_SUMMARY for the workflow run summary)", + ) add_common(sub.add_parser("startup", help="runtime gate + sys.path quality")) args = parser.parse_args() From 4fb02b4e7dcc5c4a6cb6f3df5a2fa66750a6b61b Mon Sep 17 00:00:00 2001 From: xangcastle Date: Thu, 16 Jul 2026 20:48:14 -0600 Subject: [PATCH 07/11] full analysis with dep groups --- benchmark/workspace/generate_workspace.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/benchmark/workspace/generate_workspace.py b/benchmark/workspace/generate_workspace.py index a76522d7c..c7bac0208 100644 --- a/benchmark/workspace/generate_workspace.py +++ b/benchmark/workspace/generate_workspace.py @@ -212,12 +212,19 @@ def generate_root_build(root: Path, package_count: int) -> None: //:bench depends on every local py_library so its venv scales with package_count; bench_main.py is import-free (see BENCH_MAIN_TEMPLATE). + + //:all_group_deps closes the benchmark's hub-coverage gap: it depends on the + whole active dep_group via group_deps(), which forces Bazel to load and + analyze every @pypi hub alias + the project repo graph -- the path real + py_venv consumers hit. Without it, //:bench's direct @pypi// labels only + load the reachable subset and hub/project over-generation stays inert. """ pkg_libs = [f"//src/pkg_{i}:pkg_{i}" for i in range(package_count)] pkg_bins = [f"//src/pkg_{i}:pkg_{i}_bin" for i in range(package_count)] lines = [ - 'load("@aspect_rules_py//py:defs.bzl", "py_binary")\n', - 'load("@bazel_skylib//rules:build_test.bzl", "build_test")\n\n', + 'load("@aspect_rules_py//py:defs.bzl", "py_binary", "py_library")\n', + 'load("@bazel_skylib//rules:build_test.bzl", "build_test")\n', + 'load("@pypi//:defs.bzl", "group_deps")\n\n', 'build_test(\n', ' name = "all_bins",\n', f' targets = {pkg_bins},\n', @@ -227,6 +234,13 @@ def generate_root_build(root: Path, package_count: int) -> None: ' srcs = ["bench_main.py"],\n', ' main = "bench_main.py",\n', f' deps = {pkg_libs},\n', + ')\n\n', + '# Forces analysis of the full @pypi hub + project graph for the active\n', + '# dep_group (the realistic py_venv consumer path).\n', + 'py_library(\n', + ' name = "all_group_deps",\n', + ' deps = group_deps(),\n', + ' visibility = ["//visibility:public"],\n', ')\n', ] (root / "BUILD.bazel").write_text("".join(lines)) From d7fb38f3d9c39d532279360e6d7df41c104b684d Mon Sep 17 00:00:00 2001 From: xangcastle Date: Thu, 16 Jul 2026 22:35:23 -0600 Subject: [PATCH 08/11] full analysis with dep groups --- benchmark/compare.py | 29 +++++++++++++ benchmark/profile_benchmark.py | 74 +++++++++++++++++++++++++++++++--- 2 files changed, 98 insertions(+), 5 deletions(-) diff --git a/benchmark/compare.py b/benchmark/compare.py index 3949f43fe..4e422f4a5 100644 --- a/benchmark/compare.py +++ b/benchmark/compare.py @@ -273,6 +273,31 @@ def _full_starlark_table(main_result: dict[str, Any], pr_result: dict[str, Any]) return out +def _phase_table(main_result: dict[str, Any], pr_result: dict[str, Any]) -> str: + """Inclusive wall time per profile phase, main vs PR (step summary). + + Phase durations are inclusive (spans nest, e.g. runAnalysisPhase is inside + buildTargets), so they are NOT additive. Surfaces the bzlmod/uv + module-extension eval separately from analysis -- that is where hub/project + target generation costs and which runAnalysisPhase cannot see. + """ + main_ph = {p["name"]: p for p in main_result.get("profile_phases", [])} + pr_ph = {p["name"]: p for p in pr_result.get("profile_phases", [])} + if not main_ph and not pr_ph: + return "" + names = sorted(set(main_ph) | set(pr_ph), + key=lambda n: -(pr_ph.get(n, main_ph.get(n, {})).get("mean_ms", 0.0))) + + out = "## Bazel profile phases (inclusive, non-additive)\n\n" + out += "_Where wall time goes by phase; module-extension eval is separate from analysis._\n\n" + out += "| Phase | main ms | PR ms | \u0394 ms |\n|---|---|---|---|\n" + for name in names[:25]: + m = main_ph.get(name, {}).get("mean_ms", 0.0) + p = pr_ph.get(name, {}).get("mean_ms", 0.0) + out += f"| `{_short(name, 60)}` | {m:.0f} | {p:.0f} | {p - m:+.0f} |\n" + return out + + def run_analysis(args: argparse.Namespace) -> int: bcr_path, main_path, pr_path = args.bcr, args.main, args.pr @@ -330,6 +355,10 @@ def row(label: str, d: dict[str, Any], vs_bcr: str, vs_main: str, aux: dict[str, if full: with open(args.step_summary, "a") as f: f.write(full) + phases = _phase_table(main, pr) + if phases: + with open(args.step_summary, "a") as f: + f.write(phases) if is_regression(main, pr, THRESHOLD_REGRESSION_PCT): print(f"\n\u274c REGRESSION: PR analysis is {pr_vs_main:.1f}% slower than HEAD main " diff --git a/benchmark/profile_benchmark.py b/benchmark/profile_benchmark.py index 4e2765bab..70db486d6 100644 --- a/benchmark/profile_benchmark.py +++ b/benchmark/profile_benchmark.py @@ -58,6 +58,42 @@ def extract_analysis_us(profile_path: str) -> int | None: return None +# Structural phase categories from the main --profile chrome trace. Durations +# are inclusive wall time per named span (spans nest, so they are NOT additive). +# Per-function Starlark events are excluded -- they live in the starlark_fn +# breakdown from --starlark_cpu_profile. +_PHASE_CATEGORIES = { + "general information", + "bazel module processing", + "build phase marker", + "Fetching repository", + "skyframe evaluator", + "package creation", +} + + +def extract_phases(profile_path: str) -> dict[str, float]: + """Return {event_name: inclusive_ms} for structural phase events. + + Surfaces where the run's wall time goes by phase -- critically the bzlmod / + uv module-extension eval (``evaluate module extension: ...`` under + ``bazel module processing``), which is where hub/project target generation + actually costs and which runAnalysisPhase cannot see. + """ + with gzip.open(profile_path, "rt") as f: + data = json.load(f) + phases: dict[str, float] = {} + for event in data.get("traceEvents", []): + if event.get("cat") not in _PHASE_CATEGORIES: + continue + dur = event.get("dur") + if not dur: + continue + name = event.get("name", "") + phases[name] = phases.get(name, 0.0) + dur / 1000.0 + return phases + + def stats_ms(values_ms: list[float]) -> dict[str, float]: """Aggregate millisecond durations into statistics (min/mean/median/stddev).""" count = len(values_ms) @@ -71,6 +107,24 @@ def stats_ms(values_ms: list[float]) -> dict[str, float]: } +def aggregate_phases(phase_runs: list[dict[str, float]]) -> list[dict[str, Any]]: + """Aggregate per-run {phase_name: inclusive_ms} dicts, top phases by mean.""" + names: set[str] = set() + for d in phase_runs: + names.update(d.keys()) + rows: list[dict[str, Any]] = [] + for name in names: + series = [d.get(name, 0.0) for d in phase_runs] + rows.append({ + "name": name, + "mean_ms": statistics.mean(series), + "stddev_ms": statistics.stdev(series) if len(series) > 1 else 0.0, + "runs": len(phase_runs), + }) + rows.sort(key=lambda r: r["mean_ms"], reverse=True) + return rows + + def aggregate_starlark(star_runs: list[dict[str, tuple[float, str]]]) -> dict[str, Any]: """Aggregate per-run {fn: (ms, file)} dicts into per-function stats + total. @@ -138,8 +192,11 @@ def run_once( profile_path: str, star_path: str | None, cwd: str | None = None, -) -> tuple[int, float, dict[str, float] | None]: - """Run a single measured invocation. Returns (analysis_us, wall_ms, starlark_fn|None).""" +) -> tuple[int, float, dict[str, float] | None, dict[str, float]]: + """Run a single measured invocation. + + Returns (analysis_us, wall_ms, starlark_fn|None, profile_phases). + """ if prepare: subprocess.run(prepare, shell=True, check=True) replacements = {PROFILE_PLACEHOLDER: profile_path} @@ -155,7 +212,8 @@ def run_once( if analysis_us is None: raise SystemExit(f"ERROR: runAnalysisPhase not found in {profile_path}") starlark = decode_starlark_pprof(star_path) if star_path is not None else None - return analysis_us, wall_ms, starlark + phases = extract_phases(profile_path) + return analysis_us, wall_ms, starlark, phases def _workspace_dir() -> Path: @@ -272,11 +330,13 @@ def main() -> None: analysis_us: list[float] = [] wall_ms: list[float] = [] star_runs: list[dict[str, float]] = [] + phase_runs: list[dict[str, float]] = [] for i in range(args.runs): - a, w, star = run_once(command, args.prepare, profile_path, - star_path if star_enabled else None, cwd=run_cwd) + a, w, star, phases = run_once(command, args.prepare, profile_path, + star_path if star_enabled else None, cwd=run_cwd) analysis_us.append(a) wall_ms.append(w) + phase_runs.append(phases) if star: star_runs.append(star) print( @@ -294,6 +354,7 @@ def main() -> None: output: dict[str, Any] = { "analysis_ms": stats_ms([us / 1000.0 for us in analysis_us]), "wall_ms": stats_ms(wall_ms), + "profile_phases": aggregate_phases(phase_runs), } if star_runs: @@ -308,6 +369,9 @@ def main() -> None: f"stddev={a['stddev']:.1f} (n={a['runs']})", file=sys.stderr, ) + print("profile phases (inclusive, non-additive):", file=sys.stderr) + for p in output["profile_phases"][:8]: + print(f" {p['mean_ms']:8.1f} ms {p['name']}", file=sys.stderr) if args.output: print(f"wrote {args.output}", file=sys.stderr) From e530b98f1ca0cb8be19f6148b60031d30c1883b2 Mon Sep 17 00:00:00 2001 From: xangcastle Date: Fri, 17 Jul 2026 00:08:02 -0600 Subject: [PATCH 09/11] full analysis with dep groups --- .github/workflows/performance-benchmark.yml | 318 +++++++------------- benchmark/compare.py | 148 +++++---- 2 files changed, 169 insertions(+), 297 deletions(-) diff --git a/.github/workflows/performance-benchmark.yml b/.github/workflows/performance-benchmark.yml index e43cb8185..f3dd6f1dc 100644 --- a/.github/workflows/performance-benchmark.yml +++ b/.github/workflows/performance-benchmark.yml @@ -6,18 +6,32 @@ on: branches: [main] workflow_dispatch: -permissions: - pull-requests: write - env: BCR_VERSION: "2.0.0-alpha.4" PACKAGES: "50" jobs: - startup: + bench: runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + include: + - variant: bcr + label: "BCR 2.0.0-alpha.4 (baseline)" + module: 'bcr --version "${{ env.BCR_VERSION }}"' + needs-main: false + - variant: main + label: "HEAD main" + module: 'local --path "$GITHUB_WORKSPACE/rules_py_main"' + needs-main: true + - variant: pr + label: "This PR" + module: 'local --path "$GITHUB_WORKSPACE/rules_py_pr"' + needs-main: false permissions: contents: read + pull-requests: write steps: - name: Checkout PR uses: actions/checkout@v6 @@ -26,6 +40,7 @@ jobs: path: rules_py_pr - name: Checkout HEAD main + if: matrix.needs-main uses: actions/checkout@v6 with: ref: main @@ -40,7 +55,6 @@ jobs: wget -qO "$DEB" "$URL" sudo dpkg -i "$DEB" rm -f "$DEB" - hyperfine --version - name: Generate workspace run: | @@ -48,250 +62,120 @@ jobs: cd rules_py_pr/benchmark/workspace python3 generate_workspace.py --root . --packages "${PACKAGES}" - # ── BCR baseline ──────────────────────────────────────────────────────── - - name: Benchmark BCR ${{ env.BCR_VERSION }} + - name: Benchmark ${{ matrix.variant }} run: | set -euo pipefail cd rules_py_pr/benchmark/workspace - python3 generate_module.py bcr --version "${BCR_VERSION}" + python3 generate_module.py ${{ matrix.module }} - OUT_BASE="/tmp/bazel-bcr" + OUT_BASE="/tmp/bazel-${{ matrix.variant }}" rm -rf "$OUT_BASE" - bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc build --disk_cache= //:bench - - BIN=$(bazel --output_base="$OUT_BASE" cquery //:bench --disk_cache= --output=starlark --starlark:expr='target.files_to_run.executable.path' | tail -n1) - test -x "$BIN" || { echo "ERROR: bench binary not executable: $BIN"; exit 1; } - RUNFILES_DIR="$PWD/$BIN.runfiles" hyperfine --warmup 5 --runs 50 --export-json "${GITHUB_WORKSPACE}/bcr.json" "$BIN" - RUNFILES_DIR="$PWD/$BIN.runfiles" "$BIN" "${GITHUB_WORKSPACE}/bcr-syspath.json" - - # ── HEAD main ─────────────────────────────────────────────────────────── - - name: Benchmark HEAD main - run: | - set -euo pipefail - cd rules_py_pr/benchmark/workspace - python3 generate_module.py local --path "$GITHUB_WORKSPACE/rules_py_main" - - OUT_BASE="/tmp/bazel-main" - rm -rf "$OUT_BASE" - bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc build --disk_cache= //:bench - BIN=$(bazel --output_base="$OUT_BASE" cquery //:bench --disk_cache= --output=starlark --starlark:expr='target.files_to_run.executable.path' | tail -n1) - test -x "$BIN" || { echo "ERROR: bench binary not executable: $BIN"; exit 1; } - RUNFILES_DIR="$PWD/$BIN.runfiles" hyperfine --warmup 5 --runs 50 --export-json "${GITHUB_WORKSPACE}/main.json" "$BIN" - RUNFILES_DIR="$PWD/$BIN.runfiles" "$BIN" "${GITHUB_WORKSPACE}/main-syspath.json" + # --- analysis --- + bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc fetch //... + python3 ../profile_benchmark.py \ + --runs 10 --warmup 1 \ + --prepare "rm -rf /tmp/bazel-${{ matrix.variant }}-analysis" \ + --output "${GITHUB_WORKSPACE}/${{ matrix.variant }}-analysis.json" \ + --save-profile "${GITHUB_WORKSPACE}/profiles/${{ matrix.variant }}-profile.gz" \ + --save-starlark "${GITHUB_WORKSPACE}/profiles/${{ matrix.variant }}-starlark.pprof.gz" \ + -- bazel --output_base=/tmp/bazel-${{ matrix.variant }}-analysis --bazelrc=../../.github/workflows/ci.bazelrc build --disk_cache= --nobuild --profile={PROFILE} --starlark_cpu_profile={STARPROFILE} //... - # ── Current commit (PR) ───────────────────────────────────────────────── - - name: Benchmark current PR - run: | - set -euo pipefail - cd rules_py_pr/benchmark/workspace - python3 generate_module.py local --path "$GITHUB_WORKSPACE/rules_py_pr" + bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc query //... --output=package > /tmp/${{ matrix.variant }}-packages.txt + bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc query //... > /tmp/${{ matrix.variant }}-targets.txt + echo "{\"packages\": $(wc -l < /tmp/${{ matrix.variant }}-packages.txt | tr -d ' '), \"targets\": $(wc -l < /tmp/${{ matrix.variant }}-targets.txt | tr -d ' ')}" > "${GITHUB_WORKSPACE}/${{ matrix.variant }}-analysis-aux.json" - OUT_BASE="/tmp/bazel-pr" - rm -rf "$OUT_BASE" + # --- startup --- bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc build --disk_cache= //:bench - BIN=$(bazel --output_base="$OUT_BASE" cquery //:bench --disk_cache= --output=starlark --starlark:expr='target.files_to_run.executable.path' | tail -n1) test -x "$BIN" || { echo "ERROR: bench binary not executable: $BIN"; exit 1; } - RUNFILES_DIR="$PWD/$BIN.runfiles" hyperfine --warmup 5 --runs 50 --export-json "${GITHUB_WORKSPACE}/pr.json" "$BIN" - RUNFILES_DIR="$PWD/$BIN.runfiles" "$BIN" "${GITHUB_WORKSPACE}/pr-syspath.json" + RUNFILES_DIR="$PWD/$BIN.runfiles" hyperfine --warmup 5 --runs 50 --export-json "${GITHUB_WORKSPACE}/${{ matrix.variant }}-startup.json" "$BIN" + RUNFILES_DIR="$PWD/$BIN.runfiles" "$BIN" "${GITHUB_WORKSPACE}/${{ matrix.variant }}-startup-syspath.json" - # ── Compare ───────────────────────────────────────────────────────────── - - name: Compare startup results - id: compare - continue-on-error: true + - name: Write raw summary run: | - set -euo pipefail - python3 rules_py_pr/benchmark/compare.py startup \ - --output-table startup-table.txt \ - bcr.json main.json pr.json - - - name: Save PR number - if: github.event_name == 'pull_request' - run: echo "${{ github.event.pull_request.number }}" > pr-number.txt - - - name: Upload startup artifacts - if: github.event_name == 'pull_request' + echo "## ${{ matrix.label }}" >> "$GITHUB_STEP_SUMMARY" + python3 -c " + import json + a = json.load(open('${GITHUB_WORKSPACE}/${{ matrix.variant }}-analysis.json')) + am = a['analysis_ms'] + print(f'- **Analysis**: {am[\"mean\"]:.0f}ms ±{am[\"stddev\"]:.0f}ms' if 'analysis_ms' in a else '- Analysis: pending') + s = json.load(open('${GITHUB_WORKSPACE}/${{ matrix.variant }}-startup.json')) + print(f'- **Startup**: {s[\"results\"][0][\"mean\"]*1000:.1f}ms') + " >> "$GITHUB_STEP_SUMMARY" 2>/dev/null || echo "- (partial data)" >> "$GITHUB_STEP_SUMMARY" + + - name: Upload results uses: actions/upload-artifact@v4 with: - name: startup-benchmark-results + name: bench-${{ matrix.variant }} path: | - startup-table.txt - pr-number.txt - if-no-files-found: warn - - - name: Fail on regression - if: steps.compare.outcome == 'failure' - run: exit 1 - - analysis: - runs-on: ubuntu-latest - permissions: - contents: read - steps: - - name: Checkout PR - uses: actions/checkout@v6 - with: - fetch-depth: 0 - path: rules_py_pr - - - name: Checkout HEAD main - uses: actions/checkout@v6 - with: - ref: main - path: rules_py_main - - - name: Generate workspace - run: | - set -euo pipefail - cd rules_py_pr/benchmark/workspace - python3 generate_workspace.py --root . --packages "${PACKAGES}" - - # ── BCR baseline ──────────────────────────────────────────────────────── - - name: Benchmark BCR ${{ env.BCR_VERSION }} - run: | - set -euo pipefail - cd rules_py_pr/benchmark/workspace - python3 generate_module.py bcr --version "${BCR_VERSION}" - - OUT_BASE="/tmp/bazel-bcr" - rm -rf "$OUT_BASE" - bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc fetch //... - - python3 ../profile_benchmark.py \ - --runs 10 --warmup 1 \ - --prepare 'rm -rf /tmp/bazel-bcr-analysis' \ - --output "${GITHUB_WORKSPACE}/bcr.json" \ - --save-profile "${GITHUB_WORKSPACE}/profiles/bcr-profile.gz" \ - --save-starlark "${GITHUB_WORKSPACE}/profiles/bcr-starlark.pprof.gz" \ - -- bazel --output_base=/tmp/bazel-bcr-analysis --bazelrc=../../.github/workflows/ci.bazelrc build --disk_cache= --nobuild --profile={PROFILE} --starlark_cpu_profile={STARPROFILE} //... - - bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc query //... --output=package > /tmp/bcr-packages.txt - bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc query //... > /tmp/bcr-targets.txt - echo "{\"packages\": $(wc -l < /tmp/bcr-packages.txt | tr -d ' '), \"targets\": $(wc -l < /tmp/bcr-targets.txt | tr -d ' ')}" > "${GITHUB_WORKSPACE}/bcr-aux.json" - - # ── HEAD main ─────────────────────────────────────────────────────────── - - name: Benchmark HEAD main - run: | - set -euo pipefail - cd rules_py_pr/benchmark/workspace - python3 generate_module.py local --path "$GITHUB_WORKSPACE/rules_py_main" - - OUT_BASE="/tmp/bazel-main" - rm -rf "$OUT_BASE" - bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc fetch //... - - python3 ../profile_benchmark.py \ - --runs 10 --warmup 1 \ - --prepare 'rm -rf /tmp/bazel-main-analysis' \ - --output "${GITHUB_WORKSPACE}/main.json" \ - --save-profile "${GITHUB_WORKSPACE}/profiles/main-profile.gz" \ - --save-starlark "${GITHUB_WORKSPACE}/profiles/main-starlark.pprof.gz" \ - -- bazel --output_base=/tmp/bazel-main-analysis --bazelrc=../../.github/workflows/ci.bazelrc build --disk_cache= --nobuild --profile={PROFILE} --starlark_cpu_profile={STARPROFILE} //... - - bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc query //... --output=package > /tmp/main-packages.txt - bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc query //... > /tmp/main-targets.txt - echo "{\"packages\": $(wc -l < /tmp/main-packages.txt | tr -d ' '), \"targets\": $(wc -l < /tmp/main-targets.txt | tr -d ' ')}" > "${GITHUB_WORKSPACE}/main-aux.json" - - # ── Current commit (PR) ───────────────────────────────────────────────── - - name: Benchmark current PR - run: | - set -euo pipefail - cd rules_py_pr/benchmark/workspace - python3 generate_module.py local --path "$GITHUB_WORKSPACE/rules_py_pr" - - OUT_BASE="/tmp/bazel-pr" - rm -rf "$OUT_BASE" - bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc fetch //... - - python3 ../profile_benchmark.py \ - --runs 10 --warmup 1 \ - --prepare 'rm -rf /tmp/bazel-pr-analysis' \ - --output "${GITHUB_WORKSPACE}/pr.json" \ - --save-profile "${GITHUB_WORKSPACE}/profiles/pr-profile.gz" \ - --save-starlark "${GITHUB_WORKSPACE}/profiles/pr-starlark.pprof.gz" \ - -- bazel --output_base=/tmp/bazel-pr-analysis --bazelrc=../../.github/workflows/ci.bazelrc build --disk_cache= --nobuild --profile={PROFILE} --starlark_cpu_profile={STARPROFILE} //... - - bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc query //... --output=package > /tmp/pr-packages.txt - bazel --output_base="$OUT_BASE" --bazelrc=../../.github/workflows/ci.bazelrc query //... > /tmp/pr-targets.txt - echo "{\"packages\": $(wc -l < /tmp/pr-packages.txt | tr -d ' '), \"targets\": $(wc -l < /tmp/pr-targets.txt | tr -d ' ')}" > "${GITHUB_WORKSPACE}/pr-aux.json" + ${{ matrix.variant }}-analysis.json + ${{ matrix.variant }}-analysis-aux.json + ${{ matrix.variant }}-startup.json + ${{ matrix.variant }}-startup-syspath.json + profiles/ - # ── Compare ───────────────────────────────────────────────────────────── - - name: Compare analysis results - id: compare + - name: Update PR comment + if: github.event_name == 'pull_request' continue-on-error: true run: | set -euo pipefail - python3 rules_py_pr/benchmark/compare.py analysis \ - --output-table analysis-table.txt \ - --step-summary "$GITHUB_STEP_SUMMARY" \ - bcr.json main.json pr.json - - - name: Upload analysis artifacts + # Download all sibling results that have finished. + for v in bcr main pr; do + [ "$v" = "${{ matrix.variant }}" ] && continue + gh api repos/${{ github.repository }}/actions/artifacts \ + --jq ".artifacts[] | select(.name == \"bench-$v\") | .name" 2>/dev/null | grep -q . \ + && gh api repos/${{ github.repository }}/actions/artifacts \ + --jq ".artifacts[] | select(.name == \"bench-$v\") | .archive_download_url" \ + -H "Authorization: token ${{ secrets.GITHUB_TOKEN }}" \ + > /dev/null 2>&1 \ + && echo "sibling $v available" || echo "sibling $v pending" + done + + # Try to download sibling artifacts (may not exist yet). + for v in bcr main pr; do + [ "$v" = "${{ matrix.variant }}" ] && continue + gh run download "$GITHUB_RUN_ID" -n "bench-$v" -D "/tmp/$v" 2>/dev/null \ + && cp /tmp/$v/${v}-*.json . 2>/dev/null || true + done + + # Render comparison tables (pending variants show ⏳). + python3 rules_py_pr/benchmark/compare.py analysis --partial \ + --output-table /tmp/analysis-table.txt \ + bcr-analysis.json main-analysis.json pr-analysis.json 2>/dev/null || true + python3 rules_py_pr/benchmark/compare.py startup --partial \ + --output-table /tmp/startup-table.txt \ + bcr-startup.json main-startup.json pr-startup.json 2>/dev/null || true + echo "${{ github.event.pull_request.number }}" > /tmp/pr-number.txt + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + - name: Post comment if: github.event_name == 'pull_request' - uses: actions/upload-artifact@v4 - with: - name: analysis-benchmark-results - path: | - analysis-table.txt - profiles/ - if-no-files-found: warn - - - name: Fail on regression - if: steps.compare.outcome == 'failure' - run: exit 1 - - comment: - runs-on: ubuntu-latest - needs: [startup, analysis] - if: always() && github.event_name == 'pull_request' && (needs.startup.result == 'success' || needs.startup.result == 'failure') && (needs.analysis.result == 'success' || needs.analysis.result == 'failure') - permissions: - pull-requests: write - steps: - - name: Download startup artifacts - uses: actions/download-artifact@v4 - with: - name: startup-benchmark-results - - - name: Download analysis artifacts - uses: actions/download-artifact@v4 - with: - name: analysis-benchmark-results - - - name: Post combined benchmark comment + continue-on-error: true uses: actions/github-script@v7 + env: + RUNNER_OS: ${{ runner.os }} with: script: | const fs = require('fs'); - const startup = fs.readFileSync('startup-table.txt', 'utf8').trim(); - const analysis = fs.readFileSync('analysis-table.txt', 'utf8').trim(); - const prNumber = parseInt(fs.readFileSync('pr-number.txt', 'utf8').trim()); const header = ''; - const body = `${header}\n\n${startup}\n\n${analysis}`; + let body = header + '\n\n'; + try { body += fs.readFileSync('/tmp/analysis-table.txt', 'utf8').trim() + '\n\n'; } catch {} + try { body += fs.readFileSync('/tmp/startup-table.txt', 'utf8').trim(); + '\n'; } catch {} const { data: comments } = await github.rest.issues.listComments({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: prNumber, + owner: context.repo.owner, repo: context.repo.repo, + issue_number: context.issue.number, }); const existing = comments.find(c => c.body && c.body.includes(header)); if (existing) { await github.rest.issues.updateComment({ - owner: context.repo.owner, - repo: context.repo.repo, - comment_id: existing.id, - body: body, + owner: context.repo.owner, repo: context.repo.repo, + comment_id: existing.id, body: body, }); - console.log(`Updated comment ${existing.id}`); } else { - const { data: created } = await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: prNumber, - body: body, + await github.rest.issues.createComment({ + owner: context.repo.owner, repo: context.repo.repo, + issue_number: context.issue.number, body: body, }); - console.log(`Created comment ${created.id}`); } - - - name: Fail if any benchmark regressed - if: needs.startup.result == 'failure' || needs.analysis.result == 'failure' - run: exit 1 diff --git a/benchmark/compare.py b/benchmark/compare.py index 4e422f4a5..7319d602a 100644 --- a/benchmark/compare.py +++ b/benchmark/compare.py @@ -73,10 +73,12 @@ def _is_significant(delta: float, se: float) -> bool: # analysis subcommand # --------------------------------------------------------------------------- # -def load_result(path: str) -> dict[str, Any]: +def load_result(path: str, missing_ok: bool = False) -> dict[str, Any]: """Load a profile_benchmark JSON result and validate the analysis_ms metric.""" p = Path(path) if not p.exists(): + if missing_ok: + return {"_pending": True} _fail(f"result file not found: {path}") with p.open() as f: data = json.load(f) @@ -143,6 +145,14 @@ def _relpath(file: str | None, limit: int = 56) -> str: return base if len(base) <= limit else base[: limit - 1] + "\u2026" +def _is_rules_py(file: str | None) -> bool: + """True if a pprof source file belongs to the rules_py repo itself.""" + if not file: + return False + rel = _relpath(file) + return rel.startswith("py/") or rel.startswith("uv/") + + def _starlark_section(main_result: dict[str, Any], pr_result: dict[str, Any]) -> str: """Diagnostic Starlark CPU diff (PR vs main). Informational, not a gate. @@ -188,14 +198,23 @@ def _starlark_section(main_result: dict[str, Any], pr_result: dict[str, Any]) -> movers.sort(key=lambda x: x[3], reverse=True) out = "\n### \U0001f50d Starlark CPU \u2014 where the problem is (PR vs main)\n\n" + + # Honest context: how much of the build's Starlark time is actually rules_py. + # This sets expectations -- rules_py is usually a minority (the rest is Bazel + # builtins like alias/config_setting that rules_py merely invokes, plus other + # deps). Measured levers (alias count, selects, decode) are all small. + pr_all_ms = sum(r["mean_ms"] for r in pr_fns.values()) + rules_py_ms = sum(r["mean_ms"] for r in pr_fns.values() if _is_rules_py(r.get("file"))) + builtins_ms = sum(r["mean_ms"] for r in pr_fns.values() if _relpath(r.get("file")) == "builtin") + rp_pct = (rules_py_ms / pr_all_ms * 100.0) if pr_all_ms else 0.0 out += ( - f"**Total Starlark CPU:** main {main_total_ms:.0f} ms, PR {pr_total_ms:.0f} ms " - f"(\u0394 {delta_total:+.0f} ms, {pct_total:+.1f}%; main {main_n} runs, PR {pr_n} runs)." - " Total is context only \u2014 movers below are per-function.\n\n" + f"**Total:** main {main_total_ms:.0f} ms, PR {pr_total_ms:.0f} ms " + f"(\u0394 {delta_total:+.0f} ms). Of PR's Starlark, **rules_py = {rules_py_ms:.0f} ms " + f"({rp_pct:.0f}%)**, Bazel builtins = {builtins_ms:.0f} ms, rest = other deps.\n\n" ) if movers: - out += "**Significant movers (\u0394 > 3\u03c3, independent of the total):**\n\n" + out += "**Significant movers (\u0394 > 3\u03c3):**\n\n" out += "| Function | File | main ms | PR ms | \u0394 ms | \u00b1 stderr | \u0394 % |\n|---|---|---|---|---|---|---|\n" for name, m_ms, pr_ms, d, se in movers[:10]: dpct = f"+{d / m_ms * 100:.0f}%" if m_ms > 0 else "new" @@ -204,14 +223,15 @@ def _starlark_section(main_result: dict[str, Any], pr_result: dict[str, Any]) -> f"{pr_ms:.1f} | +{d:.1f} | \u00b1{se:.1f} | {dpct} |\n" ) else: - out += ( - "\u2705 **No significant per-function regressions** \u2014 all positive deltas are " - "within run-to-run noise.\n" - ) + out += "\u2705 **No significant per-function regressions** (all deltas within noise).\n" - hot = [r for r in pr_fns.values() if r.get("pct", 0.0) >= HOTSPOT_MIN_PCT][:10] + # Hotspots: ONLY rules_py's own functions (builtins like alias/config_setting + # are NOT actionable -- measured: reducing them does not help). This shows + # where rules_py's own time goes, which IS actionable. + hot = [r for r in pr_fns.values() if _is_rules_py(r.get("file"))] + hot = [r for r in hot if r.get("pct", 0.0) >= HOTSPOT_MIN_PCT][:8] if hot: - out += "\n**Top by absolute time (PR):**\n\n" + out += "\n**rules_py's own top functions (where its time goes):**\n\n" out += "| Function | File | PR ms | % of total |\n|---|---|---|---|\n" for r in hot: out += ( @@ -219,57 +239,6 @@ def _starlark_section(main_result: dict[str, Any], pr_result: dict[str, Any]) -> f"{r['mean_ms']:.1f} | {r.get('pct', 0.0):.1f}% |\n" ) - out += ( - "\n> Sample-based attribution (Starlark CPU across the whole `--nobuild` run: " - "bzlmod loading + analysis). Significance is run-to-run stderr \u00d7 sigma; " - "builtins are attributed to the caller.\n" - ) - return out - - -def _full_starlark_table(main_result: dict[str, Any], pr_result: dict[str, Any]) -> str: - """Full per-function main-vs-PR table (ALL functions, not truncated). - - Intended for the workflow run summary ($GITHUB_STEP_SUMMARY), where someone - wants to scan every function -- as opposed to the PR comment, which stays - terse (top movers/hotspots only). Sorted by PR time desc; a significant - regression (delta > FN_SIGMA * stderr) is marked. - """ - main_sf = main_result.get("starlark_fn") - pr_sf = pr_result.get("starlark_fn") - if not isinstance(main_sf, dict) or not isinstance(pr_sf, dict): - return "" - - main_total = main_sf.get("total", {}) - pr_total = pr_sf.get("total", {}) - main_fns = {r["name"]: r for r in main_sf.get("functions", [])} - pr_fns = {r["name"]: r for r in pr_sf.get("functions", [])} - main_n = max(main_total.get("runs", 1) or 1, 1) - pr_n = max(pr_total.get("runs", 1) or 1, 1) - - rows = [] - for name, pr_r in pr_fns.items(): - m_r = main_fns.get(name) - m_ms = m_r["mean_ms"] if m_r else 0.0 - delta = pr_r["mean_ms"] - m_ms - se = _combined_se(m_r.get("stddev_ms", 0.0) if m_r else 0.0, main_n, - pr_r.get("stddev_ms", 0.0), pr_n) - file = (pr_r.get("file") or (m_r.get("file") if m_r else "") or "") - rows.append((name, file, m_ms, pr_r["mean_ms"], delta, se)) - rows.sort(key=lambda x: x[3], reverse=True) - - out = "## Starlark CPU \u2014 full per-function (main vs PR)\n\n" - out += ( - f"_All {len(rows)} functions, sorted by PR time. " - f"\u26a0\ufe0f = significant regression (\u0394 > {FN_SIGMA:g}\u03c3)._\n\n" - ) - out += "| Function | File | main ms | PR ms | \u0394 ms | \u00b1 stderr |\n|---|---|---|---|---|---|\n" - for name, file, m_ms, pr_ms, delta, se in rows: - flag = " \u26a0\ufe0f" if _is_significant(delta, se) else "" - out += ( - f"| `{_short(name)}` | `{_relpath(file)}` | {m_ms:.1f} | {pr_ms:.1f} | " - f"{delta:+.1f}{flag} | \u00b1{se:.1f} |\n" - ) return out @@ -301,17 +270,23 @@ def _phase_table(main_result: dict[str, Any], pr_result: dict[str, Any]) -> str: def run_analysis(args: argparse.Namespace) -> int: bcr_path, main_path, pr_path = args.bcr, args.main, args.pr - bcr = load_result(bcr_path) - main = load_result(main_path) - pr = load_result(pr_path) + mo = getattr(args, "partial", False) + bcr = load_result(bcr_path, missing_ok=mo) + main = load_result(main_path, missing_ok=mo) + pr = load_result(pr_path, missing_ok=mo) bcr_aux = load_auxiliary(bcr_path.replace(".json", "-aux.json")) main_aux = load_auxiliary(main_path.replace(".json", "-aux.json")) pr_aux = load_auxiliary(pr_path.replace(".json", "-aux.json")) - main_vs_bcr = pct(bcr["analysis_ms"]["mean"], main["analysis_ms"]["mean"]) - pr_vs_bcr = pct(bcr["analysis_ms"]["mean"], pr["analysis_ms"]["mean"]) - pr_vs_main = pct(main["analysis_ms"]["mean"], pr["analysis_ms"]["mean"]) + _incomplete = bcr.get("_pending") or main.get("_pending") or pr.get("_pending") + + def _mean(d): + return d["analysis_ms"]["mean"] if not d.get("_pending") else 0.0 + + main_vs_bcr = pct(_mean(bcr), _mean(main)) if not _incomplete else 0.0 + pr_vs_bcr = pct(_mean(bcr), _mean(pr)) if not _incomplete else 0.0 + pr_vs_main = pct(_mean(main), _mean(pr)) if not _incomplete else 0.0 has_aux = bcr_aux is not None or main_aux is not None or pr_aux is not None @@ -325,6 +300,8 @@ def aux_cell(aux: dict[str, Any] | None) -> str: return f"{aux.get('packages', EM)} | {aux.get('targets', EM)}" def row(label: str, d: dict[str, Any], vs_bcr: str, vs_main: str, aux: dict[str, Any] | None) -> str: + if d.get("_pending"): + return f"| {label} | \u23f3 | \u23f3 | \u23f3 | \u23f3 | \u23f3 | \u23f3 | {aux_cell(aux)} |\n" a = d["analysis_ms"] wall = d.get("wall_ms", {}).get("mean") wall_str = fmt(wall) if wall is not None else EM @@ -351,15 +328,14 @@ def row(label: str, d: dict[str, Any], vs_bcr: str, vs_main: str, aux: dict[str, _emit(table, args.output_table) if args.step_summary: - full = _full_starlark_table(main, pr) - if full: - with open(args.step_summary, "a") as f: - f.write(full) phases = _phase_table(main, pr) if phases: with open(args.step_summary, "a") as f: f.write(phases) + if _incomplete: + print("\n\u23f3 Partial results \u2014 gate deferred (some variants still running).") + return 0 if is_regression(main, pr, THRESHOLD_REGRESSION_PCT): print(f"\n\u274c REGRESSION: PR analysis is {pr_vs_main:.1f}% slower than HEAD main " f"(threshold: {THRESHOLD_REGRESSION_PCT}%)") @@ -372,10 +348,12 @@ def row(label: str, d: dict[str, Any], vs_bcr: str, vs_main: str, aux: dict[str, # startup subcommand # --------------------------------------------------------------------------- # -def load_runtime(path: str) -> dict[str, Any]: +def load_runtime(path: str, missing_ok: bool = False) -> dict[str, Any]: """Load a single hyperfine runtime JSON result.""" p = Path(path) if not p.exists(): + if missing_ok: + return {"_pending": True} _fail(f"result file not found: {path}") with p.open() as f: data = json.load(f) @@ -410,17 +388,20 @@ def load_syspath(path: str) -> dict[str, int] | None: def run_startup(args: argparse.Namespace) -> int: bcr_path, main_path, pr_path = args.bcr, args.main, args.pr - bcr = load_runtime(bcr_path) - main = load_runtime(main_path) - pr = load_runtime(pr_path) + mo = getattr(args, "partial", False) + bcr = load_runtime(bcr_path, missing_ok=mo) + main = load_runtime(main_path, missing_ok=mo) + pr = load_runtime(pr_path, missing_ok=mo) bcr_syspath = load_syspath(bcr_path.replace(".json", "-syspath.json")) main_syspath = load_syspath(main_path.replace(".json", "-syspath.json")) pr_syspath = load_syspath(pr_path.replace(".json", "-syspath.json")) - rt_main_vs_bcr = pct(bcr["mean_ms"], main["mean_ms"]) - rt_pr_vs_bcr = pct(bcr["mean_ms"], pr["mean_ms"]) - rt_pr_vs_main = pct(main["mean_ms"], pr["mean_ms"]) + _inc = bcr.get("_pending") or main.get("_pending") or pr.get("_pending") + def _ms(d): return d["mean_ms"] if not d.get("_pending") else 0.0 + rt_main_vs_bcr = pct(_ms(bcr), _ms(main)) if not _inc else 0.0 + rt_pr_vs_bcr = pct(_ms(bcr), _ms(pr)) if not _inc else 0.0 + rt_pr_vs_main = pct(_ms(main), _ms(pr)) if not _inc else 0.0 has_syspath = bcr_syspath is not None or main_syspath is not None or pr_syspath is not None @@ -429,6 +410,8 @@ def run_startup(args: argparse.Namespace) -> int: table += "|---------|-------------|-------------|----------|--------|---------|\n" def row(label: str, d: dict[str, Any], vs_bcr: str, vs_main: str) -> str: + if d.get("_pending"): + return f"| {label} | \u23f3 | \u23f3 | \u23f3 | \u23f3 | \u23f3 |\n" return ( f"| {label} | {fmt(d['mean_ms'])} | {fmt(d['median_ms'])} | " f"\u00b1{fmt(d['stddev_ms'])} | {vs_bcr} | {vs_main} |\n" @@ -474,6 +457,9 @@ def syspath_row(label: str, sp: dict[str, int] | None) -> str: _emit(table, args.output_table) + if _inc: + print("\n\u23f3 Partial results \u2014 gate deferred.") + return 0 if rt_pr_vs_main > THRESHOLD_REGRESSION_PCT: print(f"\n\u274c REGRESSION: startup {rt_pr_vs_main:.1f}% slower than HEAD main " f"(threshold: {THRESHOLD_REGRESSION_PCT}%)") @@ -518,7 +504,9 @@ def add_common(p: argparse.ArgumentParser) -> None: help="append the full per-function main-vs-PR table to this file " "(e.g. $GITHUB_STEP_SUMMARY for the workflow run summary)", ) - add_common(sub.add_parser("startup", help="runtime gate + sys.path quality")) + for p in (analysis_parser, sub.add_parser("startup", help="runtime gate + sys.path quality")): + p.add_argument("--partial", action="store_true", + help="treat missing result JSONs as pending (show ⏳ instead of failing)") args = parser.parse_args() rc = run_analysis(args) if args.kind == "analysis" else run_startup(args) From ee151ad7d2cf0414e5794226efba8b81fd42c486 Mon Sep 17 00:00:00 2001 From: xangcastle Date: Fri, 17 Jul 2026 12:24:51 -0600 Subject: [PATCH 10/11] full analysis with dep groups --- .github/workflows/performance-benchmark.yml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/performance-benchmark.yml b/.github/workflows/performance-benchmark.yml index f3dd6f1dc..f048a2e85 100644 --- a/.github/workflows/performance-benchmark.yml +++ b/.github/workflows/performance-benchmark.yml @@ -19,15 +19,12 @@ jobs: include: - variant: bcr label: "BCR 2.0.0-alpha.4 (baseline)" - module: 'bcr --version "${{ env.BCR_VERSION }}"' needs-main: false - variant: main label: "HEAD main" - module: 'local --path "$GITHUB_WORKSPACE/rules_py_main"' needs-main: true - variant: pr label: "This PR" - module: 'local --path "$GITHUB_WORKSPACE/rules_py_pr"' needs-main: false permissions: contents: read @@ -66,7 +63,11 @@ jobs: run: | set -euo pipefail cd rules_py_pr/benchmark/workspace - python3 generate_module.py ${{ matrix.module }} + if [ "${{ matrix.variant }}" = "bcr" ]; then + python3 generate_module.py bcr --version "${BCR_VERSION}" + else + python3 generate_module.py local --path "$GITHUB_WORKSPACE/rules_py_${{ matrix.variant }}" + fi OUT_BASE="/tmp/bazel-${{ matrix.variant }}" rm -rf "$OUT_BASE" From 3b0bef4ca70727a7d648ff6eccaa8e2c5735cce2 Mon Sep 17 00:00:00 2001 From: xangcastle Date: Fri, 17 Jul 2026 16:15:29 -0600 Subject: [PATCH 11/11] full analysis with dep groups --- .github/workflows/performance-benchmark.yml | 129 +++++++++++--------- 1 file changed, 70 insertions(+), 59 deletions(-) diff --git a/.github/workflows/performance-benchmark.yml b/.github/workflows/performance-benchmark.yml index f048a2e85..6877b73cb 100644 --- a/.github/workflows/performance-benchmark.yml +++ b/.github/workflows/performance-benchmark.yml @@ -28,7 +28,6 @@ jobs: needs-main: false permissions: contents: read - pull-requests: write steps: - name: Checkout PR uses: actions/checkout@v6 @@ -93,18 +92,6 @@ jobs: RUNFILES_DIR="$PWD/$BIN.runfiles" hyperfine --warmup 5 --runs 50 --export-json "${GITHUB_WORKSPACE}/${{ matrix.variant }}-startup.json" "$BIN" RUNFILES_DIR="$PWD/$BIN.runfiles" "$BIN" "${GITHUB_WORKSPACE}/${{ matrix.variant }}-startup-syspath.json" - - name: Write raw summary - run: | - echo "## ${{ matrix.label }}" >> "$GITHUB_STEP_SUMMARY" - python3 -c " - import json - a = json.load(open('${GITHUB_WORKSPACE}/${{ matrix.variant }}-analysis.json')) - am = a['analysis_ms'] - print(f'- **Analysis**: {am[\"mean\"]:.0f}ms ±{am[\"stddev\"]:.0f}ms' if 'analysis_ms' in a else '- Analysis: pending') - s = json.load(open('${GITHUB_WORKSPACE}/${{ matrix.variant }}-startup.json')) - print(f'- **Startup**: {s[\"results\"][0][\"mean\"]*1000:.1f}ms') - " >> "$GITHUB_STEP_SUMMARY" 2>/dev/null || echo "- (partial data)" >> "$GITHUB_STEP_SUMMARY" - - name: Upload results uses: actions/upload-artifact@v4 with: @@ -116,67 +103,91 @@ jobs: ${{ matrix.variant }}-startup-syspath.json profiles/ - - name: Update PR comment - if: github.event_name == 'pull_request' + report: + needs: [bench] + if: always() + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write + steps: + - name: Checkout PR + uses: actions/checkout@v6 + with: + path: rules_py_pr + + - name: Download all results + uses: actions/download-artifact@v4 + with: + pattern: bench-* + merge-multiple: true + + - name: Compare analysis + id: analysis continue-on-error: true run: | - set -euo pipefail - # Download all sibling results that have finished. - for v in bcr main pr; do - [ "$v" = "${{ matrix.variant }}" ] && continue - gh api repos/${{ github.repository }}/actions/artifacts \ - --jq ".artifacts[] | select(.name == \"bench-$v\") | .name" 2>/dev/null | grep -q . \ - && gh api repos/${{ github.repository }}/actions/artifacts \ - --jq ".artifacts[] | select(.name == \"bench-$v\") | .archive_download_url" \ - -H "Authorization: token ${{ secrets.GITHUB_TOKEN }}" \ - > /dev/null 2>&1 \ - && echo "sibling $v available" || echo "sibling $v pending" - done - - # Try to download sibling artifacts (may not exist yet). - for v in bcr main pr; do - [ "$v" = "${{ matrix.variant }}" ] && continue - gh run download "$GITHUB_RUN_ID" -n "bench-$v" -D "/tmp/$v" 2>/dev/null \ - && cp /tmp/$v/${v}-*.json . 2>/dev/null || true - done - - # Render comparison tables (pending variants show ⏳). - python3 rules_py_pr/benchmark/compare.py analysis --partial \ - --output-table /tmp/analysis-table.txt \ - bcr-analysis.json main-analysis.json pr-analysis.json 2>/dev/null || true - python3 rules_py_pr/benchmark/compare.py startup --partial \ - --output-table /tmp/startup-table.txt \ - bcr-startup.json main-startup.json pr-startup.json 2>/dev/null || true - echo "${{ github.event.pull_request.number }}" > /tmp/pr-number.txt - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - - name: Post comment - if: github.event_name == 'pull_request' + python3 rules_py_pr/benchmark/compare.py analysis \ + --output-table analysis-table.txt \ + --step-summary "$GITHUB_STEP_SUMMARY" \ + bcr-analysis.json main-analysis.json pr-analysis.json + + - name: Compare startup + id: startup continue-on-error: true + run: | + python3 rules_py_pr/benchmark/compare.py startup \ + --output-table startup-table.txt \ + bcr-startup.json main-startup.json pr-startup.json + cat startup-table.txt >> "$GITHUB_STEP_SUMMARY" + + - name: Save PR number + if: github.event_name == 'pull_request' + run: echo "${{ github.event.pull_request.number }}" > pr-number.txt + + - name: Upload report artifacts + if: github.event_name == 'pull_request' + uses: actions/upload-artifact@v4 + with: + name: benchmark-results + path: | + analysis-table.txt + startup-table.txt + pr-number.txt + profiles/ + if-no-files-found: warn + + - name: Post PR comment + if: github.event_name == 'pull_request' uses: actions/github-script@v7 - env: - RUNNER_OS: ${{ runner.os }} with: script: | const fs = require('fs'); + const startup = fs.existsSync('startup-table.txt') ? fs.readFileSync('startup-table.txt', 'utf8').trim() : ''; + const analysis = fs.existsSync('analysis-table.txt') ? fs.readFileSync('analysis-table.txt', 'utf8').trim() : ''; const header = ''; - let body = header + '\n\n'; - try { body += fs.readFileSync('/tmp/analysis-table.txt', 'utf8').trim() + '\n\n'; } catch {} - try { body += fs.readFileSync('/tmp/startup-table.txt', 'utf8').trim(); + '\n'; } catch {} + const body = `${header}\n\n${startup}\n\n${analysis}`; const { data: comments } = await github.rest.issues.listComments({ - owner: context.repo.owner, repo: context.repo.repo, + owner: context.repo.owner, + repo: context.repo.repo, issue_number: context.issue.number, }); const existing = comments.find(c => c.body && c.body.includes(header)); if (existing) { await github.rest.issues.updateComment({ - owner: context.repo.owner, repo: context.repo.repo, - comment_id: existing.id, body: body, + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: existing.id, + body: body, }); } else { - await github.rest.issues.createComment({ - owner: context.repo.owner, repo: context.repo.repo, - issue_number: context.issue.number, body: body, + const { data: created } = await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + body: body, }); } + + - name: Fail on regression + if: steps.analysis.outcome == 'failure' || steps.startup.outcome == 'failure' + run: exit 1