Skip to content

Commit

Permalink
refactor: split cutout and repair step in make (#6364)
Browse files Browse the repository at this point in the history
  • Loading branch information
jerrykingxyz authored Apr 25, 2024
1 parent b46aeb1 commit 5b8dd1d
Show file tree
Hide file tree
Showing 16 changed files with 630 additions and 642 deletions.
36 changes: 25 additions & 11 deletions crates/rspack_core/src/compiler/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use tracing::instrument;

use super::{
hmr::CompilationRecords,
make::{update_module_graph, MakeParam},
make::{update_module_graph, MakeArtifact, MakeParam},
module_executor::ModuleExecutor,
};
use crate::{
Expand Down Expand Up @@ -137,7 +137,6 @@ pub struct Compilation {
pub options: Arc<CompilerOptions>,
pub entries: Entry,
pub global_entry: EntryData,
make_module_graph: ModuleGraphPartial,
other_module_graph: Option<ModuleGraphPartial>,
pub dependency_factories: HashMap<DependencyType, Arc<dyn ModuleFactory>>,
pub make_failed_dependencies: HashSet<BuildDependency>,
Expand Down Expand Up @@ -186,6 +185,8 @@ pub struct Compilation {
import_var_map: DashMap<ModuleIdentifier, ImportVarMap>,

pub module_executor: Option<ModuleExecutor>,

make_artifact: MakeArtifact,
}

impl Compilation {
Expand Down Expand Up @@ -224,7 +225,6 @@ impl Compilation {
hot_index: 0,
records,
options,
make_module_graph: Default::default(),
other_module_graph: None,
dependency_factories: Default::default(),
make_failed_dependencies: HashSet::default(),
Expand Down Expand Up @@ -272,33 +272,47 @@ impl Compilation {
import_var_map: DashMap::new(),

module_executor,

make_artifact: Default::default(),
}
}

pub fn id(&self) -> CompilationId {
self.id
}

pub fn swap_make_module_graph_with_compilation(&mut self, other: &mut Compilation) {
std::mem::swap(&mut self.make_module_graph, &mut other.make_module_graph);
pub fn swap_make_artifact_with_compilation(&mut self, other: &mut Compilation) {
std::mem::swap(&mut self.make_artifact, &mut other.make_artifact);
}
pub fn swap_make_module_graph(&mut self, module_graph_partial: &mut ModuleGraphPartial) {
std::mem::swap(&mut self.make_module_graph, module_graph_partial);
pub fn swap_make_artifact(&mut self, make_artifact: &mut MakeArtifact) {
std::mem::swap(&mut self.make_artifact, make_artifact);
}

pub fn get_module_graph(&self) -> ModuleGraph {
if let Some(other_module_graph) = &self.other_module_graph {
ModuleGraph::new(vec![&self.make_module_graph, other_module_graph], None)
ModuleGraph::new(
vec![
self.make_artifact.get_module_graph_partial(),
other_module_graph,
],
None,
)
} else {
ModuleGraph::new(vec![&self.make_module_graph], None)
ModuleGraph::new(vec![self.make_artifact.get_module_graph_partial()], None)
}
}

pub fn get_module_graph_mut(&mut self) -> ModuleGraph {
if let Some(other) = &mut self.other_module_graph {
ModuleGraph::new(vec![&self.make_module_graph], Some(other))
ModuleGraph::new(
vec![self.make_artifact.get_module_graph_partial()],
Some(other),
)
} else {
ModuleGraph::new(vec![], Some(&mut self.make_module_graph))
ModuleGraph::new(
vec![],
Some(self.make_artifact.get_module_graph_partial_mut()),
)
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/rspack_core/src/compiler/hmr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ where
// make stage used
self
.compilation
.swap_make_module_graph_with_compilation(&mut new_compilation);
.swap_make_artifact_with_compilation(&mut new_compilation);
new_compilation.make_failed_dependencies =
std::mem::take(&mut self.compilation.make_failed_dependencies);
new_compilation.make_failed_module =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
use std::collections::VecDeque;

use rustc_hash::FxHashSet as HashSet;

use super::super::MakeArtifact;
use crate::ModuleIdentifier;

#[derive(Debug, Default)]
pub struct CleanIsolatedModule {
need_check_isolated_module_ids: HashSet<ModuleIdentifier>,
}

impl CleanIsolatedModule {
pub fn analyze_force_build_module(
&mut self,
artifact: &MakeArtifact,
module_identifier: &ModuleIdentifier,
) {
let module_graph = artifact.get_module_graph();
for connection in module_graph.get_outgoing_connections(module_identifier) {
self
.need_check_isolated_module_ids
.insert(*connection.module_identifier());
}
}

pub fn fix_artifact(self, artifact: &mut MakeArtifact) {
let mut module_graph = artifact.get_module_graph_mut();
let mut queue = VecDeque::from(
self
.need_check_isolated_module_ids
.into_iter()
.collect::<Vec<_>>(),
);
while let Some(module_identifier) = queue.pop_front() {
let Some(mgm) = module_graph.module_graph_module_by_identifier(&module_identifier) else {
tracing::trace!("Module is cleaned: {}", module_identifier);
continue;
};
if !mgm.incoming_connections().is_empty() {
tracing::trace!("Module is used: {}", module_identifier);
continue;
}

for connection in module_graph.get_outgoing_connections(&module_identifier) {
// clean child module
queue.push_back(*connection.module_identifier());
}
module_graph.revoke_module(&module_identifier);
tracing::trace!("Module is cleaned: {}", module_identifier);
}
}
}
34 changes: 34 additions & 0 deletions crates/rspack_core/src/compiler/make/cutout/fix_issuers.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
use rustc_hash::FxHashMap as HashMap;

use super::super::MakeArtifact;
use crate::{ModuleIdentifier, ModuleIssuer};

#[derive(Debug, Default)]
pub struct FixIssuers {
origin_module_issuers: HashMap<ModuleIdentifier, ModuleIssuer>,
}

impl FixIssuers {
pub fn analyze_force_build_module(
&mut self,
artifact: &MakeArtifact,
module_identifier: &ModuleIdentifier,
) {
let module_graph = artifact.get_module_graph();
let mgm = module_graph
.module_graph_module_by_identifier(module_identifier)
.expect("should have module graph module");
self
.origin_module_issuers
.insert(*module_identifier, mgm.get_issuer().clone());
}

pub fn fix_artifact(self, artifact: &mut MakeArtifact) {
let mut module_graph = artifact.get_module_graph_mut();
for (id, issuer) in self.origin_module_issuers.into_iter() {
if let Some(mgm) = module_graph.module_graph_module_by_identifier_mut(&id) {
mgm.set_issuer(issuer);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
use rustc_hash::{FxHashMap as HashMap, FxHashSet as HashSet};

use super::super::MakeArtifact;
use crate::{AsyncDependenciesBlockIdentifier, GroupOptions, ModuleGraph, ModuleIdentifier};

#[derive(Debug, Default, Eq, PartialEq)]
struct ModuleDeps {
// child module identifier of current module
child_modules: HashSet<ModuleIdentifier>,
// blocks in current module
module_blocks: HashSet<(AsyncDependenciesBlockIdentifier, Option<GroupOptions>)>,
}

impl ModuleDeps {
fn from_module(module_graph: &ModuleGraph, module_identifier: &ModuleIdentifier) -> Self {
let mut res = Self::default();
for connection in module_graph.get_outgoing_connections(module_identifier) {
res.child_modules.insert(*connection.module_identifier());
}
let module = module_graph
.module_by_identifier(module_identifier)
.expect("should have module");
for block_id in module.get_blocks() {
let block = module_graph
.block_by_id(block_id)
.expect("should have block");
res
.module_blocks
.insert((*block_id, block.get_group_options().cloned()));
}

res
}
}

#[derive(Debug, Default)]
pub struct HasModuleGraphChange {
origin_module_deps: HashMap<ModuleIdentifier, ModuleDeps>,
}

impl HasModuleGraphChange {
pub fn analyze_force_build_module(
&mut self,
artifact: &MakeArtifact,
module_identifier: &ModuleIdentifier,
) {
let module_graph = &artifact.get_module_graph();
self.origin_module_deps.insert(
*module_identifier,
ModuleDeps::from_module(module_graph, module_identifier),
);
}

pub fn fix_artifact(self, artifact: &mut MakeArtifact) {
let module_graph = &artifact.get_module_graph();
if self.origin_module_deps.is_empty() {
// origin_module_deps empty means no force_build_module and no file changed
// this only happens when build from entry
artifact.has_module_graph_change = true;
return;
}
// if artifact.has_module_graph_change is true, no need to recalculate
if !artifact.has_module_graph_change {
for (module_identifier, module_deps) in self.origin_module_deps {
if module_graph
.module_by_identifier(&module_identifier)
.is_none()
|| ModuleDeps::from_module(module_graph, &module_identifier) != module_deps
{
artifact.has_module_graph_change = true;
return;
}
}
}
}
}
112 changes: 112 additions & 0 deletions crates/rspack_core/src/compiler/make/cutout/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
mod clean_isolated_module;
mod fix_issuers;
mod has_module_graph_change;

use rustc_hash::FxHashSet as HashSet;

use self::{
clean_isolated_module::CleanIsolatedModule, fix_issuers::FixIssuers,
has_module_graph_change::HasModuleGraphChange,
};
use super::{MakeArtifact, MakeParam};
use crate::BuildDependency;

#[derive(Debug, Default)]
pub struct Cutout {
fix_issuers: FixIssuers,
clean_isolated_module: CleanIsolatedModule,
has_module_graph_change: HasModuleGraphChange,
}

impl Cutout {
pub fn cutout_artifact(
&mut self,
artifact: &mut MakeArtifact,
params: Vec<MakeParam>,
) -> HashSet<BuildDependency> {
let module_graph = artifact.get_module_graph();

let mut force_build_modules = HashSet::default();
let mut force_build_deps = HashSet::default();

for item in params {
match item {
MakeParam::ModifiedFiles(files) => {
force_build_modules.extend(module_graph.modules().values().filter_map(|module| {
// check has dependencies modified
if !module.is_available(&files) {
Some(module.identifier())
} else {
None
}
}))
}
MakeParam::DeletedFiles(files) => {
force_build_modules.extend(module_graph.modules().values().flat_map(|module| {
let mut res = vec![];

// check has dependencies modified
if !module.is_available(&files) {
// add module id
res.push(module.identifier());
// add parent module id
res.extend(
module_graph
.get_incoming_connections(&module.identifier())
.iter()
.filter_map(|connect| connect.original_module_identifier),
)
}
res
}))
}
MakeParam::ForceBuildDeps(deps) => {
for item in deps {
let (dependency_id, _) = &item;
// add deps bindings module to force_build_modules
if let Some(mid) = module_graph.module_identifier_by_dependency_id(dependency_id) {
force_build_modules.insert(*mid);
}
force_build_deps.insert(item);
}
}
MakeParam::ForceBuildModules(modules) => {
force_build_modules.extend(modules);
}
};
}

for module_identifier in &force_build_modules {
self
.fix_issuers
.analyze_force_build_module(artifact, module_identifier);
self
.clean_isolated_module
.analyze_force_build_module(artifact, module_identifier);
self
.has_module_graph_change
.analyze_force_build_module(artifact, module_identifier);
}

let mut module_graph = artifact.get_module_graph_mut();
// do revoke module and collect deps
force_build_deps.extend(
force_build_modules
.iter()
.flat_map(|id| module_graph.revoke_module(id)),
);

force_build_deps
}

pub fn fix_artifact(self, artifact: &mut MakeArtifact) {
let Self {
fix_issuers,
clean_isolated_module,
has_module_graph_change,
} = self;
fix_issuers.fix_artifact(artifact);
clean_isolated_module.fix_artifact(artifact);
has_module_graph_change.fix_artifact(artifact);
}
}
Loading

2 comments on commit 5b8dd1d

@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, self-hosted, Linux, ci ❌ failure
_selftest, ubuntu-latest ✅ success
nx, ubuntu-latest ✅ success
rspress, ubuntu-latest ✅ success
rsbuild, ubuntu-latest ✅ success
compat, ubuntu-latest ✅ success
examples, ubuntu-latest ✅ success

@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-04-25 bc61872) Current Change
10000_development-mode + exec 2.72 s ± 21 ms 2.76 s ± 31 ms +1.24 %
10000_development-mode_hmr + exec 698 ms ± 22 ms 699 ms ± 7.6 ms +0.18 %
10000_production-mode + exec 2.56 s ± 52 ms 2.56 s ± 27 ms +0.07 %
arco-pro_development-mode + exec 2.5 s ± 105 ms 2.52 s ± 75 ms +1.03 %
arco-pro_development-mode_hmr + exec 431 ms ± 1.9 ms 430 ms ± 3.7 ms -0.26 %
arco-pro_development-mode_hmr_intercept-plugin + exec 445 ms ± 7.1 ms 440 ms ± 3.9 ms -0.96 %
arco-pro_development-mode_intercept-plugin + exec 3.27 s ± 101 ms 3.26 s ± 45 ms -0.29 %
arco-pro_production-mode + exec 4.09 s ± 73 ms 3.97 s ± 43 ms -2.97 %
arco-pro_production-mode_intercept-plugin + exec 4.82 s ± 72 ms 4.75 s ± 57 ms -1.31 %
threejs_development-mode_10x + exec 2.07 s ± 25 ms 2.06 s ± 8.8 ms -0.60 %
threejs_development-mode_10x_hmr + exec 758 ms ± 8.4 ms 752 ms ± 10 ms -0.78 %
threejs_production-mode_10x + exec 5.28 s ± 42 ms 5.24 s ± 40 ms -0.84 %

Please sign in to comment.