Skip to content

Commit

Permalink
fix: runtime chunk hash should respect their reference order (#8303)
Browse files Browse the repository at this point in the history
  • Loading branch information
ahabhgk authored Nov 4, 2024
1 parent 1f08e37 commit 5c6f47a
Show file tree
Hide file tree
Showing 45 changed files with 475 additions and 256 deletions.
3 changes: 2 additions & 1 deletion crates/node_binding/binding.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,8 @@ export function formatDiagnostic(diagnostic: JsDiagnostic): ExternalObject<'Diag
export interface JsAddingRuntimeModule {
name: string
generator: () => String
cacheable: boolean
dependentHash: boolean
fullHash: boolean
isolate: boolean
stage: number
}
Expand Down
5 changes: 2 additions & 3 deletions crates/rspack_binding_values/src/codegen_result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,10 @@ impl From<CodeGenerationResult> for JsCodegenerationResult {

impl From<CodeGenerationResults> for JsCodegenerationResults {
fn from(results: CodeGenerationResults) -> Self {
let id_result_map = results.module_generation_result_map;
let (map, id_result_map) = results.into_inner();

Self {
map: results
.map
map: map
.into_iter()
.map(|(module_id, runtime_result_map)| {
let mut runtime_map: HashMap<String, JsCodegenerationResult> = Default::default();
Expand Down
6 changes: 4 additions & 2 deletions crates/rspack_binding_values/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,8 @@ pub struct JsAddingRuntimeModule {
pub name: String,
#[napi(ts_type = "() => String")]
pub generator: GenerateFn,
pub cacheable: bool,
pub dependent_hash: bool,
pub full_hash: bool,
pub isolate: bool,
pub stage: u32,
}
Expand All @@ -508,7 +509,8 @@ impl From<JsAddingRuntimeModule> for RuntimeModuleFromJs {
fn from(value: JsAddingRuntimeModule) -> Self {
Self {
name: value.name,
cacheable: value.cacheable,
full_hash: value.full_hash,
dependent_hash: value.dependent_hash,
isolate: value.isolate,
stage: RuntimeModuleStage::from(value.stage),
generator: Arc::new(move || value.generator.blocking_call_with_sync(())),
Expand Down
17 changes: 4 additions & 13 deletions crates/rspack_core/src/cgm_runtime_requirement_results.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,6 @@ impl CgmRuntimeRequirementsResults {
requirements.get(runtime)
}

pub fn set(
&mut self,
module: ModuleIdentifier,
runtime: RuntimeSpec,
runtime_requirements: RuntimeGlobals,
) {
let requirements = self
.module_to_runtime_requirements
.entry(module)
.or_default();
requirements.set(runtime, runtime_requirements);
}

pub fn set_runtime_requirements(
&mut self,
module: ModuleIdentifier,
Expand All @@ -35,4 +22,8 @@ impl CgmRuntimeRequirementsResults {
.module_to_runtime_requirements
.insert(module, runtime_requirements_map);
}

pub fn remove(&mut self, module: &ModuleIdentifier) -> Option<RuntimeSpecMap<RuntimeGlobals>> {
self.module_to_runtime_requirements.remove(module)
}
}
26 changes: 21 additions & 5 deletions crates/rspack_core/src/chunk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -426,12 +426,28 @@ impl Chunk {
.chunk_graph
.get_ordered_chunk_modules(&self.ukey, &compilation.get_module_graph())
{
if let Some(hash) = compilation
let module_identifier = module.identifier();
let hash = compilation
.code_generation_results
.get_hash(&module.identifier(), Some(&self.runtime))
{
hash.hash(hasher);
}
.get_hash(&module_identifier, Some(&self.runtime))
.unwrap_or_else(|| {
panic!("Module ({module_identifier}) should have hash result when updating chunk hash.");
});
hash.hash(hasher);
}
for (runtime_module_identifier, _) in compilation
.chunk_graph
.get_chunk_runtime_modules_in_order(&self.ukey, compilation)
{
let hash = compilation
.runtime_modules_hash
.get(runtime_module_identifier)
.unwrap_or_else(|| {
panic!(
"Runtime module ({runtime_module_identifier}) should have hash result when updating chunk hash."
);
});
hash.hash(hasher);
}
"entry".hash(hasher);
for (module, chunk_group) in compilation
Expand Down
27 changes: 26 additions & 1 deletion crates/rspack_core/src/chunk_graph/chunk_graph_chunk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,11 +314,19 @@ impl ChunkGraph {
.collect()
}

pub fn get_chunk_module_identifiers(&self, chunk: &ChunkUkey) -> &IdentifierSet {
pub fn get_chunk_modules_identifier(&self, chunk: &ChunkUkey) -> &IdentifierSet {
let chunk_graph_chunk = self.expect_chunk_graph_chunk(chunk);
&chunk_graph_chunk.modules
}

pub fn get_ordered_chunk_modules_identifier(&self, chunk: &ChunkUkey) -> Vec<ModuleIdentifier> {
let chunk_graph_chunk = self.expect_chunk_graph_chunk(chunk);
let mut modules: Vec<ModuleIdentifier> = chunk_graph_chunk.modules.iter().copied().collect();
// SAFETY: module identifier is unique
modules.sort_unstable_by_key(|m| m.as_str());
modules
}

pub fn get_ordered_chunk_modules<'module>(
&self,
chunk: &ChunkUkey,
Expand Down Expand Up @@ -427,6 +435,23 @@ impl ChunkGraph {
cgc.entry_modules.len()
}

pub fn has_chunk_full_hash_modules(
&self,
chunk: &ChunkUkey,
runtime_modules: &IdentifierMap<Box<dyn RuntimeModule>>,
) -> bool {
let cgc = self.expect_chunk_graph_chunk(chunk);
for runtime_module in &cgc.runtime_modules {
let runtime_module = runtime_modules
.get(runtime_module)
.expect("should have runtime_module");
if !runtime_module.full_hash() {
return true;
}
}
false
}

pub fn add_chunk_runtime_requirements(
&mut self,
chunk_ukey: &ChunkUkey,
Expand Down
29 changes: 27 additions & 2 deletions crates/rspack_core/src/code_generation_results.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ impl CodeGenerationResult {
source.hash(&mut hasher);
}
self.chunk_init_fragments.hash(&mut hasher);
self.runtime_requirements.hash(&mut hasher);
self.hash = Some(hasher.digest(hash_digest));
}
}
Expand All @@ -188,8 +189,8 @@ pub static CODE_GEN_RESULT_ID: AtomicU32 = AtomicU32::new(0);

#[derive(Debug, Default, Clone)]
pub struct CodeGenerationResults {
pub module_generation_result_map: HashMap<CodeGenResultId, CodeGenerationResult>,
pub map: IdentifierMap<RuntimeSpecMap<CodeGenResultId>>,
module_generation_result_map: HashMap<CodeGenResultId, CodeGenerationResult>,
map: IdentifierMap<RuntimeSpecMap<CodeGenResultId>>,
}

impl CodeGenerationResults {
Expand All @@ -210,6 +211,21 @@ impl CodeGenerationResults {
})
}

pub fn insert(
&mut self,
module_identifier: ModuleIdentifier,
codegen_res: CodeGenerationResult,
runtimes: impl IntoIterator<Item = RuntimeSpec>,
) {
let codegen_res_id = codegen_res.id;
self
.module_generation_result_map
.insert(codegen_res_id, codegen_res);
for runtime in runtimes {
self.add(module_identifier, runtime, codegen_res_id);
}
}

pub fn remove(&mut self, module_identifier: &ModuleIdentifier) -> Option<()> {
let runtime_map = self.map.remove(module_identifier)?;
for result in runtime_map.get_values() {
Expand Down Expand Up @@ -303,6 +319,15 @@ impl CodeGenerationResults {

code_generation_result.hash.as_ref()
}

pub fn into_inner(
self,
) -> (
IdentifierMap<RuntimeSpecMap<CodeGenResultId>>,
HashMap<CodeGenResultId, CodeGenerationResult>,
) {
(self.map, self.module_generation_result_map)
}
}

#[derive(Debug)]
Expand Down
Loading

2 comments on commit 5c6f47a

@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 ❌ failure
_selftest ✅ success
rspress ✅ success
rslib ✅ success
rsbuild ❌ failure
examples ❌ failure
devserver ✅ 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-11-04 a987332) Current Change
10000_big_production-mode + exec 45.9 s ± 613 ms 44 s ± 580 ms -4.27 %
10000_development-mode + exec 1.84 s ± 17 ms 1.82 s ± 32 ms -0.84 %
10000_development-mode_hmr + exec 645 ms ± 12 ms 644 ms ± 11 ms -0.13 %
10000_production-mode + exec 2.4 s ± 19 ms 2.42 s ± 31 ms +0.98 %
arco-pro_development-mode + exec 1.79 s ± 63 ms 1.78 s ± 63 ms -0.29 %
arco-pro_development-mode_hmr + exec 429 ms ± 1.2 ms 430 ms ± 2 ms +0.42 %
arco-pro_production-mode + exec 3.19 s ± 87 ms 3.18 s ± 69 ms -0.29 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.27 s ± 50 ms 3.29 s ± 65 ms +0.72 %
threejs_development-mode_10x + exec 1.58 s ± 7.7 ms 1.58 s ± 15 ms -0.18 %
threejs_development-mode_10x_hmr + exec 784 ms ± 4.2 ms 779 ms ± 11 ms -0.67 %
threejs_production-mode_10x + exec 4.93 s ± 40 ms 4.94 s ± 44 ms +0.14 %

Please sign in to comment.