Skip to content

Commit f0c1ffc

Browse files
authored
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 -->
1 parent e83bc93 commit f0c1ffc

44 files changed

Lines changed: 931 additions & 425 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

crates/next-api/src/app.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -860,7 +860,6 @@ impl AppProject {
860860
None,
861861
ResolveErrorMode::Error,
862862
)
863-
.to_resolved()
864863
.await?
865864
.first_module()
866865
.await?

crates/next-api/src/next_server_nft.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,10 +242,11 @@ impl ServerNftJsonAsset {
242242
None,
243243
ResolveErrorMode::Error,
244244
)
245+
.await?
245246
.primary_modules()
246247
.await?
247248
.into_iter()
248-
.map(|m| **m))
249+
.map(|m| *m))
249250
})
250251
.try_flat_join()
251252
.await?,

crates/next-api/src/pages.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,7 @@ impl PagesProject {
566566
None,
567567
)
568568
.await?
569+
.await?
569570
.first_module()
570571
.await?
571572
.context("expected Next.js client runtime to resolve to a module")?;

crates/next-core/src/next_client/runtime_entry.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ impl RuntimeEntry {
4040
None,
4141
ResolveErrorMode::Error,
4242
)
43-
.to_resolved()
4443
.await?
4544
.primary_modules()
4645
.await?;

crates/next-core/src/next_client_reference/ecmascript_client_reference/ecmascript_client_reference_module.rs

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use std::{io::Write, iter::once};
22

33
use anyhow::{Context, Result, bail};
4+
use async_trait::async_trait;
45
use indoc::writedoc;
56
use turbo_rcstr::{RcStr, rcstr};
67
use turbo_tasks::{ResolvedVc, ValueToString, Vc};
@@ -320,12 +321,10 @@ impl ChunkItem for EcmascriptClientReferenceProxyChunkItem {
320321
self.inner_module.ident()
321322
}
322323

323-
#[turbo_tasks::function]
324324
fn chunking_context(&self) -> Vc<Box<dyn ChunkingContext>> {
325325
*self.chunking_context
326326
}
327327

328-
#[turbo_tasks::function]
329328
fn ty(&self) -> Vc<Box<dyn ChunkType>> {
330329
Vc::upcast(Vc::<EcmascriptChunkType>::default())
331330
}
@@ -336,21 +335,19 @@ impl ChunkItem for EcmascriptClientReferenceProxyChunkItem {
336335
}
337336
}
338337

338+
#[async_trait]
339339
#[turbo_tasks::value_impl]
340340
impl EcmascriptChunkItem for EcmascriptClientReferenceProxyChunkItem {
341-
#[turbo_tasks::function]
342-
fn content(&self) -> Vc<EcmascriptChunkItemContent> {
343-
self.inner_chunk_item.content()
344-
}
345-
346-
#[turbo_tasks::function]
347-
fn content_with_async_module_info(
341+
async fn content_with_async_module_info(
348342
&self,
349343
async_module_info: Option<Vc<AsyncModuleInfo>>,
350344
estimated: bool,
351-
) -> Vc<EcmascriptChunkItemContent> {
345+
) -> Result<Vc<EcmascriptChunkItemContent>> {
352346
self.inner_chunk_item
347+
.into_trait_ref()
348+
.await?
353349
.content_with_async_module_info(async_module_info, estimated)
350+
.await
354351
}
355352
}
356353

crates/next-core/src/next_font/local/mod.rs

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,24 +52,27 @@ struct NextFontLocalFontFileOptions {
5252
#[turbo_tasks::value]
5353
pub(crate) struct NextFontLocalResolvePlugin {
5454
root: FileSystemPath,
55+
condition: ResolvedVc<BeforeResolvePluginCondition>,
5556
}
5657

5758
#[turbo_tasks::value_impl]
5859
impl NextFontLocalResolvePlugin {
5960
#[turbo_tasks::function]
60-
pub fn new(root: FileSystemPath) -> Vc<Self> {
61-
NextFontLocalResolvePlugin { root }.cell()
61+
pub async fn new(root: FileSystemPath) -> Result<Vc<Self>> {
62+
let condition = BeforeResolvePluginCondition::from_request_glob(Glob::new(
63+
rcstr!("{next,@vercel/turbopack-next/internal}/font/local/*"),
64+
GlobOptions::default(),
65+
))
66+
.to_resolved()
67+
.await?;
68+
Ok(NextFontLocalResolvePlugin { root, condition }.cell())
6269
}
6370
}
6471

6572
#[turbo_tasks::value_impl]
6673
impl BeforeResolvePlugin for NextFontLocalResolvePlugin {
67-
#[turbo_tasks::function]
6874
fn before_resolve_condition(&self) -> Vc<BeforeResolvePluginCondition> {
69-
BeforeResolvePluginCondition::from_request_glob(Glob::new(
70-
rcstr!("{next,@vercel/turbopack-next/internal}/font/local/*"),
71-
GlobOptions::default(),
72-
))
75+
*self.condition
7376
}
7477

7578
#[turbo_tasks::function]

crates/next-core/src/next_import_map.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1269,7 +1269,7 @@ pub async fn try_get_next_package(
12691269
Request::parse(Pattern::Constant(rcstr!("next/package.json"))),
12701270
node_cjs_resolve_options(root.clone()),
12711271
);
1272-
if let Some(source) = &*result.first_source().await? {
1272+
if let Some(source) = result.await?.first_source() {
12731273
Ok(Vc::cell(Some(source.ident().await?.path.parent())))
12741274
} else {
12751275
MissingNextFolderIssue {

crates/next-core/src/next_server/resolve.rs

Lines changed: 17 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -42,41 +42,38 @@ pub enum ExternalPredicate {
4242
/// possible to resolve them at runtime.
4343
#[turbo_tasks::value]
4444
pub(crate) struct ExternalCjsModulesResolvePlugin {
45-
root: FileSystemPath,
4645
predicate: ResolvedVc<ExternalPredicate>,
4746
import_externals: bool,
47+
condition: ResolvedVc<AfterResolvePluginCondition>,
4848
}
4949

5050
#[turbo_tasks::value_impl]
5151
impl ExternalCjsModulesResolvePlugin {
5252
#[turbo_tasks::function]
53-
pub fn new(
53+
pub async fn new(
5454
root: FileSystemPath,
5555
predicate: ResolvedVc<ExternalPredicate>,
5656
import_externals: bool,
57-
) -> Vc<Self> {
58-
ExternalCjsModulesResolvePlugin {
57+
) -> Result<Vc<Self>> {
58+
let condition = AfterResolvePluginCondition::new_with_glob(
5959
root,
60+
Glob::new(rcstr!("**/node_modules/**"), GlobOptions::default()),
61+
)
62+
.to_resolved()
63+
.await?;
64+
Ok(ExternalCjsModulesResolvePlugin {
6065
predicate,
6166
import_externals,
67+
condition,
6268
}
63-
.cell()
69+
.cell())
6470
}
6571
}
6672

67-
#[turbo_tasks::function]
68-
fn condition(root: FileSystemPath) -> Vc<AfterResolvePluginCondition> {
69-
AfterResolvePluginCondition::new_with_glob(
70-
root,
71-
Glob::new(rcstr!("**/node_modules/**"), GlobOptions::default()),
72-
)
73-
}
74-
7573
#[turbo_tasks::value_impl]
7674
impl AfterResolvePlugin for ExternalCjsModulesResolvePlugin {
77-
#[turbo_tasks::function]
7875
fn after_resolve_condition(&self) -> Vc<AfterResolvePluginCondition> {
79-
condition(self.root.clone())
76+
*self.condition
8077
}
8178

8279
#[turbo_tasks::function]
@@ -85,7 +82,7 @@ impl AfterResolvePlugin for ExternalCjsModulesResolvePlugin {
8582
fs_path: FileSystemPath,
8683
lookup_path: FileSystemPath,
8784
reference_type: ReferenceType,
88-
request: ResolvedVc<Request>,
85+
request: Vc<Request>,
8986
) -> Result<Vc<ResolveResultOption>> {
9087
let request_value = &*request.await?;
9188
let Request::Module {
@@ -112,10 +109,7 @@ impl AfterResolvePlugin for ExternalCjsModulesResolvePlugin {
112109
let predicate = self.predicate.await?;
113110
let must_be_external = match &*predicate {
114111
ExternalPredicate::AllExcept(exceptions) => {
115-
if *condition(self.root.clone())
116-
.matches(lookup_path.clone())
117-
.await?
118-
{
112+
if self.condition.await?.matches(&lookup_path) {
119113
return Ok(ResolveResultOption::none());
120114
}
121115

@@ -214,7 +208,7 @@ impl AfterResolvePlugin for ExternalCjsModulesResolvePlugin {
214208
Ok(ResolveResultOption::none())
215209
};
216210

217-
let mut request = *request;
211+
let mut request = request;
218212
let mut request_str = request_str.to_string();
219213

220214
let node_resolve_options = if is_esm {
@@ -230,7 +224,7 @@ impl AfterResolvePlugin for ExternalCjsModulesResolvePlugin {
230224
node_resolve_options,
231225
);
232226
let Some(result_from_original_location) =
233-
*node_resolved_from_original_location.first_source().await?
227+
node_resolved_from_original_location.await?.first_source()
234228
else {
235229
if is_esm
236230
&& !package_subpath.is_empty()
@@ -288,7 +282,7 @@ impl AfterResolvePlugin for ExternalCjsModulesResolvePlugin {
288282
request,
289283
node_resolve_options,
290284
);
291-
let resolves_equal = if let Some(result) = *node_resolved.first_source().await? {
285+
let resolves_equal = if let Some(result) = node_resolved.await?.first_source() {
292286
let ident = result.ident().await?;
293287
let cjs_path = &ident.path;
294288
cjs_path == path

0 commit comments

Comments
 (0)