Skip to content

Commit

Permalink
Turbopack: less turbo tasks for chunk_item.id() (#76110)
Browse files Browse the repository at this point in the history
- Every `EcmascriptChunkItem` already had a `chunking_context()` getter via the `ChunkItem` supertrait.
- Produce less turbo tasks for `chunk_item.id()`
- In the future, we really want to move `chunking_context.chunk_item_id()` to the module graph, (or split it out and instead pass a `Vc<ModuleIdStrategy>`). That is however a refactoring we don't need to do right now.

This showed up in the traces, but overall it doesn't seem to have an effect on perf.
Before:
![Bildschirmfoto 2025-02-17 um 09 34 05](https://github.com/user-attachments/assets/99b4816e-9056-418c-8d69-4d6535b9d296)

After:
![Bildschirmfoto 2025-02-17 um 13 43 04](https://github.com/user-attachments/assets/e4409ef6-831e-464f-900c-ec071b916a88)
  • Loading branch information
mischnic authored Feb 17, 2025
1 parent f0887fa commit 6b0724b
Show file tree
Hide file tree
Showing 43 changed files with 163 additions and 356 deletions.
1 change: 0 additions & 1 deletion crates/next-api/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1365,7 +1365,6 @@ impl AppEndpoint {
client_references_chunks,
rsc_app_entry_chunks: **app_entry_chunks,
rsc_app_entry_chunks_availability: Value::new(*app_entry_chunks_availability),
module_graph: *module_graphs.full,
client_chunking_context,
ssr_chunking_context,
next_config: project.next_config(),
Expand Down
7 changes: 3 additions & 4 deletions crates/next-api/src/dynamic_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ use turbo_tasks::{
};
use turbopack_core::{
chunk::{
availability_info::AvailabilityInfo, ChunkItem, ChunkItemExt, ChunkableModule,
ChunkingContext, ModuleId,
availability_info::AvailabilityInfo, ChunkItem, ChunkableModule, ChunkingContext,
ModuleChunkItemIdExt, ModuleId,
},
module::Module,
module_graph::{ModuleGraph, SingleModuleGraph, SingleModuleGraphModuleNode},
Expand Down Expand Up @@ -83,8 +83,7 @@ pub(crate) async fn collect_next_dynamic_chunks(
let async_chunk_group = async_loader.references().to_resolved().await?;

let module_id = dynamic_entry
.as_chunk_item(module_graph, Vc::upcast(chunking_context))
.id()
.chunk_item_id(Vc::upcast(chunking_context))
.to_resolved()
.await?;

Expand Down
2 changes: 0 additions & 2 deletions crates/next-api/src/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1647,8 +1647,6 @@ async fn whole_app_module_graph_operation(
let base = ModuleGraph::from_single_graph(base_single_module_graph);
let additional_entries = project.get_all_additional_entries(base);

base.chunk_group_info().await?;

let additional_module_graph = SingleModuleGraph::new_with_entries_visited(
additional_entries.await?.into_iter().map(|m| **m).collect(),
base_visited_modules,
Expand Down
5 changes: 0 additions & 5 deletions crates/next-core/src/hmr_entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,11 +172,6 @@ impl ChunkItem for HmrEntryChunkItem {

#[turbo_tasks::value_impl]
impl EcmascriptChunkItem for HmrEntryChunkItem {
#[turbo_tasks::function]
fn chunking_context(&self) -> Vc<Box<dyn ChunkingContext>> {
*self.chunking_context
}

#[turbo_tasks::function]
async fn content(&self) -> Result<Vc<EcmascriptChunkItemContent>> {
let this = self.module.await?;
Expand Down
5 changes: 0 additions & 5 deletions crates/next-core/src/next_app/include_modules_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,6 @@ impl ChunkItem for IncludeModulesChunkItem {

#[turbo_tasks::value_impl]
impl EcmascriptChunkItem for IncludeModulesChunkItem {
#[turbo_tasks::function]
fn chunking_context(&self) -> Vc<Box<dyn ChunkingContext>> {
*self.chunking_context
}

#[turbo_tasks::function]
fn content(&self) -> Vc<EcmascriptChunkItemContent> {
EcmascriptChunkItemContent {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -336,11 +336,6 @@ impl EcmascriptChunkItem for EcmascriptClientReferenceProxyChunkItem {
self.inner_chunk_item
.content_with_async_module_info(async_module_info)
}

#[turbo_tasks::function]
fn chunking_context(&self) -> Vc<Box<dyn ChunkingContext>> {
EcmascriptChunkItem::chunking_context(*self.inner_chunk_item)
}
}

#[turbo_tasks::value]
Expand Down
10 changes: 2 additions & 8 deletions crates/next-core/src/next_dynamic/dynamic_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use turbo_rcstr::RcStr;
use turbo_tasks::{ResolvedVc, Vc};
use turbopack_core::{
asset::{Asset, AssetContent},
chunk::{ChunkItem, ChunkItemExt, ChunkType, ChunkableModule, ChunkingContext},
chunk::{ChunkItem, ChunkType, ChunkableModule, ChunkingContext, ModuleChunkItemIdExt},
ident::AssetIdent,
module::Module,
module_graph::ModuleGraph,
Expand Down Expand Up @@ -133,19 +133,13 @@ struct NextDynamicEntryChunkItem {

#[turbo_tasks::value_impl]
impl EcmascriptChunkItem for NextDynamicEntryChunkItem {
#[turbo_tasks::function]
fn chunking_context(&self) -> Vc<Box<dyn ChunkingContext>> {
*self.chunking_context
}

#[turbo_tasks::function]
async fn content(&self) -> Result<Vc<EcmascriptChunkItemContent>> {
let inner = self.inner.await?;

let module_id = inner
.module
.as_chunk_item(*self.module_graph, Vc::upcast(*self.chunking_context))
.id()
.chunk_item_id(Vc::upcast(*self.chunking_context))
.await?;
Ok(EcmascriptChunkItemContent {
inner_code: formatdoc!(
Expand Down
40 changes: 15 additions & 25 deletions crates/next-core/src/next_manifests/client_reference_manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@ use turbo_tasks_fs::{File, FileSystemPath};
use turbopack_core::{
asset::{Asset, AssetContent},
chunk::{
availability_info::AvailabilityInfo, ChunkItemExt, ChunkableModule, ChunkingContext,
ModuleId as TurbopackModuleId,
availability_info::AvailabilityInfo, ChunkableModule, ChunkingContext,
ModuleChunkItemIdExt, ModuleId as TurbopackModuleId,
},
module_graph::ModuleGraph,
output::{OutputAsset, OutputAssets},
virtual_output::VirtualOutputAsset,
};
Expand All @@ -39,7 +38,6 @@ pub struct ClientReferenceManifestOptions {
pub client_references_chunks: Vc<ClientReferencesChunks>,
pub rsc_app_entry_chunks: Vc<OutputAssets>,
pub rsc_app_entry_chunks_availability: Value<AvailabilityInfo>,
pub module_graph: Vc<ModuleGraph>,
pub client_chunking_context: Vc<Box<dyn ChunkingContext>>,
pub ssr_chunking_context: Option<Vc<Box<dyn ChunkingContext>>>,
pub next_config: Vc<NextConfig>,
Expand All @@ -61,7 +59,6 @@ impl ClientReferenceManifest {
client_references_chunks,
rsc_app_entry_chunks,
rsc_app_entry_chunks_availability,
module_graph,
client_chunking_context,
ssr_chunking_context,
next_config,
Expand Down Expand Up @@ -149,13 +146,10 @@ impl ClientReferenceManifest {

let server_path = client_reference_module_ref.server_ident.to_string().await?;
let client_module = client_reference_module_ref.client_module;
let client_chunk_item = client_module
.as_chunk_item(module_graph, Vc::upcast(client_chunking_context))
.to_resolved()
let client_chunk_item_id = client_module
.chunk_item_id(Vc::upcast(client_chunking_context))
.await?;

let client_module_id = client_chunk_item.id().await?;

let (client_chunks_paths, client_is_async) =
if let Some((client_chunks, client_availability_info)) =
client_component_client_chunks.get(&app_client_reference_ty)
Expand Down Expand Up @@ -193,17 +187,13 @@ impl ClientReferenceManifest {

if let Some(ssr_chunking_context) = ssr_chunking_context {
let ssr_module = client_reference_module_ref.ssr_module;
let ssr_chunk_item = ssr_module
.as_chunk_item(module_graph, Vc::upcast(ssr_chunking_context))
.to_resolved()
let ssr_chunk_item_id = ssr_module
.chunk_item_id(Vc::upcast(ssr_chunking_context))
.await?;
let ssr_module_id = ssr_chunk_item.id().await?;

let rsc_chunk_item = client_reference_module
.as_chunk_item(module_graph, Vc::upcast(ssr_chunking_context))
.to_resolved()
let rsc_chunk_item_id = client_reference_module
.chunk_item_id(Vc::upcast(ssr_chunking_context))
.await?;
let rsc_module_id = rsc_chunk_item.id().await?;

let (ssr_chunks_paths, ssr_is_async) = if runtime == NextRuntime::Edge {
// the chunks get added to the middleware-manifest.json instead
Expand Down Expand Up @@ -277,7 +267,7 @@ impl ClientReferenceManifest {
get_client_reference_module_key(&server_path, "*"),
ManifestNodeEntry {
name: "*".into(),
id: (&*client_module_id).into(),
id: (&*client_chunk_item_id).into(),
chunks: client_chunks_paths,
// This should of course be client_is_async, but SSR can become
// async due to ESM externals, and
Expand All @@ -292,7 +282,7 @@ impl ClientReferenceManifest {
"*".into(),
ManifestNodeEntry {
name: "*".into(),
id: (&*ssr_module_id).into(),
id: (&*ssr_chunk_item_id).into(),
chunks: ssr_chunks_paths,
// See above
r#async: client_is_async || ssr_is_async,
Expand All @@ -304,7 +294,7 @@ impl ClientReferenceManifest {
"*".into(),
ManifestNodeEntry {
name: "*".into(),
id: (&*rsc_module_id).into(),
id: (&*rsc_chunk_item_id).into(),
chunks: rsc_chunks_paths,
r#async: rsc_is_async,
},
Expand All @@ -314,18 +304,18 @@ impl ClientReferenceManifest {
NextRuntime::NodeJs => {
entry_manifest
.ssr_module_mapping
.insert((&*client_module_id).into(), ssr_manifest_node);
.insert((&*client_chunk_item_id).into(), ssr_manifest_node);
entry_manifest
.rsc_module_mapping
.insert((&*client_module_id).into(), rsc_manifest_node);
.insert((&*client_chunk_item_id).into(), rsc_manifest_node);
}
NextRuntime::Edge => {
entry_manifest
.edge_ssr_module_mapping
.insert((&*client_module_id).into(), ssr_manifest_node);
.insert((&*client_chunk_item_id).into(), ssr_manifest_node);
entry_manifest
.edge_rsc_module_mapping
.insert((&*client_module_id).into(), rsc_manifest_node);
.insert((&*client_chunk_item_id).into(), rsc_manifest_node);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use turbo_tasks::{ResolvedVc, Vc};
use turbo_tasks_fs::FileSystemPath;
use turbopack_core::{
asset::{Asset, AssetContent},
chunk::{ChunkItem, ChunkItemExt, ChunkType, ChunkableModule, ChunkingContext},
chunk::{ChunkItem, ChunkType, ChunkableModule, ChunkingContext, ModuleChunkItemIdExt},
ident::AssetIdent,
module::Module,
module_graph::ModuleGraph,
Expand Down Expand Up @@ -128,19 +128,13 @@ struct NextServerComponentChunkItem {

#[turbo_tasks::value_impl]
impl EcmascriptChunkItem for NextServerComponentChunkItem {
#[turbo_tasks::function]
fn chunking_context(&self) -> Vc<Box<dyn ChunkingContext>> {
*self.chunking_context
}

#[turbo_tasks::function]
async fn content(&self) -> Result<Vc<EcmascriptChunkItemContent>> {
let inner = self.inner.await?;

let module_id = inner
.module
.as_chunk_item(*self.module_graph, Vc::upcast(*self.chunking_context))
.id()
.chunk_item_id(Vc::upcast(*self.chunking_context))
.await?;
Ok(EcmascriptChunkItemContent {
inner_code: formatdoc!(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use turbo_tasks::{ResolvedVc, Vc};
use turbo_tasks_fs::FileSystemPath;
use turbopack_core::{
asset::{Asset, AssetContent},
chunk::{ChunkItem, ChunkItemExt, ChunkType, ChunkableModule, ChunkingContext},
chunk::{ChunkItem, ChunkType, ChunkableModule, ChunkingContext, ModuleChunkItemIdExt},
ident::AssetIdent,
module::Module,
module_graph::ModuleGraph,
Expand Down Expand Up @@ -128,19 +128,13 @@ struct NextServerComponentChunkItem {

#[turbo_tasks::value_impl]
impl EcmascriptChunkItem for NextServerComponentChunkItem {
#[turbo_tasks::function]
fn chunking_context(&self) -> Vc<Box<dyn ChunkingContext>> {
*self.chunking_context
}

#[turbo_tasks::function]
async fn content(&self) -> Result<Vc<EcmascriptChunkItemContent>> {
let inner = self.inner.await?;

let module_id = inner
.module
.as_chunk_item(*self.module_graph, Vc::upcast(*self.chunking_context))
.id()
.chunk_item_id(Vc::upcast(*self.chunking_context))
.await?;
Ok(EcmascriptChunkItemContent {
inner_code: formatdoc!(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ use turbo_tasks_fs::{File, FileSystemPath};
use turbopack_core::{
asset::{Asset, AssetContent},
chunk::{
ChunkData, ChunkItemExt, ChunkableModule, ChunkingContext, ChunksData, EvaluatableAssets,
MinifyType, ModuleId,
ChunkData, ChunkingContext, ChunksData, EvaluatableAssets, MinifyType,
ModuleChunkItemIdExt, ModuleId,
},
code_builder::{Code, CodeBuilder},
ident::AssetIdent,
Expand Down Expand Up @@ -104,15 +104,13 @@ impl EcmascriptDevEvaluateChunk {
.iter()
.map({
let chunking_context = this.chunking_context;
let module_graph = this.module_graph;
move |entry| async move {
if let Some(placeable) =
ResolvedVc::try_sidecast::<Box<dyn EcmascriptChunkPlaceable>>(*entry)
{
Ok(Some(
placeable
.as_chunk_item(*module_graph, Vc::upcast(*chunking_context))
.id()
.chunk_item_id(Vc::upcast(*chunking_context))
.await?,
))
} else {
Expand Down
7 changes: 5 additions & 2 deletions turbopack/crates/turbopack-core/src/chunk/chunking_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,11 @@ pub trait ChunkingContext {
ident: Vc<AssetIdent>,
) -> Result<Vc<ModuleId>>;

fn chunk_item_id(self: Vc<Self>, chunk_item: Vc<Box<dyn ChunkItem>>) -> Vc<ModuleId> {
self.chunk_item_id_from_ident(chunk_item.asset_ident())
fn chunk_item_id(self: Vc<Self>, module: Vc<Box<dyn ChunkItem>>) -> Vc<ModuleId> {
self.chunk_item_id_from_ident(module.asset_ident())
}
fn chunk_item_id_from_module(self: Vc<Self>, module: Vc<Box<dyn Module>>) -> Vc<ModuleId> {
self.chunk_item_id_from_ident(module.ident())
}
}

Expand Down
19 changes: 19 additions & 0 deletions turbopack/crates/turbopack-core/src/chunk/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,25 @@ where
}
}

pub trait ModuleChunkItemIdExt {
/// Returns the chunk item id of this module.
fn chunk_item_id(
self: Vc<Self>,
chunking_context: Vc<Box<dyn ChunkingContext>>,
) -> Vc<ModuleId>;
}
impl<T> ModuleChunkItemIdExt for T
where
T: Upcast<Box<dyn Module>>,
{
fn chunk_item_id(
self: Vc<Self>,
chunking_context: Vc<Box<dyn ChunkingContext>>,
) -> Vc<ModuleId> {
chunking_context.chunk_item_id_from_module(Vc::upcast(self))
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
10 changes: 2 additions & 8 deletions turbopack/crates/turbopack-css/src/module_asset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use turbo_tasks::{FxIndexMap, ResolvedVc, Value, ValueToString, Vc};
use turbo_tasks_fs::{rope::Rope, FileSystemPath};
use turbopack_core::{
asset::{Asset, AssetContent},
chunk::{ChunkItem, ChunkItemExt, ChunkType, ChunkableModule, ChunkingContext},
chunk::{ChunkItem, ChunkType, ChunkableModule, ChunkingContext, ModuleChunkItemIdExt},
context::{AssetContext, ProcessResult},
ident::AssetIdent,
issue::{Issue, IssueExt, IssueSeverity, IssueStage, OptionStyledString, StyledString},
Expand Down Expand Up @@ -320,11 +320,6 @@ impl ChunkItem for ModuleChunkItem {

#[turbo_tasks::value_impl]
impl EcmascriptChunkItem for ModuleChunkItem {
#[turbo_tasks::function]
fn chunking_context(&self) -> Vc<Box<dyn ChunkingContext>> {
*self.chunking_context
}

#[turbo_tasks::function]
async fn content(&self) -> Result<Vc<EcmascriptChunkItemContent>> {
let classes = self.module.classes().await?;
Expand Down Expand Up @@ -378,8 +373,7 @@ impl EcmascriptChunkItem for ModuleChunkItem {
ResolvedVc::upcast(css_module);

let module_id = placeable
.as_chunk_item(*self.module_graph, Vc::upcast(*self.chunking_context))
.id()
.chunk_item_id(Vc::upcast(*self.chunking_context))
.await?;
let module_id = StringifyJs(&*module_id);
let original_name = StringifyJs(&original_name);
Expand Down
Loading

0 comments on commit 6b0724b

Please sign in to comment.