Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

perf: ⚡️ reduce lock contention #2072

Merged
merged 4 commits into from
Aug 26, 2024
Merged

perf: ⚡️ reduce lock contention #2072

merged 4 commits into from
Aug 26, 2024

Conversation

IWANABETHATGUY
Copy link
Contributor

@IWANABETHATGUY IWANABETHATGUY commented Aug 26, 2024 •
edited
Loading

@netlify Netlify
Copy link

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 GitHub Actions
Copy link

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
Contributor Author

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
Contributor Author

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

@Boshen
Copy link
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
23 checks passed
@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