Skip to content

Commit

Permalink
fix: side effects optimize for dynamic reexports (#8125)
Browse files Browse the repository at this point in the history
  • Loading branch information
ahabhgk authored Oct 15, 2024
1 parent 1229b37 commit 45c0b20
Show file tree
Hide file tree
Showing 20 changed files with 295 additions and 124 deletions.
173 changes: 154 additions & 19 deletions crates/rspack_core/src/exports_info.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::borrow::Cow;
use std::collections::hash_map::Entry;
use std::collections::BTreeMap;
use std::collections::VecDeque;
use std::rc::Rc;
use std::sync::atomic::AtomicU32;
use std::sync::atomic::Ordering::Relaxed;
Expand Down Expand Up @@ -1063,7 +1064,7 @@ impl ExportInfo {
if info.terminal_binding {
return Some(TerminalBinding::ExportInfo(*self));
}
let target = self.get_target(mg, None)?;
let target = self.get_target(mg)?;
let exports_info = mg.get_exports_info(&target.module);
let Some(export) = target.export else {
return Some(TerminalBinding::ExportsInfo(exports_info));
Expand Down Expand Up @@ -1137,15 +1138,12 @@ impl ExportInfo {
false
}

pub fn get_target(
&self,
mg: &ModuleGraph,
resolve_filter: Option<ResolveFilterFnTy>,
) -> Option<ResolvedExportInfoTarget> {
let filter = resolve_filter.unwrap_or(Rc::new(|_, _| true));

// This is a limited version of [ExportInfo::get_target_mut]:
// 1. mg is &ModuleGraph -> Won't modify module graph, more friendly for cache and parallel
// 2. don't support resolve_filter -> Just get the target module of the terminal binding
pub fn get_target(&self, mg: &ModuleGraph) -> Option<ResolvedExportInfoTarget> {
let mut already_visited = UkeySet::default();
match self._get_target(mg, filter, &mut already_visited) {
match self._get_target(mg, &mut already_visited) {
Some(ResolvedExportInfoTargetWithCircular::Circular) => None,
Some(ResolvedExportInfoTargetWithCircular::Target(target)) => Some(target),
None => None,
Expand All @@ -1155,7 +1153,6 @@ impl ExportInfo {
fn _get_target(
&self,
mg: &ModuleGraph,
resolve_filter: ResolveFilterFnTy,
already_visited: &mut UkeySet<ExportInfo>,
) -> Option<ResolvedExportInfoTargetWithCircular> {
let self_export_info = mg.get_export_info_by_id(self);
Expand All @@ -1173,7 +1170,81 @@ impl ExportInfo {
export: item.export.clone(),
});

let target = resolve_target(values.next(), already_visited, resolve_filter.clone(), mg);
let target = resolve_target(values.next(), already_visited, mg);

match target {
Some(ResolvedExportInfoTargetWithCircular::Circular) => {
Some(ResolvedExportInfoTargetWithCircular::Circular)
}
None => None,
Some(ResolvedExportInfoTargetWithCircular::Target(target)) => {
for val in values {
let resolved_target = resolve_target(Some(val), already_visited, mg);
match resolved_target {
Some(ResolvedExportInfoTargetWithCircular::Circular) => {
return Some(ResolvedExportInfoTargetWithCircular::Circular);
}
Some(ResolvedExportInfoTargetWithCircular::Target(tt)) => {
if target.module != tt.module {
return None;
}
if target.export != tt.export {
return None;
}
}
None => return None,
}
}
Some(ResolvedExportInfoTargetWithCircular::Target(target))
}
}
}

// This is the complete port of the webpack `getTarget`, with a mutable ModuleGraph
// For now only side_effects_flag_plugin need to modify module graph when getting
// the target module, to better support dynamic-reexports (webpack-test/cases/side-effects/dynamic-reexports)
pub fn get_target_mut(
&self,
mg: &mut ModuleGraph,
resolve_filter: ResolveFilterFnTy,
) -> Option<ResolvedExportInfoTarget> {
let mut already_visited = UkeySet::default();
match self._get_target_mut(mg, resolve_filter, &mut already_visited) {
Some(ResolvedExportInfoTargetWithCircular::Circular) => None,
Some(ResolvedExportInfoTargetWithCircular::Target(target)) => Some(target),
None => None,
}
}

fn _get_target_mut(
&self,
mg: &mut ModuleGraph,
resolve_filter: ResolveFilterFnTy,
already_visited: &mut UkeySet<ExportInfo>,
) -> Option<ResolvedExportInfoTargetWithCircular> {
let self_export_info = mg.get_export_info_by_id(self);
if !self_export_info.target_is_set || self_export_info.target.is_empty() {
return None;
}
if already_visited.contains(self) {
return Some(ResolvedExportInfoTargetWithCircular::Circular);
}
already_visited.insert(*self);

let max_target = self.get_max_target(mg);
let mut values = max_target
.values()
.map(|item| UnResolvedExportInfoTarget {
dependency: item.dependency,
export: item.export.clone(),
})
.collect::<VecDeque<_>>();
let target = resolve_target_mut(
values.pop_front(),
already_visited,
resolve_filter.clone(),
mg,
);

match target {
Some(ResolvedExportInfoTargetWithCircular::Circular) => {
Expand All @@ -1183,7 +1254,7 @@ impl ExportInfo {
Some(ResolvedExportInfoTargetWithCircular::Target(target)) => {
for val in values {
let resolved_target =
resolve_target(Some(val), already_visited, resolve_filter.clone(), mg);
resolve_target_mut(Some(val), already_visited, resolve_filter.clone(), mg);
match resolved_target {
Some(ResolvedExportInfoTargetWithCircular::Circular) => {
return Some(ResolvedExportInfoTargetWithCircular::Circular);
Expand Down Expand Up @@ -1296,7 +1367,7 @@ impl ExportInfo {
resolve_filter: ResolveFilterFnTy,
update_original_connection: UpdateOriginalFunctionTy,
) -> Option<ResolvedExportInfoTarget> {
let target = self._get_target(mg, resolve_filter, &mut UkeySet::default());
let target = self._get_target_mut(mg, resolve_filter, &mut UkeySet::default());

let target = match target {
Some(ResolvedExportInfoTargetWithCircular::Circular) => return None,
Expand Down Expand Up @@ -1630,8 +1701,6 @@ pub enum ResolvedExportInfoTargetWithCircular {
pub type UpdateOriginalFunctionTy =
Arc<dyn Fn(&ResolvedExportInfoTarget, &mut ModuleGraph) -> Option<DependencyId>>;

pub type ResolveFilterFnTy = Rc<dyn Fn(&ResolvedExportInfoTarget, &ModuleGraph) -> bool>;

pub type UsageFilterFnTy<T> = Box<dyn Fn(&T) -> bool>;

impl ExportInfoData {
Expand Down Expand Up @@ -1700,11 +1769,13 @@ impl ExportInfoData {
}
}

fn resolve_target(
pub type ResolveFilterFnTy = Rc<dyn Fn(&ResolvedExportInfoTarget, &mut ModuleGraph) -> bool>;

fn resolve_target_mut(
input_target: Option<UnResolvedExportInfoTarget>,
already_visited: &mut UkeySet<ExportInfo>,
resolve_filter: ResolveFilterFnTy,
mg: &ModuleGraph,
mg: &mut ModuleGraph,
) -> Option<ResolvedExportInfoTargetWithCircular> {
if let Some(input_target) = input_target {
let mut target = ResolvedExportInfoTarget {
Expand All @@ -1730,11 +1801,11 @@ fn resolve_target(
};

let exports_info = mg.get_exports_info(&target.module);
let export_info = exports_info.get_read_only_export_info(mg, name);
let export_info = exports_info.get_export_info(mg, name);
if already_visited.contains(&export_info) {
return Some(ResolvedExportInfoTargetWithCircular::Circular);
}
let new_target = export_info._get_target(mg, resolve_filter.clone(), already_visited);
let new_target = export_info._get_target_mut(mg, resolve_filter.clone(), already_visited);

match new_target {
Some(ResolvedExportInfoTargetWithCircular::Circular) => {
Expand Down Expand Up @@ -1771,6 +1842,70 @@ fn resolve_target(
}
}

fn resolve_target(
input_target: Option<UnResolvedExportInfoTarget>,
already_visited: &mut UkeySet<ExportInfo>,
mg: &ModuleGraph,
) -> Option<ResolvedExportInfoTargetWithCircular> {
if let Some(input_target) = input_target {
let mut target = ResolvedExportInfoTarget {
module: *input_target
.dependency
.and_then(|dep_id| mg.connection_by_dependency_id(&dep_id))
.expect("should have connection")
.module_identifier(),
export: input_target.export,
dependency: input_target.dependency.expect("should have dependency"),
};
if target.export.is_none() {
return Some(ResolvedExportInfoTargetWithCircular::Target(target));
}
loop {
let name = if let Some(export) = target.export.as_ref().and_then(|exports| exports.first()) {
export
} else {
return Some(ResolvedExportInfoTargetWithCircular::Target(target));
};

let exports_info = mg.get_exports_info(&target.module);
let export_info = exports_info.get_read_only_export_info(mg, name);
if already_visited.contains(&export_info) {
return Some(ResolvedExportInfoTargetWithCircular::Circular);
}
let new_target = export_info._get_target(mg, already_visited);

match new_target {
Some(ResolvedExportInfoTargetWithCircular::Circular) => {
return Some(ResolvedExportInfoTargetWithCircular::Circular);
}
None => return Some(ResolvedExportInfoTargetWithCircular::Target(target)),
Some(ResolvedExportInfoTargetWithCircular::Target(t)) => {
// SAFETY: if the target.exports is None, program will not reach here
let target_exports = target.export.as_ref().expect("should have exports");
if target_exports.len() == 1 {
target = t;
if target.export.is_none() {
return Some(ResolvedExportInfoTargetWithCircular::Target(target));
}
} else {
target.module = t.module;
target.dependency = t.dependency;
target.export = if let Some(mut exports) = t.export {
exports.extend_from_slice(&target_exports[1..]);
Some(exports)
} else {
Some(target_exports[1..].to_vec())
}
}
}
}
already_visited.insert(export_info);
}
} else {
None
}
}

#[derive(Debug, PartialEq, Copy, Clone, Default, Hash, PartialOrd, Ord, Eq)]
pub enum UsageState {
Unused = 0,
Expand Down
2 changes: 1 addition & 1 deletion crates/rspack_core/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ fn get_exports_type_impl(
if matches!(export_info.provided(mg), Some(ExportInfoProvided::False)) {
handle_default(default_object)
} else {
let Some(target) = export_info.get_target(mg, None) else {
let Some(target) = export_info.get_target(mg) else {
return ExportsType::Dynamic;
};
if target
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ impl<'a> FlagDependencyExportsState<'a> {
}

// Recalculate target exportsInfo
let target = export_info.get_target(self.mg, None);
let target = export_info.get_target(self.mg);

let mut target_exports_info: Option<ExportsInfo> = None;
if let Some(target) = target {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -772,7 +772,7 @@ impl ModuleConcatenationPlugin {
.iter()
.filter(|export_info| {
export_info.is_reexport(&module_graph)
&& export_info.get_target(&module_graph, None).is_none()
&& export_info.get_target(&module_graph).is_none()
})
.copied()
.collect::<Vec<_>>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -833,21 +833,23 @@ fn do_optimize_incoming_connection(
let cache_clone = cache.clone();
let target = export_info.move_target(
&mut module_graph,
Rc::new(move |target: &ResolvedExportInfoTarget, mg: &ModuleGraph| {
let mut cache = cache_clone.borrow_mut();
let state = if let Some(state) = cache.get(&target.module) {
*state
} else {
let state = mg
.module_by_identifier(&target.module)
.expect("should have module")
.get_side_effects_connection_state(mg, &mut IdentifierSet::default());
cache.insert(target.module, state);
state
};
Rc::new(
move |target: &ResolvedExportInfoTarget, mg: &mut ModuleGraph| {
let mut cache = cache_clone.borrow_mut();
let state = if let Some(state) = cache.get(&target.module) {
*state
} else {
let state = mg
.module_by_identifier(&target.module)
.expect("should have module")
.get_side_effects_connection_state(mg, &mut IdentifierSet::default());
cache.insert(target.module, state);
state
};

state == ConnectionState::Bool(false)
}),
state == ConnectionState::Bool(false)
},
),
Arc::new(
move |target: &ResolvedExportInfoTarget, mg: &mut ModuleGraph| {
if !mg.update_module(&dependency_id, &target.module) {
Expand Down Expand Up @@ -887,10 +889,10 @@ fn do_optimize_incoming_connection(
let export_info = cur_exports_info.get_export_info(&mut module_graph, &ids[0]);

let cache = cache.clone();
let target = export_info.get_target(
&module_graph,
Some(Rc::new(
move |target: &ResolvedExportInfoTarget, mg: &ModuleGraph| {
let target = export_info.get_target_mut(
&mut module_graph,
Rc::new(
move |target: &ResolvedExportInfoTarget, mg: &mut ModuleGraph| {
let mut cache = cache.borrow_mut();
let state = if let Some(state) = cache.get(&target.module) {
*state
Expand All @@ -904,7 +906,7 @@ fn do_optimize_incoming_connection(
};
state == ConnectionState::Bool(false)
},
)),
),
);
let Some(target) = target else {
return;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { Out } from "./lib/a"

it("should generate correct export for dynamic reexports (consume shared module)", () => {
expect(Out).toBe(42)
})
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "./b"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "./c"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const Out = 42;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"sideEffects": false
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
const rspack = require("@rspack/core")

/**@type {import("@rspack/core").Configuration}*/
module.exports = {
optimization: {
sideEffects: true,
},
plugins: [
new rspack.sharing.ConsumeSharedPlugin({
consumes: {
"./lib/c.js": {
singleton: true,
eager: true,
}
}
})
]
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { Out } from "./lib/a"

it("should generate correct export for dynamic reexports (dynamic cjs)", () => {
expect(Out).toBe(42)
})
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "./b"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "./c"
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
let e = exports;
e.Out = 42;
Loading

2 comments on commit 45c0b20

@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-10-15 25af140) Current Change
10000_development-mode + exec 2.12 s ± 39 ms 2.11 s ± 31 ms -0.45 %
10000_development-mode_hmr + exec 665 ms ± 22 ms 677 ms ± 37 ms +1.93 %
10000_production-mode + exec 2.65 s ± 33 ms 2.66 s ± 37 ms +0.41 %
arco-pro_development-mode + exec 1.82 s ± 87 ms 1.79 s ± 71 ms -1.33 %
arco-pro_development-mode_hmr + exec 426 ms ± 1.9 ms 426 ms ± 1.1 ms +0.09 %
arco-pro_production-mode + exec 3.04 s ± 65 ms 3.03 s ± 59 ms -0.23 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.15 s ± 70 ms 3.11 s ± 85 ms -1.29 %
threejs_development-mode_10x + exec 1.64 s ± 15 ms 1.61 s ± 17 ms -2.02 %
threejs_development-mode_10x_hmr + exec 788 ms ± 11 ms 756 ms ± 11 ms -4.16 %
threejs_production-mode_10x + exec 4.97 s ± 28 ms 4.92 s ± 34 ms -1.09 %

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

Please sign in to comment.