Skip to content

fix(build): stop no-change rebuilds from recompiling every TU#956

Merged
zackees merged 1 commit into
mainfrom
fix/951-noop-rebuild
Jul 3, 2026
Merged

fix(build): stop no-change rebuilds from recompiling every TU#956
zackees merged 1 commit into
mainfrom
fix/951-noop-rebuild

Conversation

@zackees

@zackees zackees commented Jul 3, 2026

Copy link
Copy Markdown
Member

Summary

The #942 baseline showed a no-change rebuild of NightDriverStrip demo taking ~108 s — compile-sketch (92 s) and compile-core-variant (10.7 s) re-ran in full every build. Two staleness-check defects:

  1. Depfile prerequisites resolved against the wrong cwd. Compiles run with cwd = the project workspace (the zccache compile-cwd normalization from ESP32 compile on Windows: 'Cannot create temporary file in C:\Windows\' — uncovered tempfile callsite #875/fix(compile): force / in path args so cc1 spec-file pass works on Windows #890), so gcc -MMD depfiles list workspace-relative prerequisites. dependency_is_newer_than_object stat'ed them against the long-lived daemon's process cwd, every stat failed, and .unwrap_or(true) marked every TU stale. Fixed by resolving relative prerequisites against zccache::compile_cwd_from_output(object) — the same workspace the compile actually ran from. Absolute prerequisites are untouched.
  2. Signature asymmetry with build_unflags. compile_c/compile_cpp apply build_unflags before hashing the written .cmdhash, but both rebuild_signature impls (trait default + Esp32Compiler) hashed the raw flags. Any project with build_unflags (NightDriverStrip's [env] sets -std=gnu++11) mismatched on every file. The check side now applies the identical filter (apply_compile_unflags promoted to pub(crate)).

Validation

  • New tests: test_needs_rebuild_resolves_relative_depfile_deps_against_workspace (relative deps must not force rebuilds when current, and must still trigger one when genuinely newer) and rebuild_signature_matches_write_path_with_unflags (check-side signature must equal the write-path signature under build_unflags). Full fbuild-build --lib: 717 passed.
  • meta: profiling-driven build performance burndown (Docker Linux, NightDriverStrip, cold vs hot cache) #942 Docker harness e2e: cold 780 s (unchanged), then steady-state no-change rebuild 108 s → 2.1 s (compile-sketch 92 s → 0.48 s stat walk, compile-core-variant 10.7 s → 0.2 s, link-convert-size 4.4 s → 1 ms).

Known residual (tracked separately, not a regression): the first rebuild after a cold build still recompiles once — the .cmdhash written mid-cold-build reflects a different flag/include assembly than steady state. Every rebuild after that is 2.1 s.

Closes #951
Part of #942

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved rebuild detection so changes to compiler flags are tracked consistently, reducing unnecessary or missed rebuilds.
    • Fixed dependency timestamp checks to correctly handle relative paths in depfiles, especially when builds run from a different working directory.
    • Resolved a signature mismatch issue for ESP32 builds when flag filtering is enabled.

Two staleness-check defects made every rebuild a full recompile
(#951, ~108s per no-op rebuild of NightDriverStrip):

1. Compiles run with cwd = the project workspace (zccache compile-cwd
   normalization), so gcc -MMD depfiles list workspace-relative
   prerequisites — but dependency_is_newer_than_object stat'ed them
   against the daemon's process cwd, failed, and .unwrap_or(true)
   marked every TU stale. Resolve relative prerequisites against
   zccache::compile_cwd_from_output(object).

2. compile_c/compile_cpp apply build_unflags before hashing the
   written .cmdhash, but both rebuild_signature impls (trait default +
   Esp32Compiler) hashed the raw flags, so any project with
   build_unflags (NightDriverStrip sets -std=gnu++11) mismatched on
   every file. Apply the same filter on the check side.

Validated in the #942 Docker harness: steady-state no-change rebuild
drops 108s -> 2.1s (compile-sketch 92s -> 0.48s stat walk,
compile-core-variant 10.7s -> 0.2s). The first rebuild after a cold
build still recompiles once (signature drift between mid-cold and
steady-state flag assembly) — tracked separately.

Closes #951
Part of #942

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a2f61d86-f7bb-42cf-b98f-73c2a3d84918

📥 Commits

Reviewing files that changed from the base of the PR and between b23512b and 51aef1b.

📒 Files selected for processing (3)
  • crates/fbuild-build/src/compiler.rs
  • crates/fbuild-build/src/compiler_tests.rs
  • crates/fbuild-build/src/esp32/esp32_compiler.rs

📝 Walkthrough

Walkthrough

This PR fixes rebuild-signature computation to apply build_unflags filtering to extra_flags in both Compiler and Esp32Compiler, and updates GCC depfile prerequisite staleness checks to resolve relative dependency paths against the compile working directory instead of the process cwd, with added regression tests.

Changes

Rebuild signature and depfile staleness fixes

Layer / File(s) Summary
Filter extra_flags in base signature
crates/fbuild-build/src/compiler.rs
rebuild_signature now applies apply_compile_unflags to base flags and extra_flags before hashing; the helper becomes pub(crate).
Resolve depfile paths against compile CWD
crates/fbuild-build/src/compiler.rs
needs_rebuild_with_signature derives a base path via compile_cwd_from_output and passes it to dependency_is_newer_than_object, which now joins relative depfile prerequisites against that base before comparing mod times.
Regression test for relative depfile deps
crates/fbuild-build/src/compiler_tests.rs
New test builds a temporary workspace with a relative-path depfile and validates rebuild decisions before/after a header update.
Apply same fix to Esp32Compiler
crates/fbuild-build/src/esp32/esp32_compiler.rs
Esp32Compiler::rebuild_signature filters flags through apply_compile_unflags before hashing; a new test asserts parity between the signature and the compile-path computation.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related issues

Possibly related PRs

  • FastLED/fbuild#63: Introduced build_unflags/apply_compile_unflags filtering that this PR now consistently applies to extra_flags in signature computation.
  • FastLED/fbuild#193: Both modify how compile_cwd_from_output derived paths are used in compiler.rs.
  • FastLED/fbuild#232: Both touch build_rebuild_signature computation and add related regression tests.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main fix: no-change rebuilds no longer recompile every translation unit.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/951-noop-rebuild

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

perf(build): no-change rebuild recompiles the entire sketch + core variant (~103s; should be seconds)

1 participant