feat(cli): #238 two-stage compile-many primitive#241
Conversation
Add `fbuild compile-many --board <board> --framework-jobs N --sketch-jobs M <sketches>...` which compiles many sketches against the same board with framework + library archives built **once** (stage 1) and per-sketch compile+link fanned out across a thread pool (stage 2). Routes through the existing `fbuild-build` orchestrator (`get_orchestrator(platform)`) so every platform automatically benefits without re-implementing platform logic. Independent parallelism knobs let memory-heavy stage 1 stay modest on constrained runners (`--framework-jobs` defaults to `min(cores, 2)`) while stage 2's per-worker memory footprint stays tiny enough to crank up (`--sketch-jobs` defaults to `cores`). Each stage-2 worker invokes the orchestrator with `jobs=1` since per-sketch work is single-TU and the outer thread pool already saturates cores. Concurrent-safety review: - Per-sketch output dirs are unique by construction (`<project_dir>/.fbuild/build/<env>/<profile>/`); each sketch in the input list has its own `project_dir`, so no two workers race on `firmware.elf`. - The zccache compile-cache wrapper is lock-free on the hot path — fbuild adds no in-process locks around it (see `crates/fbuild-build/src/zccache.rs`), and concurrent stage-2 workers contend only against the zccache daemon's own SQLite-WAL concurrency model, well below the parallelism cap we set. Tests (`crates/fbuild-build/tests/compile_many_two_stage.rs`) inject a mock `SketchBuilder` and assert: - Stage 1 runs exactly once across N sketches; stage 2 produces N-1 results. - Results are returned in input order regardless of completion order. - Stage 2 workers run truly concurrently (Barrier-of-size-N proves real parallelism — would deadlock under serial execution). - Per-sketch firmware paths are unique (HashSet insert is asserted per build). - Stage-1 failure short-circuits stage 2. - Single sketch falls through stage 1 only. The numeric perf gates in the issue (warm <= 80 s, cold <= 110 s on ubuntu-latest) are validated by the benchmark workflow after this lands and a release ships; cannot be measured locally. The benchmark.yml workflow update is a separate PR on the orphan `bench/fastled-examples` branch and is explicitly out of scope here. Closes #238 partially (CLI primitive + tests; bench workflow follows).
📝 WalkthroughWalkthroughThis PR adds a new ChangesTwo-Stage Sketch Compilation
Sequence DiagramsequenceDiagram
participant Client
participant Orchestrator
participant Stage1Worker
participant Stage2WorkerPool
participant FileSystem
Client->>Orchestrator: compile_many(request)
Orchestrator->>Orchestrator: validate sketches, resolve platforms
Orchestrator->>Stage1Worker: build sketch[0] with framework_jobs
Stage1Worker->>Stage1Worker: compile framework/libs once
Stage1Worker->>FileSystem: write compile_many.log
Stage1Worker-->>Orchestrator: SketchResult (stage 1)
alt Stage 1 Success
Orchestrator->>Stage2WorkerPool: spawn workers for sketches[1..]
par Concurrent Workers
Stage2WorkerPool->>Stage2WorkerPool: worker N: compile sketch[N]
Stage2WorkerPool->>FileSystem: write per-sketch compile_many.log
Stage2WorkerPool->>Stage2WorkerPool: preserve order via indexed slots
end
Stage2WorkerPool-->>Orchestrator: Vec~SketchResult~ (stage 2)
else Stage 1 Failure
Orchestrator-->>Client: CompileManyResult (only stage 1 result)
end
Orchestrator-->>Client: CompileManyResult (aggregate results + timing)
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related Issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/fbuild-build/src/compile_many.rs (2)
296-315: ⚡ Quick winConsider logging when log file persistence fails.
The function silently returns
Nonewhen directory creation or file write fails. Since these logs help with post-build debugging and benchmark summaries, failures should be logged so users know whylog_pathis absent.Add warning logs for failures
fn write_log_file( sketch: &Path, env_name: &str, profile: BuildProfile, build_log: fbuild_core::BuildLog, ) -> Option<PathBuf> { let build_dir = fbuild_packages::Cache::new(sketch).get_build_dir(env_name, profile); - if std::fs::create_dir_all(&build_dir).is_err() { + if let Err(e) = std::fs::create_dir_all(&build_dir) { + tracing::warn!("failed to create build dir for log: {}", e); return None; } let log_path = build_dir.join("compile_many.log"); let body = build_log.into_lines().join("\n"); match std::fs::write(&log_path, body) { Ok(()) => Some(log_path), - Err(_) => None, + Err(e) => { + tracing::warn!("failed to write log file {}: {}", log_path.display(), e); + None + } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/fbuild-build/src/compile_many.rs` around lines 296 - 315, The write_log_file function currently swallows errors; update it to log failures instead of silently returning None: when std::fs::create_dir_all(&build_dir) fails, emit a warning including the build_dir and the error; likewise when std::fs::write(&log_path, body) fails, emit a warning that includes the log_path and the error before returning None. Locate these changes in write_log_file (references: build_dir, log_path, build_log.into_lines()) and use the project's logging facility (e.g., log::warn! or tracing::warn!) so the error context is recorded.
80-85: 💤 Low valueOptional: Remove redundant
.max(1)call.The
.unwrap_or(1)already guarantees the value is at least 1, so the subsequent.max(1)is redundant.Minor cleanup
pub fn default_sketch_jobs() -> usize { std::thread::available_parallelism() .map(|n| n.get()) .unwrap_or(1) - .max(1) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/fbuild-build/src/compile_many.rs` around lines 80 - 85, The function default_sketch_jobs contains a redundant .max(1) because std::thread::available_parallelism().map(|n| n.get()).unwrap_or(1) already yields at least 1; remove the trailing .max(1) from default_sketch_jobs to simplify the return expression while keeping behavior unchanged (refer to the function default_sketch_jobs).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/fbuild-build/src/compile_many.rs`:
- Around line 296-315: The write_log_file function currently swallows errors;
update it to log failures instead of silently returning None: when
std::fs::create_dir_all(&build_dir) fails, emit a warning including the
build_dir and the error; likewise when std::fs::write(&log_path, body) fails,
emit a warning that includes the log_path and the error before returning None.
Locate these changes in write_log_file (references: build_dir, log_path,
build_log.into_lines()) and use the project's logging facility (e.g., log::warn!
or tracing::warn!) so the error context is recorded.
- Around line 80-85: The function default_sketch_jobs contains a redundant
.max(1) because std::thread::available_parallelism().map(|n|
n.get()).unwrap_or(1) already yields at least 1; remove the trailing .max(1)
from default_sketch_jobs to simplify the return expression while keeping
behavior unchanged (refer to the function default_sketch_jobs).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5e5f7e3f-78c7-423c-8492-4507239fa5eb
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
crates/fbuild-build/src/README.mdcrates/fbuild-build/src/compile_many.rscrates/fbuild-build/src/lib.rscrates/fbuild-build/tests/compile_many_two_stage.rscrates/fbuild-cli/Cargo.tomlcrates/fbuild-cli/src/main.rs
…many (#248) Adds `fbuild ci` as a drop-in replacement for `pio ci`. The new subcommand delegates to the two-stage `compile-many` primitive (#241) and exposes the PIO-compatible flag surface so existing CI workflows can swap `s/pio ci/fbuild ci/` without other changes. Flag mapping: - --board / -b -> required, matches pio ci - --lib / -l (repeatable) -> PLATFORMIO_LIB_EXTRA_DIRS (';' on Windows, ':' elsewhere) - --project-conf / -c -> PLATFORMIO_PROJECT_CONFIG (canonicalized when possible) - --keep-build-dir -> accepted for compat (no-op; fbuild always keeps build dirs) - --build-dir <path> -> accepted for compat (warns; not yet honored) - positional sketches -> .ino files are rewritten to their parent dir CompileManyRequest gains a `pio_env: HashMap<String,String>` overlay that threads through every per-sketch BuildParams, so --lib / --project-conf reach the platform orchestrators via the existing PLATFORMIO_* contract. Also fixes a Windows-debug stack overflow in clap's --help rendering: the extra subcommand pushes past the OS main thread's 1 MB default, so main() now trampolines through an 8 MB std::thread for clap's parse path. Closes #242. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Adds a new
fbuild compile-manyCLI subcommand that compiles many sketchesagainst the same board with framework + library archives built once
(stage 1) and per-sketch compile+link fanned out across a thread pool
(stage 2). Implements the primitive described in #238.
Design
--framework-jobsworkers driving intra-build parallelism. This is thememory-heavy stage (framework TU compile + library archives), so we
keep the knob modest on constrained runners.
--sketch-jobsworkers. Each worker invokes the orchestrator with
jobs=1since per-sketch work is single-TU and the outer thread pool already saturates
cores. Framework artifacts written to disk by stage 1 are reused via
the orchestrator's warm-build fast path + zccache.
fbuild_build::get_orchestrator(platform)so allplatform-specific behavior (toolchain resolution, framework
installation, LDF, link flags, size reporting) lives in one place and
compile-manyautomatically picks up future per-platform work withoutre-implementing it.
matching the issue's design goal: "one process, one toolchain load,
one LDF run, one framework build".
The implementation is parameterized over a
SketchBuildertrait so testscan inject a mock and exercise the orchestration layer without dragging
in a real toolchain. The production builder (
OrchestratorBuilder)calls
get_orchestratordirectly.Concurrent-safety review
derives the build root from
<project_dir>/.fbuild/build/<env>/<profile>/,and each sketch in the input list has its own
project_dir, so notwo workers can race on the same
firmware.elf. The test suiteinserts every observed firmware path into a
HashSetand assertsuniqueness on each insert.
zero in-process locks around zccache (
crates/fbuild-build/src/zccache.rshas no
Mutex/RwLock/file-lock). Stage-2 workers each invokezccache wrap <gcc> ...as a child process; the upstream zccachedaemon uses SQLite WAL + content-addressed object files, so concurrent
reads of the same key never block and concurrent writes of distinct
keys never block.
Test plan
uv run soldr cargo build --workspace— cleanuv run soldr cargo clippy --workspace --all-targets -- -D warnings— cleanuv run soldr cargo fmt --all -- --check— cleanuv run soldr cargo test -p fbuild-build— 508 lib tests + 5 newintegration tests in
tests/compile_many_two_stage.rsall passuv run soldr cargo test -p fbuild-cli— 13 tests passThe new integration tests assert:
firmware artifacts.
Barrierofsize N — would deadlock under serial execution).
Acceptance gates
The numeric perf gates from #238 (warm <= 80 s, cold <= 110 s on
ubuntu-latest) cannot be measured locally. They will be validated bythe benchmark workflow after this PR lands and a release ships with the
new subcommand.
Local assertions delivered by this PR:
Deferred / out-of-scope
.github/workflows/benchmark.yml) isStep 4 from feat(cli): two-stage compile-many primitive — framework once, sketches in parallel (#112) #238. That file lives on the orphan
bench/fastled-examplesbranch, not on
main, so it is a separate PR handled by anotheragent — explicitly out of scope here.
./compilewrapper switch — out of scope per feat(cli): two-stage compile-many primitive — framework once, sketches in parallel (#112) #238.Closes #238
Summary by CodeRabbit
Release Notes
New Features
compile-manycommand to the CLI for batch compiling multiple sketches with configurable parallelism--framework-jobsand--sketch-jobsflagsDocumentation