From 4e7b0b35aa088ae59764a3b6780ed77d8516314f Mon Sep 17 00:00:00 2001 From: LorrensP-2158466 Date: Mon, 6 Jul 2026 09:13:46 +0200 Subject: [PATCH 1/2] Wrap the resolver arenas into `WorkerLocal` in preperation of parallel import resolution. --- compiler/rustc_interface/src/passes.rs | 2 +- .../rustc_resolve/src/build_reduced_graph.rs | 100 ++++++++---------- compiler/rustc_resolve/src/def_collector.rs | 2 +- compiler/rustc_resolve/src/imports.rs | 10 +- compiler/rustc_resolve/src/lib.rs | 14 +-- 5 files changed, 59 insertions(+), 69 deletions(-) diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index ca6e48cb67fe5..2c1eab38190c4 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -791,7 +791,7 @@ fn resolver_for_lowering_raw<'tcx>( &'tcx Steal, &'tcx ty::ResolverGlobalCtxt, ) { - let arenas = Resolver::arenas(); + let arenas = WorkerLocal::new(|_| Resolver::new_arenas()); let _ = tcx.registered_tools(()); // Uses `crate_for_resolver`. let (krate, pre_configured_attrs) = tcx.crate_for_resolver(()).steal(); let mut resolver = Resolver::new( diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index c35dc02fb1de1..27b5db588f4b4 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -14,6 +14,7 @@ use rustc_ast::{ StmtKind, TraitAlias, TyAlias, }; use rustc_attr_parsing::AttributeParser; +use rustc_data_structures::fx::FxIndexMap; use rustc_expand::base::{ResolverExpand, SyntaxExtension, SyntaxExtensionKind}; use rustc_hir::Attribute; use rustc_hir::attrs::{AttributeKind, MacroUseArgs}; @@ -32,7 +33,7 @@ use tracing::debug; use crate::Namespace::{MacroNS, TypeNS, ValueNS}; use crate::def_collector::DefCollector; use crate::error_helper::{OnUnknownData, StructCtor}; -use crate::imports::{ImportData, ImportKind}; +use crate::imports::{ImportData, ImportKind, NameResolutionRef}; use crate::macros::{MacroRulesDecl, MacroRulesScope, MacroRulesScopeRef}; use crate::ref_mut::CmCell; use crate::{ @@ -75,46 +76,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { self.plant_decl_into_local_module(ident, orig_ident.span, ns, decl); } - /// Create a name definition from the given components, and put it into the extern module. - fn define_extern( - &self, - parent: ExternModule<'ra>, - ident: IdentKey, - orig_ident_span: Span, - ns: Namespace, - child_index: usize, - res: Res, - vis: Visibility, - span: Span, - expansion: LocalExpnId, - ambiguity: Option<(Decl<'ra>, bool)>, - ) { - let decl = self.arenas.alloc_decl(DeclData { - kind: DeclKind::Def(res), - ambiguity: CmCell::new(ambiguity), - initial_vis: vis, - ambiguity_vis_max: CmCell::new(None), - ambiguity_vis_min: CmCell::new(None), - span, - expansion, - parent_module: Some(parent.to_module()), - }); - // Even if underscore names cannot be looked up, we still need to add them to modules, - // because they can be fetched by glob imports from those modules, and bring traits - // into scope both directly and through glob imports. - let key = - BindingKey::new_disambiguated(ident, ns, || (child_index + 1).try_into().unwrap()); // 0 indicates no underscore - if self - .resolution_or_default(parent.to_module(), key, orig_ident_span) - .borrow_mut_unchecked() - .non_glob_decl - .replace(decl) - .is_some() - { - span_bug!(span, "an external binding was already defined"); - } - } - /// Walks up the tree of definitions starting at `def_id`, /// stopping at the first encountered module. /// Parent block modules for arbitrary def-ids are not recorded for the local crate, @@ -347,11 +308,22 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } } - pub(crate) fn build_reduced_graph_external(&self, module: ExternModule<'ra>) { + #[must_use = "This does not insert the resolutions in to the module itself"] + pub(crate) fn build_reduced_graph_external( + &self, + module: ExternModule<'ra>, + ) -> FxIndexMap> { + let mut resolutions = FxIndexMap::default(); let def_id = module.def_id(); let children = self.tcx.module_children(def_id); for (i, child) in children.iter().enumerate() { - self.build_reduced_graph_for_external_crate_res(child, module, i, None) + self.build_reduced_graph_for_external_crate_res( + child, + module, + i, + None, + &mut resolutions, + ) } for (i, child) in self.cstore().ambig_module_children_untracked(self.tcx, def_id).enumerate() @@ -361,8 +333,10 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { module, children.len() + i, Some(&child.second), + &mut resolutions, ) } + resolutions } /// Builds the reduced graph for a single item in an external crate. @@ -372,6 +346,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { parent: ExternModule<'ra>, child_index: usize, ambig_child: Option<&ModChild>, + resolutions: &mut FxIndexMap>, ) { let child_span = |this: &Self, reexport_chain: &[Reexport], res: def::Res<_>| { this.def_span( @@ -395,19 +370,30 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { }); // Record primary definitions. - let define_extern = |ns| { - self.define_extern( - parent, - ident, - orig_ident.span, - ns, - child_index, - res, - vis, + let mut define_extern = |ns| { + let orig_ident_span = orig_ident.span; + let decl = self.arenas.alloc_decl(DeclData { + kind: DeclKind::Def(res), + ambiguity: CmCell::new(ambig), + initial_vis: vis, + ambiguity_vis_max: CmCell::new(None), + ambiguity_vis_min: CmCell::new(None), span, expansion, - ambig, - ) + parent_module: Some(parent.to_module()), + }); + let key = + BindingKey::new_disambiguated(ident, ns, || (child_index + 1).try_into().unwrap()); + if resolutions + .entry(key) + .or_insert_with(|| self.arenas.alloc_name_resolution(orig_ident_span)) + .borrow_mut_unchecked() // only 1 thread builds extern tables + .non_glob_decl + .replace(decl) + .is_some() + { + span_bug!(span, "an external binding was already defined"); + } }; match res { Res::Def( @@ -736,7 +722,11 @@ impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> { if let PathResult::Module(ModuleOrUniformRoot::Module(module)) = path_res { self.r.prelude = Some(module); } else { - self.r.dcx().span_err(use_tree.span(), "cannot resolve a prelude import"); + let prefix = Segment::names_to_string(&prefix); + self.r.dcx().span_err( + use_tree.span(), + format!("cannot resolve a prelude import `{prefix}`: {path_res:?}"), + ); } } } diff --git a/compiler/rustc_resolve/src/def_collector.rs b/compiler/rustc_resolve/src/def_collector.rs index de3f8c380fc44..6ac922f38cb31 100644 --- a/compiler/rustc_resolve/src/def_collector.rs +++ b/compiler/rustc_resolve/src/def_collector.rs @@ -221,7 +221,7 @@ impl<'a, 'ra, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'ra, 'tcx> { i.span, |this, feed| { if let Some(ext) = opt_syn_ext { - this.r.local_macro_map.insert(feed.def_id(), self.r.arenas.alloc_macro(ext)); + this.r.local_macro_map.insert(feed.def_id(), this.r.arenas.alloc_macro(ext)); } this.with_parent(feed.def_id(), |this| { diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 379ae992c9a6e..4daa549b1e109 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -821,7 +821,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ) => { self.per_ns(|this, ns| { match import_decls[ns] { - PendingDecl::Ready(Some(import_decl)) => { + PendingDecl::Ready(Some(decl)) => { + // We need the `target`, `source` can be extracted. + let import_decl = this.new_import_decl(decl, import); if import_decl.is_assoc_item() && !this.features.import_trait_associated_functions() { @@ -1151,11 +1153,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { Some(import), ); let pending_decl = match binding_result { - Ok(binding) => { - // We need the `target`, `source` can be extracted. - let import_decl = this.new_import_decl(binding, import); - PendingDecl::Ready(Some(import_decl)) - } + Ok(binding) => PendingDecl::Ready(Some(binding)), Err(Determinacy::Determined) => PendingDecl::Ready(None), Err(Determinacy::Undetermined) => { indeterminate_count += 1; diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 911350665ed32..6d4c3a44742b1 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -47,7 +47,7 @@ use rustc_ast::{ use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet, default}; use rustc_data_structures::intern::Interned; use rustc_data_structures::steal::Steal; -use rustc_data_structures::sync::{FreezeReadGuard, FreezeWriteGuard}; +use rustc_data_structures::sync::{FreezeReadGuard, FreezeWriteGuard, WorkerLocal}; use rustc_data_structures::unord::{UnordItems, UnordMap, UnordSet}; use rustc_errors::{Applicability, Diag, ErrCode, ErrorGuaranteed, LintBuffer}; use rustc_expand::base::{DeriveResolution, SyntaxExtension, SyntaxExtensionKind}; @@ -1388,7 +1388,7 @@ pub struct Resolver<'ra, 'tcx> { /// Crate-local macro expanded `macro_export` referred to by a module-relative path. macro_expanded_macro_export_errors: BTreeSet<(Span, Span)> = BTreeSet::new(), - arenas: &'ra ResolverArenas<'ra>, + arenas: &'ra WorkerLocal>, dummy_decl: Decl<'ra>, builtin_type_decls: FxHashMap>, builtin_attr_decls: FxHashMap>, @@ -1742,7 +1742,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { attrs: &[ast::Attribute], crate_span: Span, current_crate_outer_attr_insert_span: Span, - arenas: &'ra ResolverArenas<'ra>, + arenas: &'ra WorkerLocal>, ) -> Resolver<'ra, 'tcx> { let root_def_id = CRATE_DEF_ID.to_def_id(); let graph_root = LocalModule::new( @@ -1921,7 +1921,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { &mut self.lint_buffer } - pub fn arenas() -> ResolverArenas<'ra> { + pub fn new_arenas() -> ResolverArenas<'ra> { Default::default() } @@ -2174,7 +2174,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { fn resolutions(&self, module: Module<'ra>) -> &'ra Resolutions<'ra> { if module.populate_on_access.get() { module.populate_on_access.set(false); - self.build_reduced_graph_external(module.expect_extern()); + // unchecked because extern + *module.lazy_resolutions.borrow_mut_unchecked() = + self.build_reduced_graph_external(module.expect_extern()); } &module.0.0.lazy_resolutions } @@ -2195,7 +2197,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ) -> NameResolutionRef<'ra> { *self .resolutions(module) - .borrow_mut_unchecked() + .borrow_mut(self) .entry(key) .or_insert_with(|| self.arenas.alloc_name_resolution(orig_ident_span)) } From 64da2128bb8642eaa2c8cf8bc93ca2f38b19a39f Mon Sep 17 00:00:00 2001 From: LorrensP-2158466 Date: Mon, 6 Jul 2026 10:12:10 +0200 Subject: [PATCH 2/2] Parallelize the import resolution loop using `par_map`. With the accompanying changes to make datastructures concurrent: - CmRefCell now wraps a `rustc_datastructures::sync::RwLock` because of the borrow counters (does introduce synchronized access in single-threaded mode) - Cache(Ref)Cells are now changed into atomics/(rw)locks - populating external resolution tables now use `Once` to ensure only 1 thread builds the table and all others wait if they need it This is the bare minimum to make the loop work and is not at all optimized, this will follow :). --- compiler/rustc_data_structures/src/sync.rs | 10 +- .../rustc_resolve/src/build_reduced_graph.rs | 84 ++++++----- compiler/rustc_resolve/src/error_helper.rs | 2 +- compiler/rustc_resolve/src/ident.rs | 12 +- compiler/rustc_resolve/src/imports.rs | 57 +++++--- compiler/rustc_resolve/src/lib.rs | 135 +++++++++--------- compiler/rustc_resolve/src/macros.rs | 5 +- 7 files changed, 176 insertions(+), 129 deletions(-) diff --git a/compiler/rustc_data_structures/src/sync.rs b/compiler/rustc_data_structures/src/sync.rs index 3d5bc85278286..c97a84ebfec40 100644 --- a/compiler/rustc_data_structures/src/sync.rs +++ b/compiler/rustc_data_structures/src/sync.rs @@ -16,11 +16,11 @@ //! where noted otherwise, the type in column one is defined as a //! newtype around the type from column two or three. //! -//! | Type | Serial version | Parallel version | -//! | ----------------------- | ------------------- | ------------------------------- | -//! | `Lock` | `RefCell` | `RefCell` or | -//! | | | `parking_lot::Mutex` | -//! | `RwLock` | `RefCell` | `parking_lot::RwLock` | +//! | Type | Serial version | Parallel version | +//! | ----------------------- | ------------------------ | ------------------------------- | +//! | `Lock` | `RefCell` | `RefCell` or | +//! | | | `parking_lot::Mutex` | +//! | `RwLock` | `parking_lot::RwLock` | `parking_lot::RwLock` | use std::collections::HashMap; use std::hash::{BuildHasher, Hash}; diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index 27b5db588f4b4..140910f0e074a 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -15,6 +15,7 @@ use rustc_ast::{ }; use rustc_attr_parsing::AttributeParser; use rustc_data_structures::fx::FxIndexMap; +use rustc_data_structures::sync::WriteGuard; use rustc_expand::base::{ResolverExpand, SyntaxExtension, SyntaxExtensionKind}; use rustc_hir::Attribute; use rustc_hir::attrs::{AttributeKind, MacroUseArgs}; @@ -112,38 +113,59 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { match def_id.as_local() { Some(local_def_id) => self.local_module_map.get(&local_def_id).map(|m| m.to_module()), None => { - if let module @ Some(..) = self.extern_module_map.borrow().get(&def_id) { + if let module @ Some(..) = self.extern_module_map.read().get(&def_id) { return module.map(|m| m.to_module()); } + // We need the lock on the extern_module_map for the entire duration of this call. + // It is otherwise entirely possible 2 different threads will create and allocate + // the exact same module during speculative resolution. + // FIXME(parallel_import_resolution): We lock the entire map to make sure + // no 2+ threads try to create the exact same module. Could it be possible to + // only "lock on" `def_id`? + let mut lock = self.extern_module_map.write(); + // No reentrant locking possible, so do a recurisve call with lock + // passed as argument. + self.get_extern_module_with_lock(def_id, &mut lock) + } + } + } - // Query `def_kind` is not used because query system overhead is too expensive here. - let def_kind = self.cstore().def_kind_untracked(def_id); - if def_kind.is_module_like() { - let parent = self.tcx.opt_parent(def_id).map(|parent_id| { - self.get_nearest_non_block_module(parent_id).expect_extern() - }); - // Query `expn_that_defined` is not used because - // hashing spans in its result is expensive. - let expn_id = self.cstore().expn_that_defined_untracked(self.tcx, def_id); - let module = self.new_extern_module( - parent, - ModuleKind::Def( - def_kind, - def_id, - DUMMY_NODE_ID, - Some(self.tcx.item_name(def_id)), - ), - expn_id, - self.def_span(def_id), - // FIXME: Account for `#[no_implicit_prelude]` attributes. - parent.is_some_and(|module| module.no_implicit_prelude), - ); - return Some(module.to_module()); + fn get_extern_module_with_lock( + &self, + def_id: DefId, + map_lock: &mut WriteGuard<'_, FxIndexMap>>, + ) -> Option> { + if let module @ Some(..) = map_lock.get(&def_id) { + return module.map(|m| m.to_module()); + } + // Query `def_kind` is not used because query system overhead is too expensive here. + let def_kind = self.cstore().def_kind_untracked(def_id); + if def_kind.is_module_like() { + let parent = self.tcx.opt_parent(def_id).map(|mut parent_id| { + loop { + match self.get_extern_module_with_lock(parent_id, map_lock) { + Some(module) => break module.expect_extern(), + None => parent_id = self.tcx.parent(parent_id), + } } - - None - } + }); + // Query `expn_that_defined` is not used because + // hashing spans in its result is expensive. + let expn_id = self.cstore().expn_that_defined_untracked(self.tcx, def_id); + let module = ExternModule::new( + parent, + ModuleKind::Def(def_kind, def_id, DUMMY_NODE_ID, Some(self.tcx.item_name(def_id))), + self.tcx.visibility(def_id), + expn_id, + self.def_span(def_id), + parent.is_some_and(|module| module.no_implicit_prelude), + self.arenas, + ); + map_lock.insert(def_id, module); + return Some(module.to_module()); } + + None } pub(crate) fn expn_def_scope(&self, expn_id: ExpnId) -> Module<'ra> { @@ -722,11 +744,9 @@ impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> { if let PathResult::Module(ModuleOrUniformRoot::Module(module)) = path_res { self.r.prelude = Some(module); } else { - let prefix = Segment::names_to_string(&prefix); - self.r.dcx().span_err( - use_tree.span(), - format!("cannot resolve a prelude import `{prefix}`: {path_res:?}"), - ); + self.r + .dcx() + .span_err(use_tree.span(), format!("cannot resolve a prelude import")); } } } diff --git a/compiler/rustc_resolve/src/error_helper.rs b/compiler/rustc_resolve/src/error_helper.rs index 8ab193afe0eee..6d8250bd52aef 100644 --- a/compiler/rustc_resolve/src/error_helper.rs +++ b/compiler/rustc_resolve/src/error_helper.rs @@ -1480,7 +1480,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // Never recommend deprecated helper attributes. } Scope::MacroRules(macro_rules_scope) => { - if let MacroRulesScope::Def(macro_rules_def) = macro_rules_scope.get() { + if let MacroRulesScope::Def(macro_rules_def) = *macro_rules_scope.read() { let res = macro_rules_def.decl.res(); if filter_fn(res) { suggestions.push(TypoSuggestion::new( diff --git a/compiler/rustc_resolve/src/ident.rs b/compiler/rustc_resolve/src/ident.rs index 12dd05bfe6e60..548a544d65405 100644 --- a/compiler/rustc_resolve/src/ident.rs +++ b/compiler/rustc_resolve/src/ident.rs @@ -142,9 +142,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // used to avoid long scope chains, see the comments on `MacroRulesScopeRef`. // As another consequence of this optimization visitors never observe invocation // scopes for macros that were already expanded. - while let MacroRulesScope::Invocation(invoc_id) = macro_rules_scope.get() { - if let Some(next_scope) = self.output_macro_rules_scopes.get(&invoc_id) { - macro_rules_scope.set(next_scope.get()); + let mut scope = *macro_rules_scope.read(); + while let MacroRulesScope::Invocation(invoc_id) = scope { + if let Some(next) = self.output_macro_rules_scopes.get(&invoc_id) { + scope = *next.read(); + *macro_rules_scope.write() = scope; } else { break; } @@ -185,7 +187,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } } Scope::DeriveHelpersCompat => Scope::MacroRules(parent_scope.macro_rules), - Scope::MacroRules(macro_rules_scope) => match macro_rules_scope.get() { + Scope::MacroRules(macro_rules_scope) => match *macro_rules_scope.read() { MacroRulesScope::Def(binding) => { Scope::MacroRules(binding.parent_macro_rules_scope) } @@ -590,7 +592,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } result } - Scope::MacroRules(macro_rules_scope) => match macro_rules_scope.get() { + Scope::MacroRules(macro_rules_scope) => match *macro_rules_scope.read() { MacroRulesScope::Def(macro_rules_def) if ident == macro_rules_def.ident => { Ok(macro_rules_def.decl) } diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 4daa549b1e109..441f4d197fc73 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -2,10 +2,12 @@ use std::cmp::Ordering; use std::mem; +use std::sync::atomic::{self, AtomicUsize}; use rustc_ast::NodeId; use rustc_data_structures::fx::{FxHashSet, FxIndexSet}; use rustc_data_structures::intern::Interned; +use rustc_data_structures::sync::Lock; use rustc_errors::{Applicability, BufferedEarlyLint, Diagnostic}; use rustc_expand::base::SyntaxExtensionKind; use rustc_hir::def::{self, DefKind, PartialRes}; @@ -50,6 +52,7 @@ pub(crate) enum PendingDecl<'ra> { } enum ImportResolutionKind<'ra> { + // these are the decls the import imports, not the import declarations themselves Single(PerNS>), Glob(Vec<(Decl<'ra>, BindingKey, Span /* orig_ident_span */)>), } @@ -775,31 +778,44 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let mut indeterminate_count = self.indeterminate_imports.len() * 3; while indeterminate_count < prev_indeterminate_count { prev_indeterminate_count = indeterminate_count; - indeterminate_count = 0; - let mut resolutions = Vec::new(); + let atomic_indeterminate_count = AtomicUsize::new(0); + self.assert_speculative = true; - for import in mem::take(&mut self.indeterminate_imports) { - let (resolution, import_indeterminate_count) = self.cm().resolve_import(import); - indeterminate_count += import_indeterminate_count; + + let to_resolve_imports = mem::take(&mut self.indeterminate_imports); + let cm_resolver = self.cm(); + + let determined_imports = Lock::new(Vec::new()); + let indeterminate_imports = Lock::new(Vec::new()); + let resolutions = rustc_data_structures::sync::par_map(to_resolve_imports, |import| { + let (resolution, import_indeterminate_count) = + cm_resolver.reborrow_ref().resolve_import(import); + atomic_indeterminate_count + .fetch_add(import_indeterminate_count, atomic::Ordering::Relaxed); match import_indeterminate_count { - 0 => self.determined_imports.push(import), - _ => self.indeterminate_imports.push(import), - } - if let Some(resolution) = resolution { - resolutions.push((import, resolution)); + 0 => determined_imports.lock().push(import), + _ => indeterminate_imports.lock().push(import), } - } + (import, resolution) + }); + indeterminate_count = atomic_indeterminate_count.into_inner(); + self.determined_imports.extend(determined_imports.into_inner()); + self.indeterminate_imports.extend(indeterminate_imports.into_inner()); + self.assert_speculative = false; + self.write_import_resolutions(resolutions); } } fn write_import_resolutions( &mut self, - import_resolutions: Vec<(Import<'ra>, ImportResolution<'ra>)>, + import_resolutions: Vec<(Import<'ra>, Option>)>, ) { for (import, resolution) in &import_resolutions { - let ImportResolution { imported_module, .. } = resolution; + let Some(ImportResolution { imported_module, .. }) = resolution else { + continue; + }; import.imported_module.set(Some(*imported_module), self); if import.is_glob() @@ -812,7 +828,10 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } for (import, resolution) in import_resolutions { - let ImportResolution { imported_module, kind: resolution_kind } = resolution; + let Some(ImportResolution { imported_module, kind: resolution_kind }) = resolution + else { + continue; + }; match (&import.kind, resolution_kind) { ( @@ -1139,7 +1158,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { _ => unreachable!(), }; - let mut import_decls = PerNS::default(); + let mut decls = PerNS::default(); let mut indeterminate_count = 0; self.per_ns_cm(|mut this, ns| { if bindings[ns].get() != PendingDecl::Pending { @@ -1160,12 +1179,10 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { PendingDecl::Pending } }; - import_decls[ns] = pending_decl; + decls[ns] = pending_decl; }); - let import_resolution = ImportResolution { - imported_module: module, - kind: ImportResolutionKind::Single(import_decls), - }; + let import_resolution = + ImportResolution { imported_module: module, kind: ImportResolutionKind::Single(decls) }; (Some(import_resolution), indeterminate_count) } diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 6d4c3a44742b1..71a0b21727fe3 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -21,10 +21,9 @@ #![recursion_limit = "256"] // tidy-alphabetical-end -use std::cell::Ref; use std::collections::BTreeSet; use std::ops::ControlFlow; -use std::sync::Arc; +use std::sync::{Arc, Once}; use std::{fmt, mem}; use diagnostics::{ParamKindInEnumDiscriminant, ParamKindInNonTrivialAnonConst}; @@ -47,7 +46,9 @@ use rustc_ast::{ use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet, default}; use rustc_data_structures::intern::Interned; use rustc_data_structures::steal::Steal; -use rustc_data_structures::sync::{FreezeReadGuard, FreezeWriteGuard, WorkerLocal}; +use rustc_data_structures::sync::{ + FreezeReadGuard, FreezeWriteGuard, ReadGuard, RwLock, WorkerLocal, +}; use rustc_data_structures::unord::{UnordItems, UnordMap, UnordSet}; use rustc_errors::{Applicability, Diag, ErrCode, ErrorGuaranteed, LintBuffer}; use rustc_expand::base::{DeriveResolution, SyntaxExtension, SyntaxExtensionKind}; @@ -655,7 +656,7 @@ struct ModuleData<'ra> { /// Resolutions in modules from other crates are not populated until accessed. lazy_resolutions: Resolutions<'ra>, /// True if this is a module from other crate that needs to be populated on access. - populate_on_access: CacheCell, + populate_on_access: Once, /// Used to disambiguate underscore items (`const _: T = ...`) in the module. underscore_disambiguator: CmCell, @@ -709,7 +710,6 @@ impl<'ra> ModuleData<'ra> { vis: Visibility, arenas: &'ra ResolverArenas<'ra>, ) -> Self { - let is_foreign = !kind.is_local(); let self_decl = match kind { ModuleKind::Def(def_kind, def_id, ..) => { let expn_id = expansion.as_local().unwrap_or(LocalExpnId::ROOT); @@ -721,7 +721,7 @@ impl<'ra> ModuleData<'ra> { parent, kind, lazy_resolutions: Default::default(), - populate_on_access: CacheCell::new(is_foreign), + populate_on_access: Once::new(), underscore_disambiguator: CmCell::new(0), unexpanded_invocations: Default::default(), no_implicit_prelude, @@ -1242,7 +1242,7 @@ struct ExternPreludeEntry<'ra> { item_decl: Option<(Decl<'ra>, Span, /* introduced by item */ bool)>, /// Name declaration from an `--extern` flag, lazily populated on first use. flag_decl: Option< - CacheCell<( + RwLock<( PendingDecl<'ra>, /* finalized */ bool, /* open flag (namespaced crate) */ bool, @@ -1258,14 +1258,14 @@ impl ExternPreludeEntry<'_> { fn flag() -> Self { ExternPreludeEntry { item_decl: None, - flag_decl: Some(CacheCell::new((PendingDecl::Pending, false, false))), + flag_decl: Some(RwLock::new((PendingDecl::Pending, false, false))), } } fn open_flag() -> Self { ExternPreludeEntry { item_decl: None, - flag_decl: Some(CacheCell::new((PendingDecl::Pending, false, true))), + flag_decl: Some(RwLock::new((PendingDecl::Pending, false, true))), } } @@ -1367,7 +1367,7 @@ pub struct Resolver<'ra, 'tcx> { /// Eagerly populated map of all local non-block modules. local_module_map: FxIndexMap>, /// Lazily populated cache of modules loaded from external crates. - extern_module_map: CacheRefCell>>, + extern_module_map: RwLock>>, /// Maps glob imports to the names of items actually imported. glob_map: FxIndexMap>, @@ -1400,7 +1400,7 @@ pub struct Resolver<'ra, 'tcx> { /// Eagerly populated map of all local macro definitions. local_macro_map: FxHashMap> = default::fx_hash_map(), /// Lazily populated cache of macro definitions loaded from external crates. - extern_macro_map: CacheRefCell>>, + extern_macro_map: RwLock>>, dummy_ext_bang: &'ra Arc, dummy_ext_derive: &'ra Arc, non_macro_attr: &'ra Arc, @@ -1575,7 +1575,7 @@ impl<'ra> ResolverArenas<'ra> { ) } fn alloc_macro_rules_scope(&'ra self, scope: MacroRulesScope<'ra>) -> MacroRulesScopeRef<'ra> { - self.dropless.alloc(CacheCell::new(scope)) + self.dropless.alloc(RwLock::new(scope)) } fn alloc_macro_rules_decl(&'ra self, decl: MacroRulesDecl<'ra>) -> &'ra MacroRulesDecl<'ra> { self.dropless.alloc(decl) @@ -1881,28 +1881,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { module } - fn new_extern_module( - &self, - parent: Option>, - kind: ModuleKind, - expn_id: ExpnId, - span: Span, - no_implicit_prelude: bool, - ) -> ExternModule<'ra> { - let def_id = kind.def_id(); - let module = ExternModule::new( - parent, - kind, - self.tcx.visibility(def_id), - expn_id, - span, - no_implicit_prelude, - self.arenas, - ); - self.extern_module_map.borrow_mut().insert(def_id, module); - module - } - fn next_node_id(&mut self) -> NodeId { let start = self.next_node_id; let next = start.as_u32().checked_add(1).expect("input too large; ran out of NodeIds"); @@ -2172,11 +2150,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } fn resolutions(&self, module: Module<'ra>) -> &'ra Resolutions<'ra> { - if module.populate_on_access.get() { - module.populate_on_access.set(false); - // unchecked because extern - *module.lazy_resolutions.borrow_mut_unchecked() = - self.build_reduced_graph_external(module.expect_extern()); + if !module.is_local() { + // as long as 1 thread is building this external table, all other threads will wait + module.populate_on_access.call_once(|| { + *module.lazy_resolutions.borrow_mut_unchecked() = + self.build_reduced_graph_external(module.expect_extern()); + }); } &module.0.0.lazy_resolutions } @@ -2185,10 +2164,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { &self, module: Module<'ra>, key: BindingKey, - ) -> Option>> { + ) -> Option>> { self.resolutions(module).borrow().get(&key).map(|resolution| resolution.0.borrow()) } + #[track_caller] fn resolution_or_default( &self, module: Module<'ra>, @@ -2428,7 +2408,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ) -> Option> { let entry = self.extern_prelude.get(&ident); entry.and_then(|entry| entry.flag_decl.as_ref()).and_then(|flag_decl| { - let (pending_decl, finalized, is_open) = flag_decl.get(); + let (pending_decl, finalized, is_open) = *flag_decl.read(); let decl = match pending_decl { PendingDecl::Ready(decl) => { if finalize && !finalized && !is_open { @@ -2463,7 +2443,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } } }; - flag_decl.set((PendingDecl::Ready(decl), finalize || finalized, is_open)); + *flag_decl.write() = (PendingDecl::Ready(decl), finalize || finalized, is_open); decl.or_else(|| finalize.then_some(self.dummy_decl)) }) } @@ -2805,43 +2785,60 @@ type CmResolver<'r, 'ra, 'tcx> = ref_mut::RefOrMut<'r, Resolver<'ra, 'tcx>>; // FIXME: These are cells for caches that can be populated even during speculative resolution, // and should be replaced with mutexes, atomics, or other synchronized data when migrating to // parallel name resolution. -use std::cell::{Cell as CacheCell, RefCell as CacheRefCell}; +use std::cell::RefCell as CacheRefCell; mod ref_mut { - use std::cell::{BorrowMutError, Cell, Ref, RefCell, RefMut}; - use std::fmt; + use std::cell::Cell; + use std::marker::PhantomData; use std::ops::Deref; + use std::{fmt, mem}; + + use rustc_data_structures::sync::{DynSend, DynSync, ReadGuard, RwLock, WriteGuard}; use crate::Resolver; /// A wrapper around a mutable reference that conditionally allows mutable access. pub(crate) struct RefOrMut<'a, T> { - p: &'a mut T, + p: *mut T, mutable: bool, + _marker: PhantomData<&'a mut T>, } + // SAFETY: these are safe because we never allow mutable access to the underlying + // `T` unless the user specifies that it is mutable. + // FIXME: make sure we never construct a mutable `RefOrMut` that is used in Send/Sync contexts? + unsafe impl<'a, T> DynSync for RefOrMut<'a, T> {} + unsafe impl<'a, T> DynSend for RefOrMut<'a, T> {} + impl<'a, T> Deref for RefOrMut<'a, T> { type Target = T; fn deref(&self) -> &Self::Target { - self.p + // SAFETY: `RefOrMUt` is only constructable through a `&mut T`. + unsafe { self.p.as_ref_unchecked() } } } impl<'a, T> AsRef for RefOrMut<'a, T> { fn as_ref(&self) -> &T { - self.p + // SAFETY: `RefOrMUt` is only constructable through a `&mut T`. + unsafe { self.p.as_ref_unchecked() } } } impl<'a, T> RefOrMut<'a, T> { pub(crate) fn new(p: &'a mut T, mutable: bool) -> Self { - RefOrMut { p, mutable } + RefOrMut { p, mutable, _marker: PhantomData } + } + + pub(crate) fn reborrow_ref(&self) -> RefOrMut<'_, T> { + assert!(!self.mutable); + RefOrMut { p: self.p, mutable: self.mutable, _marker: PhantomData } } /// This is needed because this wraps a `&mut T` and is therefore not `Copy`. pub(crate) fn reborrow(&mut self) -> RefOrMut<'_, T> { - RefOrMut { p: self.p, mutable: self.mutable } + RefOrMut { p: self.p, mutable: self.mutable, _marker: PhantomData } } /// Returns a mutable reference to the inner value if allowed. @@ -2852,7 +2849,9 @@ mod ref_mut { pub(crate) fn get_mut(&mut self) -> &mut T { match self.mutable { false => panic!("can't mutably borrow speculative resolver"), - true => self.p, + // SAFETY: `RefOrMut` is only constructable through a `&mut T` and we + // have tested that it may indeed be used as a `&mut T` in this match. + true => unsafe { self.p.as_mut_unchecked() }, } } } @@ -2861,6 +2860,11 @@ mod ref_mut { #[derive(Default)] pub(crate) struct CmCell(Cell); + /// SAFETY: `CmCell` is used within the resolver to make sure we do not mutate during + /// speculative resolution. This is run in parallel, thus we never mutate when in "parallel" + /// mode. It is thus safe to implement `DynSync`. + unsafe impl DynSync for CmCell {} + impl fmt::Debug for CmCell { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_tuple("CmCell").field(&self.get()).finish() @@ -2904,44 +2908,47 @@ mod ref_mut { } } - /// A wrapper around a [`RefCell`] that only allows mutable borrows based on a condition in the resolver. + /// A wrapper around a [`RwLock`] that only allows writes (mutable borrows) based on a condition in the resolver. #[derive(Default)] - pub(crate) struct CmRefCell(RefCell); + pub(crate) struct CmRefCell(RwLock); + + /// SAFETY: We wrap a `RwLock`. + unsafe impl DynSync for CmRefCell {} impl CmRefCell { - pub(crate) const fn new(value: T) -> CmRefCell { - CmRefCell(RefCell::new(value)) + pub(crate) fn new(value: T) -> CmRefCell { + CmRefCell(RwLock::new(value)) } #[track_caller] // FIXME: this should be eliminated in the process of migration // to parallel name resolution. - pub(crate) fn borrow_mut_unchecked(&self) -> RefMut<'_, T> { - self.0.borrow_mut() + pub(crate) fn borrow_mut_unchecked(&self) -> WriteGuard<'_, T> { + self.0.write() } #[track_caller] - pub(crate) fn borrow_mut<'ra, 'tcx>(&self, r: &Resolver<'ra, 'tcx>) -> RefMut<'_, T> { + pub(crate) fn borrow_mut<'ra, 'tcx>(&self, r: &Resolver<'ra, 'tcx>) -> WriteGuard<'_, T> { if r.assert_speculative { panic!("not allowed to mutably borrow a `CmRefCell` during speculative resolution"); } - self.0.borrow_mut() + self.0.write() } #[track_caller] pub(crate) fn try_borrow_mut<'ra, 'tcx>( &self, r: &Resolver<'ra, 'tcx>, - ) -> Result, BorrowMutError> { + ) -> Result, ()> { if r.assert_speculative { panic!("not allowed to mutably borrow a `CmRefCell` during speculative resolution"); } - self.0.try_borrow_mut() + self.0.try_write() } #[track_caller] - pub(crate) fn borrow(&self) -> Ref<'_, T> { - self.0.borrow() + pub(crate) fn borrow(&self) -> ReadGuard<'_, T> { + self.0.read() } } @@ -2950,7 +2957,7 @@ mod ref_mut { if r.assert_speculative { panic!("not allowed to mutate a CmRefCell during speculative resolution"); } - self.0.take() + mem::take(&mut *self.0.write()) } } } diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index 595c2fdf011dd..40b28a4c94110 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -7,6 +7,7 @@ use std::sync::Arc; use rustc_ast::{self as ast, Crate, DelegationSuffixes, NodeId}; use rustc_ast_pretty::pprust; use rustc_attr_parsing::AttributeParser; +use rustc_data_structures::sync::RwLock; use rustc_errors::{Applicability, DiagCtxtHandle, StashKey}; use rustc_expand::base::{ Annotatable, DeriveResolution, Indeterminate, ResolverExpand, SyntaxExtension, @@ -42,7 +43,7 @@ use crate::diagnostics::{ use crate::hygiene::Macros20NormalizedSyntaxContext; use crate::imports::Import; use crate::{ - BindingKey, CacheCell, CmResolver, Decl, DeclKind, DeriveData, Determinacy, Finalize, IdentKey, + BindingKey, CmResolver, Decl, DeclKind, DeriveData, Determinacy, Finalize, IdentKey, InvocationParent, ModuleKind, ModuleOrUniformRoot, ParentScope, PathResult, Res, ResolutionError, Resolver, ScopeSet, Segment, Used, }; @@ -80,7 +81,7 @@ pub(crate) enum MacroRulesScope<'ra> { /// This helps to avoid uncontrollable growth of `macro_rules!` scope chains, /// which usually grow linearly with the number of macro invocations /// in a module (including derives) and hurt performance. -pub(crate) type MacroRulesScopeRef<'ra> = &'ra CacheCell>; +pub(crate) type MacroRulesScopeRef<'ra> = &'ra RwLock>; /// Macro namespace is separated into two sub-namespaces, one for bang macros and /// one for attribute-like macros (attributes, derives).