From 1f491cf128e34532f2c672778d4e6d3f65179d7c Mon Sep 17 00:00:00 2001 From: ahabhgk Date: Thu, 19 Dec 2024 19:51:05 +0800 Subject: [PATCH 1/3] perf: parallelize side effects optimization --- crates/rspack_core/src/exports_info.rs | 101 ++++--- crates/rspack_core/src/module_graph/mod.rs | 15 +- .../src/plugin/side_effects_flag_plugin.rs | 282 ++++++++---------- 3 files changed, 193 insertions(+), 205 deletions(-) diff --git a/crates/rspack_core/src/exports_info.rs b/crates/rspack_core/src/exports_info.rs index 33c653605dd..57014c48a99 100644 --- a/crates/rspack_core/src/exports_info.rs +++ b/crates/rspack_core/src/exports_info.rs @@ -1307,43 +1307,6 @@ impl ExportInfo { false } - pub fn move_target( - &self, - mg: &mut ModuleGraph, - resolve_filter: ResolveFilterFnTy, - update_original_connection: UpdateOriginalFunctionTy, - ) -> Option { - let target = self.get_target_with_filter(mg, resolve_filter)?; - let max_target = self.get_max_target(mg); - let original_target = max_target - .values() - .next() - .expect("should have export info target"); // refer https://github.com/webpack/webpack/blob/ac7e531436b0d47cd88451f497cdfd0dad41535d/lib/ExportsInfo.js#L1388-L1394 - if original_target.dependency.as_ref() == Some(&target.dependency) - && original_target.export == target.export - { - return None; - } - let export_info_mut = self.as_export_info_mut(mg); - export_info_mut.target.clear(); - let updated_dependency_id = update_original_connection(&target, mg); - - // shadowning `export_info_mut` to reduce `&mut ModuleGraph` borrow life time, since - // `update_original_connection` also needs `&mut ModuleGraph` - let export_info_mut = self.as_export_info_mut(mg); - export_info_mut.target.insert( - None, - ExportInfoTargetValue { - dependency: updated_dependency_id, - export: target.export.clone(), - priority: 0, - }, - ); - - export_info_mut.target_is_set = true; - Some(target) - } - pub fn set_used_conditionally( &self, mg: &mut ModuleGraph, @@ -1871,6 +1834,18 @@ impl MaybeDynamicTargetExportInfo { } } + pub fn get_target_with_filter( + &self, + mg: &ModuleGraph, + resolve_filter: ResolveFilterFnTy, + ) -> Option { + match self.get_target_impl(mg, resolve_filter, &mut Default::default()) { + Some(ResolvedExportInfoTargetWithCircular::Circular) => None, + Some(ResolvedExportInfoTargetWithCircular::Target(target)) => Some(target), + None => None, + } + } + fn get_target_impl( &self, mg: &ModuleGraph, @@ -1894,6 +1869,58 @@ impl MaybeDynamicTargetExportInfo { } } } + + fn get_max_target<'a>( + &'a self, + mg: &'a ModuleGraph, + ) -> Cow<'a, HashMap, ExportInfoTargetValue>> { + match self { + MaybeDynamicTargetExportInfo::Static(export_info) => export_info.get_max_target(mg), + MaybeDynamicTargetExportInfo::Dynamic { data, .. } => data.get_max_target(), + } + } +} + +impl MaybeDynamicTargetExportInfo { + pub fn can_move_target( + &self, + mg: &ModuleGraph, + resolve_filter: ResolveFilterFnTy, + ) -> Option { + let target = self.get_target_with_filter(mg, resolve_filter)?; + let max_target = self.get_max_target(mg); + let original_target = max_target + .values() + .next() + .expect("should have export info target"); // refer https://github.com/webpack/webpack/blob/ac7e531436b0d47cd88451f497cdfd0dad41535d/lib/ExportsInfo.js#L1388-L1394 + if original_target.dependency.as_ref() == Some(&target.dependency) + && original_target.export == target.export + { + return None; + } + Some(target) + } +} + +impl ExportInfo { + pub fn do_move_target( + &self, + mg: &mut ModuleGraph, + dependency: DependencyId, + target_export: Option>, + ) { + let export_info_mut = self.as_export_info_mut(mg); + export_info_mut.target.clear(); + export_info_mut.target.insert( + None, + ExportInfoTargetValue { + dependency: Some(dependency), + export: target_export, + priority: 0, + }, + ); + export_info_mut.target_is_set = true; + } } pub type ResolveFilterFnTy<'a> = Rc bool + 'a>; diff --git a/crates/rspack_core/src/module_graph/mod.rs b/crates/rspack_core/src/module_graph/mod.rs index 0ee3e6cf8e0..fb9cd230962 100644 --- a/crates/rspack_core/src/module_graph/mod.rs +++ b/crates/rspack_core/src/module_graph/mod.rs @@ -937,16 +937,20 @@ impl<'a> ModuleGraph<'a> { .insert(dep_id, DependencyExtraMeta { ids }); } - pub fn update_module(&mut self, dep_id: &DependencyId, module_id: &ModuleIdentifier) -> bool { + pub fn can_update_module(&self, dep_id: &DependencyId, module_id: &ModuleIdentifier) -> bool { + let connection = self + .connection_by_dependency_id(dep_id) + .expect("should have connection"); + connection.module_identifier() != module_id + } + + pub fn do_update_module(&mut self, dep_id: &DependencyId, module_id: &ModuleIdentifier) { let connection = self .connection_by_dependency_id_mut(dep_id) .expect("should have connection"); let old_module_identifier = *connection.module_identifier(); - if &old_module_identifier == module_id { - return false; - } - connection.set_module_identifier(*module_id); + // remove dep_id from old module mgm incoming connection let old_mgm = self .module_graph_module_by_identifier_mut(&old_module_identifier) @@ -958,7 +962,6 @@ impl<'a> ModuleGraph<'a> { .module_graph_module_by_identifier_mut(module_id) .expect("should exist mgm"); new_mgm.add_incoming_connection(*dep_id); - true } pub fn get_exports_info(&self, module_identifier: &ModuleIdentifier) -> ExportsInfo { diff --git a/crates/rspack_plugin_javascript/src/plugin/side_effects_flag_plugin.rs b/crates/rspack_plugin_javascript/src/plugin/side_effects_flag_plugin.rs index 0544132b1c4..dafab86a9a1 100644 --- a/crates/rspack_plugin_javascript/src/plugin/side_effects_flag_plugin.rs +++ b/crates/rspack_plugin_javascript/src/plugin/side_effects_flag_plugin.rs @@ -1,13 +1,12 @@ use std::fmt::Debug; use std::rc::Rc; -use std::sync::Arc; use std::sync::LazyLock; use rayon::prelude::*; use rspack_collections::IdentifierMap; -use rspack_collections::IdentifierSet; -use rspack_collections::UkeyMap; use rspack_core::DependencyId; +use rspack_core::MaybeDynamicTargetExportInfo; +use rspack_core::ModuleGraphConnection; use rspack_core::{ BoxModule, Compilation, CompilationOptimizeDependencies, ConnectionState, FactoryMeta, ModuleFactoryCreateData, ModuleGraph, ModuleIdentifier, NormalModuleCreateData, @@ -17,6 +16,7 @@ use rspack_error::Result; use rspack_hook::{plugin, plugin_hook}; use rspack_paths::AssertUtf8; use rspack_paths::Utf8Path; +use rspack_util::atom::Atom; use sugar_path::SugarPath; use swc_core::common::comments::Comments; use swc_core::common::{comments, Span, Spanned, SyntaxContext, GLOBALS}; @@ -668,198 +668,148 @@ fn optimize_dependencies(&self, compilation: &mut Compilation) -> Result> = module_graph + + let mut do_optimizes: Vec = module_graph .modules() .keys() .par_bridge() - .filter(|module_identifier| { - side_effects_state_map[module_identifier] == ConnectionState::Bool(false) - }) - .filter_map(|module_identifier| { - let pairs: UkeyMap = module_graph - .get_incoming_connections(module_identifier) + .filter(|module| side_effects_state_map[module] == ConnectionState::Bool(false)) + .flat_map_iter(|module| { + module_graph + .get_incoming_connections(module) .filter_map(|connection| { - let origin_module = connection.original_module_identifier?; - let dep = module_graph.dependency_by_id(&connection.dependency_id)?; - let is_reexport = dep.is::(); - let is_valid_import_specifier_dep = dep - .downcast_ref::() - .map(|import_specifier_dep| !import_specifier_dep.namespace_object_as_context) - .unwrap_or_default(); - if !is_reexport && !is_valid_import_specifier_dep { - return None; - } - Some((connection.dependency_id, origin_module)) + can_optimize_connection(connection, &side_effects_state_map, &module_graph) }) - .collect(); - if pairs.is_empty() { - return None; - } - Some((*module_identifier, pairs)) }) .collect(); - let mut module_graph = compilation.get_module_graph_mut(); - let mut new_connections = Default::default(); - let to_be_optimized_modules: IdentifierSet = to_be_optimized.keys().copied().collect(); - for module in to_be_optimized_modules { - optimize_incoming_connections( - module, - &side_effects_state_map, - &mut to_be_optimized, - &mut new_connections, - &mut module_graph, - ); + while !do_optimizes.is_empty() { + let mut module_graph = compilation.get_module_graph_mut(); + let new_connections: Vec<_> = do_optimizes + .into_iter() + .map(|do_optimize| do_optimize_connection(do_optimize, &mut module_graph)) + .collect(); + let module_graph = compilation.get_module_graph(); + do_optimizes = new_connections + .into_par_iter() + .filter(|(_, module)| side_effects_state_map[module] == ConnectionState::Bool(false)) + .filter_map(|(connection, _)| module_graph.connection_by_dependency_id(&connection)) + .filter_map(|connection| { + can_optimize_connection(connection, &side_effects_state_map, &module_graph) + }) + .collect(); } + Ok(None) } -#[tracing::instrument(skip_all)] -fn optimize_incoming_connections( - module_identifier: ModuleIdentifier, - side_effects_state_map: &IdentifierMap, - to_be_optimized: &mut IdentifierMap>, - new_connections: &mut IdentifierMap>, - module_graph: &mut ModuleGraph, -) { - let Some(incoming_connections) = to_be_optimized.remove(&module_identifier) else { - return; - }; - for (dep_id, origin_module) in incoming_connections { - optimize_incoming_connection( - module_identifier, - origin_module, - dep_id, - side_effects_state_map, - to_be_optimized, - new_connections, - module_graph, - ); - } - // It is possible to add additional new connections when optimizing module's incoming connections - while let Some(connections) = new_connections.remove(&module_identifier) { - for (dep_id, origin_module) in connections { - optimize_incoming_connection( - module_identifier, - origin_module, - dep_id, - side_effects_state_map, - to_be_optimized, - new_connections, - module_graph, - ); - } - } +struct DoOptimize { + ids: Vec, + dependency: DependencyId, + target_module: ModuleIdentifier, + kind: DoOptimizeKind, } -fn optimize_incoming_connection( - module_identifier: ModuleIdentifier, - origin_module: ModuleIdentifier, - dependency_id: DependencyId, - side_effects_state_map: &IdentifierMap, - to_be_optimized: &mut IdentifierMap>, - new_connections: &mut IdentifierMap>, - module_graph: &mut ModuleGraph, -) { - // For the best optimization results, connection.origin_module must optimize before connection.module - // See: https://github.com/webpack/webpack/pull/17595 - optimize_incoming_connections( - origin_module, - side_effects_state_map, - to_be_optimized, - new_connections, - module_graph, - ); - do_optimize_incoming_connection( - dependency_id, - module_identifier, - origin_module, - side_effects_state_map, - new_connections, - module_graph, - ); +enum DoOptimizeKind { + ExportImportedSpecifier { + export_info: MaybeDynamicTargetExportInfo, + target_export: Option>, + }, + ImportSpecifier, } #[tracing::instrument(skip_all)] -fn do_optimize_incoming_connection( - dependency_id: DependencyId, - module_identifier: ModuleIdentifier, - origin_module: ModuleIdentifier, - side_effects_state_map: &IdentifierMap, - new_connections: &mut IdentifierMap>, +fn do_optimize_connection( + do_optimize: DoOptimize, module_graph: &mut ModuleGraph, -) { - if let Some(connections) = new_connections.get_mut(&module_identifier) { - connections.remove(&dependency_id); - } - if let Some(name) = module_graph - .dependency_by_id(&dependency_id) - .expect("should have dep") - .downcast_ref::() - .and_then(|dep| dep.name.clone()) +) -> (DependencyId, ModuleIdentifier) { + let DoOptimize { + ids, + dependency, + target_module, + kind, + } = do_optimize; + module_graph.do_update_module(&dependency, &target_module); + dependency.set_ids(ids, module_graph); + if let DoOptimizeKind::ExportImportedSpecifier { + export_info: MaybeDynamicTargetExportInfo::Static(export_info), + target_export, + } = kind { - let export_info = module_graph.get_export_info(origin_module, &name); - let target = export_info.move_target( + export_info.do_move_target(module_graph, dependency, target_export); + } + (dependency, target_module) +} + +#[tracing::instrument(skip_all)] +fn can_optimize_connection( + connection: &ModuleGraphConnection, + side_effects_state_map: &IdentifierMap, + module_graph: &ModuleGraph, +) -> Option { + let original_module = connection.original_module_identifier?; + let dependency_id = connection.dependency_id; + let dep = module_graph.dependency_by_id(&dependency_id)?; + let reexport_dep = dep.downcast_ref::(); + let is_reexport = reexport_dep.is_some(); + let is_valid_import_specifier_dep = dep + .downcast_ref::() + .map(|import_specifier_dep| !import_specifier_dep.namespace_object_as_context) + .unwrap_or_default(); + if !is_reexport && !is_valid_import_specifier_dep { + return None; + } + if let Some(name) = reexport_dep.and_then(|dep| dep.name.as_ref()) { + let exports_info = module_graph.get_exports_info(&original_module); + let export_info = exports_info.get_export_info_without_mut_module_graph(module_graph, name); + + let target = export_info.can_move_target( module_graph, Rc::new(|target: &ResolvedExportInfoTarget, _| { side_effects_state_map[&target.module] == ConnectionState::Bool(false) }), - Arc::new( - move |target: &ResolvedExportInfoTarget, mg: &mut ModuleGraph| { - if !mg.update_module(&dependency_id, &target.module) { - return None; - } - // TODO: Explain https://github.com/webpack/webpack/blob/ac7e531436b0d47cd88451f497cdfd0dad41535d/lib/optimize/SideEffectsFlagPlugin.js#L303-L306 - let ids = dependency_id.get_ids(mg); - let processed_ids = target - .export - .as_ref() - .map(|item| { - let mut ret = Vec::from_iter(item.iter().cloned()); - ret.extend_from_slice(ids.get(1..).unwrap_or_default()); - ret - }) - .unwrap_or_else(|| ids.get(1..).unwrap_or_default().to_vec()); - dependency_id.set_ids(processed_ids, mg); - Some(dependency_id) - }, - ), - ); - if let Some(ResolvedExportInfoTarget { - dependency, module, .. - }) = target - { - new_connections - .entry(module) - .or_default() - .insert(dependency, origin_module); - }; - return; + )?; + if !module_graph.can_update_module(&dependency_id, &target.module) { + return None; + } + + let ids = dependency_id.get_ids(module_graph); + let processed_ids = target + .export + .as_ref() + .map(|item| { + let mut ret = Vec::from_iter(item.iter().cloned()); + ret.extend_from_slice(ids.get(1..).unwrap_or_default()); + ret + }) + .unwrap_or_else(|| ids.get(1..).unwrap_or_default().to_vec()); + + return Some(DoOptimize { + ids: processed_ids, + dependency: dependency_id, + target_module: target.module, + kind: DoOptimizeKind::ExportImportedSpecifier { + export_info, + target_export: target.export, + }, + }); } let ids = dependency_id.get_ids(module_graph); if !ids.is_empty() { - let cur_exports_info = module_graph.get_exports_info(&module_identifier); - let export_info = cur_exports_info.get_export_info(module_graph, &ids[0]); + let exports_info = module_graph.get_exports_info(connection.module_identifier()); + let export_info = exports_info.get_export_info_without_mut_module_graph(module_graph, &ids[0]); let target = export_info.get_target_with_filter( module_graph, Rc::new(|target: &ResolvedExportInfoTarget, _| { side_effects_state_map[&target.module] == ConnectionState::Bool(false) }), - ); - let Some(target) = target else { - return; - }; + )?; + if !module_graph.can_update_module(&dependency_id, &target.module) { + return None; + } - if !module_graph.update_module(&dependency_id, &target.module) { - return; - }; - new_connections - .entry(target.module) - .or_default() - .insert(dependency_id, module_identifier); - // TODO: Explain https://github.com/webpack/webpack/blob/ac7e531436b0d47cd88451f497cdfd0dad41535d/lib/optimize/SideEffectsFlagPlugin.js#L303-L306 let processed_ids = target .export .map(|mut item| { @@ -867,8 +817,16 @@ fn do_optimize_incoming_connection( item }) .unwrap_or_else(|| ids[1..].to_vec()); - dependency_id.set_ids(processed_ids, module_graph); + + return Some(DoOptimize { + ids: processed_ids, + dependency: dependency_id, + target_module: target.module, + kind: DoOptimizeKind::ImportSpecifier, + }); } + + None } impl Plugin for SideEffectsFlagPlugin { From b0d20b42ec2b142b75cd8415b23f9b733db9ba9b Mon Sep 17 00:00:00 2001 From: ahabhgk Date: Thu, 19 Dec 2024 20:59:06 +0800 Subject: [PATCH 2/3] small refactor --- .../make/cutout/has_module_graph_change.rs | 14 ++-------- .../src/dependency/dependency_id.rs | 16 ----------- .../src/dependency/dependency_trait.rs | 5 ++-- .../src/dependency/runtime_template.rs | 14 ++++++---- crates/rspack_core/src/exports_info.rs | 4 +-- crates/rspack_core/src/module_graph/mod.rs | 6 ++-- .../common_js_export_require_dependency.rs | 22 +++++++-------- ...sm_export_imported_specifier_dependency.rs | 16 +++++++---- .../esm/esm_import_specifier_dependency.rs | 28 +++++++++++-------- .../plugin/flag_dependency_exports_plugin.rs | 2 +- .../src/plugin/side_effects_flag_plugin.rs | 26 ++++++++--------- 11 files changed, 69 insertions(+), 84 deletions(-) diff --git a/crates/rspack_core/src/compiler/make/cutout/has_module_graph_change.rs b/crates/rspack_core/src/compiler/make/cutout/has_module_graph_change.rs index 750e3260e19..4d80170a961 100644 --- a/crates/rspack_core/src/compiler/make/cutout/has_module_graph_change.rs +++ b/crates/rspack_core/src/compiler/make/cutout/has_module_graph_change.rs @@ -61,8 +61,9 @@ impl ModuleDeps { dep.dependency_type(), crate::DependencyType::EsmImportSpecifier ) { - let dep_ids = dep.get_ids(module_graph); - ids.extend(dep_ids.into_iter()); + // TODO: remove Dependency::get_ids once incremental build chunk graph is stable. + let dep_ids = dep._get_ids(module_graph); + ids.extend(dep_ids.iter().cloned()); } } @@ -142,7 +143,6 @@ impl HasModuleGraphChange { mod t { use std::borrow::Cow; - use itertools::Itertools; use rspack_cacheable::{cacheable, cacheable_dyn, with::Skip}; use rspack_collections::Identifiable; use rspack_error::{impl_empty_diagnosable_trait, Diagnostic, Result}; @@ -187,14 +187,6 @@ mod t { &self.id } - fn get_ids(&self, _mg: &ModuleGraph) -> Vec { - self - .ids - .iter() - .map(|id| (*id).to_string().into()) - .collect_vec() - } - fn could_affect_referencing_module(&self) -> AffectType { AffectType::True } diff --git a/crates/rspack_core/src/dependency/dependency_id.rs b/crates/rspack_core/src/dependency/dependency_id.rs index b1ff2f88edb..4f80a6c0124 100644 --- a/crates/rspack_core/src/dependency/dependency_id.rs +++ b/crates/rspack_core/src/dependency/dependency_id.rs @@ -3,9 +3,6 @@ use std::sync::atomic::Ordering::Relaxed; use rspack_cacheable::cacheable; use serde::Serialize; -use swc_core::ecma::atoms::Atom; - -use crate::ModuleGraph; #[cacheable(hashable)] #[derive(Debug, Clone, Copy, Hash, Eq, PartialEq, Ord, PartialOrd, Serialize)] @@ -17,19 +14,6 @@ impl DependencyId { pub fn new() -> Self { Self(DEPENDENCY_ID.fetch_add(1, Relaxed)) } - - pub fn set_ids(&self, ids: Vec, mg: &mut ModuleGraph) { - mg.set_dep_meta(*self, ids); - } - - /// # Panic - /// This method will panic if one of following condition is true: - /// * current dependency id is not belongs to `ESMImportSpecifierDependency` or `ESMExportImportedSpecifierDependency` - /// * current id is not in `ModuleGraph` - pub fn get_ids(&self, mg: &ModuleGraph) -> Vec { - let dep = mg.dependency_by_id(self).expect("should have dep"); - dep.get_ids(mg) - } } impl Default for DependencyId { diff --git a/crates/rspack_core/src/dependency/dependency_trait.rs b/crates/rspack_core/src/dependency/dependency_trait.rs index e5a48f496bf..de7331c2a72 100644 --- a/crates/rspack_core/src/dependency/dependency_trait.rs +++ b/crates/rspack_core/src/dependency/dependency_trait.rs @@ -4,8 +4,8 @@ use dyn_clone::{clone_trait_object, DynClone}; use rspack_cacheable::cacheable_dyn; use rspack_collections::IdentifierSet; use rspack_error::Diagnostic; +use rspack_util::atom::Atom; use rspack_util::ext::AsAny; -use swc_core::ecma::atoms::Atom; use super::dependency_template::AsDependencyTemplate; use super::module_dependency::*; @@ -89,9 +89,10 @@ pub trait Dependency: None } + // TODO: remove this once incremental build chunk graph is stable. // For now only `ESMImportSpecifierDependency` and // `ESMExportImportedSpecifierDependency` can use this method - fn get_ids(&self, _mg: &ModuleGraph) -> Vec { + fn _get_ids<'a>(&'a self, _mg: &'a ModuleGraph) -> &'a [Atom] { unreachable!() } diff --git a/crates/rspack_core/src/dependency/runtime_template.rs b/crates/rspack_core/src/dependency/runtime_template.rs index 9f9cb091f74..ae0b47259db 100644 --- a/crates/rspack_core/src/dependency/runtime_template.rs +++ b/crates/rspack_core/src/dependency/runtime_template.rs @@ -100,7 +100,7 @@ pub fn export_from_import( default_interop: bool, request: &str, import_var: &str, - mut export_name: Vec, + export_name: &[Atom], id: &DependencyId, is_call: bool, call_context: bool, @@ -124,6 +124,7 @@ pub fn export_from_import( let exports_type = get_exports_type(&compilation.get_module_graph(), id, &module.identifier()); + let mut exclude_default_export_name = None; if default_interop { if !export_name.is_empty() && let Some(first_export_name) = export_name.first() @@ -151,7 +152,7 @@ pub fn export_from_import( } } ExportsType::DefaultOnly | ExportsType::DefaultWithNamed => { - export_name = export_name[1..].to_vec(); + exclude_default_export_name = Some(export_name[1..].to_vec()); } _ => {} } @@ -204,6 +205,9 @@ pub fn export_from_import( } } + let export_name = exclude_default_export_name + .as_deref() + .unwrap_or(export_name); if !export_name.is_empty() { let used_name: Cow> = { let exports_info = compilation @@ -212,7 +216,7 @@ pub fn export_from_import( let used = exports_info.get_used_name( &compilation.get_module_graph(), *runtime, - crate::UsedName::Vec(export_name.clone()), + crate::UsedName::Vec(export_name.to_vec()), ); if let Some(used) = used { let used = match used { @@ -223,12 +227,12 @@ pub fn export_from_import( } else { return format!( "{} undefined", - to_normal_comment(&property_access(&export_name, 0)) + to_normal_comment(&property_access(export_name, 0)) ); } }; let comment = if *used_name != export_name { - to_normal_comment(&property_access(&export_name, 0)) + to_normal_comment(&property_access(export_name, 0)) } else { String::new() }; diff --git a/crates/rspack_core/src/exports_info.rs b/crates/rspack_core/src/exports_info.rs index 57014c48a99..9260b31f9e4 100644 --- a/crates/rspack_core/src/exports_info.rs +++ b/crates/rspack_core/src/exports_info.rs @@ -342,14 +342,14 @@ impl ExportsInfo { pub fn get_nested_exports_info( &self, mg: &ModuleGraph, - name: Option>, + name: Option<&[Atom]>, ) -> Option { if let Some(name) = name && !name.is_empty() { let info = self.get_read_only_export_info(mg, &name[0]); if let Some(exports_info) = info.exports_info(mg) { - return exports_info.get_nested_exports_info(mg, Some(name[1..].to_vec())); + return exports_info.get_nested_exports_info(mg, Some(&name[1..])); } else { return None; } diff --git a/crates/rspack_core/src/module_graph/mod.rs b/crates/rspack_core/src/module_graph/mod.rs index fb9cd230962..ec8f751dd12 100644 --- a/crates/rspack_core/src/module_graph/mod.rs +++ b/crates/rspack_core/src/module_graph/mod.rs @@ -928,13 +928,11 @@ impl<'a> ModuleGraph<'a> { self.loop_partials(|p| p.dep_meta_map.get(id)) } - pub fn set_dep_meta(&mut self, dep_id: DependencyId, ids: Vec) { + pub fn set_dependency_extra_meta(&mut self, dep_id: DependencyId, extra: DependencyExtraMeta) { let Some(active_partial) = &mut self.active else { panic!("should have active partial"); }; - active_partial - .dep_meta_map - .insert(dep_id, DependencyExtraMeta { ids }); + active_partial.dep_meta_map.insert(dep_id, extra); } pub fn can_update_module(&self, dep_id: &DependencyId, module_id: &ModuleIdentifier) -> bool { diff --git a/crates/rspack_plugin_javascript/src/dependency/commonjs/common_js_export_require_dependency.rs b/crates/rspack_plugin_javascript/src/dependency/commonjs/common_js_export_require_dependency.rs index 757896632e5..403cd172676 100644 --- a/crates/rspack_plugin_javascript/src/dependency/commonjs/common_js_export_require_dependency.rs +++ b/crates/rspack_plugin_javascript/src/dependency/commonjs/common_js_export_require_dependency.rs @@ -82,7 +82,7 @@ impl CommonJsExportRequireDependency { if !self.names.is_empty() { exports_info = exports_info .expect("Should get exports info from imported module") - .get_nested_exports_info(mg, Some(self.names.clone())); + .get_nested_exports_info(mg, Some(&self.names)); } let no_extra_exports = imported_exports_info.is_some_and(|imported_exports_info| { @@ -164,6 +164,12 @@ impl CommonJsExportRequireDependency { Some(exports) } + + fn get_ids<'a>(&'a self, mg: &'a ModuleGraph) -> &'a [Atom] { + mg.get_dep_meta_if_existing(&self.id) + .map(|meta| meta.ids.as_slice()) + .unwrap_or_else(|| self.ids.as_slice()) + } } #[cacheable_dyn] @@ -196,7 +202,7 @@ impl Dependency for CommonJsExportRequireDependency { export: Some(if ids.is_empty() { Nullable::Null } else { - Nullable::Value(ids) + Nullable::Value(ids.to_vec()) }), ..Default::default() })]), @@ -211,7 +217,7 @@ impl Dependency for CommonJsExportRequireDependency { reexport_info .iter() .map(|name| { - let mut export = ids.clone(); + let mut export = ids.to_vec(); export.extend(vec![name.to_owned()]); ExportNameOrSpec::ExportSpec(ExportSpec { name: name.to_owned(), @@ -255,12 +261,6 @@ impl Dependency for CommonJsExportRequireDependency { } } - fn get_ids(&self, mg: &ModuleGraph) -> Vec { - mg.get_dep_meta_if_existing(&self.id) - .map(|meta| meta.ids.clone()) - .unwrap_or_else(|| self.ids.clone()) - } - fn get_referenced_exports( &self, mg: &ModuleGraph, @@ -272,7 +272,7 @@ impl Dependency for CommonJsExportRequireDependency { vec![ExtendedReferencedExport::Array(vec![])] } else { vec![ExtendedReferencedExport::Export(ReferencedExport { - name: ids.clone(), + name: ids.to_vec(), can_mangle: false, })] } @@ -401,7 +401,7 @@ impl DependencyTemplate for CommonJsExportRequireDependency { let ids = self.get_ids(mg); if let Some(used_imported) = mg .get_exports_info(&imported_module.identifier()) - .get_used_name(mg, *runtime, UsedName::Vec(ids)) + .get_used_name(mg, *runtime, UsedName::Vec(ids.to_vec())) { require_expr = format!( "{}{}", diff --git a/crates/rspack_plugin_javascript/src/dependency/esm/esm_export_imported_specifier_dependency.rs b/crates/rspack_plugin_javascript/src/dependency/esm/esm_export_imported_specifier_dependency.rs index 74feb88f449..38445f880b5 100644 --- a/crates/rspack_plugin_javascript/src/dependency/esm/esm_export_imported_specifier_dependency.rs +++ b/crates/rspack_plugin_javascript/src/dependency/esm/esm_export_imported_specifier_dependency.rs @@ -116,6 +116,12 @@ impl ESMExportImportedSpecifierDependency { } } + pub fn get_ids<'a>(&'a self, mg: &'a ModuleGraph) -> &'a [Atom] { + mg.get_dep_meta_if_existing(&self.id) + .map(|meta| meta.ids.as_slice()) + .unwrap_or_else(|| self.ids.as_slice()) + } + // TODO cache get_mode result fn get_mode( &self, @@ -1230,10 +1236,8 @@ impl Dependency for ESMExportImportedSpecifierDependency { ConnectionState::Bool(false) } - fn get_ids(&self, mg: &ModuleGraph) -> Vec { - mg.get_dep_meta_if_existing(&self.id) - .map(|meta| meta.ids.clone()) - .unwrap_or_else(|| self.ids.clone()) + fn _get_ids<'a>(&'a self, mg: &'a ModuleGraph) -> &'a [Atom] { + self.get_ids(mg) } fn resource_identifier(&self) -> Option<&str> { @@ -1256,7 +1260,7 @@ impl Dependency for ESMExportImportedSpecifierDependency { let mut diagnostics = Vec::new(); if let Some(error) = esm_import_dependency_get_linking_error( self, - &ids, + ids, module_graph, self .name @@ -1268,7 +1272,7 @@ impl Dependency for ESMExportImportedSpecifierDependency { diagnostics.push(error); } if let Some(errors) = - self.get_conflicting_star_exports_errors(&ids, module_graph, should_error) + self.get_conflicting_star_exports_errors(ids, module_graph, should_error) { diagnostics.extend(errors); } diff --git a/crates/rspack_plugin_javascript/src/dependency/esm/esm_import_specifier_dependency.rs b/crates/rspack_plugin_javascript/src/dependency/esm/esm_import_specifier_dependency.rs index d3356446ded..b334422fcb8 100644 --- a/crates/rspack_plugin_javascript/src/dependency/esm/esm_import_specifier_dependency.rs +++ b/crates/rspack_plugin_javascript/src/dependency/esm/esm_import_specifier_dependency.rs @@ -86,16 +86,22 @@ impl ESMImportSpecifierDependency { } } + pub fn get_ids<'a>(&'a self, mg: &'a ModuleGraph) -> &'a [Atom] { + mg.get_dep_meta_if_existing(&self.id) + .map(|meta| meta.ids.as_slice()) + .unwrap_or_else(|| self.ids.as_slice()) + } + pub fn get_referenced_exports_in_destructuring( &self, - ids: Option<&Vec>, + ids: Option<&[Atom]>, ) -> Vec { if let Some(referenced_properties) = &self.referenced_properties_in_destructuring { referenced_properties .iter() .map(|prop| { if let Some(v) = ids { - let mut value = v.clone(); + let mut value = v.to_vec(); value.push(prop.clone()); ReferencedExport::new(value, false) } else { @@ -105,7 +111,7 @@ impl ESMImportSpecifierDependency { .map(ExtendedReferencedExport::Export) .collect::>() } else if let Some(v) = ids { - vec![ExtendedReferencedExport::Array(v.clone())] + vec![ExtendedReferencedExport::Array(v.to_vec())] } else { create_exports_object_referenced() } @@ -191,7 +197,7 @@ impl DependencyTemplate for ESMImportSpecifierDependency { con.module_identifier(), &ModuleReferenceOptions { asi_safe: Some(self.asi_safe), - ids, + ids: ids.to_vec(), call: self.call, direct_import: self.direct_import, ..Default::default() @@ -275,10 +281,8 @@ impl Dependency for ESMImportSpecifierDependency { ConnectionState::Bool(false) } - fn get_ids(&self, mg: &ModuleGraph) -> Vec { - mg.get_dep_meta_if_existing(&self.id) - .map(|meta| meta.ids.clone()) - .unwrap_or_else(|| self.ids.clone()) + fn _get_ids<'a>(&'a self, mg: &'a ModuleGraph) -> &'a [Atom] { + self.get_ids(mg) } fn resource_identifier(&self) -> Option<&str> { @@ -294,7 +298,7 @@ impl Dependency for ESMImportSpecifierDependency { .get_effective_export_presence(&**module) && let Some(diagnostic) = esm_import_dependency_get_linking_error( self, - &self.get_ids(module_graph)[..], + self.get_ids(module_graph), module_graph, format!("(imported as '{}')", self.name), should_error, @@ -329,7 +333,7 @@ impl Dependency for ESMImportSpecifierDependency { if ids.len() == 1 { return self.get_referenced_exports_in_destructuring(None); } - ids.drain(0..1); + ids = &ids[1..]; namespace_object_as_context = true; } ExportsType::Dynamic => { @@ -344,9 +348,9 @@ impl Dependency for ESMImportSpecifierDependency { return create_exports_object_referenced(); } // remove last one - ids.remove(ids.len() - 1); + ids = &ids[..ids.len() - 1]; } - self.get_referenced_exports_in_destructuring(Some(&ids)) + self.get_referenced_exports_in_destructuring(Some(ids)) } fn could_affect_referencing_module(&self) -> rspack_core::AffectType { diff --git a/crates/rspack_plugin_javascript/src/plugin/flag_dependency_exports_plugin.rs b/crates/rspack_plugin_javascript/src/plugin/flag_dependency_exports_plugin.rs index 978ccd83b83..8ecf8e9e2c7 100644 --- a/crates/rspack_plugin_javascript/src/plugin/flag_dependency_exports_plugin.rs +++ b/crates/rspack_plugin_javascript/src/plugin/flag_dependency_exports_plugin.rs @@ -312,7 +312,7 @@ impl<'a> FlagDependencyExportsState<'a> { if let Some(target) = target { let target_module_exports_info = self.mg.get_exports_info(&target.module); target_exports_info = - target_module_exports_info.get_nested_exports_info(self.mg, target.export); + target_module_exports_info.get_nested_exports_info(self.mg, target.export.as_deref()); match self.dependencies.entry(target.module) { Entry::Occupied(mut occ) => { occ.get_mut().insert(self.current_module_id); diff --git a/crates/rspack_plugin_javascript/src/plugin/side_effects_flag_plugin.rs b/crates/rspack_plugin_javascript/src/plugin/side_effects_flag_plugin.rs index dafab86a9a1..808bfa2ba32 100644 --- a/crates/rspack_plugin_javascript/src/plugin/side_effects_flag_plugin.rs +++ b/crates/rspack_plugin_javascript/src/plugin/side_effects_flag_plugin.rs @@ -4,6 +4,7 @@ use std::sync::LazyLock; use rayon::prelude::*; use rspack_collections::IdentifierMap; +use rspack_core::DependencyExtraMeta; use rspack_core::DependencyId; use rspack_core::MaybeDynamicTargetExportInfo; use rspack_core::ModuleGraphConnection; @@ -730,7 +731,7 @@ fn do_optimize_connection( kind, } = do_optimize; module_graph.do_update_module(&dependency, &target_module); - dependency.set_ids(ids, module_graph); + module_graph.set_dependency_extra_meta(dependency, DependencyExtraMeta { ids }); if let DoOptimizeKind::ExportImportedSpecifier { export_info: MaybeDynamicTargetExportInfo::Static(export_info), target_export, @@ -750,16 +751,10 @@ fn can_optimize_connection( let original_module = connection.original_module_identifier?; let dependency_id = connection.dependency_id; let dep = module_graph.dependency_by_id(&dependency_id)?; - let reexport_dep = dep.downcast_ref::(); - let is_reexport = reexport_dep.is_some(); - let is_valid_import_specifier_dep = dep - .downcast_ref::() - .map(|import_specifier_dep| !import_specifier_dep.namespace_object_as_context) - .unwrap_or_default(); - if !is_reexport && !is_valid_import_specifier_dep { - return None; - } - if let Some(name) = reexport_dep.and_then(|dep| dep.name.as_ref()) { + + if let Some(dep) = dep.downcast_ref::() + && let Some(name) = &dep.name + { let exports_info = module_graph.get_exports_info(&original_module); let export_info = exports_info.get_export_info_without_mut_module_graph(module_graph, name); @@ -773,7 +768,7 @@ fn can_optimize_connection( return None; } - let ids = dependency_id.get_ids(module_graph); + let ids = dep.get_ids(module_graph); let processed_ids = target .export .as_ref() @@ -795,8 +790,11 @@ fn can_optimize_connection( }); } - let ids = dependency_id.get_ids(module_graph); - if !ids.is_empty() { + if let Some(dep) = dep.downcast_ref::() + && !dep.namespace_object_as_context + && let ids = dep.get_ids(module_graph) + && !ids.is_empty() + { let exports_info = module_graph.get_exports_info(connection.module_identifier()); let export_info = exports_info.get_export_info_without_mut_module_graph(module_graph, &ids[0]); From 6bf5d4d3f073e417c4b3727270d1b586ec821707 Mon Sep 17 00:00:00 2001 From: ahabhgk Date: Fri, 20 Dec 2024 12:59:57 +0800 Subject: [PATCH 3/3] fix --- .../make/cutout/has_module_graph_change.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/crates/rspack_core/src/compiler/make/cutout/has_module_graph_change.rs b/crates/rspack_core/src/compiler/make/cutout/has_module_graph_change.rs index 4d80170a961..2be1363b979 100644 --- a/crates/rspack_core/src/compiler/make/cutout/has_module_graph_change.rs +++ b/crates/rspack_core/src/compiler/make/cutout/has_module_graph_change.rs @@ -148,7 +148,7 @@ mod t { use rspack_error::{impl_empty_diagnosable_trait, Diagnostic, Result}; use rspack_macros::impl_source_map_config; use rspack_sources::Source; - use rspack_util::source_map::SourceMapKind; + use rspack_util::{atom::Atom, source_map::SourceMapKind}; use crate::{ compiler::make::cutout::has_module_graph_change::ModuleDeps, AffectType, AsContextDependency, @@ -162,12 +162,12 @@ mod t { #[derive(Debug, Clone)] struct TestDep { #[cacheable(with=Skip)] - ids: Vec<&'static str>, + ids: Vec, id: DependencyId, } impl TestDep { - fn new(ids: Vec<&'static str>) -> Self { + fn new(ids: Vec) -> Self { Self { ids, id: DependencyId::new(), @@ -187,6 +187,10 @@ mod t { &self.id } + fn _get_ids<'a>(&'a self, _mg: &'a ModuleGraph) -> &'a [Atom] { + &self.ids + } + fn could_affect_referencing_module(&self) -> AffectType { AffectType::True } @@ -346,7 +350,7 @@ mod t { let mut partial = ModuleGraphPartial::default(); let mut mg = ModuleGraph::new(vec![], Some(&mut partial)); - let dep1 = Box::new(TestDep::new(vec!["foo"])); + let dep1 = Box::new(TestDep::new(vec!["foo".into()])); let dep1_id = *dep1.id(); let module_orig = Box::new(TestModule::new("app", vec![dep1_id])); let module_orig_id = module_orig.identifier(); @@ -366,7 +370,7 @@ mod t { assert_eq!(module_deps_1, module_deps_2); - let dep2 = Box::new(TestDep::new(vec!["bar"])); + let dep2 = Box::new(TestDep::new(vec!["bar".into()])); let dep2_id = *dep2.id(); let module_orig: &mut TestModule = mg .module_by_identifier_mut(&module_orig_id)