Skip to content

Commit

Permalink
fix: module graph change
Browse files Browse the repository at this point in the history
  • Loading branch information
JSerFeng committed Feb 25, 2025
1 parent fd867cf commit 506b32f
Show file tree
Hide file tree
Showing 23 changed files with 253 additions and 35 deletions.
4 changes: 1 addition & 3 deletions crates/rspack_core/src/build_chunk_graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,7 @@ pub fn build_chunk_graph(compilation: &mut Compilation) -> rspack_error::Result<
compilation.chunk_graph.add_module(module_identifier)
}

if enable_incremental {
compilation.code_splitting_cache.code_splitter = splitter;
}
compilation.code_splitting_cache.code_splitter = splitter;

Ok(())
}
Expand Down
4 changes: 0 additions & 4 deletions crates/rspack_core/src/cache/persistent/occasion/make/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ impl MakeOccasion {
missing_dependencies: _,
build_dependencies: _,
state: _,
has_module_graph_change: _,
make_failed_dependencies: _,
make_failed_module: _,
} = artifact;
Expand Down Expand Up @@ -69,9 +68,6 @@ impl MakeOccasion {
artifact.module_graph_partial = partial;
artifact.state = MakeArtifactState::Uninitialized(force_build_dependencies);

// TODO remove it after code splitting support incremental rebuild
artifact.has_module_graph_change = true;

// regenerate statistical data
// TODO remove set make_failed_module after all of module are cacheable
// make failed module include diagnostic that do not support cache, so recovery will not include failed module
Expand Down
7 changes: 2 additions & 5 deletions crates/rspack_core/src/compiler/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1360,11 +1360,13 @@ impl Compilation {

let start = logger.time("create chunks");
use_code_splitting_cache(self, |compilation| async {
let start = logger.time("rebuild chunk graph");
if compilation.options.experiments.parallel_code_splitting {
build_chunk_graph_new(compilation)?;
} else {
build_chunk_graph(compilation)?;
}
logger.time_end(start);
Ok(compilation)
})
.await?;
Expand Down Expand Up @@ -2286,11 +2288,6 @@ impl Compilation {
.clone()
}

// TODO remove it after code splitting support incremental rebuild
pub fn has_module_import_export_change(&self) -> bool {
self.make_artifact.has_module_graph_change
}

pub fn built_modules(&self) -> &IdentifierSet {
&self.make_artifact.built_modules
}
Expand Down
12 changes: 1 addition & 11 deletions crates/rspack_core/src/compiler/make/cutout/mod.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
mod clean_isolated_module;
mod fix_build_meta;
mod fix_issuers;
mod has_module_graph_change;

use rspack_collections::IdentifierSet;
use rustc_hash::FxHashSet as HashSet;

use self::{
clean_isolated_module::CleanIsolatedModule, fix_build_meta::FixBuildMeta,
fix_issuers::FixIssuers, has_module_graph_change::HasModuleGraphChange,
clean_isolated_module::CleanIsolatedModule, fix_build_meta::FixBuildMeta, fix_issuers::FixIssuers,
};
use super::{MakeArtifact, MakeParam};
use crate::{BuildDependency, FactorizeInfo};
Expand All @@ -18,7 +16,6 @@ pub struct Cutout {
fix_issuers: FixIssuers,
fix_build_meta: FixBuildMeta,
clean_isolated_module: CleanIsolatedModule,
has_module_graph_change: HasModuleGraphChange,
}

impl Cutout {
Expand Down Expand Up @@ -113,9 +110,6 @@ impl Cutout {
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 build_deps = HashSet::default();
Expand Down Expand Up @@ -162,8 +156,6 @@ impl Cutout {
}
artifact.entry_dependencies = entry_dependencies;

self.has_module_graph_change.analyze_artifact(artifact);

// only return available build_deps
let module_graph = artifact.get_module_graph();
build_deps
Expand All @@ -182,11 +174,9 @@ impl Cutout {
fix_issuers,
fix_build_meta,
clean_isolated_module,
has_module_graph_change,
} = self;
fix_issuers.fix_artifact(artifact);
fix_build_meta.fix_artifact(artifact);
clean_isolated_module.fix_artifact(artifact);
has_module_graph_change.fix_artifact(artifact);
}
}
5 changes: 0 additions & 5 deletions crates/rspack_core/src/compiler/make/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ impl Default for MakeArtifactState {
pub struct MakeArtifact {
// temporary data, used by subsequent steps of make
// should be reset when rebuild
pub has_module_graph_change: bool,
pub built_modules: IdentifierSet,
pub revoked_modules: IdentifierSet,
// Field to mark whether artifact has been initialized.
Expand Down Expand Up @@ -221,10 +220,6 @@ pub async fn make_module_graph(
// reset temporary data
artifact.built_modules = Default::default();
artifact.revoked_modules = Default::default();
if matches!(artifact.state, MakeArtifactState::Initialized) {
artifact.has_module_graph_change = false;
}

artifact = update_module_graph(compilation, artifact, params).await?;
Ok(artifact)
}
Expand Down
1 change: 0 additions & 1 deletion crates/rspack_core/src/compiler/module_executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ impl ModuleExecutor {
}
make_artifact.built_modules = Default::default();
make_artifact.revoked_modules = Default::default();
make_artifact.has_module_graph_change = false;

// Modules imported by `importModule` are passively loaded.
let mut build_dependencies = self.cutout.cutout_artifact(&mut make_artifact, params);
Expand Down
107 changes: 104 additions & 3 deletions crates/rspack_core/src/old_cache/local/code_splitting_cache.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
use futures::Future;
use indexmap::IndexMap;
use rspack_collections::{IdentifierIndexMap, IdentifierIndexSet};
use rspack_error::Result;
use rustc_hash::FxHashMap as HashMap;
use tracing::instrument;

use crate::{
build_chunk_graph::code_splitter::CodeSplitter, incremental::IncrementalPasses, ChunkByUkey,
ChunkGraph, ChunkGroupByUkey, ChunkGroupUkey, ChunkUkey, Compilation, ModuleIdentifier,
build_chunk_graph::code_splitter::{CodeSplitter, DependenciesBlockIdentifier},
incremental::{IncrementalPasses, Mutation},
ChunkByUkey, ChunkGraph, ChunkGroupByUkey, ChunkGroupUkey, ChunkUkey, Compilation, Logger,
ModuleIdentifier,
};

#[derive(Debug, Default)]
Expand All @@ -22,6 +25,100 @@ pub struct CodeSplittingCache {
pub(crate) module_idx: HashMap<ModuleIdentifier, (u32, u32)>,
}

impl CodeSplittingCache {
// we can skip rebuilding chunk graph if none of modules
// has changed its outgoings
// we don't need to check if module has changed its incomings
// if it changes, the incoming module changes its outgoings as well
fn can_skip_rebuilding(&self, this_compilation: &Compilation) -> bool {
let logger = this_compilation.get_logger("code splitting cache");

let Some(mutations) = this_compilation
.incremental
.mutations_read(IncrementalPasses::MAKE)
else {
// if disable incremental for make phase, we can't skip rebuilding
return false;
};

// if we have module removal, we can't skip rebuilding
if mutations
.iter()
.any(|mutation| matches!(mutation, Mutation::ModuleRemove { .. }))
{
logger.log("module removal detected");
return false;
}

let module_graph = this_compilation.get_module_graph();
let affected_modules = mutations.get_affected_modules_with_module_graph(&module_graph);
let previous_modules_map = &self.code_splitter.block_modules_runtime_map;

if previous_modules_map.is_empty() {
return false;
}

for module in affected_modules {
let outgoings: Vec<ModuleIdentifier> = module_graph
.get_ordered_outgoing_connections(&module)
.filter_map(|dep| module_graph.connection_by_dependency_id(dep))
.filter(|conn| {
// pass None to check if this connection is active in **all** runtime
conn.is_active(&module_graph, None)
})
.map(|conn| *conn.module_identifier())
.collect::<IdentifierIndexSet>()
.into_iter()
.collect();

// get outgoings from all runtimes in the previous compilation
let mut previous_modules = IdentifierIndexMap::default();
let all_runtimes = previous_modules_map.values();
let mut miss_in_previous = true;

all_runtimes.for_each(|modules| {
let Some(outgoings) = modules.get(&DependenciesBlockIdentifier::Module(module)) else {
return;
};
miss_in_previous = false;

for (outgoing, state, _) in outgoings {
// we must insert module even if state is false
// because we need to keep the import order
previous_modules
.entry(*outgoing)
.and_modify(|v| {
if state.is_not_false() {
*v = state;
}
})
.or_insert(state);
}
});

if miss_in_previous {
return false;
}

if previous_modules
.iter()
.filter(|(_, conn_state)| conn_state.is_not_false())
.map(|(m, _)| *m)
.collect::<Vec<_>>()
!= outgoings
{
// we find one module's outgoings has changed
// we cannot skip rebuilding
logger.log("module outgoings change detected");

return false;
}
}

true
}
}

#[instrument(skip_all)]
pub(crate) async fn use_code_splitting_cache<'a, T, F>(
compilation: &'a mut Compilation,
Expand All @@ -31,10 +128,12 @@ where
T: Fn(&'a mut Compilation) -> F,
F: Future<Output = Result<&'a mut Compilation>>,
{
let logger = compilation.get_logger("code splitting cache");
if !compilation
.incremental
.can_read_mutations(IncrementalPasses::MAKE)
{
logger.log("incremental disabled");
task(compilation).await?;
return Ok(());
}
Expand All @@ -45,7 +144,9 @@ where
&& compilation
.incremental
.can_read_mutations(IncrementalPasses::BUILD_CHUNK_GRAPH);
let no_change = !compilation.has_module_import_export_change();
let no_change = compilation
.code_splitting_cache
.can_skip_rebuilding(compilation);

if incremental_code_splitting || no_change {
let cache = &mut compilation.code_splitting_cache;
Expand Down
2 changes: 1 addition & 1 deletion packages/rspack-test-tools/etc/test-tools.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1567,7 +1567,7 @@ export type TTestConfig<T extends ECompilerType> = {
beforeExecute?: () => void;
afterExecute?: () => void;
moduleScope?: (ms: IBasicModuleScope, stats?: TCompilerStatsCompilation<T>) => IBasicModuleScope;
checkStats?: (stepName: string, stats: TCompilerStatsCompilation<T>) => boolean;
checkStats?: (stepName: string, jsonStats: TCompilerStatsCompilation<T>, stringStats: String) => boolean;
findBundle?: (index: number, options: TCompilerOptions<T>, stepName?: string) => string | string[];
bundlePath?: string[];
nonEsmThis?: (p: string | string[]) => Object;
Expand Down
19 changes: 18 additions & 1 deletion packages/rspack-test-tools/src/processor/watch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,25 @@ export class WatchProcessor<
return cached;
};
})();
const getStringStats = (() => {
let cached: string | null = null;
return () => {
if (!cached) {
cached = stats.toString({
logging: "verbose"
});
}
return cached;
};
})();
if (checkStats.length > 1) {
if (!checkStats(this._watchOptions.stepName, getJsonStats())) {
if (
!checkStats(
this._watchOptions.stepName,
getJsonStats(),
getStringStats()
)
) {
throw new Error("stats check failed");
}
} else {
Expand Down
3 changes: 2 additions & 1 deletion packages/rspack-test-tools/src/type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,8 @@ export type TTestConfig<T extends ECompilerType> = {
) => IBasicModuleScope;
checkStats?: (
stepName: string,
stats: TCompilerStatsCompilation<T>
jsonStats: TCompilerStatsCompilation<T>,
stringStats: String
) => boolean;
findBundle?: (
index: number,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,13 @@ module.exports = {
?.toString({ all: false, logging: "verbose" })
.replace(/\d+ ms/g, "X ms")
).toMatchInlineSnapshot(`
LOG from code splitting cache
incremental disabled
LOG from rspack.Compilation
<t> finish modules: X ms
<t> optimize dependencies: X ms
<t> rebuild chunk graph: X ms
<t> create chunks: X ms
<t> optimize: X ms
<t> module ids: X ms
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export const v1 = 42;
export const v2 = 42
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import lib from "./lib"
import value from './value'
import { v1 } from './re-exports'

it("should have correct result", () => {
expect(lib).toBe(42);
expect(value).toBe(42);
expect(v1).toBe(42);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// use eval to add sideEffects to current module
eval('')
export default 42;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from "./deep"
export const direct = 42
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// use eval to add sideEffects to current module
eval('')
export default 42;
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import lib from "./lib"
import value from './value'
import { v1 } from './re-exports'

it("should have correct result", () => {
// add specifier dependency should not rebuild chunk graph
expect(lib).toBe(42);
expect(lib).toBe(42);
expect(value).toBe(42);
expect(v1).toBe(42);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// change import order should rebuild chunk graph
import value from './value'
import lib from "./lib"
import { v1 } from './re-exports'

it("should have correct result", () => {
expect(lib).toBe(42);
expect(value).toBe(42);
expect(v1).toBe(42);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import value from './value'
import lib from "./lib"
// import member from same modules should not rebuild chunk graph
import { v1, v2 } from './re-exports'

it("should have correct result", () => {
expect(lib).toBe(42);
expect(value).toBe(42);
expect(v1).toBe(42);
expect(v2).toBe(42);
});
Loading

0 comments on commit 506b32f

Please sign in to comment.