From f3d5c561f4ceee9da9ab516d7c6dbeb920a11db3 Mon Sep 17 00:00:00 2001 From: LorrensP-2158466 Date: Mon, 6 Jul 2026 10:12:10 +0200 Subject: [PATCH] WIP --- compiler/rustc_data_structures/src/sync.rs | 14 ++-- .../src/sync/parallel.rs | 8 ++ .../rustc_resolve/src/build_reduced_graph.rs | 77 +++++++++++------ compiler/rustc_resolve/src/ident.rs | 8 +- compiler/rustc_resolve/src/imports.rs | 82 +++++++++++-------- compiler/rustc_resolve/src/lib.rs | 72 +++++++--------- 6 files changed, 150 insertions(+), 111 deletions(-) diff --git a/compiler/rustc_data_structures/src/sync.rs b/compiler/rustc_data_structures/src/sync.rs index 3d5bc85278286..85e030c8aa0c5 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}; @@ -38,8 +38,8 @@ pub use self::mode::{ FromDyn, check_dyn_thread_safe, is_dyn_thread_safe, set_dyn_thread_safe_mode, }; pub use self::parallel::{ - broadcast, par_fns, par_for_each_in, par_join, par_map, parallel_guard, spawn, - try_par_for_each_in, + broadcast, par_fns, par_for_each_in, par_for_each_slice, par_join, par_map, parallel_guard, + spawn, try_par_for_each_in, }; pub use self::vec::{AppendOnlyIndexVec, AppendOnlyVec}; pub use self::worker_local::{Registry, WorkerLocal}; diff --git a/compiler/rustc_data_structures/src/sync/parallel.rs b/compiler/rustc_data_structures/src/sync/parallel.rs index 966c34f7e1525..42d9ff5338943 100644 --- a/compiler/rustc_data_structures/src/sync/parallel.rs +++ b/compiler/rustc_data_structures/src/sync/parallel.rs @@ -185,6 +185,14 @@ pub fn par_for_each_in>( }); } +pub fn par_for_each_slice(items: &mut [T], for_each: impl Fn(&mut T)) { + parallel_guard(|guard| { + items.iter_mut().for_each(|i| { + guard.run(|| for_each(i)); + }); + }); +} + /// This runs `for_each` in parallel for each iterator item. If one or more of the /// `for_each` calls returns `Err`, the function will also return `Err`. The error returned /// will be non-deterministic, but this is expected to be used with `ErrorGuaranteed` which diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index 98c7af88550dc..35df953f1ec27 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -5,6 +5,7 @@ //! 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}; @@ -116,35 +117,57 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { if let module @ Some(..) = self.extern_module_map.borrow().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.borrow_mut(); + // 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 RefMut<'_, 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), + // FIXME: Account for `#[no_implicit_prelude]` attributes. + 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> { @@ -537,7 +560,7 @@ impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> { on_unknown_attr: OnUnknownData::from_attrs(self.r, &item.attrs), }); - self.r.indeterminate_imports.push(import); + self.r.indeterminate_imports.push((import, None, 0)); match import.kind { ImportKind::Single { target, .. } => { // Don't add underscore imports to `single_imports` diff --git a/compiler/rustc_resolve/src/ident.rs b/compiler/rustc_resolve/src/ident.rs index 26418dff45a5c..d18180ea64afc 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.get(); + 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); } else { break; } diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 25f7e28b6738c..59667c3753bc6 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -50,11 +50,12 @@ 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 */)>), } -struct ImportResolution<'ra> { +pub(crate) struct ImportResolution<'ra> { kind: ImportResolutionKind<'ra>, imported_module: ModuleOrUniformRoot<'ra>, } @@ -312,9 +313,10 @@ impl<'ra> NameResolution<'ra> { // module to keep the TLS private and only accessible through the function `enter_cycle_detector`. pub(crate) mod cycle_detection { + use std::cell::RefCell; use std::ptr; - use crate::{BindingKey, CacheRefCell, LocalModule}; + use crate::{BindingKey, LocalModule}; thread_local!( /// During import resolution, recursive imports can form cycles. @@ -326,7 +328,7 @@ pub(crate) mod cycle_detection { /// in the `Resolver Arenas` (lifetime `'ra`), it is thus stable and allows casting /// to a `*const ()` for comparison. This is done because we can't use lifetimes /// other than `'static` in thread local storage. - static ACTIVE_RESOLUTIONS: CacheRefCell> = Default::default(); + static ACTIVE_RESOLUTIONS: RefCell> = Default::default(); ); pub(crate) struct ActiveResolutionGuard { @@ -776,30 +778,43 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { while indeterminate_count < prev_indeterminate_count { prev_indeterminate_count = indeterminate_count; indeterminate_count = 0; - let mut resolutions = Vec::new(); + 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; - match import_indeterminate_count { - 0 => self.determined_imports.push(import), - _ => self.indeterminate_imports.push(import), - } - if let Some(resolution) = resolution { - resolutions.push((import, resolution)); - } - } + + let mut to_resolve_imports = mem::take(&mut self.indeterminate_imports); + let cm_resolver = self.cm(); + + rustc_data_structures::sync::par_for_each_slice( + &mut to_resolve_imports, + |(import, resolution, indeterminate_count)| { + let (new_resolution, import_indeterminate_count) = + cm_resolver.reborrow_ref().resolve_import(*import); + *resolution = new_resolution; + *indeterminate_count = import_indeterminate_count + }, + ); self.assert_speculative = false; - self.write_import_resolutions(resolutions); + + self.write_import_resolutions(&to_resolve_imports); + + self.indeterminate_imports = to_resolve_imports + .extract_if(.., |(_, _, count)| { + indeterminate_count += *count; + *count > 0 + }) + .collect(); + self.determined_imports.extend(to_resolve_imports.into_iter().map(|(i, _, _)| i)); } } fn write_import_resolutions( &mut self, - import_resolutions: Vec<(Import<'ra>, ImportResolution<'ra>)>, + import_resolutions: &[(Import<'ra>, Option>, usize)], ) { - for (import, resolution) in &import_resolutions { - let ImportResolution { imported_module, .. } = resolution; + for (import, resolution, _) in import_resolutions { + let Some(ImportResolution { imported_module, .. }) = resolution else { + continue; + }; import.imported_module.set(Some(*imported_module), self); if import.is_glob() @@ -811,8 +826,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } } - for (import, resolution) in import_resolutions { - let ImportResolution { imported_module, kind: resolution_kind } = resolution; + for (import, resolution, _) in import_resolutions { + let Some(ImportResolution { imported_module, kind: resolution_kind }) = resolution + else { + continue; + }; match (&import.kind, resolution_kind) { ( @@ -823,7 +841,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { match import_decls[ns] { PendingDecl::Ready(Some(decl)) => { // We need the `target`, `source` can be extracted. - let import_decl = this.new_import_decl(decl, import); + let import_decl = this.new_import_decl(decl, *import); if import_decl.is_assoc_item() && !this.features.import_trait_associated_functions() { @@ -852,7 +870,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { key, target.span, |_, resolution| { - resolution.single_imports.swap_remove(&import); + resolution.single_imports.swap_remove(import); }, ); } @@ -879,11 +897,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } for (binding, key, orig_ident_span) in imported_decls { - let import_decl = self.new_import_decl(binding, import); + let import_decl = self.new_import_decl(*binding, *import); let _ = self .try_plant_decl_into_local_module( key.ident, - orig_ident_span, + *orig_ident_span, key.ns, import_decl, ) @@ -918,7 +936,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { for (is_indeterminate, import) in determined_imports .iter() .map(|i| (false, i)) - .chain(indeterminate_imports.iter().map(|i| (true, i))) + .chain(indeterminate_imports.iter().map(|(i, _, _)| (true, i))) { let unresolved_import_error = self.finalize_import(*import); // If this import is unresolved then create a dummy import @@ -959,7 +977,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { return; } - for import in &indeterminate_imports { + for (import, _, _) in &indeterminate_imports { let path = import_path_to_string( &import.module_path.iter().map(|seg| seg.ident).collect::>(), &import.kind, @@ -1139,7 +1157,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 +1178,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 9d4ec67d899c8..8c2a12f6ed039 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -24,7 +24,7 @@ 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}; @@ -80,7 +80,7 @@ use tracing::{debug, instrument}; use crate::diagnostics::impls::{ ImportSuggestion, LabelSuggestion, OnUnknownData, StructCtor, Suggestion, }; -use crate::imports::NameResolutionRef; +use crate::imports::{ImportResolution, NameResolutionRef}; use crate::ref_mut::{CmCell, CmRefCell}; mod build_reduced_graph; @@ -656,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, @@ -710,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); @@ -722,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, @@ -1326,7 +1325,7 @@ pub struct Resolver<'ra, 'tcx> { determined_imports: Vec> = Vec::new(), /// All non-determined imports. - indeterminate_imports: Vec> = Vec::new(), + indeterminate_imports: Vec<(Import<'ra>, Option>, usize)> = Vec::new(), // Spans for local variables found during pattern resolution. // Used for suggestions during error reporting. @@ -1878,28 +1877,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"); @@ -2171,11 +2148,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 } @@ -2188,6 +2166,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { self.resolutions(module).borrow().get(&key).map(|resolution| resolution.0.borrow()) } + #[track_caller] fn resolution_or_default( &self, module: Module<'ra>, @@ -2807,38 +2786,47 @@ use std::cell::{Cell as CacheCell, RefCell as CacheRefCell}; mod ref_mut { use std::cell::{BorrowMutError, Cell, Ref, RefCell, RefMut}; use std::fmt; + use std::marker::PhantomData; use std::ops::Deref; 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>, } 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. @@ -2849,7 +2837,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() }, } } } @@ -2901,12 +2891,12 @@ mod ref_mut { } } - /// A wrapper around a [`RefCell`] that only allows mutable borrows based on a condition in the resolver. + /// A wrapper around a [`RefCell`] that only allows writes (mutable borrows) based on a condition in the resolver. #[derive(Default)] pub(crate) struct CmRefCell(RefCell); impl CmRefCell { - pub(crate) const fn new(value: T) -> CmRefCell { + pub(crate) fn new(value: T) -> CmRefCell { CmRefCell(RefCell::new(value)) }