perf(@angular/build): consolidate component stylesheet bundling with shared load result cache - #33906
Conversation
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors ComponentStylesheetBundler to run esbuild compilations directly instead of using BundlerContext, introducing custom caching and promise-based deduplication for both file and inline stylesheets. The review feedback highlights a potential bug where externalId remains a boolean if the cached identifier is missing, a performance optimization in the cache invalidation logic using set intersection, and a missing guard in extractResult that could lead to a TypeError if result.metafile is undefined.
a088bda to
4cb873f
Compare
There was a problem hiding this comment.
Code Review
This pull request refactors ComponentStylesheetBundler to perform direct esbuild compilation using build() instead of wrapping it in BundlerContext. It introduces in-memory caching for file and inline results, along with tracking active build promises to support incremental watch mode. The review feedback highlights critical race conditions and stale cache write issues in both file-based and inline stylesheet bundling when files are modified while a build promise is in-flight. It is recommended to clear pending promises on invalidation, ensure only active promises write to the cache, and extract a helper function to resolve absolute paths to reduce code duplication in #collectWatchFiles.
…shared load result cache Replaces per-stylesheet `esbuild.context` proliferation in `ComponentStylesheetBundler` with ephemeral single-shot `esbuild.build()` invocations and a shared, persistent `MemoryLoadResultCache`. ### Key Improvements 1. Eliminates Go process IPC context proliferation (500 active goroutines/contexts -> 0 lingering contexts). 2. Cross-component Sass partial & preprocessor cache sharing via unified `MemoryLoadResultCache`. 3. In-flight compilation deduplication coalescing concurrent requests. 4. Optimized set intersection invalidation (O(min(|A|, |B|))) accelerating large-scale file invalidations by up to 70x. ### Comprehensive Benchmarks #### 1. Cold Compilation Throughput & Latency (Mean Duration) | Workload | Baseline | Consolidated | Speedup | Latency Reduction | | :--- | :--- | :--- | :--- | :--- | | 100 Components (Cold) | 114.96 ms | 98.74 ms | 1.16x | -14.1% | | 300 Components (Cold) | 358.37 ms | 315.68 ms | 1.14x | -11.9% | | 500 Components (Cold) | 609.68 ms | 530.87 ms | 1.15x | -12.9% | #### 2. Memory Footprint (500 Components) | Metric | Baseline (Contexts) | Consolidated (Shared Cache) | Delta / Improvement | | :--- | :--- | :--- | :--- | | Persistent Go IPC Contexts | 500 active | 0 contexts | -100.0% (Eliminated) | | V8 Heap Used Delta | +10.22 MB | +5.79 MB | -43.3% Heap Reduction | | Resident Set Size (RSS) Delta | +27.60 MB | +16.30 MB | -41.0% RSS Reduction | #### 3. Watch Mode Rebuild & Invalidation Latencies | Scenario | Baseline | Consolidated | Speedup Factor | | :--- | :--- | :--- | :--- | | Leaf Component Stylesheet Edit | 7.96 ms | 6.75 ms | 1.18x Faster | | Shared Sass Partial Edit | 227.18 ms | 206.63 ms | 1.10x Faster | | Branch Switch (500 modified / 500 components) | 2.27 ms/op | 0.14 ms/op | 16.07x Faster | | Large Switch (1,000 modified / 1,000 components) | 28.38 ms/op | 0.40 ms/op | 70.65x Faster | | No-Op / Touched File Rebuild | 0.47 ms | 0.29 ms | 1.62x Faster |
4cb873f to
57de14f
Compare
| const bundlerContext = await this.#fileContexts.getOrCreate(entry, () => { | ||
| return new BundlerContext(this.options.workspaceRoot, this.incremental, (loadCache) => { | ||
| const buildOptions = createStylesheetBundleOptions(this.options, loadCache); | ||
| if (externalId) { | ||
| assert( | ||
| typeof externalId === 'string', | ||
| 'Initial external component stylesheets must have a string identifier', | ||
| ); | ||
|
|
||
| buildOptions.entryPoints = { [externalId]: entry }; | ||
| buildOptions.entryNames = '[name]'; | ||
| delete buildOptions.publicPath; | ||
| } else { | ||
| buildOptions.entryPoints = [entry]; |
There was a problem hiding this comment.
With #33914, you should be able to do something similar to the following now:
const bundlerContext = await this.#fileContexts.getOrCreate(entry, () => {
return new BundlerContext(
this.options.workspaceRoot,
this.incremental,
(loadCache) => createStylesheetBundleOptions(this.options, loadCache, ...),
/* useContext */ false, // Enables direct esbuild.build() mode
/* initialFilter */ undefined, // Default filter
this.#loadCache, // Shared LoadResultCache instance
);
});
| const hasIntersection = (setA: Set<string>, setB: Set<string>): boolean => { | ||
| if (setA.size < setB.size) { | ||
| for (const value of setA) { | ||
| if (setB.has(value)) { | ||
| return true; | ||
| } | ||
| } | ||
| } else { | ||
| for (const value of setB) { | ||
| if (setA.has(value)) { | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| }; |
There was a problem hiding this comment.
Can we move this logic into the BundlerContext's invalidate directly so that all usages could benefit? Probably need to update the signature to invalidate(files: Iterable<string> | ReadonlySet<string>): boolean.
Replaces per-stylesheet
esbuild.contextproliferation inComponentStylesheetBundlerwith ephemeral single-shotesbuild.build()invocations and a shared, persistentMemoryLoadResultCache.Key Improvements
MemoryLoadResultCache.O(min(|A|, |B|))) accelerating large-scale file invalidations by up to 70x.Comprehensive Benchmarks
1. Cold Compilation Throughput & Latency (Mean Duration)
2. Memory Footprint (500 Components)
3. Watch Mode Rebuild & Invalidation Latencies