From b99a6981d2e0d128f7e004bde4cd1895fc8e6628 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Tue, 10 Dec 2024 18:44:33 +0100 Subject: [PATCH] Migrate global module ids to single-graph --- Cargo.lock | 1 + crates/next-api/Cargo.toml | 1 + .../next-api/src/global_module_id_strategy.rs | 90 ------------------- crates/next-api/src/lib.rs | 1 - crates/next-api/src/module_graph.rs | 51 +++++++++-- crates/next-api/src/project.rs | 26 +++--- crates/next-core/src/next_config.rs | 6 +- .../src/chunk/module_id_strategies.rs | 23 ++--- .../src/global_module_id_strategy.rs | 21 ++--- 9 files changed, 85 insertions(+), 135 deletions(-) delete mode 100644 crates/next-api/src/global_module_id_strategy.rs diff --git a/Cargo.lock b/Cargo.lock index 292ba210ba74e..4beede9df11c8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3967,6 +3967,7 @@ dependencies = [ "turbo-tasks-build", "turbo-tasks-env", "turbo-tasks-fs", + "turbo-tasks-hash", "turbo-tasks-memory", "turbopack", "turbopack-browser", diff --git a/crates/next-api/Cargo.toml b/crates/next-api/Cargo.toml index af30185e2ce2f..9cfadce352138 100644 --- a/crates/next-api/Cargo.toml +++ b/crates/next-api/Cargo.toml @@ -28,6 +28,7 @@ turbo-rcstr = { workspace = true } turbo-tasks = { workspace = true } turbo-tasks-env = { workspace = true } turbo-tasks-fs = { workspace = true } +turbo-tasks-hash = { workspace = true } turbo-tasks-memory = { workspace = true } turbopack = { workspace = true } turbopack-browser = { workspace = true } diff --git a/crates/next-api/src/global_module_id_strategy.rs b/crates/next-api/src/global_module_id_strategy.rs deleted file mode 100644 index 5ec6719d5a771..0000000000000 --- a/crates/next-api/src/global_module_id_strategy.rs +++ /dev/null @@ -1,90 +0,0 @@ -use anyhow::Result; -use turbo_tasks::Vc; -use turbopack_core::chunk::module_id_strategies::{GlobalModuleIdStrategy, ModuleIdStrategy}; -use turbopack_ecmascript::global_module_id_strategy::{ - children_modules_idents, merge_preprocessed_module_ids, PreprocessedChildrenIdents, -}; - -use crate::{ - project::Project, - route::{Endpoint, Route}, -}; - -#[turbo_tasks::value] -pub struct GlobalModuleIdStrategyBuilder; - -// NOTE(LichuAcu) To access all entrypoints, we need to access an instance of `Project`, but -// `Project` is not available in `turbopack-core`, so we need need this -// `GlobalModuleIdStrategyBuilder` in `next-api`. -#[turbo_tasks::value_impl] -impl GlobalModuleIdStrategyBuilder { - #[turbo_tasks::function] - pub async fn build(project: Vc) -> Result>> { - let mut preprocessed_module_ids = Vec::new(); - - preprocessed_module_ids.push(children_modules_idents(project.client_main_modules())); - - let entrypoints = project.entrypoints().await?; - - preprocessed_module_ids.push(preprocess_module_ids(*entrypoints.pages_error_endpoint)); - preprocessed_module_ids.push(preprocess_module_ids(*entrypoints.pages_app_endpoint)); - preprocessed_module_ids.push(preprocess_module_ids(*entrypoints.pages_document_endpoint)); - - if let Some(middleware) = &entrypoints.middleware { - preprocessed_module_ids.push(preprocess_module_ids(middleware.endpoint)); - } - - if let Some(instrumentation) = &entrypoints.instrumentation { - let node_js = instrumentation.node_js; - let edge = instrumentation.edge; - preprocessed_module_ids.push(preprocess_module_ids(node_js)); - preprocessed_module_ids.push(preprocess_module_ids(edge)); - } - - for (_, route) in entrypoints.routes.iter() { - match route { - Route::Page { - html_endpoint, - data_endpoint, - } => { - preprocessed_module_ids.push(preprocess_module_ids(**html_endpoint)); - preprocessed_module_ids.push(preprocess_module_ids(**data_endpoint)); - } - Route::PageApi { endpoint } => { - preprocessed_module_ids.push(preprocess_module_ids(**endpoint)); - } - Route::AppPage(page_routes) => { - for page_route in page_routes { - preprocessed_module_ids - .push(preprocess_module_ids(page_route.html_endpoint)); - preprocessed_module_ids - .push(preprocess_module_ids(page_route.rsc_endpoint)); - } - } - Route::AppRoute { - original_name: _, - endpoint, - } => { - preprocessed_module_ids.push(preprocess_module_ids(**endpoint)); - } - Route::Conflict => { - tracing::info!("WARN: conflict"); - } - } - } - - let module_id_map = merge_preprocessed_module_ids(preprocessed_module_ids).await?; - - Ok(Vc::upcast( - GlobalModuleIdStrategy::new(module_id_map).await?, - )) - } -} - -// NOTE(LichuAcu) We can't move this function to `turbopack-core` because we need access to -// `Endpoint`, which is not available there. -#[turbo_tasks::function] -fn preprocess_module_ids(endpoint: Vc>) -> Vc { - let root_modules = endpoint.root_modules(); - children_modules_idents(root_modules) -} diff --git a/crates/next-api/src/lib.rs b/crates/next-api/src/lib.rs index 26fe340398f73..c1859ddc503d2 100644 --- a/crates/next-api/src/lib.rs +++ b/crates/next-api/src/lib.rs @@ -9,7 +9,6 @@ mod dynamic_imports; mod empty; pub mod entrypoints; mod font; -pub mod global_module_id_strategy; mod instrumentation; mod loadable_manifest; mod middleware; diff --git a/crates/next-api/src/module_graph.rs b/crates/next-api/src/module_graph.rs index df42f85da2d7f..d89cb83b7ea69 100644 --- a/crates/next-api/src/module_graph.rs +++ b/crates/next-api/src/module_graph.rs @@ -29,13 +29,15 @@ use turbo_tasks::{ CollectiblesSource, FxIndexMap, FxIndexSet, ReadRef, ResolvedVc, TryFlatJoinIterExt, TryJoinIterExt, ValueToString, Vc, }; +use turbo_tasks_hash::hash_xxh3_hash64; use turbopack_core::{ - chunk::ChunkingType, + chunk::{module_id_strategies::GlobalModuleIdStrategy, ChunkingType}, context::AssetContext, issue::{Issue, IssueExt}, module::{Module, Modules}, reference::primary_chunkable_referenced_modules, }; +use turbopack_ecmascript::global_module_id_strategy::merge_preprocessed_module_ids; use crate::{ client_references::{map_client_references, ClientReferenceMapType, ClientReferencesSet}, @@ -545,6 +547,11 @@ async fn get_module_graph_for_endpoint( Ok(Vc::cell(graphs)) } +#[turbo_tasks::function] +async fn get_module_graph_for_project(project: ResolvedVc) -> Vc { + SingleModuleGraph::new_with_entries(project.get_all_entries()) +} + #[turbo_tasks::value] pub struct NextDynamicGraph { is_single_page: bool, @@ -991,13 +998,9 @@ async fn get_reduced_graphs_for_endpoint_inner( NextMode::Build => ( false, vec![ - async move { - SingleModuleGraph::new_with_entries(project.get_all_entries()) - .to_resolved() - .await - } - .instrument(tracing::info_span!("module graph for app")) - .await?, + async move { get_module_graph_for_project(project).to_resolved().await } + .instrument(tracing::info_span!("module graph for app")) + .await?, ], ), }; @@ -1066,3 +1069,35 @@ pub async fn get_reduced_graphs_for_endpoint( } Ok(result) } + +/// Like get_reduced_graphs_for_endpoint, but may only be called for builds +/// +/// If you can, use get_reduced_graphs_for_endpoint instead. +#[turbo_tasks::function] +pub async fn get_global_module_id_strategy( + project: Vc, +) -> Result> { + let graph_op = get_module_graph_for_project(project); + // TODO get rid of this function once everything inside of + // `get_reduced_graphs_for_endpoint_inner` calls `take_collectibles()` when needed + let graph = graph_op.strongly_consistent().await?; + let _ = graph_op.take_collectibles::>(); + + let module_id_map: FxIndexMap = graph + .iter_nodes() + .map(|node| async move { + let module_ident = node.module.ident(); + let ident_str = module_ident.to_string().await?.clone_value(); + let hash = hash_xxh3_hash64(&ident_str); + Ok((ident_str, hash)) + }) + .try_join() + .await? + .into_iter() + .collect(); + + // TODO clean up this call signature + let module_id_map = merge_preprocessed_module_ids(vec![Vc::cell(module_id_map)]).await?; + + GlobalModuleIdStrategy::new(module_id_map).await +} diff --git a/crates/next-api/src/project.rs b/crates/next-api/src/project.rs index 80912c7a71d81..1b9b343a8faa8 100644 --- a/crates/next-api/src/project.rs +++ b/crates/next-api/src/project.rs @@ -64,9 +64,9 @@ use crate::{ build, empty::EmptyEndpoint, entrypoints::Entrypoints, - global_module_id_strategy::GlobalModuleIdStrategyBuilder, instrumentation::InstrumentationEndpoint, middleware::MiddlewareEndpoint, + module_graph::get_global_module_id_strategy, pages::PagesProject, route::{AppPageRoute, Endpoint, Route}, versioned_content_map::{OutputAssetsOperation, VersionedContentMap}, @@ -1412,16 +1412,22 @@ impl Project { /// Gets the module id strategy for the project. #[turbo_tasks::function] pub async fn module_id_strategy(self: Vc) -> Result>> { - let module_id_strategy = self.next_config().module_id_strategy_config(); - match *module_id_strategy.await? { - Some(ModuleIdStrategyConfig::Named) => Ok(Vc::upcast(DevModuleIdStrategy::new())), - Some(ModuleIdStrategyConfig::Deterministic) => { - Ok(Vc::upcast(GlobalModuleIdStrategyBuilder::build(self))) + let module_id_strategy = if let Some(module_id_strategy) = + &*self.next_config().module_id_strategy_config().await? + { + *module_id_strategy + } else { + match *self.next_mode().await? { + NextMode::Development => ModuleIdStrategyConfig::Named, + NextMode::Build => ModuleIdStrategyConfig::Deterministic, + } + }; + + match module_id_strategy { + ModuleIdStrategyConfig::Named => Ok(Vc::upcast(DevModuleIdStrategy::new())), + ModuleIdStrategyConfig::Deterministic => { + Ok(Vc::upcast(get_global_module_id_strategy(self))) } - None => match *self.next_mode().await? { - NextMode::Development => Ok(Vc::upcast(DevModuleIdStrategy::new())), - NextMode::Build => Ok(Vc::upcast(DevModuleIdStrategy::new())), - }, } } } diff --git a/crates/next-core/src/next_config.rs b/crates/next-core/src/next_config.rs index 8a51af19eeb96..391ac2507f116 100644 --- a/crates/next-core/src/next_config.rs +++ b/crates/next-core/src/next_config.rs @@ -449,7 +449,7 @@ pub enum LoaderItem { } #[turbo_tasks::value(non_local)] -#[derive(Clone, Debug)] +#[derive(Copy, Clone, Debug)] #[serde(rename_all = "camelCase")] pub enum ModuleIdStrategy { Named, @@ -1308,11 +1308,11 @@ impl NextConfig { .experimental .turbo .as_ref() - .and_then(|t| t.module_id_strategy.as_ref()) + .and_then(|t| t.module_id_strategy) else { return Vc::cell(None); }; - Vc::cell(Some(module_id_strategy.clone())) + Vc::cell(Some(module_id_strategy)) } #[turbo_tasks::function] diff --git a/turbopack/crates/turbopack-core/src/chunk/module_id_strategies.rs b/turbopack/crates/turbopack-core/src/chunk/module_id_strategies.rs index 6bc1b537b1f27..24fad70469a3d 100644 --- a/turbopack/crates/turbopack-core/src/chunk/module_id_strategies.rs +++ b/turbopack/crates/turbopack-core/src/chunk/module_id_strategies.rs @@ -1,7 +1,6 @@ -use anyhow::Result; +use anyhow::{bail, Result}; use turbo_rcstr::RcStr; use turbo_tasks::{FxIndexMap, ResolvedVc, ValueToString, Vc}; -use turbo_tasks_hash::hash_xxh3_hash64; use super::ModuleId; use crate::ident::AssetIdent; @@ -47,15 +46,19 @@ impl GlobalModuleIdStrategy { impl ModuleIdStrategy for GlobalModuleIdStrategy { #[turbo_tasks::function] async fn get_module_id(&self, ident: Vc) -> Result> { - let ident_string = ident.to_string().await?.clone_value(); - if let Some(module_id) = self.module_id_map.get(&ident_string) { + let ident_string = ident.to_string().await?; + if let Some(module_id) = self.module_id_map.get(&*ident_string) { return Ok(module_id.clone().cell()); } - Ok(ModuleId::String( - hash_xxh3_hash64(ident.to_string().await?) - .to_string() - .into(), - ) - .cell()) + // TODO this shouldn't happen + // It means we missed something when generating the map + bail!("ModuleId not found for ident: {:?}", ident_string) + + // Ok(ModuleId::String( + // hash_xxh3_hash64(ident.to_string().await?) + // .to_string() + // .into(), + // ) + // .cell()) } } diff --git a/turbopack/crates/turbopack-ecmascript/src/global_module_id_strategy.rs b/turbopack/crates/turbopack-ecmascript/src/global_module_id_strategy.rs index 1b80379853b38..be189347031c2 100644 --- a/turbopack/crates/turbopack-ecmascript/src/global_module_id_strategy.rs +++ b/turbopack/crates/turbopack-ecmascript/src/global_module_id_strategy.rs @@ -13,13 +13,11 @@ use turbopack_core::{ use crate::references::esm::EsmAsyncAssetReference; -#[turbo_tasks::value] -pub struct PreprocessedChildrenIdents { - // ident.to_string() -> full hash - // We save the full hash to avoid re-hashing in `merge_preprocessed_module_ids` - // if this endpoint did not change. - modules_idents: FxIndexMap, -} +// ident.to_string() -> full hash +// We save the full hash to avoid re-hashing in `merge_preprocessed_module_ids` +// if this endpoint did not change. +#[turbo_tasks::value(transparent)] +pub struct PreprocessedChildrenIdents(FxIndexMap); #[derive(Clone, Hash)] #[turbo_tasks::value(shared)] @@ -104,10 +102,7 @@ pub async fn get_children_modules( parent: ResolvedVc, ) -> Result> + Send> { let parent_module = parent.await?.module(); - let mut modules = referenced_modules(parent_module).await?.clone_value(); - for module in parent_module.additional_layers_modules().await? { - modules.push(ReferencedModule::Module(*module).resolved_cell()); - } + let modules = referenced_modules(parent_module).await?.clone_value(); Ok(modules.into_iter()) } @@ -162,7 +157,7 @@ pub async fn children_modules_idents( } } - Ok(PreprocessedChildrenIdents { modules_idents }.cell()) + Ok(Vc::cell(modules_idents)) } const JS_MAX_SAFE_INTEGER: u64 = (1u64 << 53) - 1; @@ -175,7 +170,7 @@ pub async fn merge_preprocessed_module_ids( let mut merged_module_ids = FxIndexMap::default(); for preprocessed_module_ids in preprocessed_module_ids { - for (module_ident, full_hash) in &preprocessed_module_ids.await?.modules_idents { + for (module_ident, full_hash) in &preprocessed_module_ids.await? { merged_module_ids.insert(module_ident.clone(), *full_hash); } }