mirror of
https://github.com/vercel/next.js.git
synced 2026-09-20 02:25:18 +08:00
codex/fallback-root-cache
4 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Luke Sandberg
|
f0c1ffc44e |
Remove ineffective turbo-tasks (#91341)
## Remove ineffective turbo-tasks
Identifies and removes turbo-tasks functions where the task overhead exceeds the value they provide. Each turbo-task carries ~4-6μs execution overhead per miss and ~200-500ns per cache hit, plus allocations and bookkeeping.
### What?
Removes 22 `#[turbo_tasks::function]` implementations across resolve plugins, chunk items, and resolve-result helpers — converting them to plain methods or inlining their work. Changes fall into a few buckets:
- **ResolvePlugin condition handling** (`AfterResolvePluginCondition::matches`, `BeforeResolvePluginCondition::matches`, `after_resolve_condition`, `before_resolve_condition`): conditions now store the resolved `Glob` as a `ReadRef<Glob>` on the plugin struct at construction, so `matches` is a pure sync function and the per-plugin `*_resolve_condition` getters are trivial field reads (no longer turbo-tasks). The `after_resolve` / `before_resolve` hooks themselves stay as `#[turbo_tasks::function]` — they synthesize virtual sources/modules and need memoization on `(self, lookup_path, reference_type, request)` to avoid distinct cells producing duplicate module-graph idents.
- The basic theory here is that the right level of caching is at `resolve` and at the hook bodies themselves, not the conditions or condition getters.
- `AfterResolvePluginCondition` and `BeforeResolvePluginCondition` are marked `serialization = "none"` because `ReadRef` cannot be persisted; plugin construction is cheap enough to re-derive on restore.
- **ChunkItem trait methods** (`chunking_context`, `ty`, `content_with_async_module_info`): returned constants or simple field reads, zero cache hits and no `.await` calls (no invalidation value).
- **ResolveResult / ModuleResolveResult helpers** (`primary_modules`, `first_module`, `first_source`, `primary_sources`, `is_unresolvable`, `primary_output_assets`): simple iterators over already-resolved data; converted to plain methods. Added a `Duplicate(usize)` variant to `ModuleResolveResultItem` to handle dedup at construction time instead of in a separate task.
- The basic idea here is that it is reasonable to consume `ResolveResult/ModuleResolveResult` monolithically, and we get little to no benefit from fine grained access. e.g. `is_unresolved()` in theory that is a valuable turbotask, but since it rarely changes but generally if we change how we resolve an import then we have to regenerate code, so saving a few boolean conditions is unlikely to be very valuable.
- Misc: `EcmascriptModuleAsset::analyze`, `is_types_resolving_enabled`, `next_server::resolve::condition`.
### Impact (vercel-site build, dev first-compile)
| Metric | Before | After | Δ |
|---|---:|---:|---:|
| Total cache hits | 30,885,827 | 29,201,314 | −1,684,513 |
| Total cache misses | 6,473,123 | 5,953,626 | **−519,497** |
| Overall hit rate | 82.67% | 83.06% | +0.39 pp |
| Registered task functions | 1,294 | 1,272 | −22 |
The 22 removed tasks were collectively responsible for ~519K misses per build — each miss previously paying the full execution overhead. Most of the work from `EcmascriptModuleAsset::analyze` naturally migrated into `analyze_ecmascript_module` (the task it was wrapping; +129K hits there).
### On-disk cache size (persistent caching)
Each removed task also stops allocating cache cells on disk. Measured on the same vercel-site build with `.next/cache/turbopack` (persistent cache enabled):
| | Size |
|---|---:|
| canary | 2.56 GiB |
| this branch | 2.46 GiB |
| **saved** | **~100 MiB (−3.81%)** |
### Build-time wall clock and peak memory
Ran `pnpm next build --experimental-build-mode=compile` 5 times on each branch
**Peak RSS — clear reduction:**
| | canary | branch | Δ |
|---|---:|---:|---:|
| min | 19.18 GiB | 18.94 GiB | |
| **median** | **19.22 GiB** | **19.01 GiB** | **−217 MiB (−1.10%)** |
| mean | 19.21 GiB | 19.02 GiB | −199 MiB (−1.01%) |
| max | 19.23 GiB | 19.13 GiB | |
Every branch run has lower RSS than every canary run — the distributions don't overlap. Welch's t = −6.03.
**Wall time — no measurable change:**
| | canary | branch | Δ |
|---|---:|---:|---:|
| min | 62.03s | 60.78s | |
| **median** | **62.61s** | **62.65s** | **+0.04s (+0.06%)** |
| mean | 62.83s | 63.80s | +0.96s (+1.53%) |
| max | 64.25s | 68.23s | |
| stddev | 0.84s | 3.42s | |
Median is flat. The mean difference is within noise (Welch's t = +0.61, n = 5). Branch run-to-run variance is higher — one 68.23s outlier pulls the mean up — so this is neither a regression nor a measurable speedup at this sample size.
<!-- NEXT_JS_LLM_PR -->
|
||
|
Luke Sandberg
|
0f38c522bc |
Turbopack: simplify asset ident constructors (#93213)
### What?
Removes the per-method turbo-task constructors on `AssetIdent` (`from_path`, `with_query`, `with_fragment`, `with_modifier`, `with_part`, `with_path`, `with_layer`, `with_content_type`, `with_asset`, `rename_as`, and `path`). Each of those was its own cached task that returned a small projection or a one-field-changed copy. They are now plain Rust builder methods on the owned value, with a single `into_vc()` at the end of the chain that goes through the existing cached `new_inner` constructor.
Call sites that previously chained `Vc` methods now look like:
```rust
module
.ident()
.owned()
.await?
.with_modifier(rcstr!("async loader"))
.into_vc()
```
### Why?
These constructors were tiny "projection" turbo-tasks that paid the cost of a task lookup, cell allocation, and dependency tracking but whose cache layer didn't meaningfully prevent recomputation. The trade-off is invalidation semantics:
- **Before:** a caller doing `module.ident().path()` depended on the cached `path()` projection. If the source `AssetIdent` changed but its `.path` field was unchanged (e.g. a new modifier was added), `path()` re-ran, returned the same `FileSystemPath` cell, and the caller did not re-run.
- **After:** the same caller does `module.ident().await?.path` and depends directly on the `AssetIdent` cell. Any change to the ident (modifier, query, layer, …) invalidates the caller, even if the path is unchanged.
In practice this is rarely a real loss: when an ident changes, the `Module` typically changes too, and the dependent task was going to re-run anyway. `new_inner` already deduplicates structurally-equal idents, so the wrappers were paying overhead per call without buying meaningful invalidation isolation.
Measured on a `vercel-site` build via `NEXT_TURBOPACK_TASK_STATISTICS` and `turbopack/scripts/analyze_cache_effectiveness.py`:
| Task | canary (hits / misses) | this branch |
| --------------------------------- | ---------------------- | ----------- |
| `AssetIdent::path` | 778,273 / 98,018 | removed |
| `AssetIdent::with_modifier` | 27,895 / 22,801 | removed |
| `AssetIdent::from_path` | 2,954 / 29,650 | removed |
| `AssetIdent::with_part` | 2 / 5,440 | removed |
| `AssetIdent::with_layer` | 7 / 4,356 | removed |
| `AssetIdent::rename_as` | 4,969 / 2,269 | removed |
| `AssetIdent::with_query` | 0 / 521 | removed |
| `AssetIdent::with_content_type` | 0 / 79 | removed |
| `AssetIdent::new_inner` | 628 / 129,777 | 29,213 / 120,650 |
Aggregate over the whole build:
- Total cached tasks: 1,300 → 1,292
- Total task invocations: 39,361,186 → 38,208,036 (~1.15M fewer lookups)
- Total cache misses: 6,812,198 → 6,639,937 (~172k fewer)
- Overall hit rate: 82.7% → 82.6% (essentially unchanged)
`new_inner` absorbs the construction work that used to be split across the wrappers. Four upstream tasks gained +519 cache hits each (`EsmAssetReference::resolve_reference`, `ReferencedAsset::from_resolve_result`, `NextServerUtilityModule::ident`, `NodeJsChunkingContext::chunk_item_id_strategy`); no task gained any new misses.
### How?
- `AssetIdent::from_path` and the `with_*` methods are now plain `&mut self`/`self`-by-value builder methods on the struct itself, not `#[turbo_tasks::function]`s.
- A new `AssetIdent::into_vc(self)` finalizes the builder by going through the still-cached `new_inner`.
- `AssetIdent::path()` is removed; callers use `.path` on an owned `AssetIdent`.
- All call sites across `turbopack-*` and `next-*` crates are updated. Most go from `ident.with_modifier(m)` (returning `Vc`) to `ident.owned().await?.with_modifier(m).into_vc()`.
- A follow-up commit removes a few `.clone()`s introduced in the conversion that aren't needed once lifetimes are bound to a local.
### Follow-ups (out of scope)
While migrating call sites, two pre-existing entry builders surfaced as candidates for cleanup. Not addressed here, but worth noting:
- `get_app_page_entry` (`crates/next-core/src/next_app/app_page_entry.rs`) replaces the *content* of the source returned by `load_next_js_template` (prefixing imports onto `result.build()`) but reuses the template's `ident` with a `?page=...` query suffix as a disambiguator. The new `VirtualSource` ends up with content from one place and an ident chain pointing at another. A cleaner shape would be to mint a fresh ident from the page path, since the caller already knows what it's building.
- `create_page_ssr_entry_module` (`crates/next-pages/page_entry.rs`) has the same shape on the instrumentation-conflict branch: it appends `export const register = hoist(...)` to the template content and constructs a `VirtualSource` with the original `source.ident()` unchanged. Lower-frequency than the app-page case (fires at most once per build), but the ident still misrepresents the constructed content.
<!-- NEXT_JS_LLM_PR -->
|
||
|
Luke Sandberg
|
236a76dd0f |
[turbopack] Remove turbo_tasks::function from ModuleReference getters (#91229)
### What? Refactors the `ModuleReference` trait to make `chunking_type()` and `binding_usage()` methods return direct values instead of `Vc<T>` wrapped values, removing the need for async task functions. Also removes the `get_referenced_asset` task from `EsmAssetReference`, inlining its logic into the callers. ### Why? This change simplifies the API by eliminating unnecessary async overhead for methods that typically return simple, computed values. The previous implementation required `#[turbo_tasks::function]` annotations and `Vc<T>` wrappers even when the methods didn't need to perform async operations or benefit from caching. ### Impact | Metric | Base | Change | Delta | |--------|------|--------|-------| | Hits | 35,678,143 | 35,845,124 | **+166,981** | | Misses | 9,418,378 | 7,910,986 | **-1,507,392** | | Total | 45,096,521 | 43,756,110 | **-1,340,411** | | Task types | 1,306 | 1,277 | **-29** | 29 task types were removed, eliminating **2.6M total task invocations** (1.1M hits + 1.5M misses): - **`chunking_type`** — 21 task types removed across all `ModuleReference` implementors (~952k invocations) - **`binding_usage`** — 6 task types removed (~527k invocations) - **`BindingUsage::all`** — helper task removed (~36k invocations) - **`EsmAssetReference::get_referenced_asset`** — removed and inlined (~1.08M invocations: 628k hits + 451k misses) The removed `get_referenced_asset` hits reappear as +628k hits on `EsmAssetReference::resolve_reference` and `ReferencedAsset::from_resolve_result` (with zero increase in misses), confirming the work is now served from cache through the existing callers. No tasks had increased misses — the removal is clean with no cache invalidation spillover. I also ran some builds to measure latency ``` # This branch $ hyperfine -p 'rm -rf .next' -w 2 -r 10 'pnpm next build --turbopack --experimental-build-mode=compile' Benchmark 1: pnpm next build --turbopack --experimental-build-mode=compile Time (mean ± σ): 52.752 s ± 0.658 s [User: 376.575 s, System: 106.375 s] Range (min … max): 51.913 s … 54.161 s 10 runs # on canary $ hyperfine -p 'rm -rf .next' -w 2 -r 10 'pnpm next build --turbopack --experimental-build-mode=compile' Benchmark 1: pnpm next build --turbopack --experimental-build-mode=compile Time (mean ± σ): 54.675 s ± 1.394 s [User: 389.273 s, System: 114.642 s] Range (min … max): 53.434 s … 58.189 s 10 runs ``` so a solid win of almost 2 seconds MaxRSS also went from 16,474,324,992 bytes to 16,359,309,312 bytes (from one measurement) so a savings of ~100M of max heap size. ### How? - Changed `chunking_type()` method signature from `Vc<ChunkingTypeOption>` to `Option<ChunkingType>` - Changed `binding_usage()` method signature from `Vc<BindingUsage>` to `BindingUsage` - Removed `ChunkingTypeOption` type alias as it's no longer needed - Updated all implementations across the codebase to return direct values instead of wrapped ones - Removed `#[turbo_tasks::function]` annotations from these methods - Updated call sites to use `into_trait_ref().await?` pattern when accessing these methods from `Vc<dyn ModuleReference>` - Removed `EsmAssetReference::get_referenced_asset`, inlining its logic into callers - Added validation for `turbopack-chunking-type` annotation values in import analysis - Fixed cache effectiveness analysis script |
||
|
Luke Sandberg
|
7cc4990f49 |
[turbopack] Track task durations in the task_statistics file (#83522)
# Track task execution duration in TaskStatistics ## What? This PR adds tracking of task execution duration in Turbopack's task system, enabling better performance analysis and optimization. ## Why? Understanding how long tasks take to execute helps identify optimization opportunities, especially for determining whether caching is beneficial for specific tasks. ## How? - Added `track_task_duration` method to record execution time for tasks - Updated task statistics to track execution count and duration - Added a Python script `analyze_cache_effectiveness.py` to identify tasks that would benefit from removing caching - Updated tests to account for the new statistics fields Sample output from the script ``` Tasks ranked by estimated time savings from removing caching layer Savings Hit Rate Exec Time Operations Task Name --------------------------------------------------- 2.33s 39.5% 765ns 661,176 turbopack-ecmascript@turbopack_ecmascript::references::esm::base::EsmAssetReference::ChunkableModuleReference::chunking_type 2.29s 18.5% 1.6μs 490,488 turbopack-ecmascript@turbopack_ecmascript::references::esm::base::EsmAssetReference::ChunkableModuleReference::export_usage 1.99s 9.0% 9.8μs 430,149 turbopack@turbopack::ModuleAssetContext::AssetContext::resolve_asset 1.17s 51.7% 1.2μs 462,164 turbopack-ecmascript@turbopack_ecmascript::EcmascriptModuleAsset::ResolveOrigin::get_inner_asset 1.10s 54.1% 1.2μs 462,387 turbopack-core@turbopack_core::resolve::ModuleResolveResult::is_unresolvable 916.01ms 0.0% 19.2μs 152,669 turbopack@turbopack::apply_module_type 807.37ms 74.5% 1.1μs 722,106 turbopack-ecmascript@turbopack_ecmascript::references::esm::base::ReferencedAsset::from_resolve_result 782.16ms 69.7% 1.5μs 680,828 turbopack-core@turbopack_core::resolve::ModuleResolveResult::primary_modules 749.54ms 4.0% 80ns 129,625 turbopack-core@turbopack_core::ident::AssetIdent::new_inner 717.59ms 94.5% 5ns 887,040 turbopack-ecmascript@turbopack_ecmascript::EcmascriptModuleAsset::ResolveOrigin::asset_context 522.31ms 30.2% 1.7μs 136,180 turbopack-core@turbopack_core::resolve::ResolveResult::is_unresolvable 452.88ms 0.0% 5.2μs 75,484 next-core@next_core::next_server::resolve::ExternalCjsModulesResolvePlugin::AfterResolvePlugin::after_resolve 415.54ms 45.2% 937ns 134,377 turbopack-core@turbopack_core::resolve::pattern::Pattern::new_internal 388.03ms 0.0% 191.1μs 64,672 turbopack-ecmascript@turbopack_ecmascript::parse::parse ``` The script analyzes task statistics to find tasks where the overhead of caching exceeds the benefit, providing recommendations for optimization based on execution patterns. It leverages data from the overhead.rs benchmark which is also enhanced to provide an estimate on the delta between the measured duration and the actual duration. ## Conclusions? There are a few items of low hanging fruit but the real issue is `trait` items. We need to provide more flexibility to `value_trait` items to make it possible to have non-turbotask items that are `async` |