Skip to content

Commit

Permalink
fix: use topological order when SideEffectsFlagPlugin optimize incomi…
Browse files Browse the repository at this point in the history
…ng connections (#7717)
  • Loading branch information
ahabhgk authored Aug 29, 2024
1 parent cc9c1f2 commit 216e6d4
Show file tree
Hide file tree
Showing 12 changed files with 286 additions and 154 deletions.
27 changes: 16 additions & 11 deletions crates/rspack_core/src/compiler/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1077,20 +1077,11 @@ impl Compilation {
.finish_modules
.call(self)
.await?;
logger.time_end(start);
// Collect dependencies diagnostics at here to make sure:
// 1. after finish_modules: has provide exports info
// 2. before optimize dependencies: side effects free module hasn't been skipped (move_target)
let module_graph = self.get_module_graph();
let diagnostics: Vec<_> = module_graph
.module_graph_modules()
.par_iter()
.flat_map(|(_, mgm)| &mgm.all_dependencies)
.filter_map(|dependency_id| module_graph.dependency_by_id(dependency_id))
.filter_map(|dependency| dependency.get_diagnostics(&module_graph))
.flat_map(|ds| ds)
.collect();
self.extend_diagnostics(diagnostics);
logger.time_end(start);
self.collect_dependencies_diagnostics();

// take make diagnostics
let diagnostics = self.make_artifact.take_diagnostics();
Expand All @@ -1110,6 +1101,20 @@ impl Compilation {
Ok(())
}

#[tracing::instrument(skip_all)]
fn collect_dependencies_diagnostics(&mut self) {
let module_graph = self.get_module_graph();
let diagnostics: Vec<_> = module_graph
.module_graph_modules()
.par_iter()
.flat_map(|(_, mgm)| &mgm.all_dependencies)
.filter_map(|dependency_id| module_graph.dependency_by_id(dependency_id))
.filter_map(|dependency| dependency.get_diagnostics(&module_graph))
.flat_map(|ds| ds)
.collect();
self.extend_diagnostics(diagnostics);
}

#[instrument(name = "compilation:seal", skip_all)]
pub async fn seal(&mut self, plugin_driver: SharedPluginDriver) -> Result<()> {
self.other_module_graph = Some(ModuleGraphPartial::default());
Expand Down
2 changes: 1 addition & 1 deletion crates/rspack_core/src/exports_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1590,7 +1590,7 @@ pub struct ResolvedExportInfoTarget {
pub module: ModuleIdentifier,
pub export: Option<Vec<Atom>>,
/// using dependency id to retrieve Connection
connection: DependencyId,
pub connection: DependencyId,
}

#[derive(Clone, Debug)]
Expand Down
9 changes: 7 additions & 2 deletions crates/rspack_core/src/module_graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1034,15 +1034,19 @@ impl<'a> ModuleGraph<'a> {
.insert(dep_id, DependencyExtraMeta { ids });
}

pub fn update_module(&mut self, dep_id: &DependencyId, module_id: &ModuleIdentifier) {
pub fn update_module(
&mut self,
dep_id: &DependencyId,
module_id: &ModuleIdentifier,
) -> Option<ConnectionId> {
let connection_id = *self
.connection_id_by_dependency_id(dep_id)
.expect("should have connection id");
let connection = self
.connection_by_connection_id_mut(&connection_id)
.expect("should have connection");
if connection.module_identifier() == module_id {
return;
return None;
}

// clone connection
Expand Down Expand Up @@ -1097,6 +1101,7 @@ impl<'a> ModuleGraph<'a> {
mgm.add_incoming_connection(new_connection_id);
mgm.remove_incoming_connection(&connection_id);
}
Some(new_connection_id)
}

pub fn get_exports_info(&self, module_identifier: &ModuleIdentifier) -> ExportsInfo {
Expand Down
Loading

2 comments on commit 216e6d4

@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-08-29 cc9c1f2) Current Change
10000_development-mode + exec 2.28 s ± 33 ms 2.28 s ± 34 ms +0.25 %
10000_development-mode_hmr + exec 706 ms ± 19 ms 711 ms ± 16 ms +0.71 %
10000_production-mode + exec 2.94 s ± 18 ms 2.96 s ± 29 ms +0.89 %
arco-pro_development-mode + exec 1.89 s ± 63 ms 1.9 s ± 62 ms +0.42 %
arco-pro_development-mode_hmr + exec 436 ms ± 4 ms 435 ms ± 2.3 ms -0.33 %
arco-pro_production-mode + exec 3.44 s ± 73 ms 3.44 s ± 75 ms +0.07 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.51 s ± 60 ms 3.5 s ± 61 ms -0.16 %
threejs_development-mode_10x + exec 1.69 s ± 14 ms 1.69 s ± 14 ms -0.12 %
threejs_development-mode_10x_hmr + exec 812 ms ± 7.9 ms 812 ms ± 12 ms +0.04 %
threejs_production-mode_10x + exec 5.49 s ± 31 ms 5.47 s ± 45 ms -0.24 %

@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
nx ❌ failure
rspress ✅ success
rslib ❌ failure
rsbuild ✅ success
examples ✅ success

Please sign in to comment.