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

compiler/rustc_resolve/src/diagnostics/impls.rs

@@ -1482,7 +1482,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)
                 }

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<PendingDecl<'ra>>),
     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<ImportResolution<'ra>>)>,
     ) {
         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)
     }