refactor(linter): use RwLock<FxHashMap> instead of FxDashMap for module record data (#8061)

This commit is contained in:
Boshen 2024-12-22 07:10:42 +00:00
parent 7cb84f34d1
commit 547c102b6b
12 changed files with 90 additions and 74 deletions

View file

@ -200,29 +200,29 @@ impl ModuleGraphVisitor {
} }
}; };
} }
for module_record_ref in &module_record.loaded_modules { for pair in module_record.loaded_modules.read().unwrap().iter() {
if self.depth > self.max_depth { if self.depth > self.max_depth {
return VisitFoldWhile::Stop(accumulator.into_inner()); return VisitFoldWhile::Stop(accumulator.into_inner());
} }
let path = &module_record_ref.resolved_absolute_path; let path = &pair.1.resolved_absolute_path;
if !self.traversed.insert(path.clone()) { if !self.traversed.insert(path.clone()) {
continue; continue;
} }
if !filter(module_record_ref.pair(), module_record) { if !filter(pair, module_record) {
continue; continue;
} }
self.depth += 1; self.depth += 1;
event(ModuleGraphVisitorEvent::Enter, module_record_ref.pair(), module_record); event(ModuleGraphVisitorEvent::Enter, pair, module_record);
enter(module_record_ref.pair(), module_record); enter(pair, module_record);
accumulate!(fold(accumulator.into_inner(), module_record_ref.pair(), module_record)); accumulate!(fold(accumulator.into_inner(), pair, module_record));
accumulate!(self.filter_fold_recursive( accumulate!(self.filter_fold_recursive(
accumulator, accumulator,
module_record_ref.value(), pair.1,
filter, filter,
fold, fold,
event, event,
@ -230,8 +230,8 @@ impl ModuleGraphVisitor {
leave leave
)); ));
event(ModuleGraphVisitorEvent::Leave, module_record_ref.pair(), module_record); event(ModuleGraphVisitorEvent::Leave, pair, module_record);
leave(module_record_ref.pair(), module_record); leave(pair, module_record);
self.depth -= 1; self.depth -= 1;
} }

View file

@ -3,18 +3,15 @@
use std::{ use std::{
fmt, fmt,
path::{Path, PathBuf}, path::{Path, PathBuf},
sync::Arc, sync::{Arc, RwLock},
}; };
use dashmap::DashMap; use rustc_hash::FxHashMap;
use rustc_hash::{FxBuildHasher, FxHashMap};
use oxc_semantic::Semantic; use oxc_semantic::Semantic;
use oxc_span::{CompactStr, Span}; use oxc_span::{CompactStr, Span};
pub use oxc_syntax::module_record::RequestedModule; pub use oxc_syntax::module_record::RequestedModule;
type FxDashMap<K, V> = DashMap<K, V, FxBuildHasher>;
/// ESM Module Record /// ESM Module Record
/// ///
/// All data inside this data structure are for ESM, no commonjs data is allowed. /// All data inside this data structure are for ESM, no commonjs data is allowed.
@ -49,7 +46,7 @@ pub struct ModuleRecord {
/// ///
/// Note that Oxc does not support cross-file analysis, so this map will be empty after /// Note that Oxc does not support cross-file analysis, so this map will be empty after
/// [`ModuleRecord`] is created. You must link the module records yourself. /// [`ModuleRecord`] is created. You must link the module records yourself.
pub loaded_modules: FxDashMap<CompactStr, Arc<ModuleRecord>>, pub loaded_modules: RwLock<FxHashMap<CompactStr, Arc<ModuleRecord>>>,
/// `[[ImportEntries]]` /// `[[ImportEntries]]`
/// ///
@ -81,7 +78,7 @@ pub struct ModuleRecord {
/// Reexported bindings from `export * from 'specifier'` /// Reexported bindings from `export * from 'specifier'`
/// Keyed by resolved path /// Keyed by resolved path
pub exported_bindings_from_star_export: FxDashMap<PathBuf, Vec<CompactStr>>, pub exported_bindings_from_star_export: RwLock<FxHashMap<PathBuf, Vec<CompactStr>>>,
/// `export default name` /// `export default name`
/// ^^^^^^^ span /// ^^^^^^^ span
@ -93,8 +90,10 @@ impl fmt::Debug for ModuleRecord {
// recursively formatting loaded modules can crash when the module graph is cyclic // recursively formatting loaded modules can crash when the module graph is cyclic
let loaded_modules = self let loaded_modules = self
.loaded_modules .loaded_modules
.read()
.unwrap()
.iter() .iter()
.map(|entry| (entry.key().to_string())) .map(|(key, _)| (key.to_string()))
.reduce(|acc, key| format!("{acc}, {key}")) .reduce(|acc, key| format!("{acc}, {key}"))
.unwrap_or_default(); .unwrap_or_default();
let loaded_modules = format!("{{ {loaded_modules} }}"); let loaded_modules = format!("{{ {loaded_modules} }}");

View file

@ -53,19 +53,20 @@ declare_oxc_lint!(
impl Rule for Default { impl Rule for Default {
fn run_once(&self, ctx: &LintContext<'_>) { fn run_once(&self, ctx: &LintContext<'_>) {
let module_record = ctx.module_record(); let module_record = ctx.module_record();
let loaded_modules = module_record.loaded_modules.read().unwrap();
for import_entry in &module_record.import_entries { for import_entry in &module_record.import_entries {
let ImportImportName::Default(default_span) = import_entry.import_name else { let ImportImportName::Default(default_span) = import_entry.import_name else {
continue; continue;
}; };
let specifier = import_entry.module_request.name(); let specifier = import_entry.module_request.name();
let Some(remote_module_record_ref) = module_record.loaded_modules.get(specifier) else { let Some(remote_module_record) = loaded_modules.get(specifier) else {
continue; continue;
}; };
if !remote_module_record_ref.has_module_syntax { if !remote_module_record.has_module_syntax {
continue; continue;
} }
if !remote_module_record_ref if !remote_module_record
.resolved_absolute_path .resolved_absolute_path
.extension() .extension()
.and_then(|ext| ext.to_str()) .and_then(|ext| ext.to_str())
@ -73,8 +74,8 @@ impl Rule for Default {
{ {
continue; continue;
} }
if remote_module_record_ref.export_default.is_none() if remote_module_record.export_default.is_none()
&& !remote_module_record_ref.exported_bindings.contains_key("default") && !remote_module_record.exported_bindings.contains_key("default")
{ {
ctx.diagnostic(default_diagnostic(specifier, default_span)); ctx.diagnostic(default_diagnostic(specifier, default_span));
} }

View file

@ -55,6 +55,7 @@ impl Rule for Export {
let mut all_export_names = FxHashMap::default(); let mut all_export_names = FxHashMap::default();
let mut visited = FxHashSet::default(); let mut visited = FxHashSet::default();
let loaded_modules = module_record.loaded_modules.read().unwrap();
module_record.star_export_entries.iter().for_each(|star_export_entry| { module_record.star_export_entries.iter().for_each(|star_export_entry| {
if star_export_entry.is_type { if star_export_entry.is_type {
return; return;
@ -64,17 +65,11 @@ impl Rule for Export {
let Some(module_request) = &star_export_entry.module_request else { let Some(module_request) = &star_export_entry.module_request else {
return; return;
}; };
let Some(remote_module_record_ref) = let Some(remote_module_record) = loaded_modules.get(module_request.name()) else {
module_record.loaded_modules.get(module_request.name())
else {
return; return;
}; };
walk_exported_recursive( walk_exported_recursive(remote_module_record, &mut export_names, &mut visited);
remote_module_record_ref.value(),
&mut export_names,
&mut visited,
);
if export_names.is_empty() { if export_names.is_empty() {
ctx.diagnostic(no_named_export(module_request.name(), module_request.span())); ctx.diagnostic(no_named_export(module_request.name(), module_request.span()));
@ -126,16 +121,15 @@ fn walk_exported_recursive(
for name in module_record.exported_bindings.keys() { for name in module_record.exported_bindings.keys() {
result.insert(name.clone()); result.insert(name.clone());
} }
let loaded_modules = module_record.loaded_modules.read().unwrap();
for export_entry in &module_record.star_export_entries { for export_entry in &module_record.star_export_entries {
let Some(module_request) = &export_entry.module_request else { let Some(module_request) = &export_entry.module_request else {
continue; continue;
}; };
let Some(remote_module_record_ref) = let Some(remote_module_record) = loaded_modules.get(module_request.name()) else {
module_record.loaded_modules.get(module_request.name())
else {
continue; continue;
}; };
walk_exported_recursive(remote_module_record_ref.value(), result, visited); walk_exported_recursive(remote_module_record, result, visited);
} }
} }

View file

@ -91,6 +91,7 @@ impl Rule for Named {
let module_record = ctx.module_record(); let module_record = ctx.module_record();
let loaded_modules = module_record.loaded_modules.read().unwrap();
for import_entry in &module_record.import_entries { for import_entry in &module_record.import_entries {
// Get named import // Get named import
let ImportImportName::Name(import_name) = &import_entry.import_name else { let ImportImportName::Name(import_name) = &import_entry.import_name else {
@ -98,10 +99,9 @@ impl Rule for Named {
}; };
let specifier = import_entry.module_request.name(); let specifier = import_entry.module_request.name();
// Get remote module record // Get remote module record
let Some(remote_module_record_ref) = module_record.loaded_modules.get(specifier) else { let Some(remote_module_record) = loaded_modules.get(specifier) else {
continue; continue;
}; };
let remote_module_record = remote_module_record_ref.value();
if !remote_module_record.has_module_syntax { if !remote_module_record.has_module_syntax {
continue; continue;
} }
@ -118,8 +118,10 @@ impl Rule for Named {
// check re-export // check re-export
if remote_module_record if remote_module_record
.exported_bindings_from_star_export .exported_bindings_from_star_export
.read()
.unwrap()
.iter() .iter()
.any(|entry| entry.value().contains(&import_name.name)) .any(|(_, value)| value.contains(&import_name.name))
{ {
continue; continue;
} }
@ -127,6 +129,7 @@ impl Rule for Named {
ctx.diagnostic(named_diagnostic(name, specifier, import_span)); ctx.diagnostic(named_diagnostic(name, specifier, import_span));
} }
let loaded_modules = module_record.loaded_modules.read().unwrap();
for export_entry in &module_record.indirect_export_entries { for export_entry in &module_record.indirect_export_entries {
let Some(module_request) = &export_entry.module_request else { let Some(module_request) = &export_entry.module_request else {
continue; continue;
@ -136,10 +139,9 @@ impl Rule for Named {
}; };
let specifier = module_request.name(); let specifier = module_request.name();
// Get remote module record // Get remote module record
let Some(remote_module_record_ref) = module_record.loaded_modules.get(specifier) else { let Some(remote_module_record) = loaded_modules.get(specifier) else {
continue; continue;
}; };
let remote_module_record = remote_module_record_ref.value();
if !remote_module_record.has_module_syntax { if !remote_module_record.has_module_syntax {
continue; continue;
} }

View file

@ -123,32 +123,33 @@ impl Rule for Namespace {
return; return;
} }
let loaded_modules = module_record.loaded_modules.read().unwrap();
for entry in &module_record.import_entries { for entry in &module_record.import_entries {
let (source, module) = match &entry.import_name { let (source, module) = match &entry.import_name {
ImportImportName::NamespaceObject => { ImportImportName::NamespaceObject => {
let source = entry.module_request.name(); let source = entry.module_request.name();
if let Some(module) = module_record.loaded_modules.get(source) { if let Some(module) = loaded_modules.get(source) {
(source.to_string(), Arc::clone(module.value())) (source.to_string(), Arc::clone(module))
} else { } else {
return; return;
} }
} }
ImportImportName::Name(name) => { ImportImportName::Name(name) => {
let Some(loaded_module) = let Some(loaded_module) = loaded_modules.get(entry.module_request.name())
module_record.loaded_modules.get(entry.module_request.name())
else { else {
return; return;
}; };
let Some(source) = get_module_request_name(name.name(), &loaded_module) else { let Some(source) = get_module_request_name(name.name(), loaded_module) else {
return; return;
}; };
let Some(loaded_module) = &loaded_module.loaded_modules.get(source.as_str()) let loaded_module = loaded_module.loaded_modules.read().unwrap();
else { let Some(loaded_module) = loaded_module.get(source.as_str()) else {
return; return;
}; };
(source, Arc::clone(loaded_module.value())) (source, Arc::clone(loaded_module))
} }
ImportImportName::Default(_) => { ImportImportName::Default(_) => {
// TODO: Hard to confirm if it's a namespace object // TODO: Hard to confirm if it's a namespace object
@ -275,10 +276,11 @@ fn check_deep_namespace_for_node(
if let Some(module_source) = get_module_request_name(name, module) { if let Some(module_source) = get_module_request_name(name, module) {
let parent_node = ctx.nodes().parent_node(node.id())?; let parent_node = ctx.nodes().parent_node(node.id())?;
let module_record = module.loaded_modules.get(module_source.as_str())?; let loaded_modules = module.loaded_modules.read().unwrap();
let module_record = loaded_modules.get(module_source.as_str())?;
let mut namespaces = namespaces.to_owned(); let mut namespaces = namespaces.to_owned();
namespaces.push(name.into()); namespaces.push(name.into());
check_deep_namespace_for_node(parent_node, source, &namespaces, module_record.value(), ctx); check_deep_namespace_for_node(parent_node, source, &namespaces, module_record, ctx);
} else { } else {
check_binding_exported( check_binding_exported(
name, name,
@ -313,11 +315,13 @@ fn check_deep_namespace_for_object_pattern(
if let Some(module_source) = get_module_request_name(&name, module) { if let Some(module_source) = get_module_request_name(&name, module) {
let mut next_namespaces = namespaces.to_owned(); let mut next_namespaces = namespaces.to_owned();
next_namespaces.push(name.to_string()); next_namespaces.push(name.to_string());
let loaded_modules = module.loaded_modules.read().unwrap();
check_deep_namespace_for_object_pattern( check_deep_namespace_for_object_pattern(
pattern, pattern,
source, source,
next_namespaces.as_slice(), next_namespaces.as_slice(),
module.loaded_modules.get(module_source.as_str()).unwrap().value(), loaded_modules.get(module_source.as_str()).unwrap(),
ctx, ctx,
); );
continue; continue;
@ -353,8 +357,10 @@ fn check_binding_exported(
|| (name == "default" && module.export_default.is_some()) || (name == "default" && module.export_default.is_some())
|| module || module
.exported_bindings_from_star_export .exported_bindings_from_star_export
.read()
.unwrap()
.iter() .iter()
.any(|entry| entry.value().iter().any(|s| s.as_str() == name)) .any(|(_, value)| value.iter().any(|s| s.as_str() == name))
{ {
return; return;
} }

View file

@ -91,11 +91,12 @@ impl Rule for NoDuplicates {
fn run_once(&self, ctx: &LintContext<'_>) { fn run_once(&self, ctx: &LintContext<'_>) {
let module_record = ctx.module_record(); let module_record = ctx.module_record();
let loaded_modules = module_record.loaded_modules.read().unwrap();
let groups = module_record let groups = module_record
.requested_modules .requested_modules
.iter() .iter()
.map(|(source, requested_modules)| { .map(|(source, requested_modules)| {
let resolved_absolute_path = module_record.loaded_modules.get(source).map_or_else( let resolved_absolute_path = loaded_modules.get(source).map_or_else(
|| source.to_string(), || source.to_string(),
|module| module.resolved_absolute_path.to_string_lossy().to_string(), |module| module.resolved_absolute_path.to_string_lossy().to_string(),
); );

View file

@ -66,12 +66,13 @@ impl Rule for NoNamedAsDefault {
}; };
let specifier = import_entry.module_request.name(); let specifier = import_entry.module_request.name();
let Some(remote_module_record_ref) = module_record.loaded_modules.get(specifier) else { let remote_module_record = module_record.loaded_modules.read().unwrap();
let Some(remote_module_record) = remote_module_record.get(specifier) else {
continue; continue;
}; };
let import_name = import_entry.local_name.name(); let import_name = import_entry.local_name.name();
if remote_module_record_ref.exported_bindings.contains_key(import_name) { if remote_module_record.exported_bindings.contains_key(import_name) {
ctx.diagnostic(no_named_as_default_diagnostic( ctx.diagnostic(no_named_as_default_diagnostic(
*import_span, *import_span,
import_name, import_name,

View file

@ -1,3 +1,5 @@
use std::sync::Arc;
use oxc_ast::{ use oxc_ast::{
ast::{BindingPatternKind, Expression, IdentifierReference, MemberExpression}, ast::{BindingPatternKind, Expression, IdentifierReference, MemberExpression},
AstKind, AstKind,
@ -82,11 +84,12 @@ impl Rule for NoNamedAsDefaultMember {
}; };
let specifier = import_entry.module_request.name(); let specifier = import_entry.module_request.name();
let Some(remote_module_record_ref) = module_record.loaded_modules.get(specifier) else { let remote_module_record = module_record.loaded_modules.read().unwrap();
let Some(remote_module_record) = remote_module_record.get(specifier) else {
continue; continue;
}; };
if remote_module_record_ref.exported_bindings.is_empty() { if remote_module_record.exported_bindings.is_empty() {
continue; continue;
} }
@ -95,8 +98,10 @@ impl Rule for NoNamedAsDefaultMember {
return; return;
}; };
has_members_map has_members_map.insert(
.insert(symbol_id, (remote_module_record_ref, import_entry.module_request.clone())); symbol_id,
(Arc::clone(remote_module_record), import_entry.module_request.clone()),
);
} }
if has_members_map.is_empty() { if has_members_map.is_empty() {

View file

@ -45,10 +45,11 @@ impl Rule for NoSelfImport {
let module_record = ctx.module_record(); let module_record = ctx.module_record();
let resolved_absolute_path = &module_record.resolved_absolute_path; let resolved_absolute_path = &module_record.resolved_absolute_path;
for (request, requested_modules) in &module_record.requested_modules { for (request, requested_modules) in &module_record.requested_modules {
let Some(remote_module_record_ref) = module_record.loaded_modules.get(request) else { let remote_module_record = module_record.loaded_modules.read().unwrap();
let Some(remote_module_record) = remote_module_record.get(request) else {
continue; continue;
}; };
if remote_module_record_ref.value().resolved_absolute_path == *resolved_absolute_path { if remote_module_record.resolved_absolute_path == *resolved_absolute_path {
for requested_module in requested_modules { for requested_module in requested_modules {
ctx.diagnostic(no_self_import_diagnostic(requested_module.span)); ctx.diagnostic(no_self_import_diagnostic(requested_module.span));
} }

View file

@ -95,8 +95,10 @@ impl Rule for NoBarrelFile {
let mut total: usize = 0; let mut total: usize = 0;
for module_request in module_requests { for module_request in module_requests {
if let Some(remote_module) = module_record.loaded_modules.get(module_request.name()) { if let Some(remote_module) =
if let Some(count) = count_loaded_modules(remote_module.value()) { module_record.loaded_modules.read().unwrap().get(module_request.name())
{
if let Some(count) = count_loaded_modules(remote_module) {
total += count; total += count;
labels.push(module_request.span().label(format!("{count} modules"))); labels.push(module_request.span().label(format!("{count} modules")));
} }
@ -111,7 +113,7 @@ impl Rule for NoBarrelFile {
} }
fn count_loaded_modules(module_record: &ModuleRecord) -> Option<usize> { fn count_loaded_modules(module_record: &ModuleRecord) -> Option<usize> {
if module_record.loaded_modules.is_empty() { if module_record.loaded_modules.read().unwrap().is_empty() {
return None; return None;
} }
Some( Some(

View file

@ -255,6 +255,8 @@ impl Runtime {
if let ModuleState::Resolved(target_module_record) = target_ref.value() { if let ModuleState::Resolved(target_module_record) = target_ref.value() {
module_record module_record
.loaded_modules .loaded_modules
.write()
.unwrap()
.insert(specifier.clone(), Arc::clone(target_module_record)); .insert(specifier.clone(), Arc::clone(target_module_record));
} }
}; };
@ -264,19 +266,20 @@ impl Runtime {
// Resolve and append `star_export_bindings` // Resolve and append `star_export_bindings`
for export_entry in &module_record.star_export_entries { for export_entry in &module_record.star_export_entries {
let Some(remote_module_record_ref) = let Some(module_request) = &export_entry.module_request else {
export_entry.module_request.as_ref().and_then(|module_request| { continue;
module_record.loaded_modules.get(module_request.name()) };
}) let loaded_modules = module_record.loaded_modules.read().unwrap();
else { let Some(remote_module_record) = loaded_modules.get(module_request.name()) else {
continue; continue;
}; };
let remote_module_record = remote_module_record_ref.value();
// Append both remote `bindings` and `exported_bindings_from_star_export` // Append both remote `bindings` and `exported_bindings_from_star_export`
let remote_exported_bindings_from_star_export = remote_module_record let remote_exported_bindings_from_star_export =
.exported_bindings_from_star_export remote_module_record.exported_bindings_from_star_export.read().unwrap();
let remote_exported_bindings_from_star_export =
remote_exported_bindings_from_star_export
.iter() .iter()
.flat_map(|r| r.value().clone()); .flat_map(|(_, value)| value.clone());
let remote_bindings = remote_module_record let remote_bindings = remote_module_record
.exported_bindings .exported_bindings
.keys() .keys()
@ -285,9 +288,10 @@ impl Runtime {
.collect::<Vec<_>>(); .collect::<Vec<_>>();
module_record module_record
.exported_bindings_from_star_export .exported_bindings_from_star_export
.write()
.unwrap()
.entry(remote_module_record.resolved_absolute_path.clone()) .entry(remote_module_record.resolved_absolute_path.clone())
.or_default() .or_default()
.value_mut()
.extend(remote_bindings); .extend(remote_bindings);
} }