From f24ec3709e83270655d52a0583368c437cfe956a Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Mon, 2 Oct 2023 09:21:59 +0000 Subject: [PATCH 1/6] run code generation only once and not per entrypoint --- crates/turbopack-ecmascript/src/lib.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/turbopack-ecmascript/src/lib.rs b/crates/turbopack-ecmascript/src/lib.rs index a1691a6b3dd7f..f4ce1abf56ed0 100644 --- a/crates/turbopack-ecmascript/src/lib.rs +++ b/crates/turbopack-ecmascript/src/lib.rs @@ -507,6 +507,11 @@ impl EcmascriptChunkItem for ModuleChunkItem { availability_info: Value, ) -> Result> { let this = self.await?; + let availability_info = if *this.module.analyze().needs_availability_info().await? { + availability_info + } else { + Value::new(AvailabilityInfo::Untracked) + }; let content = this .module .module_content(this.chunking_context, availability_info); From 538290b3bc7dc9728b5107821e15fa1670fb67c8 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Mon, 2 Oct 2023 15:04:58 +0000 Subject: [PATCH 2/6] improve caching when avaiability info is needed --- .../src/chunk/availability_info.rs | 81 ++++++++++++++++++- .../src/chunk/esm_scope.rs | 8 +- crates/turbopack-ecmascript/src/chunk/item.rs | 11 ++- crates/turbopack-ecmascript/src/code_gen.rs | 9 ++- crates/turbopack-ecmascript/src/lib.rs | 25 +++--- .../src/references/async_module.rs | 66 ++++++++++----- .../src/references/mod.rs | 31 +++++-- .../src/tree_shake/chunk_item.rs | 26 +++--- 8 files changed, 191 insertions(+), 66 deletions(-) diff --git a/crates/turbopack-core/src/chunk/availability_info.rs b/crates/turbopack-core/src/chunk/availability_info.rs index a1cb3c01f100e..a62493d7af65a 100644 --- a/crates/turbopack-core/src/chunk/availability_info.rs +++ b/crates/turbopack-core/src/chunk/availability_info.rs @@ -1,8 +1,42 @@ +use std::ops::{Add, AddAssign}; + use turbo_tasks::Vc; use super::available_modules::AvailableAssets; use crate::module::Module; +#[turbo_tasks::value(shared)] +#[derive(Copy, Clone, Debug)] +pub enum AvailabilityInfoNeeds { + None, + Root, + AvailableModules, + Complete, +} + +impl Add for AvailabilityInfoNeeds { + type Output = AvailabilityInfoNeeds; + + fn add(self, rhs: Self) -> Self::Output { + match (self, rhs) { + (Self::None, rhs) => rhs, + (lhs, Self::None) => lhs, + (Self::Complete, _) | (_, Self::Complete) => Self::Complete, + (Self::Root, Self::Root) => Self::Root, + (Self::AvailableModules, Self::AvailableModules) => Self::AvailableModules, + (Self::Root, Self::AvailableModules) | (Self::AvailableModules, Self::Root) => { + Self::Complete + } + } + } +} + +impl AddAssign for AvailabilityInfoNeeds { + fn add_assign(&mut self, rhs: Self) { + *self = *self + rhs; + } +} + #[turbo_tasks::value(serialization = "auto_for_input")] #[derive(PartialOrd, Ord, Hash, Clone, Copy, Debug)] pub enum AvailabilityInfo { @@ -10,10 +44,13 @@ pub enum AvailabilityInfo { Root { current_availability_root: Vc>, }, - Inner { + Complete { available_modules: Vc, current_availability_root: Vc>, }, + OnlyAvailableModules { + available_modules: Vc, + }, } impl AvailabilityInfo { @@ -23,10 +60,11 @@ impl AvailabilityInfo { Self::Root { current_availability_root, } => Some(*current_availability_root), - Self::Inner { + Self::Complete { current_availability_root, .. } => Some(*current_availability_root), + Self::OnlyAvailableModules { .. } => None, } } @@ -34,9 +72,46 @@ impl AvailabilityInfo { match self { Self::Untracked => None, Self::Root { .. } => None, - Self::Inner { + Self::Complete { available_modules, .. } => Some(*available_modules), + Self::OnlyAvailableModules { + available_modules, .. + } => Some(*available_modules), + } + } + + pub fn reduce_to_needs(self, needs: AvailabilityInfoNeeds) -> Self { + match needs { + AvailabilityInfoNeeds::None => Self::Untracked, + AvailabilityInfoNeeds::Root => match self { + Self::Untracked => Self::Untracked, + Self::OnlyAvailableModules { .. } => Self::Untracked, + Self::Root { + current_availability_root, + } => Self::Root { + current_availability_root, + }, + Self::Complete { + current_availability_root, + .. + } => Self::Root { + current_availability_root, + }, + }, + AvailabilityInfoNeeds::AvailableModules => match self { + Self::Untracked => Self::Untracked, + Self::Root { .. } => Self::Untracked, + Self::Complete { + available_modules, + current_availability_root, + } => Self::Complete { + available_modules, + current_availability_root, + }, + Self::OnlyAvailableModules { .. } => Self::Untracked, + }, + AvailabilityInfoNeeds::Complete => self, } } } diff --git a/crates/turbopack-ecmascript/src/chunk/esm_scope.rs b/crates/turbopack-ecmascript/src/chunk/esm_scope.rs index e5e3d36d5bd38..c79edd75ef5b7 100644 --- a/crates/turbopack-ecmascript/src/chunk/esm_scope.rs +++ b/crates/turbopack-ecmascript/src/chunk/esm_scope.rs @@ -2,9 +2,9 @@ use std::collections::HashMap; use anyhow::{Context, Result}; use petgraph::{algo::tarjan_scc, prelude::DiGraphMap}; -use turbo_tasks::{TryFlatJoinIterExt, Value, Vc}; +use turbo_tasks::{TryFlatJoinIterExt, Vc}; use turbopack_core::{ - chunk::{availability_info::AvailabilityInfo, available_modules::chunkable_modules_set}, + chunk::available_modules::chunkable_modules_set, module::{Module, ModulesSet}, }; @@ -38,8 +38,8 @@ pub(crate) struct EsmScopeSccs(Vec>); impl EsmScope { /// Create a new [EsmScope] from the availability root given. #[turbo_tasks::function] - pub(crate) async fn new(availability_info: Value) -> Result> { - let assets = if let Some(root) = availability_info.current_availability_root() { + pub(crate) async fn new(availability_root: Option>>) -> Result> { + let assets = if let Some(root) = availability_root { chunkable_modules_set(root) } else { ModulesSet::empty() diff --git a/crates/turbopack-ecmascript/src/chunk/item.rs b/crates/turbopack-ecmascript/src/chunk/item.rs index 7d4210af21b2d..48bea8b887aaa 100644 --- a/crates/turbopack-ecmascript/src/chunk/item.rs +++ b/crates/turbopack-ecmascript/src/chunk/item.rs @@ -262,7 +262,7 @@ impl FromChunkableModule for Box { return Ok(None); }; - Ok(Some(placeable.as_chunk_item(context))) + Ok(Some(placeable.as_chunk_item(context).resolve().await?)) } async fn from_async_asset( @@ -281,17 +281,20 @@ impl FromChunkableModule for Box { AvailabilityInfo::Untracked => AvailabilityInfo::Untracked, AvailabilityInfo::Root { current_availability_root, - } => AvailabilityInfo::Inner { + } => AvailabilityInfo::Complete { available_modules: AvailableAssets::new(vec![current_availability_root]), current_availability_root: Vc::upcast(module), }, - AvailabilityInfo::Inner { + AvailabilityInfo::Complete { available_modules, current_availability_root, - } => AvailabilityInfo::Inner { + } => AvailabilityInfo::Complete { available_modules: available_modules.with_roots(vec![current_availability_root]), current_availability_root: Vc::upcast(module), }, + AvailabilityInfo::OnlyAvailableModules { .. } => { + panic!("OnlyAvailableModules should not appear here") + } }; let manifest_asset = diff --git a/crates/turbopack-ecmascript/src/code_gen.rs b/crates/turbopack-ecmascript/src/code_gen.rs index d2d812354c6d7..6c7fe03529a5a 100644 --- a/crates/turbopack-ecmascript/src/code_gen.rs +++ b/crates/turbopack-ecmascript/src/code_gen.rs @@ -1,7 +1,7 @@ use serde::{Deserialize, Serialize}; use swc_core::ecma::visit::{AstParentKind, VisitMut}; use turbo_tasks::{debug::ValueDebugFormat, trace::TraceRawVcs, Value, Vc}; -use turbopack_core::chunk::availability_info::AvailabilityInfo; +use turbopack_core::chunk::availability_info::{AvailabilityInfo, AvailabilityInfoNeeds}; use crate::chunk::EcmascriptChunkingContext; @@ -39,6 +39,13 @@ pub trait CodeGenerateableWithAvailabilityInfo { chunking_context: Vc>, availability_info: Value, ) -> Vc; + + fn get_availability_info_needs( + self: Vc, + _is_async_module: bool, + ) -> Vc { + AvailabilityInfoNeeds::Complete.cell() + } } #[derive(Clone, Copy, PartialEq, Eq, Serialize, Deserialize, TraceRawVcs, ValueDebugFormat)] diff --git a/crates/turbopack-ecmascript/src/lib.rs b/crates/turbopack-ecmascript/src/lib.rs index f4ce1abf56ed0..d5db061a8bda2 100644 --- a/crates/turbopack-ecmascript/src/lib.rs +++ b/crates/turbopack-ecmascript/src/lib.rs @@ -353,11 +353,6 @@ impl EcmascriptModuleAsset { availability_info: Value, ) -> Result> { let this = self.await?; - let availability_info = if *self.analyze().needs_availability_info().await? { - availability_info - } else { - Value::new(AvailabilityInfo::Untracked) - }; let parsed = parse(this.source, Value::new(this.ty), this.transforms); @@ -507,18 +502,20 @@ impl EcmascriptChunkItem for ModuleChunkItem { availability_info: Value, ) -> Result> { let this = self.await?; - let availability_info = if *this.module.analyze().needs_availability_info().await? { - availability_info - } else { - Value::new(AvailabilityInfo::Untracked) - }; - let content = this - .module - .module_content(this.chunking_context, availability_info); let async_module_options = this .module .get_async_module() - .module_options(availability_info); + .module_options(availability_info.current_availability_root()); + let is_async_module = async_module_options.await?.is_some(); + let availability_info_needs = *this + .module + .analyze() + .get_availability_info_needs(is_async_module) + .await?; + let availability_info = availability_info.reduce_to_needs(availability_info_needs); + let content = this + .module + .module_content(this.chunking_context, Value::new(availability_info)); Ok(EcmascriptChunkItemContent::new( content, diff --git a/crates/turbopack-ecmascript/src/references/async_module.rs b/crates/turbopack-ecmascript/src/references/async_module.rs index cda5cb48d14ed..32457d21a8abc 100644 --- a/crates/turbopack-ecmascript/src/references/async_module.rs +++ b/crates/turbopack-ecmascript/src/references/async_module.rs @@ -7,7 +7,10 @@ use swc_core::{ quote, }; use turbo_tasks::{trace::TraceRawVcs, TryFlatJoinIterExt, Value, Vc}; -use turbopack_core::chunk::availability_info::AvailabilityInfo; +use turbopack_core::{ + chunk::availability_info::{AvailabilityInfo, AvailabilityInfoNeeds}, + module::Module, +}; use super::esm::base::ReferencedAsset; use crate::{ @@ -67,20 +70,20 @@ impl OptionAsyncModule { #[turbo_tasks::function] pub async fn is_async( self: Vc, - availability_info: Value, + availability_root: Option>>, ) -> Result> { Ok(Vc::cell( - self.module_options(availability_info).await?.is_some(), + self.module_options(availability_root).await?.is_some(), )) } #[turbo_tasks::function] pub async fn module_options( self: Vc, - availability_info: Value, + availability_root: Option>>, ) -> Result> { if let Some(async_module) = &*self.await? { - return Ok(async_module.module_options(availability_info)); + return Ok(async_module.module_options(availability_root)); } Ok(OptionAsyncModuleOptions::none()) @@ -124,7 +127,12 @@ impl AsyncModuleScc { for scc in &*this.scope.get_scc_children(this.scc).await? { // Because we generated SCCs there can be no loops in the children, so calling // recursively is fine. - if *AsyncModuleScc::new(*scc, this.scope).is_async().await? { + if *AsyncModuleScc::new(*scc, this.scope) + .resolve() + .await? + .is_async() + .await? + { return Ok(Vc::cell(true)); } } @@ -141,7 +149,7 @@ impl AsyncModule { #[turbo_tasks::function] async fn get_async_idents( self: Vc, - availability_info: Value, + availability_root: Option>>, ) -> Result> { let this = self.await?; @@ -158,7 +166,7 @@ impl AsyncModule { ReferencedAsset::Some(placeable) => { if *placeable .get_async_module() - .is_async(availability_info) + .is_async(availability_root) .await? { referenced_asset.get_ident().await? @@ -183,11 +191,11 @@ impl AsyncModule { #[turbo_tasks::function] async fn get_scc( self: Vc, - availability_info: Value, + availability_root: Option>>, ) -> Result> { let this = self.await?; - let scope = EsmScope::new(availability_info); + let scope = EsmScope::new(availability_root); let Some(scc) = &*scope.get_scc(this.placeable).await? else { // I'm not sure if this should be possible. return Ok(Vc::cell(None)); @@ -195,7 +203,7 @@ impl AsyncModule { let scc = AsyncModuleScc::new(*scc, scope); - Ok(Vc::cell(Some(scc))) + Ok(Vc::cell(Some(scc.resolve().await?))) } /// Check if the current module or any of it's ESM children contain a top @@ -203,10 +211,10 @@ impl AsyncModule { #[turbo_tasks::function] pub async fn is_async( self: Vc, - availability_info: Value, + availability_root: Option>>, ) -> Result> { Ok( - if let Some(scc) = &*self.get_scc(availability_info).await? { + if let Some(scc) = &*self.get_scc(availability_root).await? { scc.is_async() } else { self.is_self_async() @@ -218,9 +226,9 @@ impl AsyncModule { #[turbo_tasks::function] pub async fn module_options( self: Vc, - availability_info: Value, + availability_root: Option>>, ) -> Result> { - if !*self.is_async(availability_info).await? { + if !*self.is_async(availability_root).await? { return Ok(Vc::cell(None)); } @@ -240,16 +248,34 @@ impl CodeGenerateableWithAvailabilityInfo for AsyncModule { ) -> Result> { let mut visitors = Vec::new(); - let async_idents = self.get_async_idents(availability_info).await?; + let availability_info = availability_info.into_value(); - if !async_idents.is_empty() { - visitors.push(create_visitor!(visit_mut_program(program: &mut Program) { - add_async_dependency_handler(program, &async_idents); - })); + if !matches!(availability_info, AvailabilityInfo::Untracked) { + let async_idents = self + .get_async_idents(availability_info.current_availability_root()) + .await?; + + if !async_idents.is_empty() { + visitors.push(create_visitor!(visit_mut_program(program: &mut Program) { + add_async_dependency_handler(program, &async_idents); + })); + } } Ok(CodeGeneration { visitors }.into()) } + + #[turbo_tasks::function] + async fn get_availability_info_needs( + self: Vc, + is_async_module: bool, + ) -> Result> { + Ok(if is_async_module { + AvailabilityInfoNeeds::Root.cell() + } else { + AvailabilityInfoNeeds::None.cell() + }) + } } fn add_async_dependency_handler(program: &mut Program, idents: &IndexSet) { diff --git a/crates/turbopack-ecmascript/src/references/mod.rs b/crates/turbopack-ecmascript/src/references/mod.rs index 6300e0a58bdda..703c87ad522ca 100644 --- a/crates/turbopack-ecmascript/src/references/mod.rs +++ b/crates/turbopack-ecmascript/src/references/mod.rs @@ -48,6 +48,7 @@ use swc_core::{ use turbo_tasks::{TryJoinIterExt, Upcast, Value, Vc}; use turbo_tasks_fs::{FileJsonContent, FileSystemPath}; use turbopack_core::{ + chunk::availability_info::AvailabilityInfoNeeds, compile_time_info::{CompileTimeInfo, FreeVarReference}, error::PrettyPrintError, issue::{analyze::AnalyzeIssue, IssueExt, IssueSeverity, IssueSource, OptionIssueSource}, @@ -140,26 +141,40 @@ pub struct AnalyzeEcmascriptModuleResult { #[turbo_tasks::value_impl] impl AnalyzeEcmascriptModuleResult { #[turbo_tasks::function] - pub async fn needs_availability_info(self: Vc) -> Result> { + pub async fn get_availability_info_needs( + self: Vc, + is_async_module: bool, + ) -> Result> { let AnalyzeEcmascriptModuleResult { references, code_generation, .. } = &*self.await?; + let mut needs = AvailabilityInfoNeeds::None; for c in code_generation.await?.iter() { - if matches!(c, CodeGen::CodeGenerateableWithAvailabilityInfo(..)) { - return Ok(Vc::cell(true)); + if let CodeGen::CodeGenerateableWithAvailabilityInfo(code_gen) = c { + needs += *code_gen + .get_availability_info_needs(is_async_module) + .await?; + if matches!(needs, AvailabilityInfoNeeds::Complete) { + return Ok(needs.cell()); + } } } for r in references.await?.iter() { - if Vc::try_resolve_sidecast::>(*r) - .await? - .is_some() + if let Some(code_gen) = + Vc::try_resolve_sidecast::>(*r) + .await? { - return Ok(Vc::cell(true)); + needs += *code_gen + .get_availability_info_needs(is_async_module) + .await?; + if matches!(needs, AvailabilityInfoNeeds::Complete) { + return Ok(needs.cell()); + } } } - Ok(Vc::cell(false)) + return Ok(needs.cell()); } } diff --git a/crates/turbopack-ecmascript/src/tree_shake/chunk_item.rs b/crates/turbopack-ecmascript/src/tree_shake/chunk_item.rs index 4e9939a1b866b..c06d94326e2df 100644 --- a/crates/turbopack-ecmascript/src/tree_shake/chunk_item.rs +++ b/crates/turbopack-ecmascript/src/tree_shake/chunk_item.rs @@ -39,13 +39,20 @@ impl EcmascriptChunkItem for EcmascriptModulePartChunkItem { availability_info: Value, ) -> Result> { let this = self.await?; - let availability_info = if *this.module.analyze().needs_availability_info().await? { - availability_info - } else { - Value::new(AvailabilityInfo::Untracked) - }; - let module = this.module.await?; + let async_module_options = module + .full_module + .get_async_module() + .module_options(availability_info.current_availability_root()); + let is_async_module = async_module_options.await?.is_some(); + + let availability_info_needs = *this + .module + .analyze() + .get_availability_info_needs(is_async_module) + .await?; + let availability_info = availability_info.reduce_to_needs(availability_info_needs); + let split_data = split_module(module.full_module); let parsed = part_of_module(split_data, module.part); @@ -54,14 +61,9 @@ impl EcmascriptChunkItem for EcmascriptModulePartChunkItem { module.full_module.ident(), this.chunking_context, this.module.analyze(), - availability_info, + Value::new(availability_info), ); - let async_module_options = module - .full_module - .get_async_module() - .module_options(availability_info); - Ok(EcmascriptChunkItemContent::new( content, this.chunking_context, From 8af04ebc4e964f2a615edcfe05391aa01ffc80a9 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Mon, 2 Oct 2023 15:05:16 +0000 Subject: [PATCH 3/6] avoid resolve tasks for None as argument --- crates/turbo-tasks/src/manager.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/turbo-tasks/src/manager.rs b/crates/turbo-tasks/src/manager.rs index ff59cf309fb23..d3a293505270d 100644 --- a/crates/turbo-tasks/src/manager.rs +++ b/crates/turbo-tasks/src/manager.rs @@ -410,7 +410,7 @@ impl TurboTasks { /// Calls a native function with arguments. Resolves arguments when needed /// with a wrapper [Task]. pub fn dynamic_call(&self, func: FunctionId, inputs: Vec) -> RawVc { - if inputs.iter().all(|i| i.is_resolved() && !i.is_nothing()) { + if inputs.iter().all(|i| i.is_resolved()) { self.native_call(func, inputs) } else { RawVc::TaskOutput(self.backend.get_or_create_persistent_task( From fb237029efd3de5d47351097b9a6230a1c83763b Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Wed, 4 Oct 2023 06:45:42 +0000 Subject: [PATCH 4/6] clippy --- crates/turbopack-ecmascript/src/references/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/turbopack-ecmascript/src/references/mod.rs b/crates/turbopack-ecmascript/src/references/mod.rs index 703c87ad522ca..e156d0389a5d5 100644 --- a/crates/turbopack-ecmascript/src/references/mod.rs +++ b/crates/turbopack-ecmascript/src/references/mod.rs @@ -174,7 +174,7 @@ impl AnalyzeEcmascriptModuleResult { } } } - return Ok(needs.cell()); + Ok(needs.cell()) } } From ca4bce870327e8b4c4a72339483194d6174bd5e3 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Thu, 5 Oct 2023 18:53:04 +0000 Subject: [PATCH 5/6] Docs, changed to boolean flags --- .../src/chunk/availability_info.rs | 100 ++++++++++++------ crates/turbopack-ecmascript/src/code_gen.rs | 16 +-- crates/turbopack-ecmascript/src/lib.rs | 4 + .../src/references/async_module.rs | 2 + .../src/references/mod.rs | 2 + 5 files changed, 84 insertions(+), 40 deletions(-) diff --git a/crates/turbopack-core/src/chunk/availability_info.rs b/crates/turbopack-core/src/chunk/availability_info.rs index a62493d7af65a..316f760c8b238 100644 --- a/crates/turbopack-core/src/chunk/availability_info.rs +++ b/crates/turbopack-core/src/chunk/availability_info.rs @@ -1,20 +1,35 @@ -use std::ops::{Add, AddAssign}; +use std::ops::{Add, AddAssign, BitOr, BitOrAssign}; use turbo_tasks::Vc; use super::available_modules::AvailableAssets; use crate::module::Module; +/// This specifies which information is needed from an AvailabilityInfo #[turbo_tasks::value(shared)] #[derive(Copy, Clone, Debug)] -pub enum AvailabilityInfoNeeds { - None, - Root, - AvailableModules, - Complete, +pub struct AvailabilityInfoNeeds { + pub current_availability_root: bool, + pub available_modules: bool, } -impl Add for AvailabilityInfoNeeds { +impl AvailabilityInfoNeeds { + pub fn all() -> Self { + Self { + current_availability_root: true, + available_modules: true, + } + } + + pub fn none() -> Self { + Self { + current_availability_root: false, + available_modules: false, + } + } +} + +impl BitOr for AvailabilityInfoNeeds { type Output = AvailabilityInfoNeeds; fn add(self, rhs: Self) -> Self::Output { @@ -29,25 +44,43 @@ impl Add for AvailabilityInfoNeeds { } } } + + fn bitor(mut self, rhs: Self) -> Self::Output { + let Self { + available_modules, + current_availability_root, + } = rhs; + self.current_availability_root |= current_availability_root; + self.available_modules |= available_modules; + self + } } -impl AddAssign for AvailabilityInfoNeeds { - fn add_assign(&mut self, rhs: Self) { - *self = *self + rhs; +impl BitOrAssign for AvailabilityInfoNeeds { + fn bitor_assign(&mut self, rhs: Self) { + *self = *self | rhs; } } #[turbo_tasks::value(serialization = "auto_for_input")] #[derive(PartialOrd, Ord, Hash, Clone, Copy, Debug)] pub enum AvailabilityInfo { + /// Available modules are not tracked or the information is not available + /// due to specified needs. Untracked, + /// There are no modules available (or the information is not available due + /// to specified needs), but it's tracked for nested chunk groups + /// and the current chunk group root is defined. Root { current_availability_root: Vc>, }, + /// There are modules already available and the current chunk group root is + /// defined. Complete { available_modules: Vc, current_availability_root: Vc>, }, + /// Only partial information is available, about the available modules. OnlyAvailableModules { available_modules: Vc, }, @@ -81,37 +114,38 @@ impl AvailabilityInfo { } } + /// Returns AvailabilityInfo, but only with the information that is needed pub fn reduce_to_needs(self, needs: AvailabilityInfoNeeds) -> Self { - match needs { - AvailabilityInfoNeeds::None => Self::Untracked, - AvailabilityInfoNeeds::Root => match self { - Self::Untracked => Self::Untracked, - Self::OnlyAvailableModules { .. } => Self::Untracked, + let AvailabilityInfoNeeds { + available_modules, + current_availability_root, + } = needs; + match (current_availability_root, available_modules, self) { + (false, false, _) => Self::Untracked, + (true, true, _) => self, + ( + true, + false, Self::Root { current_availability_root, - } => Self::Root { - current_availability_root, - }, - Self::Complete { + } + | Self::Complete { current_availability_root, .. - } => Self::Root { - current_availability_root, }, + ) => Self::Root { + current_availability_root, }, - AvailabilityInfoNeeds::AvailableModules => match self { - Self::Untracked => Self::Untracked, - Self::Root { .. } => Self::Untracked, + (true, false, _) => Self::Untracked, + ( + false, + true, Self::Complete { - available_modules, - current_availability_root, - } => Self::Complete { - available_modules, - current_availability_root, - }, - Self::OnlyAvailableModules { .. } => Self::Untracked, - }, - AvailabilityInfoNeeds::Complete => self, + available_modules, .. + } + | Self::OnlyAvailableModules { available_modules }, + ) => Self::OnlyAvailableModules { available_modules }, + (false, true, _) => Self::Untracked, } } } diff --git a/crates/turbopack-ecmascript/src/code_gen.rs b/crates/turbopack-ecmascript/src/code_gen.rs index 6c7fe03529a5a..edfe02eb1a7aa 100644 --- a/crates/turbopack-ecmascript/src/code_gen.rs +++ b/crates/turbopack-ecmascript/src/code_gen.rs @@ -34,18 +34,20 @@ pub trait CodeGenerateable { #[turbo_tasks::value_trait] pub trait CodeGenerateableWithAvailabilityInfo { - fn code_generation( - self: Vc, - chunking_context: Vc>, - availability_info: Value, - ) -> Vc; - + /// Returns the pieces of AvailabilityInfo that are needed for + /// `code_generation`. fn get_availability_info_needs( self: Vc, _is_async_module: bool, ) -> Vc { - AvailabilityInfoNeeds::Complete.cell() + AvailabilityInfoNeeds::all().cell() } + + fn code_generation( + self: Vc, + chunking_context: Vc>, + availability_info: Value, + ) -> Vc; } #[derive(Clone, Copy, PartialEq, Eq, Serialize, Deserialize, TraceRawVcs, ValueDebugFormat)] diff --git a/crates/turbopack-ecmascript/src/lib.rs b/crates/turbopack-ecmascript/src/lib.rs index d5db061a8bda2..745851175b802 100644 --- a/crates/turbopack-ecmascript/src/lib.rs +++ b/crates/turbopack-ecmascript/src/lib.rs @@ -512,6 +512,10 @@ impl EcmascriptChunkItem for ModuleChunkItem { .analyze() .get_availability_info_needs(is_async_module) .await?; + // We reduce the availability info to the needs of the chunk item to improve + // caching of the methods that are called with availability info. e. g. + // module_content() can be cached for different availability info when it + // doesn't really need that info. let availability_info = availability_info.reduce_to_needs(availability_info_needs); let content = this .module diff --git a/crates/turbopack-ecmascript/src/references/async_module.rs b/crates/turbopack-ecmascript/src/references/async_module.rs index 32457d21a8abc..dfd1dc2d21444 100644 --- a/crates/turbopack-ecmascript/src/references/async_module.rs +++ b/crates/turbopack-ecmascript/src/references/async_module.rs @@ -127,6 +127,8 @@ impl AsyncModuleScc { for scc in &*this.scope.get_scc_children(this.scc).await? { // Because we generated SCCs there can be no loops in the children, so calling // recursively is fine. + // AsyncModuleScc::new is resolved here to avoid unnecessary resolve tasks for + // is_async in this hot code path. if *AsyncModuleScc::new(*scc, this.scope) .resolve() .await? diff --git a/crates/turbopack-ecmascript/src/references/mod.rs b/crates/turbopack-ecmascript/src/references/mod.rs index e156d0389a5d5..93b3b45959537 100644 --- a/crates/turbopack-ecmascript/src/references/mod.rs +++ b/crates/turbopack-ecmascript/src/references/mod.rs @@ -140,6 +140,8 @@ pub struct AnalyzeEcmascriptModuleResult { #[turbo_tasks::value_impl] impl AnalyzeEcmascriptModuleResult { + /// Returns the pieces of AvailabilityInfo that are required for code + /// generation. #[turbo_tasks::function] pub async fn get_availability_info_needs( self: Vc, From e62a87d7d3cac6c8de1a4a2da9bee47a9a22da21 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Fri, 6 Oct 2023 06:00:01 +0000 Subject: [PATCH 6/6] fixup --- .../src/chunk/availability_info.rs | 23 ++++++++----------- .../src/references/async_module.rs | 10 ++++---- .../src/references/mod.rs | 10 ++++---- 3 files changed, 19 insertions(+), 24 deletions(-) diff --git a/crates/turbopack-core/src/chunk/availability_info.rs b/crates/turbopack-core/src/chunk/availability_info.rs index 316f760c8b238..ad5eb650f5caa 100644 --- a/crates/turbopack-core/src/chunk/availability_info.rs +++ b/crates/turbopack-core/src/chunk/availability_info.rs @@ -1,4 +1,4 @@ -use std::ops::{Add, AddAssign, BitOr, BitOrAssign}; +use std::ops::{BitOr, BitOrAssign}; use turbo_tasks::Vc; @@ -27,24 +27,19 @@ impl AvailabilityInfoNeeds { available_modules: false, } } + + pub fn is_complete(self) -> bool { + let Self { + current_availability_root, + available_modules, + } = self; + current_availability_root && available_modules + } } impl BitOr for AvailabilityInfoNeeds { type Output = AvailabilityInfoNeeds; - fn add(self, rhs: Self) -> Self::Output { - match (self, rhs) { - (Self::None, rhs) => rhs, - (lhs, Self::None) => lhs, - (Self::Complete, _) | (_, Self::Complete) => Self::Complete, - (Self::Root, Self::Root) => Self::Root, - (Self::AvailableModules, Self::AvailableModules) => Self::AvailableModules, - (Self::Root, Self::AvailableModules) | (Self::AvailableModules, Self::Root) => { - Self::Complete - } - } - } - fn bitor(mut self, rhs: Self) -> Self::Output { let Self { available_modules, diff --git a/crates/turbopack-ecmascript/src/references/async_module.rs b/crates/turbopack-ecmascript/src/references/async_module.rs index dfd1dc2d21444..f5d3be82d12ca 100644 --- a/crates/turbopack-ecmascript/src/references/async_module.rs +++ b/crates/turbopack-ecmascript/src/references/async_module.rs @@ -272,11 +272,11 @@ impl CodeGenerateableWithAvailabilityInfo for AsyncModule { self: Vc, is_async_module: bool, ) -> Result> { - Ok(if is_async_module { - AvailabilityInfoNeeds::Root.cell() - } else { - AvailabilityInfoNeeds::None.cell() - }) + let mut needs = AvailabilityInfoNeeds::none(); + if is_async_module { + needs.current_availability_root = true; + } + Ok(needs.cell()) } } diff --git a/crates/turbopack-ecmascript/src/references/mod.rs b/crates/turbopack-ecmascript/src/references/mod.rs index 93b3b45959537..d919380fdb547 100644 --- a/crates/turbopack-ecmascript/src/references/mod.rs +++ b/crates/turbopack-ecmascript/src/references/mod.rs @@ -152,13 +152,13 @@ impl AnalyzeEcmascriptModuleResult { code_generation, .. } = &*self.await?; - let mut needs = AvailabilityInfoNeeds::None; + let mut needs = AvailabilityInfoNeeds::none(); for c in code_generation.await?.iter() { if let CodeGen::CodeGenerateableWithAvailabilityInfo(code_gen) = c { - needs += *code_gen + needs |= *code_gen .get_availability_info_needs(is_async_module) .await?; - if matches!(needs, AvailabilityInfoNeeds::Complete) { + if needs.is_complete() { return Ok(needs.cell()); } } @@ -168,10 +168,10 @@ impl AnalyzeEcmascriptModuleResult { Vc::try_resolve_sidecast::>(*r) .await? { - needs += *code_gen + needs |= *code_gen .get_availability_info_needs(is_async_module) .await?; - if matches!(needs, AvailabilityInfoNeeds::Complete) { + if needs.is_complete() { return Ok(needs.cell()); } }