perf(uv): rebuild fbuild ~14x faster on no-source-change reinstall#743
Conversation
The slow path was `uv sync --reinstall-package fbuild` (also fires on version bumps / lockfile churn): 14.9s of cargo work even when no Rust source had changed, because PEP 517 build isolation copies the source tree to a temp dir each time and cargo's incremental cache lives at `<cwd>/target/` — discarded after the build. Three coordinated changes: - `setup.py`: pin `CARGO_TARGET_DIR` to `~/.fbuild/cargo-target/wheel-build` so the incremental cache survives. Add an mtime-skip in `BuildWithCargo.run` that short-circuits cargo entirely when the staged `ci/bin/fbuild[.exe]` is newer than every `crates/**/*.rs`, `Cargo.toml`, `Cargo.lock`, `rust-toolchain.toml`. - `pyproject.toml`: `[tool.uv] no-build-isolation-package = ["fbuild"]` so the build runs in the real repo (not a temp copy). `default-groups = ["dev"]` + setuptools in the `dev` group so setuptools is in the venv when uv tries to rebuild the wheel (no chicken-and-egg). - `pyproject.toml`: `cache-keys = [..., "crates/**/*.rs", ...]` so uv's editable install actually re-syncs on Rust edits — without this the compiled `_native.pyd` silently stayed stale until manual reinstall. Measurements (on Windows, soldr 0.7.55, uv 0.8.11): | Scenario | Before | After | |-------------------------------------------|--------:|--------:| | forced reinstall, no source change | 14.9s | 1.1s | | real `.rs` edit + `uv run python --ver` | 15.7s | 14.3s | | warm `uv run python --version` | ~110ms | ~100ms | The 13.6x win is on the no-source-change path (mtime-skip never invokes cargo). The real-edit path is at cargo's incremental + linker floor on this workspace; pushing further would need a faster linker (rust-lld/mold) — separate change. See `ci/bench-results/REPORT.md` for full methodology and raw timings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds a persistent Changesuv run Rebuild Speedup
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Stop tracking ad-hoc `port_scan_*.txt` captures. PR #741 added the `fbuild port scan` command; users sometimes redirect its output to a local file for diagnostic comparison. Those captures aren't useful to anyone else and never belonged in the working tree. - Refresh `ci/bench-results/REPORT.md` to cover the full cumulative story after PR #743 and PR #744: - No-source-change reinstall: 14.9s -> 1.1s (13.6x, PR #743). - Real `.rs` edit + uv run: 100.1s -> 18.9s (5.3x, PR #744). - The remaining ~14s touch-only floor is soldr/zccache wrapper overhead; profiled it down to the second and filed upstream as zackees/soldr#883. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tems 4+5) (#5) Two complementary changes that together let cargo's incremental fingerprint cache survive across `pip install .` / `uv build` invocations. Ported from FastLED/fbuild#743. 1. `os.environ.setdefault("CARGO_TARGET_DIR", ~/.template-python-rust-cmd/cargo-target/wheel-build)` at the top of `ci/build_wheel.py`. PEP 517 isolated builds copy the source tree to a temp dir and throw `<temp>/target/` away after each install — every install runs cargo cold (25-30s wall-clock without this). Pinning to a stable home-dir path preserves the fingerprint across invocations. Deliberately separate from `<repo>/target/` so iteration with bare `cargo check` doesn't churn the wheel-build cache and vice versa. 2. `[tool.uv] no-build-isolation-package = ["template-python-rust-cmd"]` in `pyproject.toml`. Without this, `uv build` (and `uv pip install .`) copy the source tree to a temp dir, and (a) cargo's incremental cache lands in that temp dir then gets discarded, (b) the future mtime-skip fast path can't see the staged binary because it's outside the temp tree. `maturin` already ships in the dev dependency group, so the build env has what it needs without isolation. 3. Refactor: pull `_cargo_target_root()` out so it respects `CARGO_TARGET_DIR`. Previously hardcoded `ROOT / "target" / "release"` — would have masked the pinned path. Refs #2 (items 4 + 5). Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
… items 6+8) (#6) Two upgrades to `ci/build_wheel.py` ported from FastLED/fbuild#743. 1. mtime-skip fast path. `_staged_binary_is_up_to_date()` walks the workspace's cargo inputs (Cargo.toml, Cargo.lock, rust-toolchain.toml, crates/**/Cargo.toml, crates/**/*.rs) and compares each st_mtime against the staged `src/template_python_rust_cmd/_bin/template-cli`. If staged is newer, skip cargo entirely — `_ensure_staged_cli_binary()` returns the already-staged file. Triggered on no-op reinstalls (version bumps, lockfile churn, --reinstall-package). Even cargo's "Fresh" pass walks the workspace and burns wall-clock seconds; an mtime check is milliseconds. fbuild measured 14.9s → 1.1s on its forced reinstall path. 2. Structured artifact discovery. `build_cli_binary()` now invokes cargo with `--message-format=json-render-diagnostics` and walks the JSON artifact stream for `reason == "compiler-artifact"` + `target.name == "template-cli"` + non-null `executable`. Fallback: `_find_cli_executable_by_search()` probes the target root (respecting CARGO_TARGET_DIR) and every per-host-triple subdir for cases where cargo emits no compiler-artifact line (fully cached `Fresh` runs) or where the build is configured with a host triple (CARGO_BUILD_TARGET). Replaces the previous hardcoded `target/release/template-cli` path, which would have masked the pinned CARGO_TARGET_DIR landed in the previous PR. Refs #2 (items 6 + 8). Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
….pth (not a PEP 660 finder) (#748) The per-package `package-dir = {fbuild = "python/fbuild", "fbuild.api" = "python/fbuild/api"}` map worked for the wheel build but made setuptools' editable install emit a PEP 660 meta-path finder (`__editable___fbuild_*_finder.py`) whose `MAPPING` only registered `fbuild`. `fbuild.api` resolved at runtime via `importlib.machinery.PathFinder` — fine for `import fbuild.api`, but invisible to static analyzers (ty, pyright, mypy) that walk `top_level.txt` and `.pth` files. Downstream consumers (FastLED hit this on `from fbuild.api import SerialMonitor`) saw `Cannot resolve imported module 'fbuild.api'` even though runtime imports worked. Replace the per-package map with the canonical src-layout pattern `package-dir = {"" = "python"}`. Setuptools now emits a plain `.pth` pointing at `python/`, which every static analyzer handles, and the wheel-build path is unaffected. Verified: - Wheel (`python -m build`): byte-for-byte identical to baseline (10,454,675 B). Same contents: `fbuild/__init__.py`, `fbuild/_native.pyd`, `fbuild/api/__init__.py`, `fbuild-2.2.31.data/scripts/fbuild.exe`, dist-info. - Sdist: +2,811 B / +0.12%, entirely from `fbuild.egg-info/` relocating to `python/fbuild.egg-info/`. Cosmetic. - `ci/publish.py::build_wheel` is independent of setuptools — it hand-rolls wheels from the hard-coded `PYTHON_SHIMS_DIR = ROOT / "python"`. Smoke-tested: still finds the same 2 shims (`fbuild/__init__.py`, `fbuild/api/__init__.py`). Release path unaffected. - Editable reinstall path: ~8s (FastLED → fbuild via uv sources), within noise of baseline. No regression to the perf work in #743 / #744. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
uv sync --reinstall-package fbuild(and version bumps / lockfile churn that trigger the same path): 14.9s → 1.1s (13.6×). PEP 517 build isolation used to copy the source tree to a temp dir and discard cargo's incremental cache; nowCARGO_TARGET_DIRis pinned + a mtime-skip short-circuits cargo entirely when the staged binary is already current._native.pydafter.rsedits:cache-keyswatchescrates/**/*.rs,Cargo.{toml,lock},pyproject.toml, souv runtriggers re-sync on Rust changes. The real-edit rebuild is at the cargo + linker floor (~14s on Windows); pushing further needs a linker swap, which is a separate change.uv runpath (~100ms either way).Numbers
.rsedit +uv run python --versionuv run python --versionFull methodology + raw
ci/bench_uv_run.pyJSON inci/bench-results/REPORT.md.Files
setup.py—CARGO_TARGET_DIRpin (separate from<repo>/target/so dev-CLIsoldr cargoruns don't churn the same cache);_staged_binary_is_up_to_datemtime check inBuildWithCargo.run.pyproject.toml—[tool.uv] no-build-isolation-package = ["fbuild"],default-groups = ["dev"], setuptools added todevgroup (no chicken-and-egg with no-build-isolation),cache-keysso editable installs catch Rust source changes.ci/bench_uv_run.py+ci/bench-results/— benchmark script + raw timings + writeup.Test plan
uv sync --reinstall-package fbuildwith no source change → measured 1.1s\ntocrates/fbuild-core/src/lib.rsthenuv run python --version→ measured 14.3s, fresh binaryuv run python --version→ ~100msfrom fbuild.api import SerialMonitorsucceeds against the rebuilt wheel🤖 Generated with Claude Code
Summary by CodeRabbit