Skip to content

Commit

Permalink
perf: avoid heap allocation for getting connections (#8625)
Browse files Browse the repository at this point in the history
  • Loading branch information
ahabhgk authored Dec 4, 2024
1 parent 1702843 commit c26a11a
Show file tree
Hide file tree
Showing 10 changed files with 48 additions and 67 deletions.
6 changes: 1 addition & 5 deletions crates/rspack_core/src/build_chunk_graph/code_splitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1567,17 +1567,13 @@ Or do you want to use the entrypoints '{name}' and '{runtime}' independently on
map.insert(b.into(), Vec::new());
}

let sorted_connections = module_graph
.get_ordered_connections(&module)
.expect("should have module");

// keep the dependency order sorted by span
let mut connection_map: IndexMap<
(DependenciesBlockIdentifier, ModuleIdentifier),
Vec<DependencyId>,
> = IndexMap::default();

for dep_id in sorted_connections {
for dep_id in module_graph.get_ordered_all_dependencies(&module) {
let dep = module_graph
.dependency_by_id(dep_id)
.expect("should have dep");
Expand Down
38 changes: 16 additions & 22 deletions crates/rspack_core/src/chunk_graph/chunk_graph_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,29 +268,23 @@ impl ChunkGraph {
.hash(&mut hasher);
let strict = module.get_strict_esm_module();
let mg = compilation.get_module_graph();
let connections = mg
.get_outgoing_connections(&module.identifier())
.into_iter()
.collect::<Vec<_>>();
if !connections.is_empty() {
let mut visited_modules = IdentifierSet::default();
visited_modules.insert(module.identifier());
for connection in connections {
let module_identifier = connection.module_identifier();
if visited_modules.contains(module_identifier) {
continue;
}
if connection.active_state(&mg, runtime).is_false() {
continue;
}
visited_modules.insert(*module_identifier);
let module = mg
.module_by_identifier(module_identifier)
.expect("should have module")
.as_ref();
module.get_exports_type(&mg, strict).hash(&mut hasher);
self.get_module_graph_hash_without_connections(module, compilation, runtime);
let mut visited_modules = IdentifierSet::default();
visited_modules.insert(module.identifier());
for connection in mg.get_outgoing_connections(&module.identifier()) {
let module_identifier = connection.module_identifier();
if visited_modules.contains(module_identifier) {
continue;
}
if connection.active_state(&mg, runtime).is_false() {
continue;
}
visited_modules.insert(*module_identifier);
let module = mg
.module_by_identifier(module_identifier)
.expect("should have module")
.as_ref();
module.get_exports_type(&mg, strict).hash(&mut hasher);
self.get_module_graph_hash_without_connections(module, compilation, runtime);
}
hasher.finish()
}
Expand Down
1 change: 0 additions & 1 deletion crates/rspack_core/src/compiler/make/cutout/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ impl Cutout {
res.extend(
module_graph
.get_incoming_connections(&module.identifier())
.iter()
.filter_map(|connect| connect.original_module_identifier),
)
}
Expand Down
24 changes: 10 additions & 14 deletions crates/rspack_core/src/concatenated_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,8 @@ impl ConcatenationEntryExternal {
}

#[derive(Debug)]
pub struct ConcatenatedModuleImportInfo {
connection: ModuleGraphConnection,
pub struct ConcatenatedModuleImportInfo<'a> {
connection: &'a ModuleGraphConnection,
source_order: i32,
range_start: Option<u32>,
}
Expand Down Expand Up @@ -201,8 +201,8 @@ pub struct ExternalModuleInfo {
pub name: Option<Atom>,
}

pub struct ConnectionWithRuntimeCondition {
pub connection: ModuleGraphConnection,
pub struct ConnectionWithRuntimeCondition<'a> {
pub connection: &'a ModuleGraphConnection,
pub runtime_condition: RuntimeCondition,
}

Expand Down Expand Up @@ -1530,7 +1530,7 @@ impl ConcatenatedModule {
module_set: &mut IdentifierIndexSet,
runtime: Option<&RuntimeSpec>,
mg: &ModuleGraph,
con: ModuleGraphConnection,
con: &ModuleGraphConnection,
runtime_condition: RuntimeCondition,
exists_entry: &mut IdentifierMap<RuntimeCondition>,
list: &mut Vec<ConcatenationEntry>,
Expand Down Expand Up @@ -1592,21 +1592,17 @@ impl ConcatenatedModule {
}
}

fn get_concatenated_imports(
fn get_concatenated_imports<'a>(
&self,
module_id: &ModuleIdentifier,
root_module_id: &ModuleIdentifier,
runtime: Option<&RuntimeSpec>,
mg: &ModuleGraph,
) -> Vec<ConnectionWithRuntimeCondition> {
let mut connections = mg
.get_outgoing_connections(module_id)
.into_iter()
.cloned()
.collect::<Vec<_>>();
mg: &'a ModuleGraph,
) -> Vec<ConnectionWithRuntimeCondition<'a>> {
let mut connections = mg.get_outgoing_connections(module_id).collect::<Vec<_>>();
if module_id == root_module_id {
for c in mg.get_outgoing_connections(&self.id) {
connections.push(c.clone());
connections.push(c);
}
}

Expand Down
1 change: 0 additions & 1 deletion crates/rspack_core/src/incremental/mutations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ impl Mutations {
updated_modules.extend(
module_graph
.get_incoming_connections(module)
.into_iter()
.filter_map(|c| c.original_module_identifier),
);
}
Expand Down
23 changes: 12 additions & 11 deletions crates/rspack_core/src/module_graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::collections::hash_map::Entry;
use rspack_collections::{IdentifierMap, UkeyMap};
use rspack_error::Result;
use rspack_hash::RspackHashDigest;
use rustc_hash::{FxHashMap as HashMap, FxHashSet as HashSet};
use rustc_hash::FxHashMap as HashMap;
use swc_core::ecma::atoms::Atom;

use crate::{
Expand Down Expand Up @@ -789,18 +789,19 @@ impl<'a> ModuleGraph<'a> {
exports_info.get_export_info(self, export_name)
}

pub(crate) fn get_ordered_connections(
pub(crate) fn get_ordered_all_dependencies(
&self,
module_identifier: &ModuleIdentifier,
) -> Option<Vec<&DependencyId>> {
) -> impl Iterator<Item = &DependencyId> {
self
.module_graph_module_by_identifier(module_identifier)
.map(|m| {
m.all_dependencies
.iter()
.filter(|dep_id| self.connection_by_dependency_id(dep_id).is_some())
.collect()
})
.into_iter()
.flatten()
}

pub fn connection_by_dependency_id(
Expand Down Expand Up @@ -848,7 +849,7 @@ impl<'a> ModuleGraph<'a> {

pub fn is_optional(&self, module_id: &ModuleIdentifier) -> bool {
let mut has_connections = false;
for connection in self.get_incoming_connections(module_id).iter() {
for connection in self.get_incoming_connections(module_id) {
let Some(dependency) = self
.dependency_by_id(&connection.dependency_id)
.and_then(|dep| dep.as_module_dependency())
Expand Down Expand Up @@ -886,33 +887,33 @@ impl<'a> ModuleGraph<'a> {
pub fn get_outgoing_connections(
&self,
module_id: &ModuleIdentifier,
) -> HashSet<&ModuleGraphConnection> {
) -> impl Iterator<Item = &ModuleGraphConnection> + Clone {
self
.module_graph_module_by_identifier(module_id)
.map(|mgm| {
mgm
.outgoing_connections()
.iter()
.filter_map(|id| self.connection_by_dependency_id(id))
.collect()
})
.unwrap_or_default()
.into_iter()
.flatten()
}

pub fn get_incoming_connections(
&self,
module_id: &ModuleIdentifier,
) -> HashSet<&ModuleGraphConnection> {
) -> impl Iterator<Item = &ModuleGraphConnection> + Clone {
self
.module_graph_module_by_identifier(module_id)
.map(|mgm| {
mgm
.incoming_connections()
.iter()
.filter_map(|id| self.connection_by_dependency_id(id))
.collect()
})
.unwrap_or_default()
.into_iter()
.flatten()
}

pub fn get_module_hash(&self, module_id: &ModuleIdentifier) -> Option<&RspackHashDigest> {
Expand Down
4 changes: 1 addition & 3 deletions crates/rspack_plugin_dll/src/lib_manifest_plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,7 @@ async fn emit(&self, compilation: &mut Compilation) -> Result<()> {
for module in chunk_graph.get_ordered_chunk_modules(&chunk.ukey(), &module_graph) {
if self.options.entry_only.unwrap_or_default()
&& !some_in_iterable(
module_graph
.get_incoming_connections(&module.identifier())
.into_iter(),
module_graph.get_incoming_connections(&module.identifier()),
|conn| {
let dep = module_graph.dependency_by_id(&conn.dependency_id);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ fn set_sync_modules(
let module_graph = compilation.get_module_graph();
if module_graph
.get_outgoing_connections(&module)
.iter()
.filter_map(|con| module_graph.module_identifier_by_dependency_id(&con.dependency_id))
.filter(|&out| &module != out)
.any(|module| ModuleGraph::is_async(compilation, module))
Expand All @@ -124,7 +123,6 @@ fn set_sync_modules(
let module_graph = compilation.get_module_graph();
module_graph
.get_incoming_connections(&module)
.iter()
.filter(|con| {
module_graph
.dependency_by_id(&con.dependency_id)
Expand Down Expand Up @@ -160,7 +158,6 @@ fn set_async_modules(
let module_graph = compilation.get_module_graph();
module_graph
.get_incoming_connections(&module)
.iter()
.filter(|con| {
module_graph
.dependency_by_id(&con.dependency_id)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ async fn finish_modules(&self, compilation: &mut Compilation) -> Result<()> {
let mut deps_to_replace = Vec::new();
let module = mg.module_by_identifier(module_id).expect("should have mgm");

let connections = mg.get_outgoing_connections(module_id);
let connections: HashSet<_> = mg.get_outgoing_connections(module_id).collect();
let block_ids = module.get_blocks();

for block_id in block_ids {
Expand Down Expand Up @@ -290,7 +290,7 @@ async fn finish_modules(&self, compilation: &mut Compilation) -> Result<()> {
let mut deps_to_replace = Vec::new();
let mut external_connections = Vec::new();
let module = mg.module_by_identifier(module_id).expect("should have mgm");
let connections = mg.get_outgoing_connections(module_id);
let connections: HashSet<_> = mg.get_outgoing_connections(module_id).collect();
let dep_ids = module.get_dependencies();

let mut module_id_to_connections: IdentifierMap<Vec<DependencyId>> = IdentifierMap::default();
Expand Down
11 changes: 6 additions & 5 deletions crates/rspack_plugin_warn_sensitive_module/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,18 @@ impl WarnCaseSensitiveModulesPlugin {

for m in modules {
if let Some(boxed_m) = graph.module_by_identifier(&m) {
let mut module_msg = format!(" - {}\n", m);
message.push_str(" - ");
message.push_str(&m);
message.push('\n');
graph
.get_incoming_connections(&boxed_m.identifier())
.iter()
.for_each(|c| {
if let Some(original_identifier) = c.original_module_identifier {
module_msg.push_str(&format!(" - used by {}\n", original_identifier));
message.push_str(" - used by ");
message.push_str(&original_identifier);
message.push('\n');
}
});

message.push_str(&module_msg);
}
}

Expand Down

2 comments on commit c26a11a

@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-12-04 044e8e3) Current Change
10000_big_production-mode_disable-minimize + exec 42.6 s ± 1.25 s 36.5 s ± 83 ms -14.26 %
10000_development-mode + exec 1.83 s ± 47 ms 1.79 s ± 27 ms -2.04 %
10000_development-mode_hmr + exec 647 ms ± 3.9 ms 642 ms ± 22 ms -0.82 %
10000_production-mode + exec 2.41 s ± 41 ms 2.33 s ± 20 ms -3.40 %
arco-pro_development-mode + exec 1.75 s ± 70 ms 1.74 s ± 60 ms -0.72 %
arco-pro_development-mode_hmr + exec 426 ms ± 2 ms 424 ms ± 0.8 ms -0.55 %
arco-pro_production-mode + exec 3.1 s ± 71 ms 3.13 s ± 70 ms +0.84 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.13 s ± 77 ms 3.15 s ± 92 ms +0.65 %
threejs_development-mode_10x + exec 1.63 s ± 20 ms 1.63 s ± 15 ms +0.37 %
threejs_development-mode_10x_hmr + exec 807 ms ± 11 ms 804 ms ± 13 ms -0.43 %
threejs_production-mode_10x + exec 4.91 s ± 26 ms 4.9 s ± 34 ms -0.13 %
10000_big_production-mode_disable-minimize + rss memory 13449 MiB ± 447 MiB 10289 MiB ± 44 MiB -23.49 %
10000_development-mode + rss memory 784 MiB ± 25.4 MiB 780 MiB ± 16.2 MiB -0.48 %
10000_development-mode_hmr + rss memory 1896 MiB ± 541 MiB 1918 MiB ± 344 MiB +1.16 %
10000_production-mode + rss memory 663 MiB ± 27.2 MiB 667 MiB ± 43.9 MiB +0.60 %
arco-pro_development-mode + rss memory 731 MiB ± 34.9 MiB 716 MiB ± 30 MiB -2.04 %
arco-pro_development-mode_hmr + rss memory 926 MiB ± 68.8 MiB 879 MiB ± 136 MiB -5.05 %
arco-pro_production-mode + rss memory 834 MiB ± 35.9 MiB 817 MiB ± 66.6 MiB -2.10 %
arco-pro_production-mode_generate-package-json-webpack-plugin + rss memory 879 MiB ± 37.5 MiB 808 MiB ± 58.3 MiB -8.08 %
threejs_development-mode_10x + rss memory 823 MiB ± 46.7 MiB 779 MiB ± 46.6 MiB -5.33 %
threejs_development-mode_10x_hmr + rss memory 2147 MiB ± 152 MiB 1783 MiB ± 231 MiB -16.98 %
threejs_production-mode_10x + rss memory 1034 MiB ± 53.5 MiB 1103 MiB ± 70.1 MiB +6.66 %

@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 ✅ success
_selftest ✅ success
rsdoctor ✅ success
rspress ✅ success
rslib ✅ success
rsbuild ✅ success
examples ✅ success
devserver ✅ success
nuxt ✅ success

Please sign in to comment.