Resolver: Parallelize the import resolution loop

compiler/rustc_data_structures/src/sync/parallel.rs

@@ -185,12 +185,18 @@ pub fn par_for_each_in<I: DynSend, T: IntoIterator<Item = I>>(
     });
 }
 
-// FIXME: actually make parallel and `T: DynSend`
-pub fn par_for_each_slice<T>(items: &mut [T], for_each: impl Fn(&mut T)) {
+pub fn par_for_each_slice<T: DynSend>(
+    items: &mut [T],
+    for_each: impl Fn(&mut T) + DynSync + DynSend,
+) {
     parallel_guard(|guard| {
-        items.iter_mut().for_each(|i| {
-            guard.run(|| for_each(i));
-        });
+        if let Some(proof) = mode::check_dyn_thread_safe() {
+            par_slice(items, guard, |i| for_each(i), proof)
+        } else {
+            items.iter_mut().for_each(|i| {
+                guard.run(|| for_each(i));
+            });
+        }
     });
 }
 

compiler/rustc_resolve/src/build_reduced_graph.rs

@@ -5,7 +5,6 @@
 //! unexpanded macros in the fragment are visited and registered.
 //! Imports are also considered items and placed into modules here, but not resolved yet.
 
-use std::cell::RefMut;
 use std::sync::Arc;
 
 use rustc_ast::visit::{self, AssocCtxt, Visitor, WalkItemKind};
@@ -16,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};
@@ -135,7 +135,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
     fn get_extern_module_with_lock(
         &self,
         def_id: DefId,
-        map_lock: &mut RefMut<'_, FxIndexMap<DefId, ExternModule<'ra>>>,
+        map_lock: &mut WriteGuard<'_, FxIndexMap<DefId, ExternModule<'ra>>>,
     ) -> Option<ExternModule<'ra>> {
         if let module @ Some(..) = map_lock.get(&def_id) {
             return module.copied();

compiler/rustc_resolve/src/diagnostics/impls.rs

@@ -1487,7 +1487,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(

compiler/rustc_resolve/src/ident.rs

@@ -142,11 +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.
-                    let mut scope = macro_rules_scope.get();
+                    let mut scope = *macro_rules_scope.borrow();
                     while let MacroRulesScope::Invocation(invoc_id) = scope {
                         if let Some(next) = self.output_macro_rules_scopes.get(&invoc_id) {
-                            scope = next.get();
-                            macro_rules_scope.set(scope);
+                            scope = *next.borrow();
+                            *macro_rules_scope.borrow_mut() = scope;
                         } else {
                             break;
                         }
@@ -187,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)
                     }
@@ -592,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)
                 }

compiler/rustc_resolve/src/lib.rs

@@ -24,7 +24,7 @@
 use std::cell::RefMut;
 use std::collections::BTreeSet;
 use std::ops::ControlFlow;
-use std::sync::{Arc, OnceLock};
+use std::sync::{Arc, Mutex, OnceLock};
 use std::{fmt, mem};
 
 use diagnostics::{ParamKindInEnumDiscriminant, ParamKindInNonTrivialAnonConst};
@@ -46,7 +46,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, WorkerLocal};
+use rustc_data_structures::sync::{FreezeReadGuard, FreezeWriteGuard, 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};
@@ -1274,7 +1274,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<(
+        Mutex<(
             PendingDecl<'ra>,
             /* finalized */ bool,
             /* open flag (namespaced crate) */ bool,
@@ -1290,14 +1290,14 @@ impl ExternPreludeEntry<'_> {
     fn flag() -> Self {
         ExternPreludeEntry {
             item_decl: None,
-            flag_decl: Some(CacheCell::new((PendingDecl::Pending, false, false))),
+            flag_decl: Some(Mutex::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(Mutex::new((PendingDecl::Pending, false, true))),
         }
     }
 
@@ -1397,7 +1397,7 @@ pub struct Resolver<'ra, 'tcx> {
     /// Eagerly populated map of all local non-block modules.
     local_module_map: FxIndexMap<LocalDefId, LocalModule<'ra>>,
     /// Lazily populated cache of modules loaded from external crates.
-    extern_module_map: CacheRefCell<FxIndexMap<DefId, ExternModule<'ra>>>,
+    extern_module_map: RwLock<FxIndexMap<DefId, ExternModule<'ra>>>,
 
     /// Maps glob imports to the names of items actually imported.
     glob_map: FxIndexMap<LocalDefId, FxIndexSet<Symbol>>,
@@ -1429,7 +1429,7 @@ pub struct Resolver<'ra, 'tcx> {
     /// Eagerly populated map of all local macro definitions.
     local_macro_map: FxHashMap<LocalDefId, &'ra Arc<SyntaxExtension>> = default::fx_hash_map(),
     /// Lazily populated cache of macro definitions loaded from external crates.
-    extern_macro_map: CacheRefCell<FxHashMap<DefId, &'ra Arc<SyntaxExtension>>>,
+    extern_macro_map: RwLock<FxHashMap<DefId, &'ra Arc<SyntaxExtension>>>,
     dummy_ext_bang: &'ra Arc<SyntaxExtension>,
     dummy_ext_derive: &'ra Arc<SyntaxExtension>,
     non_macro_attr: &'ra Arc<SyntaxExtension>,
@@ -1602,7 +1602,7 @@ impl<'ra> ResolverArenas<'ra> {
         Interned::new_unchecked(self.name_resolutions.alloc(CmRefCell::new(resolution)))
     }
     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)
@@ -2456,7 +2456,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
     ) -> Option<Decl<'ra>> {
         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 mut flag_decl = flag_decl.lock().unwrap(); // Lock for this entire process
+            let (pending_decl, finalized, is_open) = *flag_decl;
             let decl = match pending_decl {
                 PendingDecl::Ready(decl) => {
                     if finalize && !finalized && !is_open {
@@ -2491,7 +2492,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                     }
                 }
             };
-            flag_decl.set((PendingDecl::Ready(decl), finalize || finalized, is_open));
+            *flag_decl = (PendingDecl::Ready(decl), finalize || finalized, is_open);
             decl.or_else(|| finalize.then_some(self.dummy_decl))
         })
     }
@@ -2831,16 +2832,31 @@ pub fn provide(providers: &mut Providers) {
 /// Prefer constructing it through `Resolver::cm(_mut)` to ensure correctness.
 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};
-
+/// The [`Resolver`] has different resolution phases, which share most of the resolution logic.
+/// Some of these phases mutate the resolver (`&mut Resolver`) during resolution. However, we are
+/// transforming import resolution (one of these phases), often referred to as
+/// "speculative resolution", to a parallel algorithm, which requires 2 things to change:
+///     - A `&Resolver`, because cannot mutate any fields in it.
+///     - Some fields with interior mutability may not be changed during import resolution, as
+///       this can conflict with other in progress resolutions (so locks are not the solution).
+///       But these fields do require interior mutability during the other phases.
+///
+/// Because refactoring the entire name resolution code to be split into "immutable" and "mutable"
+/// logic, we opted for a more "developer friendly" and "unsafe" approach using data structures
+/// that are immutable during import resolution. This module is the collection of 3 data structures
+/// that offer use the above 2 points:
+///     - a `RefOrMut<'a, T>` smart pointer that gates a `&'a mut T` through a flag
+///       (i.e. are we in import res?).
+///     - `CmCell` and `CmRefCell`, which are conditionally mutable through a flag.
+///
+/// We hope one day to not have to do this :D
 mod ref_mut {
     use std::cell::{BorrowMutError, Cell, Ref, RefCell, RefMut};
     use std::fmt;
     use std::ops::Deref;
 
+    use rustc_data_structures::sync::DynSync;
+
     use crate::Resolver;
 
     /// A reference type that conditionally allows mutable access.
@@ -2893,6 +2909,12 @@ mod ref_mut {
     #[derive(Default)]
     pub(crate) struct CmCell<T>(Cell<T>);
 
+    // SAFETY: `CmCell<T>` is `Sync` only because every path that can call `Cell::set`
+    // (i.e. `CmCell::set`, and `update`, which is built on top of it) first checks
+    // `r.assert_speculative` and panics if it's set, refusing to mutate. Soundness
+    // therefore depends on proper usage of the `assert_speculative` field in the `Resolver`.
+    unsafe impl<T: DynSync> DynSync for CmCell<T> {}
+
     impl<T: Copy + fmt::Debug> fmt::Debug for CmCell<T> {
         fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
             f.debug_tuple("CmCell").field(&self.get()).finish()
@@ -2981,10 +3003,20 @@ mod ref_mut {
         }
     }
 
-    /// A wrapper around a [`RefCell`] that only allows writes (mutable borrows) based on a condition in the resolver.
     #[derive(Default)]
     pub(crate) struct CmRefCell<T>(RefCell<T>);
 
+    // SAFETY: This is safe because we can only mutate the inner state (borrow counter and `T`)
+    // if we are not in speculative resolution, which is run in parallel. This is checked
+    // dynamically with the `resolver.speculative_flag` field:
+    //
+    // - Any form of `borrow_mut` causes an immediate panic if that flag is set to ture.
+    // - We can only ever update the read counter in `borrow` if we have a `&mut Resolver`,
+    //   thus proving no speculative resolution exists. If we do need a shared borrow, the
+    //   `borrow_checked` function can be used, which gives out a `CmRef`. (see the safety comment
+    //   in `borrow_checked` for why this works)
+    unsafe impl<T: DynSync> DynSync for CmRefCell<T> {}
+
     impl<T> CmRefCell<T> {
         pub(crate) fn new(value: T) -> CmRefCell<T> {
             CmRefCell(RefCell::new(value))

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, StashKey};
 use rustc_expand::base::{
     Annotatable, DeriveResolution, Indeterminate, ResolverExpand, SyntaxExtension,
@@ -41,7 +42,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,
 };
@@ -79,7 +80,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<MacroRulesScope<'ra>>;
+pub(crate) type MacroRulesScopeRef<'ra> = &'ra RwLock<MacroRulesScope<'ra>>;
 
 /// Macro namespace is separated into two sub-namespaces, one for bang macros and
 /// one for attribute-like macros (attributes, derives).