From c9f4b548cbd6996bec79f92844ca47822543efae Mon Sep 17 00:00:00 2001 From: jinrui Date: Mon, 22 Apr 2024 17:33:10 +0800 Subject: [PATCH] fix: compilation.import_module use immutable ref (#6311) --- .../rspack_binding_values/src/compilation.rs | 9 +-- .../rspack_core/src/compiler/compilation.rs | 10 ++++ crates/rspack_core/src/compiler/mod.rs | 4 +- .../src/compiler/module_executor.rs | 58 +++++++------------ 4 files changed, 37 insertions(+), 44 deletions(-) diff --git a/crates/rspack_binding_values/src/compilation.rs b/crates/rspack_binding_values/src/compilation.rs index 0635f9021fc..449f2ec6e57 100644 --- a/crates/rspack_binding_values/src/compilation.rs +++ b/crates/rspack_binding_values/src/compilation.rs @@ -432,7 +432,7 @@ impl JsCompilation { #[allow(clippy::too_many_arguments)] #[napi] pub fn import_module( - &'static mut self, + &'static self, env: Env, request: String, public_path: Option, @@ -452,7 +452,7 @@ impl JsCompilation { let module_executor = self .0 .module_executor - .as_mut() + .as_ref() .expect("should have module executor"); let result = module_executor .import_module( @@ -491,12 +491,9 @@ impl JsCompilation { .into_iter() .map(|d| d.to_string_lossy().to_string()) .collect(), - assets: res.assets.keys().cloned().collect(), + assets: res.assets.into_iter().collect(), id: res.id, }; - for (filename, asset) in res.assets { - self.0.emit_asset(filename, asset) - } Ok(js_result) } Err(e) => Err(Error::new(napi::Status::GenericFailure, format!("{e}"))), diff --git a/crates/rspack_core/src/compiler/compilation.rs b/crates/rspack_core/src/compiler/compilation.rs index 1fac38b162b..158a72f44fa 100644 --- a/crates/rspack_core/src/compiler/compilation.rs +++ b/crates/rspack_core/src/compiler/compilation.rs @@ -1009,6 +1009,16 @@ impl Compilation { self.create_chunk_assets(plugin_driver.clone()).await?; logger.time_end(start); + // sync assets to compilation from module_executor + let assets = self + .module_executor + .as_mut() + .map(|module_executor| std::mem::take(&mut module_executor.assets)) + .unwrap_or_default(); + for (filename, asset) in assets { + self.emit_asset(filename, asset) + } + let start = logger.time("process assets"); plugin_driver .compilation_hooks diff --git a/crates/rspack_core/src/compiler/mod.rs b/crates/rspack_core/src/compiler/mod.rs index 903d8e8ca38..1b539e3b45c 100644 --- a/crates/rspack_core/src/compiler/mod.rs +++ b/crates/rspack_core/src/compiler/mod.rs @@ -88,7 +88,7 @@ where let (plugin_driver, options) = PluginDriver::new(options, plugins, resolver_factory.clone()); let cache = Arc::new(Cache::new(options.clone())); assert!(!(options.is_new_tree_shaking() && options.builtins.tree_shaking.enable()), "Can't enable builtins.tree_shaking and `experiments.rspack_future.new_treeshaking` at the same time"); - let module_executor = ModuleExecutor::new(options.is_new_tree_shaking()); + let module_executor = ModuleExecutor::default(); Self { options: options.clone(), compilation: Compilation::new( @@ -121,7 +121,7 @@ where // TODO: maybe it's better to use external entries. self.plugin_driver.resolver_factory.clear_cache(); - let module_executor = ModuleExecutor::new(self.options.is_new_tree_shaking()); + let module_executor = ModuleExecutor::default(); fast_set( &mut self.compilation, Compilation::new( diff --git a/crates/rspack_core/src/compiler/module_executor.rs b/crates/rspack_core/src/compiler/module_executor.rs index 46a6cbfb27d..520b73e8919 100644 --- a/crates/rspack_core/src/compiler/module_executor.rs +++ b/crates/rspack_core/src/compiler/module_executor.rs @@ -1,6 +1,7 @@ use std::sync::{atomic::AtomicU32, Arc}; -use std::{collections::hash_map, hash::BuildHasherDefault, iter::once}; +use std::{hash::BuildHasherDefault, iter::once}; +use dashmap::DashMap; use rayon::prelude::*; use rspack_error::Result; use rspack_identifier::{Identifiable, IdentifierSet}; @@ -9,10 +10,10 @@ use rustc_hash::{FxHashMap as HashMap, FxHashSet as HashSet, FxHasher}; use crate::cache::Cache; use crate::{ Chunk, ChunkGraph, ChunkKind, CodeGenerationDataAssetInfo, CodeGenerationDataFilename, - CodeGenerationResult, CompilationAssets, Dependency, DependencyType, EntryDependency, - EntryOptions, Entrypoint, ModuleFactory, ModuleGraphPartial, SourceType, + CodeGenerationResult, Dependency, DependencyType, EntryDependency, EntryOptions, Entrypoint, + ModuleFactory, SourceType, }; -use crate::{Compilation, CompilationAsset, DependencyId, MakeParam}; +use crate::{Compilation, CompilationAsset, MakeParam}; use crate::{CompilerOptions, Context, ResolverFactory, SharedPluginDriver}; static EXECUTE_MODULE_ID: AtomicU32 = AtomicU32::new(0); @@ -24,27 +25,19 @@ pub struct ExecuteModuleResult { pub context_dependencies: HashSet, pub missing_dependencies: HashSet, pub build_dependencies: HashSet, - pub assets: CompilationAssets, + pub assets: HashSet, pub id: ExecuteModuleId, } -#[derive(Debug)] +#[derive(Debug, Default)] pub struct ModuleExecutor { - pub make_module_graph: ModuleGraphPartial, - request_dep_map: HashMap, + pub assets: DashMap, } impl ModuleExecutor { - pub fn new(is_new_treeshaking: bool) -> Self { - Self { - make_module_graph: ModuleGraphPartial::new(is_new_treeshaking), - request_dep_map: HashMap::default(), - } - } - #[allow(clippy::too_many_arguments)] pub async fn import_module( - &mut self, + &self, options: Arc, plugin_driver: SharedPluginDriver, resolver_factory: Arc, @@ -67,21 +60,16 @@ impl ModuleExecutor { None, ); compilation.dependency_factories = dependency_factories; - compilation.swap_make_module_graph(&mut self.make_module_graph); let mut mg = compilation.get_module_graph_mut(); - let dep_id = match self.request_dep_map.entry(request.clone()) { - hash_map::Entry::Vacant(v) => { - let dep = EntryDependency::new( - request, - original_module_context.unwrap_or(Context::from("")), - ); - let dep_id = *dep.id(); - mg.add_dependency(Box::new(dep)); - v.insert(dep_id); - dep_id - } - hash_map::Entry::Occupied(v) => *v.get(), + let dep_id = { + let dep = EntryDependency::new( + request, + original_module_context.unwrap_or(Context::from("")), + ); + let dep_id = *dep.id(); + mg.add_dependency(Box::new(dep)); + dep_id }; compilation @@ -278,18 +266,16 @@ impl ModuleExecutor { }; if let Ok(ref mut result) = execute_result { - std::mem::swap(&mut result.assets, compilation.assets_mut()); + let assets = std::mem::take(compilation.assets_mut()); + for (key, value) in assets { + result.assets.insert(key.clone()); + self.assets.insert(key, value); + } } - let mut has_error = false; for error in compilation.get_errors() { - has_error = true; error.render_report(true)?; } - if !has_error { - // save make module_graph for next import_module - compilation.swap_make_module_graph(&mut self.make_module_graph); - } execute_result }