Skip to content

Turbopack: refactor side effects optimization #82466

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 8, 2025
+93 −197
Merged

Turbopack: refactor side effects optimization #82466

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5ee4f0c
refactor side effects optimization
sokra Aug 7, 2025
e991b80
test is fixed now
sokra Aug 8, 2025
c7c1dec
test works now
sokra Aug 8, 2025
8ecc286
update snapshot
sokra Aug 8, 2025
ac71779
update snapshots
sokra Aug 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
refactor side effects optimization
  • Loading branch information
@sokra
sokra committed Aug 8, 2025
commit 5ee4f0c768a70b87d7ec39afb17e0efa1ac6e6af
2 changes: 1 addition & 1 deletion turbopack/crates/turbopack-ecmascript/src/chunk/placeable.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ pub enum EcmascriptExports {
#[turbo_tasks::value_impl]
impl EcmascriptExports {
#[turbo_tasks::function]
pub async fn needs_facade(&self) -> Result<Vc<bool>> {
pub async fn split_locals_and_reexports(&self) -> Result<Vc<bool>> {
Ok(match self {
EcmascriptExports::EsmExports(exports) => {
let exports = exports.await?;
Expand Down
14 changes: 7 additions & 7 deletions turbopack/crates/turbopack-ecmascript/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -850,8 +850,8 @@ pub struct EcmascriptModuleContentOptions {
specified_module_type: SpecifiedModuleType,
chunking_context: ResolvedVc<Box<dyn ChunkingContext>>,
references: ResolvedVc<ModuleReferences>,
esm_references: ResolvedVc<EsmAssetReferences>,
part_references: Vec<ResolvedVc<EcmascriptModulePartReference>>,
esm_references: ResolvedVc<EsmAssetReferences>,
code_generation: ResolvedVc<CodeGens>,
async_module: ResolvedVc<OptionAsyncModule>,
generate_source_map: bool,
Expand All @@ -872,8 +872,8 @@ impl EcmascriptModuleContentOptions {
module,
chunking_context,
references,
esm_references,
part_references,
esm_references,
code_generation,
async_module,
exports,
Expand Down Expand Up @@ -912,14 +912,14 @@ impl EcmascriptModuleContentOptions {
},
];

let esm_code_gens = esm_references
.await?
let part_code_gens = part_references
.iter()
.map(|r| r.code_generation(**chunking_context, scope_hoisting_context))
.try_join()
.await?;

let part_code_gens = part_references
let esm_code_gens = esm_references
.await?
.iter()
.map(|r| r.code_generation(**chunking_context, scope_hoisting_context))
.try_join()
Expand All @@ -933,9 +933,9 @@ impl EcmascriptModuleContentOptions {
.await?;

anyhow::Ok(
esm_code_gens
part_code_gens
.into_iter()
.chain(part_code_gens.into_iter())
.chain(esm_code_gens.into_iter())
.chain(additional_code_gens.into_iter().flatten())
.chain(code_gens.into_iter())
.collect(),
Expand Down
45 changes: 20 additions & 25 deletions turbopack/crates/turbopack-ecmascript/src/references/esm/export.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,17 +141,8 @@ pub async fn follow_reexports(
side_effect_free_packages: Vc<Glob>,
ignore_side_effect_of_entry: bool,
) -> Result<Vc<FollowExportsResult>> {
if !ignore_side_effect_of_entry
&& !*module
.is_marked_as_side_effect_free(side_effect_free_packages)
.await?
{
return Ok(FollowExportsResult::cell(FollowExportsResult {
module,
export_name: Some(export_name),
ty: FoundExportType::SideEffects,
}));
}
let mut ignore_side_effects = ignore_side_effect_of_entry;

let mut module = module;
let mut export_name = export_name;
loop {
Expand All @@ -164,12 +155,26 @@ pub async fn follow_reexports(
}));
};

if !ignore_side_effects
&& !*module
.is_marked_as_side_effect_free(side_effect_free_packages)
.await?
{
// TODO It's unfortunate that we have to use the whole module here.
// This is often the Facade module, which includes all reexports.
// Often we could use Locals + the followed reexports instead.
return Ok(FollowExportsResult::cell(FollowExportsResult {
module,
export_name: Some(export_name),
ty: FoundExportType::SideEffects,
}));
}
ignore_side_effects = false;

// Try to find the export in the local exports
let exports_ref = exports.await?;
if let Some(export) = exports_ref.exports.get(&export_name) {
match handle_declared_export(module, export_name, export, side_effect_free_packages)
.await?
{
match handle_declared_export(module, export_name, export).await? {
ControlFlow::Continue((m, n)) => {
module = m.to_resolved().await?;
export_name = n;
Expand Down Expand Up @@ -222,23 +227,12 @@ async fn handle_declared_export(
module: ResolvedVc<Box<dyn EcmascriptChunkPlaceable>>,
export_name: RcStr,
export: &EsmExport,
side_effect_free_packages: Vc<Glob>,
) -> Result<ControlFlow<FollowExportsResult, (Vc<Box<dyn EcmascriptChunkPlaceable>>, RcStr)>> {
match export {
EsmExport::ImportedBinding(reference, name, _) => {
if let ReferencedAsset::Some(module) =
*ReferencedAsset::from_resolve_result(reference.resolve_reference()).await?
{
if !*module
.is_marked_as_side_effect_free(side_effect_free_packages)
.await?
{
return Ok(ControlFlow::Break(FollowExportsResult {
module,
export_name: Some(name.clone()),
ty: FoundExportType::SideEffects,
}));
}
return Ok(ControlFlow::Continue((*module, name.clone())));
}
}
Expand Down Expand Up @@ -286,6 +280,7 @@ async fn find_export_from_reexports(
module: ResolvedVc<Box<dyn EcmascriptChunkPlaceable>>,
export_name: RcStr,
) -> Result<Vc<FindExportFromReexportsResult>> {
// TODO why do we need a special case for this?
Comment on lines 282 to +283
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the discussions in #78047

if let Some(module) =
Vc::try_resolve_downcast_type::<EcmascriptModulePartAsset>(*module).await?
&& matches!(module.await?.part, ModulePart::Exports)
Expand Down
14 changes: 0 additions & 14 deletions turbopack/crates/turbopack-ecmascript/src/references/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@ pub struct AnalyzeEcmascriptModuleResult {
pub esm_references: ResolvedVc<EsmAssetReferences>,
pub esm_local_references: ResolvedVc<EsmAssetReferences>,
pub esm_reexport_references: ResolvedVc<EsmAssetReferences>,
pub esm_evaluation_references: ResolvedVc<EsmAssetReferences>,

pub code_generation: ResolvedVc<CodeGens>,
pub exports: ResolvedVc<EcmascriptExports>,
Expand Down Expand Up @@ -217,7 +216,6 @@ pub struct AnalyzeEcmascriptModuleResultBuilder {
esm_references: FxHashSet<usize>,
esm_local_references: FxHashSet<usize>,
esm_reexport_references: FxHashSet<usize>,
esm_evaluation_references: FxHashSet<usize>,

esm_references_free_var: FxIndexMap<RcStr, ResolvedVc<EsmAssetReference>>,
// Ad-hoc created import references that are resolved `import * as x from ...; x.foo` accesses
Expand All @@ -239,7 +237,6 @@ impl AnalyzeEcmascriptModuleResultBuilder {
esm_references: Default::default(),
esm_local_references: Default::default(),
esm_reexport_references: Default::default(),
esm_evaluation_references: Default::default(),
esm_references_rewritten: Default::default(),
esm_references_free_var: Default::default(),
code_gens: Default::default(),
Expand Down Expand Up @@ -282,7 +279,6 @@ impl AnalyzeEcmascriptModuleResultBuilder {
pub fn add_esm_evaluation_reference(&mut self, idx: usize) {
self.esm_references.insert(idx);
self.esm_local_references.insert(idx);
self.esm_evaluation_references.insert(idx);
}

/// Adds a codegen to the analysis result.
Expand Down Expand Up @@ -369,8 +365,6 @@ impl AnalyzeEcmascriptModuleResultBuilder {
});
let mut esm_reexport_references = track_reexport_references
.then(|| Vec::with_capacity(self.esm_reexport_references.len()));
let mut esm_evaluation_references = track_reexport_references
.then(|| Vec::with_capacity(self.esm_evaluation_references.len()));
for (i, reference) in import_references.iter().enumerate() {
if self.esm_references.contains(&i) {
esm_references.push(*reference);
Expand All @@ -392,11 +386,6 @@ impl AnalyzeEcmascriptModuleResultBuilder {
.flat_map(|m| m.values().copied()),
);
}
if let Some(esm_evaluation_references) = &mut esm_evaluation_references
&& self.esm_evaluation_references.contains(&i)
{
esm_evaluation_references.push(*reference);
}
if let Some(esm_reexport_references) = &mut esm_reexport_references
&& self.esm_reexport_references.contains(&i)
{
Expand All @@ -415,9 +404,6 @@ impl AnalyzeEcmascriptModuleResultBuilder {
esm_reexport_references: ResolvedVc::cell(
esm_reexport_references.unwrap_or_default(),
),
esm_evaluation_references: ResolvedVc::cell(
esm_evaluation_references.unwrap_or_default(),
),
code_generation: ResolvedVc::cell(self.code_gens),
exports: self.exports.resolved_cell(),
async_module: self.async_module,
Expand Down
Loading