perf(pre-commit): cut make pre-commit wall time by ~35%

Profiling `make pre-commit` on a staged litellm/*.py change showed basedpyright
was ~90% of it: 137s of a 157s type-check gate, on a run that took ~152s total.

Three changes, all measured:

Run basedpyright across worker threads instead of one. The width is pinned to a
constant rather than following the host's core count, because partitioning files
across threads reorders a few order-dependent inferences: serial, 2-thread and
4-thread passes disagree on a handful of diagnostics, while passes at the same
width are byte-identical run after run. An auto width would make a 16-core
laptop and a 4-core runner report different totals for the same tree, so the
width joins the dependency-group set in the environment fingerprint and any
cache entry or CI artifact recorded at another width is recomputed, never
matched. 137s -> 87s over this tree.

Memoize the path resolution in all three budget gates. Each called
`Path.resolve()`, a filesystem round trip, once per violation rather than once
per file: 149k realpath walks for 2.2k files in the basedpyright gate alone,
6.6s -> 0.9s there, 5.0s -> 2.8s for the ruff strict gate.

Split `bootstrap` into `bootstrap-python` and `bootstrap-dashboard`, and have
`pre-commit` take only the Python half. The dashboard's npm install is seconds
a Python-only commit has no use for, and on a machine whose node predates the
dashboard's engines floor it fails outright, which blocked `make pre-commit`
entirely for changes that never touch the dashboard. The script now tops the
dashboard up itself for the commits that reach it, after the Python block forks
so the install overlaps that lint and before both node blocks fork so two npm
installs cannot race.

Also fixes a temp file the dashboard job leaked on Ctrl-C: bash skips EXIT traps
on an uncaught fatal signal, so the eslint report survived the interrupt.

Measured over 3 reps each, staged litellm/*.py change, 4-core box:
  before 178.9 / 148.8 / 152.0s  (median 152.0)
  after  106.2 /  99.2 /  94.8s  (median 99.2)
Cold path, where the base tree has to be measured too, 5m33s -> 3m17s.
This commit is contained in:
Claude 2026-08-08 09:53:34 +00:00
parent e24a9146e3
commit 2559bfe345
No known key found for this signature in database
9 changed files with 178 additions and 36 deletions

View file

@ -9,12 +9,14 @@
lint-ruff-budget lint-ruff-budget-update lint-budget-update lint-gate \
install-dev install-proxy-dev install-test-deps install-hooks \
install-helm-unittest check-circular-imports check-import-safety pre-commit \
lint-install lint-fetch-base bootstrap
lint-install lint-fetch-base bootstrap bootstrap-python bootstrap-dashboard
# Default target
help:
@echo "Available commands:"
@echo " make bootstrap - Provision a fresh clone/worktree"
@echo " make bootstrap-python - Provision only the Python env (no dashboard npm install)"
@echo " make bootstrap-dashboard - Provision only the dashboard's node_modules"
@echo " make install-dev - Install development dependencies"
@echo " make install-proxy-dev - Install proxy development dependencies"
@echo " make install-dev-ci - Install dev dependencies (CI-compatible, pins OpenAI)"
@ -72,17 +74,24 @@ info:
install-dev:
$(UV) sync --inexact --frozen
bootstrap:
bootstrap: bootstrap-python bootstrap-dashboard
@echo "bootstrap: done"
# The halves are separate targets so a caller can provision only what it needs:
# `make pre-commit` on a Python-only change takes bootstrap-python and never pays
# for (nor hard-fails on) the dashboard's node toolchain.
bootstrap-python:
$(UV) sync --inexact --frozen --extra proxy --group proxy-dev --group e2e-dev
$(UV_RUN) python scripts/prisma_generate_if_needed.py
cd ui/litellm-dashboard && ../../scripts/with_dashboard_node.sh npm install --no-audit --no-fund
@main_root=$$(git worktree list --porcelain | head -1 | sed 's/^worktree //'); \
if [ "$$main_root" != "$$(git rev-parse --show-toplevel)" ] && [ -f "$$main_root/.env" ] && [ ! -f .env ]; then \
cp "$$main_root/.env" .env && echo "bootstrap: copied .env from $$main_root"; \
else \
echo "bootstrap: .env left untouched"; \
fi
@echo "bootstrap: done"
bootstrap-dashboard:
cd ui/litellm-dashboard && ../../scripts/with_dashboard_node.sh npm install --no-audit --no-fund
install-proxy-dev:
$(UV) sync --frozen --group proxy-dev --extra proxy
@ -240,7 +249,7 @@ lint-dev: lint-format-changed check-circular-imports check-import-safety
# test-linting.yml (Python), test-litellm-ui-build.yml's frontend-lint (dashboard), and
# check-ui-api-types.yml (API-type drift), skipping any whose files you didn't stage.
# Not auto-installed as a git hook so it never slows an unrelated human commit.
pre-commit: bootstrap
pre-commit: bootstrap-python
./scripts/pre_commit_lint.sh
# Testing targets

View file

@ -98,9 +98,12 @@ EOF
# counts are not diff-scoped, so a local pass here means the budget step will
# pass in CI too.
report=$(mktemp)
# Ctrl-C reaches this job as a SIGTERM from on_interrupt; bash skips EXIT
# traps on an uncaught fatal signal, so catch it and exit through one.
trap 'rm -f "$report"' EXIT
trap 'exit 130' INT TERM
npx eslint . -f json -o "$report" || true
node scripts/check-lint-budgets.mjs "$report" eslint-budgets.json || rc=1
rm -f "$report"
exit $rc
)
}
@ -156,6 +159,15 @@ if [ -n "$e2e_py_files" ]; then
|| { echo "✗ Raw HTTP client import in tests/e2e. Route the call through tests/e2e/e2e_http.py, then re-run make pre-commit." >&2; status=1; }
fi
# `make pre-commit` provisions the Python env only, so top up the dashboard's
# node_modules for the commits that reach it. Placed after the Python block forked
# (so the install overlaps that lint) and before both node blocks fork (so two npm
# installs never race in the same directory).
if [ -n "$ui_prettier_files" ] || [ -n "$ui_eslint_files" ] || [ -n "$spec_files" ]; then
echo "pre-commit: provisioning the dashboard toolchain (make bootstrap-dashboard)"
make bootstrap-dashboard || status=1
fi
dashboard_checks() {
echo "pre-commit: linting dashboard (prettier + eslint + lint budgets)"
if [ ! -d ui/litellm-dashboard/node_modules ]; then

View file

@ -10,6 +10,7 @@ branch point (the merge-base).
"""
import argparse
import functools
import json
import re
import shutil
@ -77,18 +78,27 @@ def _ruff_json(cwd: Path, config: Path) -> list:
return json.loads(raw or "[]")
# Memoized because `resolve()` is a filesystem round trip and ruff reports many
# more violations than there are files: the cache makes it one realpath walk per
# file rather than one per violation.
@functools.lru_cache(maxsize=None)
def _relative_to_repo(filename: str) -> str:
name = Path(filename)
return (
(name if name.is_absolute() else REPO_ROOT / name)
.resolve()
.relative_to(REPO_ROOT)
.as_posix()
)
def head_violations() -> list:
out = []
for item in _ruff_json(REPO_ROOT, STRICT_CONFIG):
name = Path(item["filename"])
rel = (
(name if name.is_absolute() else REPO_ROOT / name)
.resolve()
.relative_to(REPO_ROOT)
.as_posix()
return [
Violation(
_relative_to_repo(item["filename"]), item["location"]["row"], item["code"]
)
out.append(Violation(rel, item["location"]["row"], item["code"]))
return out
for item in _ruff_json(REPO_ROOT, STRICT_CONFIG)
]
def count_by_rule(violations: list) -> dict:

View file

@ -24,6 +24,18 @@ set by construction; re-syncs of an up-to-date env are a near-instant no-op.
The group set is folded into the cache and artifact fingerprint, so counts
recorded under a different set are never matched, only recomputed.
Checking is parallelized across a *pinned* number of worker threads rather than
the host's core count. Threading is what makes the pass roughly 1.6x faster
(137s -> 87s over this tree on four cores), but partitioning files across
threads also reorders a handful of order-dependent inferences, so a few
diagnostics differ between a serial pass, a two-thread pass and a four-thread
pass; passes at the same width agree exactly, run after run. Width is therefore
part of the measurement in exactly the way the installed package set is, and
letting it follow ``nproc`` would make a 16-core laptop and a 4-core CI runner
report different totals for the same tree. It is a constant here and is folded
into the fingerprint alongside the group set, so a cache entry or CI artifact
recorded at another width is never matched, only recomputed.
The gate runs basedpyright itself, for both the head and the base pass, with
``NODE_OPTIONS`` raised to the heap this repo needs: basedpyright's node
process OOMs at the ~4 GB default, and when callers had to remember the flag,
@ -52,6 +64,7 @@ carries an unambiguous ``rule`` field.
import argparse
import contextlib
import functools
import hashlib
import io
import json
@ -91,6 +104,10 @@ PRISMA_SCHEMA = REPO_ROOT / "litellm" / "proxy" / "schema.prisma"
# caller-set value while preserving the caller's other NODE_OPTIONS flags.
NODE_HEAP_OPTION = "--max-old-space-size=8192"
# Worker threads every pass is measured at. Four is the GitHub-hosted runner's
# vCPU count, so CI gets full parallelism and any dev box measures what CI does.
BASEDPYRIGHT_THREADS: Final = 4
# Bucket for a basedpyright diagnostic with no `rule`. Counted so it's gated.
UNCODED = "<uncoded>"
@ -107,6 +124,10 @@ class Breach(NamedTuple):
added: int
# Memoized because it is called once per diagnostic, and `resolve()` is a
# filesystem round trip: this tree reports ~149k errors across ~2.2k files, so
# the cache turns ~149k realpath walks into one per file (~6s -> ~0.3s).
@functools.lru_cache(maxsize=None)
def _to_relative(raw: str, root: Path) -> str | None:
path = Path(raw)
absolute = path if path.is_absolute() else root / path
@ -217,9 +238,10 @@ def run_basedpyright(cwd: Path = REPO_ROOT, env_dir: Path = TYPECHECK_ENV_DIR) -
the only pin that works, because basedpyright auto-detects a `.venv` in the
project root and that beats both PATH order and VIRTUAL_ENV, silently
measuring the caller's fatter venv (whose extra typed packages flip
diagnostics) whenever the repo has one. Exit 0 (clean) and 1 (errors
found) are both output-bearing runs; anything else is a crash and fails
loudly instead of reading as zero errors."""
diagnostics) whenever the repo has one. `--threads` is pinned for the same
reason: it is a measurement parameter, not a local tuning knob. Exit 0
(clean) and 1 (errors found) are both output-bearing runs; anything else is
a crash and fails loudly instead of reading as zero errors."""
bin_dir: Final = env_dir / "bin"
proc = subprocess.run(
[
@ -227,6 +249,8 @@ def run_basedpyright(cwd: Path = REPO_ROOT, env_dir: Path = TYPECHECK_ENV_DIR) -
"--outputjson",
"--pythonpath",
str(bin_dir / "python"),
"--threads",
str(BASEDPYRIGHT_THREADS),
],
cwd=cwd,
capture_output=True,
@ -301,6 +325,7 @@ def over_ceiling(
def environment_fingerprints(
dep_groups: tuple[str, ...] = TYPECHECK_DEP_GROUPS,
threads: int = BASEDPYRIGHT_THREADS,
) -> tuple[str, ...]:
return (
*(
@ -309,6 +334,7 @@ def environment_fingerprints(
if path.exists()
),
"groups:" + ",".join(dep_groups),
f"threads:{threads}",
)

View file

@ -28,6 +28,7 @@ otherwise be misread as "fixed", collapsing the deliberate headroom to zero.
"""
import argparse
import functools
import json
import re
import shutil
@ -88,21 +89,30 @@ def resolve_base_point(base_ref: str, cwd: Path = REPO_ROOT) -> str:
return merge_point if older == head_point else head_point
# Memoized because `resolve()` is a filesystem round trip and the checker reports
# many more violations than there are files: the cache makes it one realpath walk
# per file rather than one per violation.
@functools.lru_cache(maxsize=None)
def _relative_to_root(filename: str, root: Path) -> str:
name = Path(filename)
full = name if name.is_absolute() else root / name
return full.resolve().relative_to(root).as_posix()
def _check(root: Path, checker: Path) -> list:
# Resolve root first: on macOS tempfile dirs (/var/...) resolve to /private/var/...,
# and the checker prints already-resolved absolute paths, so relative_to would fail.
root = root.resolve()
out = _run([sys.executable, str(checker), str(root / TARGET)], cwd=root)
found = []
for line in out.splitlines():
m = _LINE.match(line)
if m is None:
continue
name = Path(m.group("file"))
full = name if name.is_absolute() else root / name
rel = full.resolve().relative_to(root).as_posix()
found.append(Violation(rel, int(m.group("line")), m.group("code")))
return found
resolved_root = root.resolve()
out = _run([sys.executable, str(checker), str(resolved_root / TARGET)], cwd=resolved_root)
return [
Violation(
_relative_to_root(m.group("file"), resolved_root),
int(m.group("line")),
m.group("code"),
)
for line in out.splitlines()
if (m := _LINE.match(line)) is not None
]
def head_violations() -> list:

View file

@ -29,6 +29,7 @@ BARRIER_HELPER = """barrier_sync() {
MAKE_STUB = """#!/bin/sh
. "$STUB_BIN/barrier.sh"
[ -n "${STUB_MAKE_LOG:-}" ] && echo "$*" >> "$STUB_MAKE_LOG"
case "$*" in
lint)
[ "${STUB_FAIL:-}" = "make-lint" ] && exit 1
@ -85,7 +86,7 @@ def _write_executable(path: Path, body: str) -> None:
path.chmod(0o755)
def _sandbox(tmp_path: Path) -> tuple[Path, Path]:
def _sandbox(tmp_path: Path, stage: tuple[str, ...] = (".",)) -> tuple[Path, Path]:
repo = tmp_path / "repo"
(repo / "litellm" / "proxy").mkdir(parents=True)
(repo / "litellm" / "foo.py").write_text("x = 1\n")
@ -95,7 +96,7 @@ def _sandbox(tmp_path: Path) -> tuple[Path, Path]:
(dashboard / "node_modules").mkdir()
(dashboard / "src" / "app.ts").write_text("export {}\n")
subprocess.run(["git", "init", "-q"], cwd=repo, check=True)
subprocess.run(["git", "add", "."], cwd=repo, check=True)
subprocess.run(["git", "add", *stage], cwd=repo, check=True)
bin_dir = tmp_path / "bin"
bin_dir.mkdir()
@ -148,6 +149,29 @@ def test_all_blocks_passing_exits_zero(tmp_path: Path) -> None:
assert proc.returncode == 0, proc.stdout + proc.stderr
def _make_invocations(tmp_path: Path, repo: Path, bin_dir: Path) -> list[str]:
log = tmp_path / "make.log"
proc = _run(repo, bin_dir, {"STUB_MAKE_LOG": str(log)})
assert proc.returncode == 0, proc.stdout + proc.stderr
return log.read_text().split()
def test_a_python_only_commit_never_provisions_the_dashboard_toolchain(tmp_path: Path) -> None:
# npm install costs seconds this commit has no use for, and on a machine whose
# node predates the dashboard's engines floor it fails outright, which used to
# block `make pre-commit` for changes that never touch the dashboard.
repo, bin_dir = _sandbox(tmp_path, stage=("litellm/foo.py",))
invocations = _make_invocations(tmp_path, repo, bin_dir)
assert "lint" in invocations
assert "bootstrap-dashboard" not in invocations
@pytest.mark.parametrize("staged", ["ui/litellm-dashboard/src/app.ts", "litellm/proxy/spec.py"])
def test_a_commit_that_reaches_the_dashboard_provisions_it(tmp_path: Path, staged: str) -> None:
repo, bin_dir = _sandbox(tmp_path, stage=(staged,))
assert "bootstrap-dashboard" in _make_invocations(tmp_path, repo, bin_dir)
def test_full_output_is_saved_to_a_log_file_in_the_git_dir(tmp_path: Path) -> None:
repo, bin_dir = _sandbox(tmp_path)
(repo / "scratch.txt").write_text("")

View file

@ -16,6 +16,15 @@ def rule(name, limit):
return {name: {"limit": limit}}
def test_paths_are_resolved_once_per_file_not_once_per_violation():
# resolve() is a filesystem round trip and ruff reports ~20k strict violations
# over far fewer files, so resolving per violation is pure overhead.
gate._relative_to_repo.cache_clear()
same_file = str(gate.REPO_ROOT / "litellm" / "a.py")
assert {gate._relative_to_repo(same_file) for _ in range(20)} == {"litellm/a.py"}
assert gate._relative_to_repo.cache_info().misses == 1
def test_under_ceiling_passes():
assert gate.evaluate({"ANN001": 100}, {"ANN001": 100}, rule("ANN001", 110)) == []

View file

@ -46,6 +46,18 @@ def test_basedpyright_error_without_a_rule_is_bucketed():
assert gate.count_basedpyright(payload) == {gate.UNCODED: 1}
def test_counting_resolves_each_file_once_not_once_per_diagnostic():
# resolve() is a filesystem round trip and this tree reports ~149k errors over
# ~2.2k files, so a resolve per diagnostic costs ~6s of the gate for nothing.
gate._to_relative.cache_clear()
same_file = ROOT / "litellm" / "a.py"
payload = json.dumps(
{"generalDiagnostics": [_bpr(same_file, "error", "reportAny")] * 50}
)
assert gate.count_basedpyright(payload) == {"reportAny": 50}
assert gate._to_relative.cache_info().misses == 1
def test_paths_outside_repo_are_skipped():
payload = json.dumps(
{
@ -122,6 +134,20 @@ def test_run_basedpyright_pins_import_resolution_to_the_owned_env(tmp_path):
assert argv[argv.index("--pythonpath") + 1] == str(env_dir / "bin" / "python")
def test_run_basedpyright_pins_the_thread_width_instead_of_inheriting_the_hosts(tmp_path):
# Partitioning files across threads reorders a few order-dependent inferences,
# so counts are only comparable at equal width; letting it follow the core
# count would make a 16-core laptop and a 4-core runner disagree on one tree.
captured = tmp_path / "argv.txt"
env_dir = _stub_env(
tmp_path,
f'echo "$@" > "{captured}"\necho \'{{"generalDiagnostics": []}}\'',
)
gate.run_basedpyright(cwd=tmp_path, env_dir=env_dir)
argv = captured.read_text().split()
assert argv[argv.index("--threads") + 1] == str(gate.BASEDPYRIGHT_THREADS)
def test_run_basedpyright_fails_loudly_on_a_crash_exit_code(tmp_path):
import pytest
@ -275,9 +301,14 @@ def test_fingerprints_carry_the_dependency_group_set():
assert gate.environment_fingerprints(
dep_groups=("proxy-dev",)
) != gate.environment_fingerprints(dep_groups=("proxy-dev", "e2e-dev"))
assert gate.environment_fingerprints()[-1] == "groups:" + ",".join(
gate.TYPECHECK_DEP_GROUPS
)
assert "groups:" + ",".join(gate.TYPECHECK_DEP_GROUPS) in gate.environment_fingerprints()
def test_fingerprints_carry_the_thread_width():
# Same reasoning as the group set: a count taken at another width is not
# comparable, so its cache entry and artifact name must not be reachable.
assert f"threads:{gate.BASEDPYRIGHT_THREADS}" in gate.environment_fingerprints()
assert gate.environment_fingerprints(threads=2) != gate.environment_fingerprints(threads=4)
def test_fingerprints_cover_the_prisma_schema():

View file

@ -19,6 +19,17 @@ def _budget(limit):
return {"LIT006": {"limit": limit}}
def test_paths_are_resolved_once_per_file_not_once_per_violation():
# resolve() is a filesystem round trip and the checker reports tens of
# thousands of violations over ~2.2k files, so resolving per violation is
# pure overhead on the pre-commit and CI critical path.
gate._relative_to_root.cache_clear()
root = gate.REPO_ROOT.resolve()
same_file = str(root / "litellm" / "a.py")
assert {gate._relative_to_root(same_file, root) for _ in range(20)} == {"litellm/a.py"}
assert gate._relative_to_root.cache_info().misses == 1
def test_over_ceiling_flags_only_counts_above_the_limit():
budget = _budget(12)
assert gate.over_ceiling({"LIT006": 12}, budget) == frozenset() # at limit