Skip to content

perf(install): warm pip install . ~23s -> ~2s (in-tree backend, per-repo target dir) - #1683

Merged
zackees merged 3 commits into
mainfrom
perf/local-wheel-stored
Oct 16, 2026
Merged

zackees merged 3 commits into
mainfrom
perf/local-wheel-stored

Conversation

@zackees

@zackees zackees commented Oct 16, 2026 •

Copy link
Copy Markdown
Member
  • perf(test): one integration-test binary each for core, cli, packages, library-select
  • perf(install): store local wheel uncompressed (warm pip install . 23s -> ~10s)

Summary by CodeRabbit

  • Documentation
    • Added guidance for organizing integration tests into shared test suites.
  • Chores
    • Grouped integration tests into shared binaries across several components and updated related test references.
    • Changed compression for locally built source-install wheels; release wheel compression is unchanged.

@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 43 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: FastLED/fbuild/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b468fb99-29a8-4dc4-8ba7-a0635813e9cd
📥 Commits

Reviewing files that changed from the base of the PR and between 799979b and f06c05b.

⛔ Files ignored due to path filters: 1
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing: 4
  • .github/workflows/lint-subprocess.yml
  • ci/build_backend.py
  • ci/local_gate.py
  • pyproject.toml

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: FastLED/fbuild/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: cad4efac-94a0-4eac-b5c8-ec16d3731fbc
📥 Commits

Reviewing files that changed from the base of the PR and between cf231cf and 799979b.

⛔ Files ignored due to path filters: 2
  • ci/platform_boundary_ledger.tsv is excluded by !**/*.tsv
  • ci/platform_boundary_research.tsv is excluded by !**/*.tsv
📒 Files selected for processing: 27
  • crates/fbuild-cli/tests/it/README.md
  • crates/fbuild-cli/tests/it/ci_command.rs
  • crates/fbuild-cli/tests/it/daemon_crash_recovery.rs
  • crates/fbuild-cli/tests/it/lib_select.rs
  • crates/fbuild-cli/tests/it/main.rs
  • crates/fbuild-cli/tests/it/test_emu_exit_code.rs
  • crates/fbuild-core/tests/it/README.md
  • crates/fbuild-core/tests/it/cpu_profiling.rs
  • crates/fbuild-core/tests/it/dep_identity.rs
  • crates/fbuild-core/tests/it/main.rs
  • crates/fbuild-core/tests/it/platformio_avr_resolution.rs
  • crates/fbuild-core/tests/it/platformio_ch32v_resolution.rs
  • crates/fbuild-core/tests/it/platformio_esp8266_resolution.rs
  • crates/fbuild-core/tests/it/platformio_package_resolution.rs
  • crates/fbuild-library-select/tests/it/README.md
  • crates/fbuild-library-select/tests/it/main.rs
  • crates/fbuild-library-select/tests/it/perf_tdd.rs
  • crates/fbuild-library-select/tests/it/teensy41_ldf_diag.rs
  • crates/fbuild-packages/tests/it/README.md
  • crates/fbuild-packages/tests/it/disk_cache_schema_migration.rs
  • crates/fbuild-packages/tests/it/lnk_e2e.rs
  • crates/fbuild-packages/tests/it/main.rs
  • dylints/ban_raw_path_prefix_compare/src/lib.rs
  • dylints/ban_raw_subprocess/src/lib.rs
  • dylints/ban_std_pathbuf/src/allowlist.txt
  • dylints/enforce_platform_boundary/src/baseline.txt
  • setup.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Four crates now declare integration-test modules in a shared it binary, with related documentation and path references updated. Local source-install wheels use stored compression; release wheels assembled by ci/publish.py remain deflated.

Changes: Integration Test Consolidation

Layer / File(s) Summary
Declare integration-test modules
crates/fbuild-cli/tests/it/README.md, crates/fbuild-cli/tests/it/main.rs, crates/fbuild-core/tests/it/README.md, crates/fbuild-core/tests/it/main.rs, crates/fbuild-library-select/tests/it/README.md, crates/fbuild-library-select/tests/it/main.rs, crates/fbuild-packages/tests/it/README.md, crates/fbuild-packages/tests/it/main.rs
Each crate documents the shared it integration-test binary and declares its test modules.
Update integration-test paths
crates/fbuild-library-select/tests/it/teensy41_ldf_diag.rs, dylints/ban_raw_path_prefix_compare/src/lib.rs, dylints/ban_raw_subprocess/src/lib.rs, dylints/ban_std_pathbuf/src/allowlist.txt, dylints/enforce_platform_boundary/src/baseline.txt
The diagnostic command, lint path examples, allowlist entries, and baseline entries now use the tests/it layout.

Local Wheel Compression

Layer / File(s) Summary
Set local wheel compression
setup.py
The local bdist_wheel configuration uses stored compression. Comments state that release wheels assembled by ci/publish.py remain deflated.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 79997

No actionable merge-blocking issue was identified. The test relocation and local wheel change are ready for normal checks.

Pre-merge checks: Passed 5
Check name Status Explanation
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 8 files. (6 skipped: 6 …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly identifies the main performance change for local pip install . and includes relevant implementation context. It does not mention the integration-test consolidation, but the title d…
Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 8 files. (6 skipped: 6 unsupported.)

Full details: Title check

Explanation

The title clearly identifies the main performance change for local pip install . and includes relevant implementation context. It does not mention the integration-test consolidation, but the title does not need to cover every change.

✨ Finishing Touches: 💡 1
⚔️ Resolve merge conflicts: 💡
  • Resolve merge conflict in branch perf/local-wheel-stored
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

… -> ~10s)

Warm `pip install .` already skips cargo (staged artifacts up to date),
but setuptools deflated ~240 MB of dev-profile binaries at zlib's default
level: ~12.5 s to zip plus ~3 s to unzip. The wheel is a throwaway local
artifact, so bdist_wheel now uses compression=stored. Release wheels are
assembled by ci/publish.py and are unaffected.
@zackees
zackees force-pushed the perf/local-wheel-stored branch from 799979b to 06e2359 Compare October 16, 2026 06:57
The shared bosn daemon rejects clients from another release, so the exact
pin made the gate fail whenever a newer daemon (0.1.22) was running. Use a
floor and refresh so the gate always runs the latest bosn.
Replace the setuptools backend with ci/build_backend.py (git mv of
setup.py). It has no build requirements (pip's isolated env is empty,
no setuptools install), skips the three setup.py subprocesses and the
build/ + bdist/ copies of ~240 MB of binaries, and writes the wheel in
one stored pass. Editable installs write a plain .pth onto python/, as
before.

CARGO_TARGET_DIR is now persistent per repo root:
~/.fbuild/cargo-target/wheel-build/<repo>-<sha256(root)[:12]>, so each
checkout keeps a stable warm cache and checkouts never thrash each other.

Measured on Linux, warm (cargo skipped): pip install . 1.6-2.8 s,
uv sync 1.6 s.
@zackees zackees changed the title perf/local wheel stored perf(install): warm pip install . ~23s -> ~2s (in-tree backend, per-repo target dir) Oct 16, 2026
@zackees
zackees merged commit 76aef49 into main Oct 16, 2026
18 of 20 checks passed
@zackees
zackees deleted the perf/local-wheel-stored branch October 16, 2026 08:23
zackees added a commit that referenced this pull request Oct 16, 2026
The pyproject cleanup in #1683 deleted the [build-system] table along with
the setuptools sections, so pip fell back to setuptools' default backend
and `pip install .` failed with 'Multiple top-level packages discovered'.

Verified: cold pip install . (full cargo build) succeeds; warm reinstalls
3.0-5.7 s on a loaded machine; fbuild runs and fbuild._native imports.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant