Skip to content

perf: ⚡️ reduce lock contention#2072

Merged
IWANABETHATGUY merged 4 commits intomainfrom
fix/reduce-lock-contention
Aug 26, 2024
Merged

perf: ⚡️ reduce lock contention#2072
IWANABETHATGUY merged 4 commits intomainfrom
fix/reduce-lock-contention

Conversation

@IWANABETHATGUY
Copy link
Copy Markdown
Member

@IWANABETHATGUY IWANABETHATGUY commented Aug 26, 2024 •
edited
Loading

@netlify
Copy link
Copy Markdown

netlify Bot commented Aug 26, 2024 •
edited
Loading

✅ Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 48de659
🔍 Latest deploy log https://app.netlify.com/sites/rolldown-rs/deploys/66cc6ee5eac9580008f1ed10

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 26, 2024 •
edited
Loading

Benchmarks Rust

group                                                               pr                                     target
-----                                                               --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol                     1.00     52.0±1.13ms        ? ?/sec    1.01     52.6±0.87ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-minify              1.00     73.8±0.62ms        ? ?/sec    1.05     77.6±0.96ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-minify-sourcemap    1.02     96.7±1.75ms        ? ?/sec    1.00     94.9±1.57ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap           1.00     59.8±0.98ms        ? ?/sec    1.03     61.4±0.76ms        ? ?/sec
bundle/bundle@rome-ts                                               1.01    103.1±0.97ms        ? ?/sec    1.00    101.9±1.29ms        ? ?/sec
bundle/bundle@rome-ts-minify                                        1.09    205.9±2.86ms        ? ?/sec    1.00    189.6±5.28ms        ? ?/sec
bundle/bundle@rome-ts-minify-sourcemap                              1.02    248.8±7.53ms        ? ?/sec    1.00    244.6±7.75ms        ? ?/sec
bundle/bundle@rome-ts-sourcemap                                     1.02    120.5±1.60ms        ? ?/sec    1.00    118.5±1.51ms        ? ?/sec
bundle/bundle@threejs                                               1.03     37.9±0.62ms        ? ?/sec    1.00     36.9±0.74ms        ? ?/sec
bundle/bundle@threejs-minify                                        1.06     88.7±2.78ms        ? ?/sec    1.00     83.8±1.91ms        ? ?/sec
bundle/bundle@threejs-minify-sourcemap                              1.05    110.7±2.72ms        ? ?/sec    1.00    105.6±1.89ms        ? ?/sec
bundle/bundle@threejs-sourcemap                                     1.04     47.5±0.75ms        ? ?/sec    1.00     45.5±0.41ms        ? ?/sec
bundle/bundle@threejs10x                                            1.02    405.6±7.19ms        ? ?/sec    1.00    395.8±4.95ms        ? ?/sec
bundle/bundle@threejs10x-minify                                     1.02  1017.5±14.71ms        ? ?/sec    1.00    994.2±9.12ms        ? ?/sec
bundle/bundle@threejs10x-minify-sourcemap                           1.02  1267.0±11.41ms        ? ?/sec    1.00  1247.9±10.33ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                                  1.00    467.5±4.29ms        ? ?/sec    1.00    465.9±3.50ms        ? ?/sec
remapping/remapping                                                 1.00     32.2±0.22ms        ? ?/sec    1.00     32.4±0.29ms        ? ?/sec
remapping/render-chunk-remapping                                    1.00     80.8±0.43ms        ? ?/sec    1.01     81.7±0.36ms        ? ?/sec
scan/scan@rome-ts                                                   1.04     84.1±0.88ms        ? ?/sec    1.00     80.7±0.71ms        ? ?/sec
scan/scan@threejs                                                   1.01     29.1±0.96ms        ? ?/sec    1.00     28.7±0.35ms        ? ?/sec
scan/scan@threejs10x                                                1.02    291.4±2.35ms        ? ?/sec    1.00    286.8±1.98ms        ? ?/sec

@IWANABETHATGUY IWANABETHATGUY force-pushed the fix/reduce-lock-contention branch from 127c331 to 0d71e4f Compare August 26, 2024 11:23
@IWANABETHATGUY
Copy link
Copy Markdown
Member Author

IWANABETHATGUY commented Aug 26, 2024

The benchmark did change because the lock contention only happens when there are multiple chunks, if there is only one chunk the code just running in single-threaded

rolldown/crates/rolldown/src/stages/generate_stage/compute_cross_chunk_links.rs

Lines 171 to 261 in f2bffcb

)| {
let mut symbol_needs_to_assign = vec![];
chunk.modules.iter().copied().for_each(|module_id| {
let Module::Ecma(module) = &self.link_output.module_table.modules[module_id] else {
return;
};
module
.import_records
.iter()
.inspect(|rec| {
if let Module::Ecma(importee_module) =
&self.link_output.module_table.modules[rec.resolved_module]
{
// the the resolved module is not included in module graph, skip
// TODO: Is that possible that the module of the record is a external module?
if !importee_module.is_included {
return;
}
if matches!(rec.kind, ImportKind::DynamicImport) {
let importee_chunk = chunk_graph.module_to_chunk[importee_module.idx]
.expect("importee chunk should exist");
cross_chunk_dynamic_imports.insert(importee_chunk);
}
}
})
.filter(|rec| matches!(rec.kind, ImportKind::Import))
.filter_map(|rec| {
self.link_output.module_table.modules[rec.resolved_module].as_external()
})
.for_each(|importee| {
// Ensure the external module is imported in case it has side effects.
imports_from_external_modules.entry(importee.idx).or_default();
});
module.named_imports.iter().for_each(|(_, import)| {
let rec = &module.import_records[import.record_id];
if let Module::External(importee) =
&self.link_output.module_table.modules[rec.resolved_module]
{
imports_from_external_modules.entry(importee.idx).or_default().push(import.clone());
}
});
module.stmt_infos.iter().for_each(|stmt_info| {
if !stmt_info.is_included {
return;
}
stmt_info.declared_symbols.iter().for_each(|declared| {
symbol_needs_to_assign.push(*declared);
});
stmt_info.referenced_symbols.iter().for_each(|referenced| {
let referenced = referenced.symbol_ref();
let mut canonical_ref = symbols.par_canonical_ref_for(*referenced);
if let Some(namespace_alias) = &symbols.get(canonical_ref).namespace_alias {
canonical_ref = namespace_alias.namespace_ref;
}
depended_symbols.insert(canonical_ref);
});
});
});
if let ChunkKind::EntryPoint { module: entry_id, .. } = &chunk.kind {
let entry = &self.link_output.module_table.modules[*entry_id].as_ecma().unwrap();
let entry_meta = &self.link_output.metas[entry.idx];
if !matches!(entry_meta.wrap_kind, WrapKind::Cjs) {
for export_ref in entry_meta.resolved_exports.values() {
let mut canonical_ref = symbols.par_canonical_ref_for(export_ref.symbol_ref);
let symbol = symbols.get(canonical_ref);
if let Some(ns_alias) = &symbol.namespace_alias {
canonical_ref = ns_alias.namespace_ref;
}
depended_symbols.insert(canonical_ref);
}
}
if !matches!(entry_meta.wrap_kind, WrapKind::None) {
depended_symbols
.insert(entry_meta.wrapper_ref.expect("cjs should be wrapped in esm output"));
}
if matches!(self.options.format, OutputFormat::Cjs)
&& matches!(entry.exports_kind, ExportsKind::Esm)
{
depended_symbols.insert(self.link_output.runtime.resolve_symbol("__toCommonJS"));
depended_symbols.insert(entry.namespace_object_ref);
}
}
symbol_to_chunk_id_vec.push((chunk_id, symbol_needs_to_assign));
},
,

@IWANABETHATGUY
Copy link
Copy Markdown
Member Author

IWANABETHATGUY commented Aug 26, 2024

oxc_monitor has thousands of dynamic import which will generate thousands of chunks

@Boshen
Copy link
Copy Markdown
Member

Boshen commented Aug 26, 2024

Cool, can confirm it's gone.

@IWANABETHATGUY IWANABETHATGUY marked this pull request as ready for review August 26, 2024 12:47
@IWANABETHATGUY IWANABETHATGUY added this pull request to the merge queue Aug 26, 2024
Merged via the queue into main with commit 80dc48d Aug 26, 2024
@IWANABETHATGUY IWANABETHATGUY deleted the fix/reduce-lock-contention branch August 26, 2024 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hyf0 hyf0 hyf0 approved these changes

Assignees

No one assigned

Labels

needs-triage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Performance]: heavy mutex lock in generate_stage/compute_cross_chunk_links

3 participants

@IWANABETHATGUY @Boshen @hyf0