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 {
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()) {
continue;
}
if !filter(module_record_ref.pair(), module_record) {
if !filter(pair, module_record) {
continue;
}
self.depth += 1;
event(ModuleGraphVisitorEvent::Enter, module_record_ref.pair(), module_record);
enter(module_record_ref.pair(), module_record);
event(ModuleGraphVisitorEvent::Enter, 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(
accumulator,
module_record_ref.value(),
pair.1,
filter,
fold,
event,
@ -230,8 +230,8 @@ impl ModuleGraphVisitor {
leave
));
event(ModuleGraphVisitorEvent::Leave, module_record_ref.pair(), module_record);
leave(module_record_ref.pair(), module_record);
event(ModuleGraphVisitorEvent::Leave, pair, module_record);
leave(pair, module_record);
self.depth -= 1;
}

View file

@ -3,18 +3,15 @@
use std::{
fmt,
path::{Path, PathBuf},
sync::Arc,
sync::{Arc, RwLock},
};
use dashmap::DashMap;
use rustc_hash::{FxBuildHasher, FxHashMap};
use rustc_hash::FxHashMap;
use oxc_semantic::Semantic;
use oxc_span::{CompactStr, Span};
pub use oxc_syntax::module_record::RequestedModule;
type FxDashMap<K, V> = DashMap<K, V, FxBuildHasher>;
/// ESM Module Record
///
/// 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
/// [`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]]`
///
@ -81,7 +78,7 @@ pub struct ModuleRecord {
/// Reexported bindings from `export * from 'specifier'`
/// 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`
/// ^^^^^^^ span
@ -93,8 +90,10 @@ impl fmt::Debug for ModuleRecord {
// recursively formatting loaded modules can crash when the module graph is cyclic
let loaded_modules = self
.loaded_modules
.read()
.unwrap()
.iter()
.map(|entry| (entry.key().to_string()))
.map(|(key, _)| (key.to_string()))
.reduce(|acc, key| format!("{acc}, {key}"))
.unwrap_or_default();
let loaded_modules = format!("{{ {loaded_modules} }}");

View file

@ -53,19 +53,20 @@ declare_oxc_lint!(
impl Rule for Default {
fn run_once(&self, ctx: &LintContext<'_>) {
let module_record = ctx.module_record();
let loaded_modules = module_record.loaded_modules.read().unwrap();
for import_entry in &module_record.import_entries {
let ImportImportName::Default(default_span) = import_entry.import_name else {
continue;
};
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;
};
if !remote_module_record_ref.has_module_syntax {
if !remote_module_record.has_module_syntax {
continue;
}
if !remote_module_record_ref
if !remote_module_record
.resolved_absolute_path
.extension()
.and_then(|ext| ext.to_str())
@ -73,8 +74,8 @@ impl Rule for Default {
{
continue;
}
if remote_module_record_ref.export_default.is_none()
&& !remote_module_record_ref.exported_bindings.contains_key("default")
if remote_module_record.export_default.is_none()
&& !remote_module_record.exported_bindings.contains_key("default")
{
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 visited = FxHashSet::default();
let loaded_modules = module_record.loaded_modules.read().unwrap();
module_record.star_export_entries.iter().for_each(|star_export_entry| {
if star_export_entry.is_type {
return;
@ -64,17 +65,11 @@ impl Rule for Export {
let Some(module_request) = &star_export_entry.module_request else {
return;
};
let Some(remote_module_record_ref) =
module_record.loaded_modules.get(module_request.name())
else {
let Some(remote_module_record) = loaded_modules.get(module_request.name()) else {
return;
};
walk_exported_recursive(
remote_module_record_ref.value(),
&mut export_names,
&mut visited,
);
walk_exported_recursive(remote_module_record, &mut export_names, &mut visited);
if export_names.is_empty() {
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() {
result.insert(name.clone());
}
let loaded_modules = module_record.loaded_modules.read().unwrap();
for export_entry in &module_record.star_export_entries {
let Some(module_request) = &export_entry.module_request else {
continue;
};
let Some(remote_module_record_ref) =
module_record.loaded_modules.get(module_request.name())
else {
let Some(remote_module_record) = loaded_modules.get(module_request.name()) else {
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 loaded_modules = module_record.loaded_modules.read().unwrap();
for import_entry in &module_record.import_entries {
// Get named import
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();
// 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;
};
let remote_module_record = remote_module_record_ref.value();
if !remote_module_record.has_module_syntax {
continue;
}
@ -118,8 +118,10 @@ impl Rule for Named {
// check re-export
if remote_module_record
.exported_bindings_from_star_export
.read()
.unwrap()
.iter()
.any(|entry| entry.value().contains(&import_name.name))
.any(|(_, value)| value.contains(&import_name.name))
{
continue;
}
@ -127,6 +129,7 @@ impl Rule for Named {
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 {
let Some(module_request) = &export_entry.module_request else {
continue;
@ -136,10 +139,9 @@ impl Rule for Named {
};
let specifier = module_request.name();
// 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;
};
let remote_module_record = remote_module_record_ref.value();
if !remote_module_record.has_module_syntax {
continue;
}

View file

@ -123,32 +123,33 @@ impl Rule for Namespace {
return;
}
let loaded_modules = module_record.loaded_modules.read().unwrap();
for entry in &module_record.import_entries {
let (source, module) = match &entry.import_name {
ImportImportName::NamespaceObject => {
let source = entry.module_request.name();
if let Some(module) = module_record.loaded_modules.get(source) {
(source.to_string(), Arc::clone(module.value()))
if let Some(module) = loaded_modules.get(source) {
(source.to_string(), Arc::clone(module))
} else {
return;
}
}
ImportImportName::Name(name) => {
let Some(loaded_module) =
module_record.loaded_modules.get(entry.module_request.name())
let Some(loaded_module) = loaded_modules.get(entry.module_request.name())
else {
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;
};
let Some(loaded_module) = &loaded_module.loaded_modules.get(source.as_str())
else {
let loaded_module = loaded_module.loaded_modules.read().unwrap();
let Some(loaded_module) = loaded_module.get(source.as_str()) else {
return;
};
(source, Arc::clone(loaded_module.value()))
(source, Arc::clone(loaded_module))
}
ImportImportName::Default(_) => {
// 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) {
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();
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 {
check_binding_exported(
name,
@ -313,11 +315,13 @@ fn check_deep_namespace_for_object_pattern(
if let Some(module_source) = get_module_request_name(&name, module) {
let mut next_namespaces = namespaces.to_owned();
next_namespaces.push(name.to_string());
let loaded_modules = module.loaded_modules.read().unwrap();
check_deep_namespace_for_object_pattern(
pattern,
source,
next_namespaces.as_slice(),
module.loaded_modules.get(module_source.as_str()).unwrap().value(),
loaded_modules.get(module_source.as_str()).unwrap(),
ctx,
);
continue;
@ -353,8 +357,10 @@ fn check_binding_exported(
|| (name == "default" && module.export_default.is_some())
|| module
.exported_bindings_from_star_export
.read()
.unwrap()
.iter()
.any(|entry| entry.value().iter().any(|s| s.as_str() == name))
.any(|(_, value)| value.iter().any(|s| s.as_str() == name))
{
return;
}

View file

@ -91,11 +91,12 @@ impl Rule for NoDuplicates {
fn run_once(&self, ctx: &LintContext<'_>) {
let module_record = ctx.module_record();
let loaded_modules = module_record.loaded_modules.read().unwrap();
let groups = module_record
.requested_modules
.iter()
.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(),
|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 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;
};
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(
*import_span,
import_name,

View file

@ -1,3 +1,5 @@
use std::sync::Arc;
use oxc_ast::{
ast::{BindingPatternKind, Expression, IdentifierReference, MemberExpression},
AstKind,
@ -82,11 +84,12 @@ impl Rule for NoNamedAsDefaultMember {
};
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;
};
if remote_module_record_ref.exported_bindings.is_empty() {
if remote_module_record.exported_bindings.is_empty() {
continue;
}
@ -95,8 +98,10 @@ impl Rule for NoNamedAsDefaultMember {
return;
};
has_members_map
.insert(symbol_id, (remote_module_record_ref, import_entry.module_request.clone()));
has_members_map.insert(
symbol_id,
(Arc::clone(remote_module_record), import_entry.module_request.clone()),
);
}
if has_members_map.is_empty() {

View file

@ -45,10 +45,11 @@ impl Rule for NoSelfImport {
let module_record = ctx.module_record();
let resolved_absolute_path = &module_record.resolved_absolute_path;
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;
};
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 {
ctx.diagnostic(no_self_import_diagnostic(requested_module.span));
}

View file

@ -95,8 +95,10 @@ impl Rule for NoBarrelFile {
let mut total: usize = 0;
for module_request in module_requests {
if let Some(remote_module) = module_record.loaded_modules.get(module_request.name()) {
if let Some(count) = count_loaded_modules(remote_module.value()) {
if let Some(remote_module) =
module_record.loaded_modules.read().unwrap().get(module_request.name())
{
if let Some(count) = count_loaded_modules(remote_module) {
total += count;
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> {
if module_record.loaded_modules.is_empty() {
if module_record.loaded_modules.read().unwrap().is_empty() {
return None;
}
Some(

View file

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