Repository: vercel/next.js
Pull request: Turbopack: instrument scope hoisting with tracing spans

<!-- Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that you follow the checklist sections below.
Choose the right checklist for the change(s) that you're making:

## For Contributors

### Improving Documentation

- Run `pnpm prettier-fix` to fix formatting issues before opening the PR.
- Read the Docs Contribution Guide to ensure your contribution follows the docs guidelines: https://nextjs.org/docs/community/contribution-guide

### Adding or Updating Examples

- The "examples guidelines" are followed from our contributing doc https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md
- Make sure the linting passes by running `pnpm build && pnpm lint`. See https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md

### Fixing a bug

- Related issues linked using `fixes #number`
- Tests added. See: https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md

### Adding a feature

- Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. (A discussion must be opened, see https://github.com/vercel/next.js/discussions/new?category=ideas)
- Related issues/discussions are linked using `fixes #number`
- e2e tests added (https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs)
- Documentation added
- Telemetry added. In case of a feature if it's used or not.
- Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md


## For Maintainers

- Minimal description (aim for explaining to someone not on the team to understand the PR)
- When linking to a Slack thread, you might want to share details of the conclusion
- Link both the Linear (Fixes NEXT-xxx) and the GitHub issues
- Add review comments if necessary to explain to the reviewer the logic behind a change

### What?

### Why?

### How?

Closes NEXT-
Fixes #

-->


Closes PACK-4952

Changed files:
filename: turbopack/crates/turbopack-cli/benches/small_apps.rs
patch: @@ -83,6 +83,7 @@ fn bench_small_apps(c: &mut Criterion) {
                             no_sourcemap: false,
                             no_minify: false,
                             force_memory_cleanup: true,
+                            no_scope_hoist: false,
                         })
                         .await
                     })

filename: turbopack/crates/turbopack-cli/src/arguments.rs
patch: @@ -136,6 +136,10 @@ pub struct BuildArguments {
     #[clap(long)]
     pub no_minify: bool,
 
+    /// Don't perform scope hoisting.
+    #[clap(long)]
+    pub no_scope_hoist: bool,
+
     /// Drop the `TurboTasks` object upon exit. By default we intentionally leak this memory, as
     /// we're about to exit the process anyways, but that can cause issues with valgrind or other
     /// leak detectors.

filename: turbopack/crates/turbopack-cli/src/build/mod.rs
patch: @@ -76,6 +76,7 @@ pub struct TurbopackBuildBuilder {
     source_maps_type: SourceMapsType,
     minify_type: MinifyType,
     target: Target,
+    scope_hoist: bool,
 }
 
 impl TurbopackBuildBuilder {
@@ -96,6 +97,7 @@ impl TurbopackBuildBuilder {
                 mangle: Some(MangleType::OptimalSize),
             },
             target: Target::Node,
+            scope_hoist: true,
         }
     }
 
@@ -134,6 +136,11 @@ impl TurbopackBuildBuilder {
         self
     }
 
+    pub fn scope_hoist(mut self, scope_hoist: bool) -> Self {
+        self.scope_hoist = scope_hoist;
+        self
+    }
+
     pub fn target(mut self, target: Target) -> Self {
         self.target = target;
         self
@@ -149,6 +156,7 @@ impl TurbopackBuildBuilder {
                 self.source_maps_type,
                 self.minify_type,
                 self.target,
+                self.scope_hoist,
             );
 
             // Await the result to propagate any errors.
@@ -196,6 +204,7 @@ async fn build_internal(
     source_maps_type: SourceMapsType,
     minify_type: MinifyType,
     target: Target,
+    scope_hoist: bool,
 ) -> Result<Vc<()>> {
     let output_fs = output_fs(project_dir.clone());
     let project_fs = project_fs(root_dir.clone(), /* watch= */ false);
@@ -359,7 +368,7 @@ async fn build_internal(
                             },
                         )
                         .use_content_hashing(ContentHashing::Direct { length: 16 })
-                        .module_merging(true);
+                        .module_merging(scope_hoist);
                 }
             }
 
@@ -404,7 +413,7 @@ async fn build_internal(
                                 ..Default::default()
                             },
                         )
-                        .module_merging(true);
+                        .module_merging(scope_hoist);
                 }
             }
 
@@ -533,6 +542,7 @@ pub async fn build(args: &BuildArguments) -> Result<()> {
                 mangle: Some(MangleType::OptimalSize),
             }
         })
+        .scope_hoist(!args.no_scope_hoist)
         .target(args.common.target.unwrap_or(Target::Node))
         .show_all(args.common.show_all);
 

filename: turbopack/crates/turbopack-ecmascript/src/lib.rs
patch: @@ -77,7 +77,7 @@ use swc_core::{
     },
     quote,
 };
-use tracing::Instrument;
+use tracing::{Instrument, Level, instrument};
 pub use transform::{
     CustomTransformer, EcmascriptInputTransform, EcmascriptInputTransforms, TransformContext,
     TransformPlugin, UnsupportedServerActionIssue,
@@ -1066,106 +1066,121 @@ impl EcmascriptModuleContent {
         module_options: Vec<Vc<EcmascriptModuleContentOptions>>,
         entry_points: Vec<ResolvedVc<Box<dyn EcmascriptAnalyzable>>>,
     ) -> Result<Vc<Self>> {
-        let modules = modules
-            .into_iter()
-            .map(|(m, exposed)| {
-                (
-                    ResolvedVc::try_sidecast::<Box<dyn EcmascriptChunkPlaceable>>(m).unwrap(),
-                    exposed,
-                )
-            })
-            .collect::<FxIndexMap<_, _>>();
-        let entry_points = entry_points
-            .into_iter()
-            .map(|m| {
-                let m = ResolvedVc::try_sidecast::<Box<dyn EcmascriptChunkPlaceable>>(m).unwrap();
-                (m, modules.get_index_of(&m).unwrap())
-            })
-            .collect::<Vec<_>>();
+        async {
+            let modules = modules
+                .into_iter()
+                .map(|(m, exposed)| {
+                    (
+                        ResolvedVc::try_sidecast::<Box<dyn EcmascriptChunkPlaceable>>(m).unwrap(),
+                        exposed,
+                    )
+                })
+                .collect::<FxIndexMap<_, _>>();
+            let entry_points = entry_points
+                .into_iter()
+                .map(|m| {
+                    let m =
+                        ResolvedVc::try_sidecast::<Box<dyn EcmascriptChunkPlaceable>>(m).unwrap();
+                    (m, modules.get_index_of(&m).unwrap())
+                })
+                .collect::<Vec<_>>();
 
-        let globals_merged = Globals::default();
+            let globals_merged = Globals::default();
 
-        let contents = module_options
-            .iter()
-            .zip(modules.keys().copied())
-            .map(async |(options, module)| {
-                let options = options.await?;
-                let EcmascriptModuleContentOptions {
-                    chunking_context,
-                    parsed,
-                    ident,
-                    specified_module_type,
-                    generate_source_map,
-                    original_source_map,
-                    ..
-                } = &*options;
-
-                let result = process_parse_result(
-                    *parsed,
-                    **ident,
-                    *specified_module_type,
-                    *generate_source_map,
-                    *original_source_map,
-                    *chunking_context.minify_type().await?,
-                    Some(&*options),
-                    Some(ScopeHoistingOptions {
-                        module,
-                        modules: &modules,
-                    }),
-                )
-                .await?;
+            let contents = module_options
+                .iter()
+                .zip(modules.keys().copied())
+                .map(async |(options, module)| {
+                    async {
+                        let options = options.await?;
+                        let EcmascriptModuleContentOptions {
+                            chunking_context,
+                            parsed,
+                            ident,
+                            specified_module_type,
+                            generate_source_map,
+                            original_source_map,
+                            ..
+                        } = &*options;
+
+                        let result = process_parse_result(
+                            *parsed,
+                            **ident,
+                            *specified_module_type,
+                            *generate_source_map,
+                            *original_source_map,
+                            *chunking_context.minify_type().await?,
+                            Some(&*options),
+                            Some(ScopeHoistingOptions {
+                                module,
+                                modules: &modules,
+                            }),
+                        )
+                        .await?;
 
-                Ok((module, result))
-            })
-            .try_join()
-            .await?;
+                        Ok((module, result))
+                    }
+                    .instrument(tracing::trace_span!("process individual module"))
+                    .await
+                })
+                .try_join()
+                .await?;
 
-        let (merged_ast, comments, source_maps, original_source_maps) =
-            merge_modules(contents, &entry_points, &globals_merged).await?;
+            let (merged_ast, comments, source_maps, original_source_maps) =
+                merge_modules(contents, &entry_points, &globals_merged).await?;
 
-        // Use the options from an arbitrary module, since they should all be the same with regards
-        // to minify_type and chunking_context.
-        let options = module_options.last().unwrap().await?;
+            // Use the options from an arbitrary module, since they should all be the same with
+            // regards to minify_type and chunking_context.
+            let options = module_options.last().unwrap().await?;
 
-        let modules_header_width = modules.len().next_power_of_two().trailing_zeros();
-        let content = CodeGenResult {
-            program: merged_ast,
-            source_map: CodeGenResultSourceMap::ScopeHoisting {
-                modules_header_width,
-                source_maps,
-            },
-            comments: CodeGenResultComments::ScopeHoisting {
-                modules_header_width,
-                comments,
-            },
-            export_contexts: None,
-            is_esm: true,
-            strict: true,
-            original_source_map: CodeGenResultOriginalSourceMap::ScopeHoisting(
-                original_source_maps,
-            ),
-            minify: *options.chunking_context.minify_type().await?,
-            scope_hoisting_syntax_contexts: None,
-        };
+            let modules_header_width = modules.len().next_power_of_two().trailing_zeros();
+            let content = CodeGenResult {
+                program: merged_ast,
+                source_map: CodeGenResultSourceMap::ScopeHoisting {
+                    modules_header_width,
+                    source_maps,
+                },
+                comments: CodeGenResultComments::ScopeHoisting {
+                    modules_header_width,
+                    comments,
+                },
+                export_contexts: None,
+                is_esm: true,
+                strict: true,
+                original_source_map: CodeGenResultOriginalSourceMap::ScopeHoisting(
+                    original_source_maps,
+                ),
+                minify: *options.chunking_context.minify_type().await?,
+                scope_hoisting_syntax_contexts: None,
+            };
 
-        let first_entry = entry_points.first().unwrap().0;
-        let additional_ids = modules
-            .keys()
-            // Additionally set this module factory for all modules that are exposed. The whole
-            // group might be imported via a different entry import in different chunks (we only
-            // ensure that the modules are in the same order, not that they form a subgraph that is
-            // always imported from the same root module).
-            //
-            // Also skip the first entry, which is the name of the chunk item.
-            .filter(|m| {
-                **m != first_entry && *modules.get(*m).unwrap() == MergeableModuleExposure::External
-            })
-            .map(|m| m.chunk_item_id(*options.chunking_context).to_resolved())
-            .try_join()
-            .await?
-            .into();
+            let first_entry = entry_points.first().unwrap().0;
+            let additional_ids = modules
+                .keys()
+                // Additionally set this module factory for all modules that are exposed. The whole
+                // group might be imported via a different entry import in different chunks (we only
+                // ensure that the modules are in the same order, not that they form a subgraph that
+                // is always imported from the same root module).
+                //
+                // Also skip the first entry, which is the name of the chunk item.
+                .filter(|m| {
+                    **m != first_entry
+                        && *modules.get(*m).unwrap() == MergeableModuleExposure::External
+                })
+                .map(|m| m.chunk_item_id(*options.chunking_context).to_resolved())
+                .try_join()
+                .await?
+                .into();
 
-        emit_content(content, additional_ids).await
+            emit_content(content, additional_ids)
+                .instrument(tracing::info_span!("emit"))
+                .await
+        }
+        .instrument(tracing::info_span!(
+            "merged EcmascriptModuleContent",
+            modules = module_options.len()
+        ))
+        .await
     }
 }
 
@@ -1177,6 +1192,7 @@ impl EcmascriptModuleContent {
 /// - `sym` being the name of the import.
 ///
 /// This is then used to map back to the variable name and context of the exporting module.
+#[instrument(level = Level::TRACE, skip_all, name = "merge")]
 #[allow(clippy::type_complexity)]
 async fn merge_modules(
     mut contents: Vec<(ResolvedVc<Box<dyn EcmascriptChunkPlaceable>>, CodeGenResult)>,
@@ -1314,13 +1330,15 @@ async fn merge_modules(
         .collect::<Result<FxHashMap<_, _>>>()?;
 
     let (merged_ast, inserted) = GLOBALS.set(globals_merged, || {
+        let _ = tracing::trace_span!("merge inner").entered();
         // As an optimization, assume an average number of 5 contexts per module.
         let mut unique_contexts_cache =
             FxHashMap::with_capacity_and_hasher(contents.len() * 5, Default::default());
 
         let mut prepare_module =
             |(module, content): &(ResolvedVc<Box<dyn EcmascriptChunkPlaceable>>, CodeGenResult),
              program: &mut Program| {
+                let _ = tracing::trace_span!("prepare module").entered();
                 if let CodeGenResult {
                     scope_hoisting_syntax_contexts: Some(module_contexts),
                     ..
@@ -1366,6 +1384,7 @@ async fn merge_modules(
 
         let mut inserted_imports = FxHashMap::default();
 
+        let span = tracing::trace_span!("merge ASTs");
         // Replace inserted `__turbopack_merged_esm__(i);` statements with the corresponding
         // ith-module.
         let mut queue = entry_points
@@ -1438,13 +1457,16 @@ async fn merge_modules(
 
             result.push(item);
         }
+        drop(span);
 
+        let span = tracing::trace_span!("hygiene").entered();
         let mut merged_ast = Program::Module(swc_core::ecma::ast::Module {
             body: result,
             span: DUMMY_SP,
             shebang: None,
         });
         merged_ast.visit_mut_with(&mut swc_core::ecma::transforms::base::hygiene::hygiene());
+        drop(span);
 
         anyhow::Ok((merged_ast, inserted))
     })?;