Skip to content

Commit

Permalink
perf: improve determine_export_assignments (#6431)
Browse files Browse the repository at this point in the history
* 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.

* perf: improve determine_export_assignments (2)

* eliminate Atom clone
* rewrite collect hidden_exports

* Fix clippy

* Fix fmt

* perf: improve determine_export_assignments (3)

* Use indexmap again

* Use usize::from
  • Loading branch information
quininer authored May 16, 2024
1 parent dc02c55 commit 9572d63
Showing 1 changed file with 28 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -309,14 +309,13 @@ 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::<HashSet<_>>()
});
if !no_extra_exports && !no_extra_imports {
return StarReexportsInfo {
Expand Down Expand Up @@ -419,10 +418,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<DiscoverActiveExportsFromOtherStarExportsRet> {
module_graph: &'a ModuleGraph,
) -> Option<DiscoverActiveExportsFromOtherStarExportsRet<'a>> {
if let Some(other_star_exports) = &self.other_star_exports {
if other_star_exports.is_empty() {
return None;
Expand All @@ -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,
Expand All @@ -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],
Expand Down Expand Up @@ -829,8 +828,8 @@ impl HarmonyExportImportedSpecifierDependency {
}

#[derive(Debug)]
pub struct DiscoverActiveExportsFromOtherStarExportsRet {
names: Vec<Atom>,
pub struct DiscoverActiveExportsFromOtherStarExportsRet<'a> {
names: Vec<&'a Atom>,
names_slice: usize,
pub dependency_indices: Vec<usize>,
pub dependency_index: usize,
Expand Down Expand Up @@ -1350,39 +1349,34 @@ pub struct StarReexportsInfo {
}

/// return (names, dependency_indices)
fn determine_export_assignments(
module_graph: &ModuleGraph,
mut dependencies: Vec<DependencyId>,
fn determine_export_assignments<'a>(
module_graph: &'a ModuleGraph,
dependencies: &[DependencyId],
additional_dependency: Option<DependencyId>,
) -> (Vec<Atom>, Vec<usize>) {
if let Some(additional_dependency) = additional_dependency {
dependencies.push(additional_dependency);
}

) -> (Vec<&'a Atom>, Vec<usize>) {
// 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<Atom, BuildHasherDefault<FxHasher>> = IndexSet::default();
let mut dependency_indices = vec![];
let mut names: IndexSet<&Atom, BuildHasherDefault<FxHasher>> = IndexSet::default();
let mut dependency_indices =
Vec::with_capacity(dependencies.len() + usize::from(additional_dependency.is_some()));

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().expect("export name is empty");
if matches!(export_info.provided, Some(ExportInfoProvided::True))
&& &export_info_name != "default"
&& !names.contains(&export_info_name)
&& export_info_name != "default"
&& !names.contains(export_info_name)
{
names.insert(export_info_name.clone());
let cur_len = dependency_indices.len();
dependency_indices[cur_len - 1] = names.len();
names.insert(export_info_name);
}
}
}
dependency_indices.push(names.len());
}

(names.into_iter().collect::<Vec<Atom>>(), dependency_indices)
(names.into_iter().collect(), dependency_indices)
}

2 comments on commit 9572d63

@rspack-bot
Copy link

Choose a reason for hiding this comment

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

📝 Ran ecosystem CI: Open

suite result
modernjs, self-hosted, Linux, ci ❌ failure
_selftest, ubuntu-latest ✅ success
nx, ubuntu-latest ❌ failure
rspress, ubuntu-latest ✅ success
rsbuild, ubuntu-latest ✅ success
compat, ubuntu-latest ✅ success
examples, ubuntu-latest ❌ failure

@rspack-bot
Copy link

Choose a reason for hiding this comment

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

📝 Benchmark detail: Open

Name Base (2024-05-16 dc02c55) Current Change
10000_development-mode + exec 2.6 s ± 23 ms 2.57 s ± 20 ms -1.31 %
10000_development-mode_hmr + exec 702 ms ± 4.9 ms 683 ms ± 8.3 ms -2.73 %
10000_production-mode + exec 2.49 s ± 26 ms 2.44 s ± 22 ms -2.05 %
arco-pro_development-mode + exec 2.48 s ± 65 ms 2.41 s ± 47 ms -2.75 %
arco-pro_development-mode_hmr + exec 432 ms ± 1.9 ms 430 ms ± 2.3 ms -0.55 %
arco-pro_development-mode_hmr_intercept-plugin + exec 441 ms ± 2.3 ms 439 ms ± 2.7 ms -0.57 %
arco-pro_development-mode_intercept-plugin + exec 3.28 s ± 88 ms 3.25 s ± 79 ms -0.94 %
arco-pro_production-mode + exec 4.07 s ± 89 ms 3.98 s ± 105 ms -2.01 %
arco-pro_production-mode_intercept-plugin + exec 4.86 s ± 77 ms 4.81 s ± 98 ms -1.05 %
threejs_development-mode_10x + exec 1.97 s ± 16 ms 1.95 s ± 21 ms -1.41 %
threejs_development-mode_10x_hmr + exec 755 ms ± 5.7 ms 749 ms ± 8 ms -0.70 %
threejs_production-mode_10x + exec 5.19 s ± 48 ms 5.1 s ± 31 ms -1.75 %

Please sign in to comment.