perf: reduce large pre-allocations for JavascriptParser::new (#7286)

perf: init
web-infra-dev · Jul 23, 2024 · f6c8932 · f6c8932 · rspack-bot · Jul 23, 2024
1 parent 355d50d
commit f6c8932
Show file tree

Hide file tree

Showing 14 changed files with 39 additions and 27 deletions.
diff --git a/crates/rspack_core/src/compiler/make/repair/build.rs b/crates/rspack_core/src/compiler/make/repair/build.rs
@@ -138,9 +138,9 @@ impl Task<MakeTaskContext> for BuildResultTask {
     let mut queue = VecDeque::new();
     let mut all_dependencies = vec![];
     let mut handle_block = |dependencies: Vec<BoxDependency>,
-                            blocks: Vec<AsyncDependenciesBlock>,
-                            current_block: Option<AsyncDependenciesBlock>|
-     -> Vec<AsyncDependenciesBlock> {
+                            blocks: Vec<Box<AsyncDependenciesBlock>>,
+                            current_block: Option<Box<AsyncDependenciesBlock>>|
+     -> Vec<Box<AsyncDependenciesBlock>> {
       for dependency in dependencies {
         let dependency_id = *dependency.id();
         if current_block.is_none() {

diff --git a/crates/rspack_core/src/context_module.rs b/crates/rspack_core/src/context_module.rs
@@ -1085,7 +1085,10 @@ impl ContextModule {
     Ok(())
   }
 
-  fn resolve_dependencies(&self) -> Result<(Vec<BoxDependency>, Vec<AsyncDependenciesBlock>)> {
+  // Vec<Box<T: Sized>> makes sense if T is a large type (see #3530, 1st comment).
+  // #3530: https://github.com/rust-lang/rust-clippy/issues/3530
+  #[allow(clippy::vec_box)]
+  fn resolve_dependencies(&self) -> Result<(Vec<BoxDependency>, Vec<Box<AsyncDependenciesBlock>>)> {
     tracing::trace!("resolving context module path {}", self.options.resource);
 
     let resolver = &self.resolve_factory.get(ResolveOptionsWithDependencyType {
@@ -1133,7 +1136,7 @@ impl ContextModule {
       if let Some(group_options) = &self.options.context_options.group_options {
         block.set_group_options(group_options.clone());
       }
-      blocks.push(block);
+      blocks.push(Box::new(block));
     } else if matches!(self.options.context_options.mode, ContextMode::Lazy) {
       let mut index = 0;
       for context_element_dependency in context_element_dependencies {
@@ -1175,7 +1178,7 @@ impl ContextModule {
           prefetch_order,
           fetch_priority,
         )));
-        blocks.push(block);
+        blocks.push(Box::new(block));
       }
     } else {
       dependencies = context_element_dependencies

diff --git a/crates/rspack_core/src/dependencies_block.rs b/crates/rspack_core/src/dependencies_block.rs
@@ -92,7 +92,10 @@ impl From<String> for AsyncDependenciesBlockIdentifier {
 pub struct AsyncDependenciesBlock {
   id: AsyncDependenciesBlockIdentifier,
   group_options: Option<GroupOptions>,
-  blocks: Vec<AsyncDependenciesBlock>,
+  // Vec<Box<T: Sized>> makes sense if T is a large type (see #3530, 1st comment).
+  // #3530: https://github.com/rust-lang/rust-clippy/issues/3530
+  #[allow(clippy::vec_box)]
+  blocks: Vec<Box<AsyncDependenciesBlock>>,
   block_ids: Vec<AsyncDependenciesBlockIdentifier>,
   dependency_ids: Vec<DependencyId>,
   dependencies: Vec<BoxDependency>,
@@ -164,7 +167,7 @@ impl AsyncDependenciesBlock {
     // self.blocks.push(block);
   }
 
-  pub fn take_blocks(&mut self) -> Vec<AsyncDependenciesBlock> {
+  pub fn take_blocks(&mut self) -> Vec<Box<AsyncDependenciesBlock>> {
     std::mem::take(&mut self.blocks)
   }
 

diff --git a/crates/rspack_core/src/module.rs b/crates/rspack_core/src/module.rs
@@ -157,7 +157,7 @@ pub struct BuildResult {
   pub build_meta: BuildMeta,
   pub build_info: BuildInfo,
   pub dependencies: Vec<BoxDependency>,
-  pub blocks: Vec<AsyncDependenciesBlock>,
+  pub blocks: Vec<Box<AsyncDependenciesBlock>>,
   pub optimization_bailouts: Vec<String>,
 }
 

diff --git a/crates/rspack_core/src/module_graph/mod.rs b/crates/rspack_core/src/module_graph/mod.rs
@@ -46,7 +46,7 @@ pub struct ModuleGraphPartial {
   dependencies: HashMap<DependencyId, Option<BoxDependency>>,
 
   /// AsyncDependenciesBlocks indexed by `AsyncDependenciesBlockIdentifier`.
-  blocks: HashMap<AsyncDependenciesBlockIdentifier, Option<AsyncDependenciesBlock>>,
+  blocks: HashMap<AsyncDependenciesBlockIdentifier, Option<Box<AsyncDependenciesBlock>>>,
 
   /// ModuleGraphModule indexed by `ModuleIdentifier`.
   module_graph_modules: IdentifierMap<Option<ModuleGraphModule>>,
@@ -601,7 +601,7 @@ impl<'a> ModuleGraph<'a> {
     }
   }
 
-  pub fn add_block(&mut self, block: AsyncDependenciesBlock) {
+  pub fn add_block(&mut self, block: Box<AsyncDependenciesBlock>) {
     let Some(active_partial) = &mut self.active else {
       panic!("should have active partial");
     };
@@ -641,7 +641,10 @@ impl<'a> ModuleGraph<'a> {
     &self,
     block_id: &AsyncDependenciesBlockIdentifier,
   ) -> Option<&AsyncDependenciesBlock> {
-    self.loop_partials(|p| p.blocks.get(block_id))?.as_ref()
+    self
+      .loop_partials(|p| p.blocks.get(block_id))?
+      .as_ref()
+      .map(|b| &**b)
   }
 
   pub fn block_by_id_expect(

diff --git a/crates/rspack_core/src/parser_and_generator.rs b/crates/rspack_core/src/parser_and_generator.rs
@@ -65,7 +65,7 @@ impl SideEffectsBailoutItemWithSpan {
 #[derive(Debug)]
 pub struct ParseResult {
   pub dependencies: Vec<BoxDependency>,
-  pub blocks: Vec<AsyncDependenciesBlock>,
+  pub blocks: Vec<Box<AsyncDependenciesBlock>>,
   pub presentational_dependencies: Vec<Box<dyn DependencyTemplate>>,
   pub code_generation_dependencies: Vec<Box<dyn ModuleDependency>>,
   pub source: BoxSource,

diff --git a/crates/rspack_plugin_javascript/src/parser_plugin/import_parser_plugin.rs b/crates/rspack_plugin_javascript/src/parser_plugin/import_parser_plugin.rs
@@ -142,7 +142,7 @@ impl JavascriptParserPlugin for ImportParserPlugin {
         chunk_prefetch,
         fetch_priority,
       )));
-      parser.blocks.push(block);
+      parser.blocks.push(Box::new(block));
       Some(true)
     } else {
       let ContextModuleScanResult {

diff --git a/crates/rspack_plugin_javascript/src/parser_plugin/worker_plugin.rs b/crates/rspack_plugin_javascript/src/parser_plugin/worker_plugin.rs
@@ -120,7 +120,7 @@ fn add_dependencies(
     depend_on: None,
   })));
 
-  parser.blocks.push(block);
+  parser.blocks.push(Box::new(block));
   if let Some(range) = range {
     parser
       .presentational_dependencies

diff --git a/crates/rspack_plugin_javascript/src/visitors/dependency/mod.rs b/crates/rspack_plugin_javascript/src/visitors/dependency/mod.rs
@@ -26,7 +26,7 @@ use crate::BoxJavascriptParserPlugin;
 
 pub struct ScanDependenciesResult {
   pub dependencies: Vec<BoxDependency>,
-  pub blocks: Vec<AsyncDependenciesBlock>,
+  pub blocks: Vec<Box<AsyncDependenciesBlock>>,
   pub presentational_dependencies: Vec<BoxDependencyTemplate>,
   pub warning_diagnostics: Vec<Box<dyn Diagnostic + Send + Sync>>,
 }

diff --git a/crates/rspack_plugin_javascript/src/visitors/dependency/parser/mod.rs b/crates/rspack_plugin_javascript/src/visitors/dependency/parser/mod.rs
@@ -207,7 +207,10 @@ pub struct JavascriptParser<'parser> {
   pub(crate) warning_diagnostics: Vec<Box<dyn Diagnostic + Send + Sync>>,
   pub dependencies: Vec<BoxDependency>,
   pub(crate) presentational_dependencies: Vec<Box<dyn DependencyTemplate>>,
-  pub(crate) blocks: Vec<AsyncDependenciesBlock>,
+  // Vec<Box<T: Sized>> makes sense if T is a large type (see #3530, 1st comment).
+  // #3530: https://github.com/rust-lang/rust-clippy/issues/3530
+  #[allow(clippy::vec_box)]
+  pub(crate) blocks: Vec<Box<AsyncDependenciesBlock>>,
   // TODO: remove `additional_data` once we have builtin:css-extract-loader
   pub additional_data: AdditionalData,
   pub(crate) comments: Option<&'parser dyn Comments>,
@@ -262,11 +265,11 @@ impl<'parser> JavascriptParser<'parser> {
     parser_plugins: &'parser mut Vec<BoxJavascriptParserPlugin>,
     additional_data: AdditionalData,
   ) -> Self {
-    let warning_diagnostics: Vec<Box<dyn Diagnostic + Send + Sync>> = Vec::with_capacity(32);
-    let errors = Vec::with_capacity(32);
-    let dependencies = Vec::with_capacity(256);
-    let blocks = Vec::with_capacity(256);
-    let presentational_dependencies = Vec::with_capacity(256);
+    let warning_diagnostics: Vec<Box<dyn Diagnostic + Send + Sync>> = Vec::with_capacity(4);
+    let errors = Vec::with_capacity(4);
+    let dependencies = Vec::with_capacity(64);
+    let blocks = Vec::with_capacity(64);
+    let presentational_dependencies = Vec::with_capacity(64);
     let parser_exports_state: Option<bool> = None;
 
     let mut plugins: Vec<parser_plugin::BoxJavascriptParserPlugin> = Vec::with_capacity(32);

diff --git a/crates/rspack_plugin_lazy_compilation/src/module.rs b/crates/rspack_plugin_lazy_compilation/src/module.rs
@@ -161,13 +161,13 @@ impl Module for LazyCompilationProxyModule {
     if self.active {
       let dep = LazyCompilationDependency::new(self.create_data.clone());
 
-      blocks.push(AsyncDependenciesBlock::new(
+      blocks.push(Box::new(AsyncDependenciesBlock::new(
         self.identifier,
         None,
         None,
         vec![Box::new(dep)],
         None,
-      ));
+      )));
     }
 
     let mut files = FxHashSet::default();

diff --git a/crates/rspack_plugin_mf/src/container/container_entry_module.rs b/crates/rspack_plugin_mf/src/container/container_entry_module.rs
@@ -151,7 +151,7 @@ impl Module for ContainerEntryModule {
       block.set_group_options(GroupOptions::ChunkGroup(
         ChunkGroupOptions::default().name_optional(options.name.clone()),
       ));
-      blocks.push(block);
+      blocks.push(Box::new(block));
     }
     dependencies.push(Box::new(StaticExportsDependency::new(
       StaticExportsSpec::Array(vec!["get".into(), "init".into()]),

diff --git a/crates/rspack_plugin_mf/src/sharing/consume_shared_module.rs b/crates/rspack_plugin_mf/src/sharing/consume_shared_module.rs
@@ -160,7 +160,7 @@ impl Module for ConsumeSharedModule {
         dependencies.push(dep as BoxDependency);
       } else {
         let block = AsyncDependenciesBlock::new(self.identifier, None, None, vec![dep], None);
-        blocks.push(block);
+        blocks.push(Box::new(block));
       }
     }
 

diff --git a/crates/rspack_plugin_mf/src/sharing/provide_shared_module.rs b/crates/rspack_plugin_mf/src/sharing/provide_shared_module.rs
@@ -142,7 +142,7 @@ impl Module for ProvideSharedModule {
       dependencies.push(dep as BoxDependency);
     } else {
       let block = AsyncDependenciesBlock::new(self.identifier, None, None, vec![dep], None);
-      blocks.push(block);
+      blocks.push(Box::new(block));
     }
 
     Ok(BuildResult {
suite	result
modernjs	❌ failure
_selftest	✅ success
nx	✅ success
rspress	✅ success
rsbuild	❌ failure
examples	✅ success
Name	Base (2024-07-23 26d1cb6)	Current	Change
10000_development-mode + exec	2.25 s ± 22 ms	2.26 s ± 35 ms	+0.45 %
10000_development-mode_hmr + exec	699 ms ± 10 ms	694 ms ± 5 ms	-0.71 %
10000_production-mode + exec	2.84 s ± 38 ms	2.84 s ± 22 ms	+0.08 %
arco-pro_development-mode + exec	1.91 s ± 73 ms	1.92 s ± 78 ms	+0.62 %
arco-pro_development-mode_hmr + exec	434 ms ± 0.95 ms	434 ms ± 2.1 ms	+0.05 %
arco-pro_production-mode + exec	3.47 s ± 109 ms	3.47 s ± 87 ms	+0.16 %
threejs_development-mode_10x + exec	1.77 s ± 20 ms	1.78 s ± 17 ms	+0.51 %
threejs_development-mode_10x_hmr + exec	865 ms ± 6.6 ms	874 ms ± 5.7 ms	+1.09 %
threejs_production-mode_10x + exec	5.74 s ± 25 ms	5.79 s ± 36 ms	+0.72 %