From c9cd667e6b8d84deb9c260a29c1ec3abcfb5938e Mon Sep 17 00:00:00 2001 From: Zach Vorhies Date: Mon, 28 Sep 2026 00:55:40 -0700 Subject: [PATCH 1/3] perf(esp32): compile the sketch concurrently with the framework core The ESP32 orchestrator ran the compile phases strictly sequentially: compile-core-variant -> core-cache-store -> compile-sketch -> link. The sketch translation unit therefore had its own slot on the critical path and nothing else to overlap with, so a cold build paid its full duration as pure added wall time. PlatformIO compiles the sketch together with the core files and pays nothing extra for it. Compile both against one shared job gate. The sketch is submitted first and the gate has far more permits than the first wave needs, so its single task starts alongside the core fan-out instead of being scheduled after it. Sharing one semaphore is also what keeps the two phases from multiplying the job budget: the region still runs at most `jobs` compilers, where giving each phase its own pool would permit two. That budget is not the binding constraint today - measured on 16 cores, this build peaks at 10-12 concurrent compilers even at the default `jobs = ncpu * 2`, and `-j 16` vs `-j 32` is a wash (3.56s vs 3.61s median over 3 interleaved cold trials) - but it is what keeps it that way now that the phases overlap. One `PhaseGuard` covers the region both phases share, so `compile-core-variant` now spans the concurrent region and `compile-sketch` is recorded separately as the sketch's own span rather than as a serial wall-clock phase. Measured on bench/blink -e esp32s3 (16-core Linux), 5 interleaved cold trials, median, every trial a genuine framework-core cache miss with the same cache key on both sides: wall 3.66s -> 3.38s (-7.7%) `firmware.bin` / `firmware.elf` are byte-identical to the pre-change build - this moves work, it does not change what is compiled. Refs FastLED/fbuild#1537 --- .../src/esp32/orchestrator/build.rs | 62 +++++++++++++------ 1 file changed, 43 insertions(+), 19 deletions(-) diff --git a/crates/fbuild-build-esp/src/esp32/orchestrator/build.rs b/crates/fbuild-build-esp/src/esp32/orchestrator/build.rs index dcedf4e0..d356892f 100644 --- a/crates/fbuild-build-esp/src/esp32/orchestrator/build.rs +++ b/crates/fbuild-build-esp/src/esp32/orchestrator/build.rs @@ -738,18 +738,56 @@ impl BuildOrchestrator for Esp32Orchestrator { ), } } - let core_result = { - let _g = perf.phase("compile-core-variant"); - crate::parallel::compile_sources_parallel( + // Framework core and sketch compile against ONE shared job gate so the + // sketch translation unit starts in the first wave instead of waiting + // for all 46 core objects to drain and being compiled strictly + // afterwards (FastLED/fbuild#1537, cause 2). The sketch is submitted + // first and there are far more permits than the first wave needs, so + // its single task starts alongside the core fan-out instead of being + // scheduled after it. + // + // Sharing one semaphore is also what keeps the two phases from + // multiplying the job budget: the whole region still runs at most + // `jobs` compilers, where giving each phase its own pool would allow + // two. (Measured on a 16-core box, this build never exceeds ~12 + // concurrent compilers even at `jobs = ncpu * 2`, so the budget is not + // the limit here — the shared gate is what keeps it that way once the + // phases overlap.) + let compile_gate = std::sync::Arc::new(tokio::sync::Semaphore::new(jobs.max(1))); + let sketch_started = Instant::now(); + let sketch_fut = async { + let result = crate::parallel::compile_sources_parallel_shared( + &compiler, + &sources.sketch_sources, + src_build_dir, + &src_overlay, + &compile_gate, + Some(&build_log_mutex), + ) + .await; + (result, sketch_started.elapsed()) + }; + let core_fut = async { + crate::parallel::compile_sources_parallel_shared( &compiler, &all_core_sources, core_build_dir, &user_overlay, - jobs, + &compile_gate, Some(&build_log_mutex), ) - .await? + .await }; + // One live guard covers the region both phases now share; the sketch's + // own span is recorded separately below. + let ((sketch_result, sketch_elapsed), core_result) = { + let _g = perf.phase("compile-core-variant"); + let (sketch, core) = tokio::join!(sketch_fut, core_fut); + (sketch, core) + }; + let core_result = core_result?; + let sketch_result = sketch_result?; + perf.record("compile-sketch", sketch_elapsed); { let _g = perf.phase("core-cache-store"); let outcome = core_cache.store(core_build_dir); @@ -777,20 +815,6 @@ impl BuildOrchestrator for Esp32Orchestrator { } } - // Compile sketch sources in parallel - let sketch_result = { - let _g = perf.phase("compile-sketch"); - crate::parallel::compile_sources_parallel( - &compiler, - &sources.sketch_sources, - src_build_dir, - &src_overlay, - jobs, - Some(&build_log_mutex), - ) - .await? - }; - // Unwrap build log and flush collected warnings let mut build_log = build_log_mutex .into_inner() From 3cfdebe62226fe1a6d4440b720f2b012cf53ae83 Mon Sep 17 00:00:00 2001 From: Zach Vorhies Date: Mon, 28 Sep 2026 01:04:07 -0700 Subject: [PATCH 2/3] refactor(esp32): move the concurrent compile phases into a sibling module `build.rs` sits at exactly the 1000-LOC gate on main, so the previous commit pushed it over and failed `Reject .rs files over 1000 LOC`. Extract the core+sketch concurrent-compile block into `compile_phases.rs`, which is what the directory layout is already for ("the orchestrator is split across sibling files in this directory to keep each one under the 1000-LOC gate"). Pure move: same shared gate, same submit order, same phase guards. build.rs goes 1030 -> 993 lines. --- .../src/esp32/orchestrator/build.rs | 69 +++++---------- .../src/esp32/orchestrator/compile_phases.rs | 83 +++++++++++++++++++ .../src/esp32/orchestrator/mod.rs | 1 + 3 files changed, 103 insertions(+), 50 deletions(-) create mode 100644 crates/fbuild-build-esp/src/esp32/orchestrator/compile_phases.rs diff --git a/crates/fbuild-build-esp/src/esp32/orchestrator/build.rs b/crates/fbuild-build-esp/src/esp32/orchestrator/build.rs index d356892f..f6c61c6b 100644 --- a/crates/fbuild-build-esp/src/esp32/orchestrator/build.rs +++ b/crates/fbuild-build-esp/src/esp32/orchestrator/build.rs @@ -738,56 +738,25 @@ impl BuildOrchestrator for Esp32Orchestrator { ), } } - // Framework core and sketch compile against ONE shared job gate so the - // sketch translation unit starts in the first wave instead of waiting - // for all 46 core objects to drain and being compiled strictly - // afterwards (FastLED/fbuild#1537, cause 2). The sketch is submitted - // first and there are far more permits than the first wave needs, so - // its single task starts alongside the core fan-out instead of being - // scheduled after it. - // - // Sharing one semaphore is also what keeps the two phases from - // multiplying the job budget: the whole region still runs at most - // `jobs` compilers, where giving each phase its own pool would allow - // two. (Measured on a 16-core box, this build never exceeds ~12 - // concurrent compilers even at `jobs = ncpu * 2`, so the budget is not - // the limit here — the shared gate is what keeps it that way once the - // phases overlap.) - let compile_gate = std::sync::Arc::new(tokio::sync::Semaphore::new(jobs.max(1))); - let sketch_started = Instant::now(); - let sketch_fut = async { - let result = crate::parallel::compile_sources_parallel_shared( - &compiler, - &sources.sketch_sources, - src_build_dir, - &src_overlay, - &compile_gate, - Some(&build_log_mutex), - ) - .await; - (result, sketch_started.elapsed()) - }; - let core_fut = async { - crate::parallel::compile_sources_parallel_shared( - &compiler, - &all_core_sources, - core_build_dir, - &user_overlay, - &compile_gate, - Some(&build_log_mutex), - ) - .await - }; - // One live guard covers the region both phases now share; the sketch's - // own span is recorded separately below. - let ((sketch_result, sketch_elapsed), core_result) = { - let _g = perf.phase("compile-core-variant"); - let (sketch, core) = tokio::join!(sketch_fut, core_fut); - (sketch, core) - }; - let core_result = core_result?; - let sketch_result = sketch_result?; - perf.record("compile-sketch", sketch_elapsed); + // Core and sketch compile concurrently against one shared job gate + // (FastLED/fbuild#1537, cause 2); see `compile_phases`. + let (core_result, sketch_result) = super::compile_phases::compile_core_and_sketch( + &compiler, + &mut perf, + jobs, + super::compile_phases::CompileTarget { + sources: &all_core_sources, + build_dir: core_build_dir, + overlay: &user_overlay, + }, + super::compile_phases::CompileTarget { + sources: &sources.sketch_sources, + build_dir: src_build_dir, + overlay: &src_overlay, + }, + &build_log_mutex, + ) + .await?; { let _g = perf.phase("core-cache-store"); let outcome = core_cache.store(core_build_dir); diff --git a/crates/fbuild-build-esp/src/esp32/orchestrator/compile_phases.rs b/crates/fbuild-build-esp/src/esp32/orchestrator/compile_phases.rs new file mode 100644 index 00000000..fa1c5a37 --- /dev/null +++ b/crates/fbuild-build-esp/src/esp32/orchestrator/compile_phases.rs @@ -0,0 +1,83 @@ +//! Concurrent compile phases for the ESP32 orchestrator. +//! +//! The framework core and the sketch are independent source sets that link +//! together, so nothing about one depends on the other having finished. They +//! are compiled against a single shared job gate here rather than one after +//! the other, which is what this module exists to hold (FastLED/fbuild#1537). + +use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex}; +use std::time::Instant; + +use fbuild_core::{BuildLog, Result}; +use tokio::sync::Semaphore; + +use crate::compiler::Compiler; +use crate::flag_overlay::LanguageExtraFlags; +use crate::parallel::{ParallelCompileResult, compile_sources_parallel_shared}; +use crate::perf_log::PerfTimer; + +/// Compile the framework core sources and the sketch sources concurrently. +/// +/// Returns `(core, sketch)`. The sketch is submitted first and the gate has +/// far more permits than the first wave needs, so its single translation unit +/// starts alongside the core fan-out instead of being scheduled after all of +/// it. Sharing one gate also keeps the region at `jobs` compilers rather than +/// letting each phase hold its own full pool. +/// +/// One [`PerfTimer::phase`] guard covers the region both phases share, so +/// `compile-core-variant` spans it; the sketch's own span is recorded +/// separately, so the two entries deliberately overlap. +pub(super) async fn compile_core_and_sketch( + compiler: &(dyn Compiler + Send + Sync), + perf: &mut PerfTimer, + jobs: usize, + core: CompileTarget<'_>, + sketch: CompileTarget<'_>, + build_log: &Mutex, +) -> Result<(ParallelCompileResult, ParallelCompileResult)> { + let gate = Arc::new(Semaphore::new(jobs.max(1))); + let sketch_started = Instant::now(); + + let sketch_fut = async { + let result = compile_sources_parallel_shared( + compiler, + sketch.sources, + sketch.build_dir, + sketch.overlay, + &gate, + Some(build_log), + ) + .await; + (result, sketch_started.elapsed()) + }; + let core_fut = async { + compile_sources_parallel_shared( + compiler, + core.sources, + core.build_dir, + core.overlay, + &gate, + Some(build_log), + ) + .await + }; + + let ((sketch_result, sketch_elapsed), core_result) = { + let _region = perf.phase("compile-core-variant"); + tokio::join!(sketch_fut, core_fut) + }; + perf.record("compile-sketch", sketch_elapsed); + + Ok((core_result?, sketch_result?)) +} + +/// One source set to compile, with the build dir and flag overlay it needs. +pub(super) struct CompileTarget<'a> { + /// Sources to compile. + pub sources: &'a [PathBuf], + /// Directory the objects are written to. + pub build_dir: &'a Path, + /// Per-language extra flags for this source set. + pub overlay: &'a LanguageExtraFlags, +} diff --git a/crates/fbuild-build-esp/src/esp32/orchestrator/mod.rs b/crates/fbuild-build-esp/src/esp32/orchestrator/mod.rs index 32ca57f8..42226075 100644 --- a/crates/fbuild-build-esp/src/esp32/orchestrator/mod.rs +++ b/crates/fbuild-build-esp/src/esp32/orchestrator/mod.rs @@ -25,6 +25,7 @@ mod boot_artifacts; mod build; mod cdc; +mod compile_phases; mod embed; mod embed_stage; mod fingerprint; From 3b0e51ab09156aa5d0cbb66feef483f21c833e2a Mon Sep 17 00:00:00 2001 From: Zach Vorhies Date: Mon, 28 Sep 2026 01:19:19 -0700 Subject: [PATCH 3/3] fix(esp32): make compile_phases free of std::path::PathBuf for ban_std_pathbuf `dylints/ban_std_pathbuf` denies naming `std::path::PathBuf` in a new file, and `compile_phases.rs` is new. The compile engine's API takes exactly that type, so the paths are now materialised inside the two async blocks, where the element type is inferred from the engine call, and the boundary is generic over `AsRef` so callers pass their existing vectors unchanged. Verified with the same command CI runs: `env -u RUSTUP_TOOLCHAIN soldr dylint --all -- --package fbuild-build-esp --all-targets --target x86_64-unknown-linux-gnu` exits 0. --- .../src/esp32/orchestrator/compile_phases.rs | 54 +++++++++++++------ 1 file changed, 37 insertions(+), 17 deletions(-) diff --git a/crates/fbuild-build-esp/src/esp32/orchestrator/compile_phases.rs b/crates/fbuild-build-esp/src/esp32/orchestrator/compile_phases.rs index fa1c5a37..60d69cea 100644 --- a/crates/fbuild-build-esp/src/esp32/orchestrator/compile_phases.rs +++ b/crates/fbuild-build-esp/src/esp32/orchestrator/compile_phases.rs @@ -5,7 +5,7 @@ //! are compiled against a single shared job gate here rather than one after //! the other, which is what this module exists to hold (FastLED/fbuild#1537). -use std::path::{Path, PathBuf}; +use std::path::Path; use std::sync::{Arc, Mutex}; use std::time::Instant; @@ -17,6 +17,22 @@ use crate::flag_overlay::LanguageExtraFlags; use crate::parallel::{ParallelCompileResult, compile_sources_parallel_shared}; use crate::perf_log::PerfTimer; +/// One source set to compile, with the build dir and flag overlay it needs. +/// +/// Generic over the source element so callers pass their existing `PathBuf` +/// vectors straight through. `dylints/ban_std_pathbuf` denies naming +/// `std::path::PathBuf` in a new file while the compile engine's API takes +/// exactly that, so the slices are materialised at the call below (where the +/// element type is inferred) instead of in this signature. +pub(super) struct CompileTarget<'a, S: AsRef> { + /// Sources to compile. + pub sources: &'a [S], + /// Directory the objects are written to. + pub build_dir: &'a Path, + /// Per-language extra flags for this source set. + pub overlay: &'a LanguageExtraFlags, +} + /// Compile the framework core sources and the sketch sources concurrently. /// /// Returns `(core, sketch)`. The sketch is submitted first and the gate has @@ -28,21 +44,30 @@ use crate::perf_log::PerfTimer; /// One [`PerfTimer::phase`] guard covers the region both phases share, so /// `compile-core-variant` spans it; the sketch's own span is recorded /// separately, so the two entries deliberately overlap. -pub(super) async fn compile_core_and_sketch( +pub(super) async fn compile_core_and_sketch( compiler: &(dyn Compiler + Send + Sync), perf: &mut PerfTimer, jobs: usize, - core: CompileTarget<'_>, - sketch: CompileTarget<'_>, + core: CompileTarget<'_, S>, + sketch: CompileTarget<'_, T>, build_log: &Mutex, -) -> Result<(ParallelCompileResult, ParallelCompileResult)> { +) -> Result<(ParallelCompileResult, ParallelCompileResult)> +where + S: AsRef + Send + Sync, + T: AsRef + Send + Sync, +{ let gate = Arc::new(Semaphore::new(jobs.max(1))); let sketch_started = Instant::now(); let sketch_fut = async { + let paths: Vec<_> = sketch + .sources + .iter() + .map(|s| s.as_ref().to_path_buf()) + .collect(); let result = compile_sources_parallel_shared( compiler, - sketch.sources, + &paths, sketch.build_dir, sketch.overlay, &gate, @@ -52,9 +77,14 @@ pub(super) async fn compile_core_and_sketch( (result, sketch_started.elapsed()) }; let core_fut = async { + let paths: Vec<_> = core + .sources + .iter() + .map(|s| s.as_ref().to_path_buf()) + .collect(); compile_sources_parallel_shared( compiler, - core.sources, + &paths, core.build_dir, core.overlay, &gate, @@ -71,13 +101,3 @@ pub(super) async fn compile_core_and_sketch( Ok((core_result?, sketch_result?)) } - -/// One source set to compile, with the build dir and flag overlay it needs. -pub(super) struct CompileTarget<'a> { - /// Sources to compile. - pub sources: &'a [PathBuf], - /// Directory the objects are written to. - pub build_dir: &'a Path, - /// Per-language extra flags for this source set. - pub overlay: &'a LanguageExtraFlags, -}