Resolver: Parallelize the import resolution loop

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<T>`               | `RefCell<T>`        | `RefCell<T>` or                 |
-//! |                         |                     | `parking_lot::Mutex<T>`         |
-//! | `RwLock<T>`             | `RefCell<T>`        | `parking_lot::RwLock<T>`        |
+//! | Type                    | Serial version           | Parallel version                |
+//! | ----------------------- | ------------------------ | ------------------------------- |
+//! | `Lock<T>`               | `RefCell<T>`             | `RefCell<T>` or                 |
+//! |                         |                          | `parking_lot::Mutex<T>`         |
+//! | `RwLock<T>`             | `parking_lot::RwLock<T>` | `parking_lot::RwLock<T>`        |
 
 use std::collections::HashMap;
 use std::hash::{BuildHasher, Hash};

compiler/rustc_interface/src/passes.rs

@@ -791,7 +791,7 @@ fn resolver_for_lowering_raw<'tcx>(
     &'tcx Steal<ast::Crate>,
     &'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(

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<DefId>,
-        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<DefId, ExternModule<'ra>>>,
+    ) -> Option<Module<'ra>> {
+        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<BindingKey, NameResolutionRef<'ra>> {
+        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<BindingKey, NameResolutionRef<'ra>>,
     ) {
         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"));
                     }
                 }
             }

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| {

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(

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)
                 }