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_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..140910f0e074a 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -14,6 +14,8 @@ use rustc_ast::{ StmtKind, TraitAlias, TyAlias, }; 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}; @@ -32,7 +34,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 +77,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, @@ -151,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> { @@ -347,11 +330,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 +355,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 +368,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 +392,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 +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 { - self.r.dcx().span_err(use_tree.span(), "cannot resolve a prelude import"); + self.r + .dcx() + .span_err(use_tree.span(), format!("cannot resolve a prelude import")); } } } 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/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 379ae992c9a6e..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) { ( @@ -821,7 +840,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() { @@ -1137,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 { @@ -1151,23 +1172,17 @@ 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; 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 911350665ed32..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}; +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>, @@ -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>, @@ -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) @@ -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( @@ -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"); @@ -1921,7 +1899,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { &mut self.lint_buffer } - pub fn arenas() -> ResolverArenas<'ra> { + pub fn new_arenas() -> ResolverArenas<'ra> { Default::default() } @@ -2172,9 +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); - 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 } @@ -2183,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>, @@ -2195,7 +2177,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)) } @@ -2426,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 { @@ -2461,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)) }) } @@ -2803,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. @@ -2850,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() }, } } } @@ -2859,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() @@ -2902,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() } } @@ -2948,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).