Skip to content

Commit

Permalink
perf: ⚡️ reduce alloc in ModuleGraphAccessor impl (#6136)
Browse files Browse the repository at this point in the history 
* perf: ⚡️ reduce alloc

* chore: 🤖 lint
  • Loading branch information
@IWANABETHATGUY
IWANABETHATGUY authored Apr 9, 2024
1 parent 10b9605 commit e77874e
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 45 deletions.
123 changes: 79 additions & 44 deletions crates/rspack_core/src/exports_info.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -969,14 +969,41 @@ impl ExportInfoId {
}
already_visited.insert(*self);

let values = mga
.get_max_target(self)
.values()
.map(|item| UnResolvedExportInfoTarget {
connection: item.connection,
export: item.export.clone(),
})
.collect::<Vec<_>>();
let values = match mga.ty() {
ModuleGraphAccessorMutability::Mutable => mga
.get_max_target(self)
.values()
.map(|item| UnResolvedExportInfoTarget {
connection: item.connection,
export: item.export.clone(),
})
.collect::<Vec<_>>(),
ModuleGraphAccessorMutability::Immutable => {
let export_info = mga.inner().get_export_info_by_id(self);

let map = if export_info.max_target_is_set {
export_info
.get_max_target_readonly()
.values()
.map(|item| UnResolvedExportInfoTarget {
connection: item.connection,
export: item.export.clone(),
})
.collect::<Vec<_>>()
} else {
export_info
._get_max_target()
.values()
.map(|item| UnResolvedExportInfoTarget {
connection: item.connection,
export: item.export.clone(),
})
.collect::<Vec<_>>()
};
map
}
};

let target = ExportInfo::resolve_target(
values.first().cloned(),
already_visited,
Expand Down Expand Up @@ -1980,15 +2007,23 @@ macro_rules! debug_exports_info {
};
}

pub enum ModuleGraphAccessorMutability {
Mutable,
Immutable,
}

pub trait ModuleGraphAccessor<'a> {
fn ty(&self) -> ModuleGraphAccessorMutability;
fn inner(&self) -> &ModuleGraph;

fn connection_by_dependency(&self, dependency_id: DependencyId)
-> Option<&ModuleGraphConnection>;
fn run_resolve_filter(
&mut self,
target: &ResolvedExportInfoTarget,
resolve_filter: &ResolveFilterFnTy,
) -> bool;
fn get_export_info(&mut self, export_info_id: &ExportInfoId) -> Arc<ExportInfo>;
fn get_export_info(&mut self, export_info_id: &ExportInfoId) -> &ExportInfo;
fn get_export_info_id(
&mut self,
name: &Atom,
Expand All @@ -1997,7 +2032,7 @@ pub trait ModuleGraphAccessor<'a> {
fn get_max_target(
&mut self,
export_info_id: &ExportInfoId,
) -> Arc<HashMap<Option<DependencyId>, ExportInfoTargetValue>>;
) -> &HashMap<Option<DependencyId>, ExportInfoTargetValue>;
fn get_module_meta_exports_type(
&mut self,
module_identifier: &ModuleIdentifier,
Expand All @@ -2006,7 +2041,7 @@ pub trait ModuleGraphAccessor<'a> {
&mut self,
name: &Atom,
module_identifier: &ModuleIdentifier,
) -> Option<Arc<ExportInfo>>;
) -> Option<&ExportInfo>;
}

pub struct MutableModuleGraph<'a, 'b> {
Expand Down Expand Up @@ -2036,14 +2071,11 @@ impl<'a, 'b> ModuleGraphAccessor<'a> for MutableModuleGraph<'a, 'b> {
fn get_max_target(
&mut self,
export_info_id: &ExportInfoId,
) -> Arc<HashMap<Option<DependencyId>, ExportInfoTargetValue>> {
Arc::new(
self
.inner
.get_export_info_mut_by_id(export_info_id)
.get_max_target()
.to_owned(),
)
) -> &HashMap<Option<DependencyId>, ExportInfoTargetValue> {
self
.inner
.get_export_info_mut_by_id(export_info_id)
.get_max_target()
}

fn run_resolve_filter(
Expand All @@ -2054,19 +2086,18 @@ impl<'a, 'b> ModuleGraphAccessor<'a> for MutableModuleGraph<'a, 'b> {
resolve_filter(target, self.inner)
}

fn get_export_info(&mut self, export_info_id: &ExportInfoId) -> Arc<ExportInfo> {
Arc::new(self.inner.get_export_info_by_id(export_info_id).to_owned())
fn get_export_info(&mut self, export_info_id: &ExportInfoId) -> &ExportInfo {
self.inner.get_export_info_by_id(export_info_id)
}

fn get_read_only_export_info(
&mut self,
name: &Atom,
module_identifier: &ModuleIdentifier,
) -> Option<Arc<ExportInfo>> {
) -> Option<&ExportInfo> {
self
.inner
.get_read_only_export_info(module_identifier, name.to_owned())
.map(|i| Arc::new(i.to_owned()))
}

fn get_module_meta_exports_type(
Expand All @@ -2086,6 +2117,14 @@ impl<'a, 'b> ModuleGraphAccessor<'a> for MutableModuleGraph<'a, 'b> {
) -> Option<&ModuleGraphConnection> {
self.inner.connection_by_dependency(&dependency_id)
}

fn ty(&self) -> ModuleGraphAccessorMutability {
ModuleGraphAccessorMutability::Mutable
}

fn inner(&self) -> &ModuleGraph {
&*self.inner
}
}

pub struct ImmutableModuleGraph<'a> {
Expand Down Expand Up @@ -2113,25 +2152,14 @@ impl<'a> ModuleGraphAccessor<'a> for ImmutableModuleGraph<'a> {
.id
}

// Due rustc limitation, we can't reference a hashmap generate in function, the impl is just a
// dummy place holder, call site should determine the type of `ModuleGraphAccessor`, and inline
// the function in the call site
fn get_max_target(
&mut self,
export_info_id: &ExportInfoId,
) -> Arc<HashMap<Option<DependencyId>, ExportInfoTargetValue>> {
Arc::new({
let export_info_id = self.inner.get_export_info_by_id(export_info_id);

if export_info_id.max_target_is_set {
export_info_id.get_max_target_readonly().to_owned()
} else {
// FIXME:
// max target should be cached
// but when moduleGraph is immutable and no cached max target
// generate a new one, this should be removed
// when module graph in get_exports and get_referenced_exports
// is refactored to mutable
export_info_id._get_max_target().clone()
}
})
_export_info_id: &ExportInfoId,
) -> &HashMap<Option<DependencyId>, ExportInfoTargetValue> {
unreachable!()
}

fn run_resolve_filter(
Expand All @@ -2142,19 +2170,18 @@ impl<'a> ModuleGraphAccessor<'a> for ImmutableModuleGraph<'a> {
resolve_filter(target, self.inner)
}

fn get_export_info(&mut self, export_info_id: &ExportInfoId) -> Arc<ExportInfo> {
Arc::new(self.inner.get_export_info_by_id(export_info_id).to_owned())
fn get_export_info(&mut self, export_info_id: &ExportInfoId) -> &ExportInfo {
self.inner.get_export_info_by_id(export_info_id)
}

fn get_read_only_export_info(
&mut self,
name: &Atom,
module_identifier: &ModuleIdentifier,
) -> Option<Arc<ExportInfo>> {
) -> Option<&ExportInfo> {
self
.inner
.get_read_only_export_info(module_identifier, name.to_owned())
.map(|i| Arc::new(i.to_owned()))
}

fn get_module_meta_exports_type(
Expand All @@ -2174,6 +2201,14 @@ impl<'a> ModuleGraphAccessor<'a> for ImmutableModuleGraph<'a> {
) -> Option<&ModuleGraphConnection> {
self.inner.connection_by_dependency(&dependency_id)
}

fn ty(&self) -> ModuleGraphAccessorMutability {
ModuleGraphAccessorMutability::Immutable
}

fn inner(&self) -> &ModuleGraph {
self.inner
}
}

pub fn prepare_get_exports_type<'a>(mg: &'a mut ModuleGraph<'a>) {
Expand Down
3 changes: 2 additions & 1 deletion crates/rspack_core/src/module.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -401,10 +401,11 @@ fn get_exports_type_impl(
if let Some(export_info) =
mga.get_read_only_export_info(&Atom::from("__esModule"), &identifier)
{
let export_info_id = export_info.id;
if matches!(export_info.provided, Some(ExportInfoProvided::False)) {
handle_default(default_object)
} else {
let Some(target) = export_info.id.get_target(mga, None) else {
let Some(target) = export_info_id.get_target(mga, None) else {
return ExportsType::Dynamic;
};
if target
Expand Down

2 comments on commit e77874e

@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 ✅ success
rspress, ubuntu-latest ✅ success
rsbuild, ubuntu-latest ❌ failure
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-04-09 10b9605) Current Change
10000_development-mode + exec 2.65 s ± 36 ms 2.66 s ± 39 ms +0.35 %
10000_development-mode_hmr + exec 670 ms ± 3.3 ms 671 ms ± 17 ms +0.12 %
10000_production-mode + exec 2.7 s ± 40 ms 2.66 s ± 30 ms -1.57 %
arco-pro_development-mode + exec 2.51 s ± 40 ms 2.52 s ± 57 ms +0.69 %
arco-pro_development-mode_hmr + exec 444 ms ± 2.3 ms 442 ms ± 3.6 ms -0.39 %
arco-pro_development-mode_hmr_intercept-plugin + exec 454 ms ± 2.7 ms 453 ms ± 2.5 ms -0.21 %
arco-pro_development-mode_intercept-plugin + exec 3.25 s ± 57 ms 3.25 s ± 66 ms +0.02 %
arco-pro_production-mode + exec 3.97 s ± 79 ms 3.93 s ± 101 ms -1.05 %
arco-pro_production-mode_intercept-plugin + exec 4.72 s ± 87 ms 4.75 s ± 153 ms +0.67 %
threejs_development-mode_10x + exec 2 s ± 22 ms 2 s ± 21 ms -0.42 %
threejs_development-mode_10x_hmr + exec 727 ms ± 21 ms 710 ms ± 17 ms -2.38 %
threejs_production-mode_10x + exec 5.28 s ± 37 ms 5.35 s ± 129 ms +1.33 %

Please sign in to comment.