From ee9360ba2960bdb2a764d68e74c74aa2ee5c441b Mon Sep 17 00:00:00 2001 From: quininer Date: Wed, 1 May 2024 13:27:23 +0800 Subject: [PATCH 1/6] perf: improve determine_export_assignments * Use .chain instead of .clone + .push * eliminate Atom drop by avoiding redundant Atom clone * Remove IndexSet and insert directly into Vec. --- ...ny_export_imported_specifier_dependency.rs | 41 +++++++++---------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs b/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs index 53e96934fafe..41500c84d01c 100644 --- a/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs +++ b/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs @@ -1,7 +1,6 @@ -use std::hash::BuildHasherDefault; +use std::cell::OnceCell; use std::{collections::HashMap, sync::Arc}; -use indexmap::IndexSet; use rspack_core::{ create_exports_object_referenced, create_no_exports_referenced, export_from_import, get_exports_type, process_export_info, property_access, property_name, string_of_used_name, @@ -13,7 +12,7 @@ use rspack_core::{ RuntimeGlobals, RuntimeSpec, Template, TemplateContext, TemplateReplaceSource, UsageState, UsedName, }; -use rustc_hash::{FxHashSet as HashSet, FxHasher}; +use rustc_hash::FxHashSet as HashSet; use swc_core::ecma::atoms::Atom; use super::{create_resource_identifier_for_esm_dependency, harmony_import_dependency_apply}; @@ -435,7 +434,7 @@ impl HarmonyExportImportedSpecifierDependency { let all_star_exports = self.all_star_exports(module_graph); if !all_star_exports.is_empty() { let (names, dependency_indices) = - determine_export_assignments(module_graph, all_star_exports.clone(), None); + determine_export_assignments(module_graph, &all_star_exports, None); return Some(DiscoverActiveExportsFromOtherStarExportsRet { names, @@ -447,7 +446,7 @@ impl HarmonyExportImportedSpecifierDependency { if let Some(other_star_exports) = &self.other_star_exports { let (names, dependency_indices) = - determine_export_assignments(module_graph, other_star_exports.clone(), Some(self.id)); + determine_export_assignments(module_graph, other_star_exports, Some(self.id)); return Some(DiscoverActiveExportsFromOtherStarExportsRet { names, names_slice: dependency_indices[i - 1], @@ -1352,37 +1351,37 @@ pub struct StarReexportsInfo { /// return (names, dependency_indices) fn determine_export_assignments( module_graph: &ModuleGraph, - mut dependencies: Vec, + dependencies: &[DependencyId], additional_dependency: Option, ) -> (Vec, Vec) { - if let Some(additional_dependency) = additional_dependency { - dependencies.push(additional_dependency); - } - // https://github.com/webpack/webpack/blob/ac7e531436b0d47cd88451f497cdfd0dad41535d/lib/dependencies/HarmonyExportImportedSpecifierDependency.js#L109 // js `Set` keep the insertion order, use `IndexSet` to align there behavior - let mut names: IndexSet> = IndexSet::default(); - let mut dependency_indices = vec![]; + let mut names = Vec::new(); + let mut hints = HashSet::default(); + let mut dependency_indices = + Vec::with_capacity(dependencies.len() + additional_dependency.is_some() as usize); + let empty = OnceCell::new(); - for dependency in dependencies.iter() { - dependency_indices.push(names.len()); + for dependency in dependencies.iter().chain(additional_dependency.iter()) { if let Some(module_identifier) = module_graph.module_identifier_by_dependency_id(dependency) { let exports_info = module_graph.get_exports_info(module_identifier); for export_info_id in exports_info.exports.values() { let export_info = module_graph.get_export_info_by_id(export_info_id); // SAFETY: This is safe because a real export can't export empty string - let export_info_name = export_info.name.clone().unwrap_or_default(); + let export_info_name = export_info.name + .as_ref() + .unwrap_or_else(|| empty.get_or_init(Atom::default)); if matches!(export_info.provided, Some(ExportInfoProvided::True)) - && &export_info_name != "default" - && !names.contains(&export_info_name) + && export_info_name != "default" + && !hints.contains(export_info_name) { - names.insert(export_info_name.clone()); - let cur_len = dependency_indices.len(); - dependency_indices[cur_len - 1] = names.len(); + hints.insert(export_info_name); + names.push(export_info_name.clone()); } } } + dependency_indices.push(names.len()); } - (names.into_iter().collect::>(), dependency_indices) + (names, dependency_indices) } From aec0dbf1d81e9901514564572be723b5e60ac143 Mon Sep 17 00:00:00 2001 From: quininer Date: Wed, 1 May 2024 13:47:22 +0800 Subject: [PATCH 2/6] perf: improve determine_export_assignments (2) * eliminate Atom clone * rewrite collect hidden_exports --- ...ny_export_imported_specifier_dependency.rs | 39 ++++++++----------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs b/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs index 41500c84d01c..7532ba364cc8 100644 --- a/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs +++ b/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs @@ -1,4 +1,3 @@ -use std::cell::OnceCell; use std::{collections::HashMap, sync::Arc}; use rspack_core::{ @@ -308,14 +307,11 @@ impl HarmonyExportImportedSpecifierDependency { let hidden_exports = self .discover_active_exports_from_other_star_exports(module_graph) .map(|other_star_exports| { - let mut hide_exports = HashSet::default(); - for i in 0..other_star_exports.names_slice { - hide_exports.insert(other_star_exports.names[i].clone()); - } - for e in ignored_exports.iter() { - hide_exports.remove(e); - } - hide_exports + other_star_exports.names.into_iter() + .take(other_star_exports.names_slice) + .filter(|name| !ignored_exports.contains(name)) + .cloned() + .collect::>() }); if !no_extra_exports && !no_extra_imports { return StarReexportsInfo { @@ -418,10 +414,10 @@ impl HarmonyExportImportedSpecifierDependency { } } - pub fn discover_active_exports_from_other_star_exports( + pub fn discover_active_exports_from_other_star_exports<'a>( &self, - module_graph: &ModuleGraph, - ) -> Option { + module_graph: &'a ModuleGraph, + ) -> Option> { if let Some(other_star_exports) = &self.other_star_exports { if other_star_exports.is_empty() { return None; @@ -828,8 +824,8 @@ impl HarmonyExportImportedSpecifierDependency { } #[derive(Debug)] -pub struct DiscoverActiveExportsFromOtherStarExportsRet { - names: Vec, +pub struct DiscoverActiveExportsFromOtherStarExportsRet<'a> { + names: Vec<&'a Atom>, names_slice: usize, pub dependency_indices: Vec, pub dependency_index: usize, @@ -1349,18 +1345,17 @@ pub struct StarReexportsInfo { } /// return (names, dependency_indices) -fn determine_export_assignments( - module_graph: &ModuleGraph, +fn determine_export_assignments<'a>( + module_graph: &'a ModuleGraph, dependencies: &[DependencyId], additional_dependency: Option, -) -> (Vec, Vec) { +) -> (Vec<&'a Atom>, Vec) { // https://github.com/webpack/webpack/blob/ac7e531436b0d47cd88451f497cdfd0dad41535d/lib/dependencies/HarmonyExportImportedSpecifierDependency.js#L109 - // js `Set` keep the insertion order, use `IndexSet` to align there behavior + // js `Set` keep the insertion order let mut names = Vec::new(); let mut hints = HashSet::default(); let mut dependency_indices = Vec::with_capacity(dependencies.len() + additional_dependency.is_some() as usize); - let empty = OnceCell::new(); for dependency in dependencies.iter().chain(additional_dependency.iter()) { if let Some(module_identifier) = module_graph.module_identifier_by_dependency_id(dependency) { @@ -1368,15 +1363,13 @@ fn determine_export_assignments( for export_info_id in exports_info.exports.values() { let export_info = module_graph.get_export_info_by_id(export_info_id); // SAFETY: This is safe because a real export can't export empty string - let export_info_name = export_info.name - .as_ref() - .unwrap_or_else(|| empty.get_or_init(Atom::default)); + let export_info_name = export_info.name.as_ref().expect("export name is empty"); if matches!(export_info.provided, Some(ExportInfoProvided::True)) && export_info_name != "default" && !hints.contains(export_info_name) { + names.push(export_info_name); hints.insert(export_info_name); - names.push(export_info_name.clone()); } } } From c3809cc8c2f7faac31e946576dbf13511f5fb0ec Mon Sep 17 00:00:00 2001 From: quininer Date: Wed, 1 May 2024 14:04:37 +0800 Subject: [PATCH 3/6] Fix clippy --- .../esm/harmony_export_imported_specifier_dependency.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs b/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs index 7532ba364cc8..9d5f43c2af5b 100644 --- a/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs +++ b/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs @@ -430,7 +430,7 @@ impl HarmonyExportImportedSpecifierDependency { let all_star_exports = self.all_star_exports(module_graph); if !all_star_exports.is_empty() { let (names, dependency_indices) = - determine_export_assignments(module_graph, &all_star_exports, None); + determine_export_assignments(module_graph, all_star_exports, None); return Some(DiscoverActiveExportsFromOtherStarExportsRet { names, From ae7c46560fb9268ecf1f5df5016a520030adbf2c Mon Sep 17 00:00:00 2001 From: quininer Date: Wed, 1 May 2024 14:19:35 +0800 Subject: [PATCH 4/6] Fix fmt --- ...harmony_export_imported_specifier_dependency.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs b/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs index 9d5f43c2af5b..29f43131d552 100644 --- a/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs +++ b/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs @@ -307,11 +307,13 @@ impl HarmonyExportImportedSpecifierDependency { let hidden_exports = self .discover_active_exports_from_other_star_exports(module_graph) .map(|other_star_exports| { - other_star_exports.names.into_iter() - .take(other_star_exports.names_slice) - .filter(|name| !ignored_exports.contains(name)) - .cloned() - .collect::>() + other_star_exports + .names + .into_iter() + .take(other_star_exports.names_slice) + .filter(|name| !ignored_exports.contains(name)) + .cloned() + .collect::>() }); if !no_extra_exports && !no_extra_imports { return StarReexportsInfo { @@ -1355,7 +1357,7 @@ fn determine_export_assignments<'a>( let mut names = Vec::new(); let mut hints = HashSet::default(); let mut dependency_indices = - Vec::with_capacity(dependencies.len() + additional_dependency.is_some() as usize); + Vec::with_capacity(dependencies.len() + additional_dependency.is_some() as usize); for dependency in dependencies.iter().chain(additional_dependency.iter()) { if let Some(module_identifier) = module_graph.module_identifier_by_dependency_id(dependency) { From b22d0a8f0e5015c76198a22c06d59b5feccdb6f0 Mon Sep 17 00:00:00 2001 From: quininer Date: Thu, 2 May 2024 10:39:46 +0800 Subject: [PATCH 5/6] perf: improve determine_export_assignments (3) * Use indexmap again --- ...rmony_export_imported_specifier_dependency.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs b/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs index 29f43131d552..78c0b301bcd2 100644 --- a/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs +++ b/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs @@ -1,5 +1,7 @@ +use std::hash::BuildHasherDefault; use std::{collections::HashMap, sync::Arc}; +use indexmap::IndexSet; use rspack_core::{ create_exports_object_referenced, create_no_exports_referenced, export_from_import, get_exports_type, process_export_info, property_access, property_name, string_of_used_name, @@ -11,7 +13,7 @@ use rspack_core::{ RuntimeGlobals, RuntimeSpec, Template, TemplateContext, TemplateReplaceSource, UsageState, UsedName, }; -use rustc_hash::FxHashSet as HashSet; +use rustc_hash::{FxHashSet as HashSet, FxHasher}; use swc_core::ecma::atoms::Atom; use super::{create_resource_identifier_for_esm_dependency, harmony_import_dependency_apply}; @@ -1353,9 +1355,8 @@ fn determine_export_assignments<'a>( additional_dependency: Option, ) -> (Vec<&'a Atom>, Vec) { // https://github.com/webpack/webpack/blob/ac7e531436b0d47cd88451f497cdfd0dad41535d/lib/dependencies/HarmonyExportImportedSpecifierDependency.js#L109 - // js `Set` keep the insertion order - let mut names = Vec::new(); - let mut hints = HashSet::default(); + // js `Set` keep the insertion order, use `IndexSet` to align there behavior + let mut names: IndexSet<&Atom, BuildHasherDefault> = IndexSet::default(); let mut dependency_indices = Vec::with_capacity(dependencies.len() + additional_dependency.is_some() as usize); @@ -1368,15 +1369,14 @@ fn determine_export_assignments<'a>( let export_info_name = export_info.name.as_ref().expect("export name is empty"); if matches!(export_info.provided, Some(ExportInfoProvided::True)) && export_info_name != "default" - && !hints.contains(export_info_name) + && !names.contains(export_info_name) { - names.push(export_info_name); - hints.insert(export_info_name); + names.insert(export_info_name); } } } dependency_indices.push(names.len()); } - (names, dependency_indices) + (names.into_iter().collect(), dependency_indices) } From 151a5a3f45d56cf2f7f798da7239e0b969256277 Mon Sep 17 00:00:00 2001 From: quininer Date: Thu, 2 May 2024 10:43:56 +0800 Subject: [PATCH 6/6] Use usize::from --- .../esm/harmony_export_imported_specifier_dependency.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs b/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs index 78c0b301bcd2..6b243914c3e2 100644 --- a/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs +++ b/crates/rspack_plugin_javascript/src/dependency/esm/harmony_export_imported_specifier_dependency.rs @@ -1358,7 +1358,7 @@ fn determine_export_assignments<'a>( // js `Set` keep the insertion order, use `IndexSet` to align there behavior let mut names: IndexSet<&Atom, BuildHasherDefault> = IndexSet::default(); let mut dependency_indices = - Vec::with_capacity(dependencies.len() + additional_dependency.is_some() as usize); + Vec::with_capacity(dependencies.len() + usize::from(additional_dependency.is_some())); for dependency in dependencies.iter().chain(additional_dependency.iter()) { if let Some(module_identifier) = module_graph.module_identifier_by_dependency_id(dependency) {