Rollup merge of #119589 - petrochenkov:cdatalock, r=Mark-Simulacrum

cstore: Remove unnecessary locking from `CrateMetadata`

Locks and atomics in `CrateMetadata` fields were necessary before #107765 when `CStore` was cloneable, but now they are not necessary and can be removed after restructuring the code a bit to please borrow checker.

All remaining locked fields in `CrateMetadata` are lazily populated caches.

Author: matthiaskrgr

compiler/rustc_metadata/src/creader.rs

@@ -196,6 +196,10 @@ impl CStore {
         CrateMetadataRef { cdata, cstore: self }
     }
 
+    pub(crate) fn get_crate_data_mut(&mut self, cnum: CrateNum) -> &mut CrateMetadata {
+        self.metas[cnum].as_mut().unwrap_or_else(|| panic!("Failed to get crate data for {cnum:?}"))
+    }
+
     fn set_crate_data(&mut self, cnum: CrateNum, data: CrateMetadata) {
         assert!(self.metas[cnum].is_none(), "Overwriting crate metadata entry");
         self.metas[cnum] = Some(Box::new(data));
@@ -207,6 +211,12 @@ impl CStore {
             .filter_map(|(cnum, data)| data.as_deref().map(|data| (cnum, data)))
     }
 
+    fn iter_crate_data_mut(&mut self) -> impl Iterator<Item = (CrateNum, &mut CrateMetadata)> {
+        self.metas
+            .iter_enumerated_mut()
+            .filter_map(|(cnum, data)| data.as_deref_mut().map(|data| (cnum, data)))
+    }
+
     fn push_dependencies_in_postorder(&self, deps: &mut Vec<CrateNum>, cnum: CrateNum) {
         if !deps.contains(&cnum) {
             let data = self.get_crate_data(cnum);
@@ -586,11 +596,11 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
 
         match result {
             (LoadResult::Previous(cnum), None) => {
-                let data = self.cstore.get_crate_data(cnum);
+                let data = self.cstore.get_crate_data_mut(cnum);
                 if data.is_proc_macro_crate() {
                     dep_kind = CrateDepKind::MacrosOnly;
                 }
-                data.update_dep_kind(|data_dep_kind| cmp::max(data_dep_kind, dep_kind));
+                data.set_dep_kind(cmp::max(data.dep_kind(), dep_kind));
                 if let Some(private_dep) = private_dep {
                     data.update_and_private_dep(private_dep);
                 }
@@ -637,17 +647,6 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
         }))
     }
 
-    fn update_extern_crate(&self, cnum: CrateNum, extern_crate: ExternCrate) {
-        let cmeta = self.cstore.get_crate_data(cnum);
-        if cmeta.update_extern_crate(extern_crate) {
-            // Propagate the extern crate info to dependencies if it was updated.
-            let extern_crate = ExternCrate { dependency_of: cnum, ..extern_crate };
-            for dep_cnum in cmeta.dependencies() {
-                self.update_extern_crate(dep_cnum, extern_crate);
-            }
-        }
-    }
-
     // Go through the crate metadata and load any crates that it references
     fn resolve_crate_deps(
         &mut self,
@@ -726,17 +725,19 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
         let mut runtime_found = false;
         let mut needs_panic_runtime = attr::contains_name(&krate.attrs, sym::needs_panic_runtime);
 
+        let mut panic_runtimes = Vec::new();
         for (cnum, data) in self.cstore.iter_crate_data() {
             needs_panic_runtime = needs_panic_runtime || data.needs_panic_runtime();
             if data.is_panic_runtime() {
                 // Inject a dependency from all #![needs_panic_runtime] to this
                 // #![panic_runtime] crate.
-                self.inject_dependency_if(cnum, "a panic runtime", &|data| {
-                    data.needs_panic_runtime()
-                });
+                panic_runtimes.push(cnum);
                 runtime_found = runtime_found || data.dep_kind() == CrateDepKind::Explicit;
             }
         }
+        for cnum in panic_runtimes {
+            self.inject_dependency_if(cnum, "a panic runtime", &|data| data.needs_panic_runtime());
+        }
 
         // If an explicitly linked and matching panic runtime was found, or if
         // we just don't need one at all, then we're done here and there's
@@ -917,7 +918,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
     }
 
     fn inject_dependency_if(
-        &self,
+        &mut self,
         krate: CrateNum,
         what: &str,
         needs_dep: &dyn Fn(&CrateMetadata) -> bool,
@@ -947,7 +948,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
         // crate provided for this compile, but in order for this compilation to
         // be successfully linked we need to inject a dependency (to order the
         // crates on the command line correctly).
-        for (cnum, data) in self.cstore.iter_crate_data() {
+        for (cnum, data) in self.cstore.iter_crate_data_mut() {
             if needs_dep(data) {
                 info!("injecting a dep from {} to {}", cnum, krate);
                 data.add_dependency(krate);
@@ -1031,7 +1032,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
                 let cnum = self.resolve_crate(name, item.span, dep_kind)?;
 
                 let path_len = definitions.def_path(def_id).data.len();
-                self.update_extern_crate(
+                self.cstore.update_extern_crate(
                     cnum,
                     ExternCrate {
                         src: ExternCrateSource::Extern(def_id.to_def_id()),
@@ -1049,7 +1050,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
     pub fn process_path_extern(&mut self, name: Symbol, span: Span) -> Option<CrateNum> {
         let cnum = self.resolve_crate(name, span, CrateDepKind::Explicit)?;
 
-        self.update_extern_crate(
+        self.cstore.update_extern_crate(
             cnum,
             ExternCrate {
                 src: ExternCrateSource::Path,

compiler/rustc_metadata/src/rmeta/decoder.rs

@@ -8,7 +8,7 @@ use rustc_ast as ast;
 use rustc_data_structures::captures::Captures;
 use rustc_data_structures::fingerprint::Fingerprint;
 use rustc_data_structures::owned_slice::OwnedSlice;
-use rustc_data_structures::sync::{AppendOnlyVec, AtomicBool, Lock, Lrc, OnceLock};
+use rustc_data_structures::sync::{Lock, Lrc, OnceLock};
 use rustc_data_structures::unhash::UnhashMap;
 use rustc_expand::base::{SyntaxExtension, SyntaxExtensionKind};
 use rustc_expand::proc_macro::{AttrProcMacro, BangProcMacro, DeriveProcMacro};
@@ -31,7 +31,6 @@ use rustc_span::{BytePos, Pos, SpanData, SyntaxContext, DUMMY_SP};
 use proc_macro::bridge::client::ProcMacro;
 use std::iter::TrustedLen;
 use std::path::Path;
-use std::sync::atomic::Ordering;
 use std::{io, iter, mem};
 
 pub(super) use cstore_impl::provide;
@@ -96,15 +95,15 @@ pub(crate) struct CrateMetadata {
     /// IDs as they are seen from the current compilation session.
     cnum_map: CrateNumMap,
     /// Same ID set as `cnum_map` plus maybe some injected crates like panic runtime.
-    dependencies: AppendOnlyVec<CrateNum>,
+    dependencies: Vec<CrateNum>,
     /// How to link (or not link) this crate to the currently compiled crate.
-    dep_kind: Lock<CrateDepKind>,
+    dep_kind: CrateDepKind,
     /// Filesystem location of this crate.
     source: Lrc<CrateSource>,
     /// Whether or not this crate should be consider a private dependency.
     /// Used by the 'exported_private_dependencies' lint, and for determining
     /// whether to emit suggestions that reference this crate.
-    private_dep: AtomicBool,
+    private_dep: bool,
     /// The hash for the host proc macro. Used to support `-Z dual-proc-macro`.
     host_hash: Option<Svh>,
 
@@ -118,7 +117,7 @@ pub(crate) struct CrateMetadata {
     // --- Data used only for improving diagnostics ---
     /// Information about the `extern crate` item or path that caused this crate to be loaded.
     /// If this is `None`, then the crate was injected (e.g., by the allocator).
-    extern_crate: Lock<Option<ExternCrate>>,
+    extern_crate: Option<ExternCrate>,
 }
 
 /// Holds information about a rustc_span::SourceFile imported from another crate.
@@ -1818,11 +1817,11 @@ impl CrateMetadata {
             cnum,
             cnum_map,
             dependencies,
-            dep_kind: Lock::new(dep_kind),
+            dep_kind,
             source: Lrc::new(source),
-            private_dep: AtomicBool::new(private_dep),
+            private_dep,
             host_hash,
-            extern_crate: Lock::new(None),
+            extern_crate: None,
             hygiene_context: Default::default(),
             def_key_cache: Default::default(),
         };
@@ -1839,18 +1838,18 @@ impl CrateMetadata {
     }
 
     pub(crate) fn dependencies(&self) -> impl Iterator<Item = CrateNum> + '_ {
-        self.dependencies.iter()
+        self.dependencies.iter().copied()
     }
 
-    pub(crate) fn add_dependency(&self, cnum: CrateNum) {
+    pub(crate) fn add_dependency(&mut self, cnum: CrateNum) {
         self.dependencies.push(cnum);
     }
 
-    pub(crate) fn update_extern_crate(&self, new_extern_crate: ExternCrate) -> bool {
-        let mut extern_crate = self.extern_crate.borrow_mut();
-        let update = Some(new_extern_crate.rank()) > extern_crate.as_ref().map(ExternCrate::rank);
+    pub(crate) fn update_extern_crate(&mut self, new_extern_crate: ExternCrate) -> bool {
+        let update =
+            Some(new_extern_crate.rank()) > self.extern_crate.as_ref().map(ExternCrate::rank);
         if update {
-            *extern_crate = Some(new_extern_crate);
+            self.extern_crate = Some(new_extern_crate);
         }
         update
     }
@@ -1860,15 +1859,15 @@ impl CrateMetadata {
     }
 
     pub(crate) fn dep_kind(&self) -> CrateDepKind {
-        *self.dep_kind.lock()
+        self.dep_kind
     }
 
-    pub(crate) fn update_dep_kind(&self, f: impl FnOnce(CrateDepKind) -> CrateDepKind) {
-        self.dep_kind.with_lock(|dep_kind| *dep_kind = f(*dep_kind))
+    pub(crate) fn set_dep_kind(&mut self, dep_kind: CrateDepKind) {
+        self.dep_kind = dep_kind;
     }
 
-    pub(crate) fn update_and_private_dep(&self, private_dep: bool) {
-        self.private_dep.fetch_and(private_dep, Ordering::SeqCst);
+    pub(crate) fn update_and_private_dep(&mut self, private_dep: bool) {
+        self.private_dep &= private_dep;
     }
 
     pub(crate) fn required_panic_strategy(&self) -> Option<PanicStrategy> {

compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs

@@ -19,7 +19,7 @@ use rustc_middle::query::LocalCrate;
 use rustc_middle::ty::fast_reject::SimplifiedType;
 use rustc_middle::ty::{self, TyCtxt};
 use rustc_middle::util::Providers;
-use rustc_session::cstore::CrateStore;
+use rustc_session::cstore::{CrateStore, ExternCrate};
 use rustc_session::{Session, StableCrateId};
 use rustc_span::hygiene::{ExpnHash, ExpnId};
 use rustc_span::symbol::{kw, Symbol};
@@ -290,13 +290,7 @@ provide! { tcx, def_id, other, cdata,
     cross_crate_inlinable => { cdata.cross_crate_inlinable(def_id.index) }
 
     dylib_dependency_formats => { cdata.get_dylib_dependency_formats(tcx) }
-    is_private_dep => {
-        // Parallel compiler needs to synchronize type checking and linting (which use this flag)
-        // so that they happen strictly crate loading. Otherwise, the full list of available
-        // impls aren't loaded yet.
-        use std::sync::atomic::Ordering;
-        cdata.private_dep.load(Ordering::Acquire)
-    }
+    is_private_dep => { cdata.private_dep }
     is_panic_runtime => { cdata.root.panic_runtime }
     is_compiler_builtins => { cdata.root.compiler_builtins }
     has_global_allocator => { cdata.root.has_global_allocator }
@@ -305,10 +299,7 @@ provide! { tcx, def_id, other, cdata,
     is_profiler_runtime => { cdata.root.profiler_runtime }
     required_panic_strategy => { cdata.root.required_panic_strategy }
     panic_in_drop_strategy => { cdata.root.panic_in_drop_strategy }
-    extern_crate => {
-        let r = *cdata.extern_crate.lock();
-        r.map(|c| &*tcx.arena.alloc(c))
-    }
+    extern_crate => { cdata.extern_crate.map(|c| &*tcx.arena.alloc(c)) }
     is_no_builtins => { cdata.root.no_builtins }
     symbol_mangling_version => { cdata.root.symbol_mangling_version }
     reachable_non_generics => {
@@ -339,10 +330,7 @@ provide! { tcx, def_id, other, cdata,
     implementations_of_trait => { cdata.get_implementations_of_trait(tcx, other) }
     crate_incoherent_impls => { cdata.get_incoherent_impls(tcx, other) }
 
-    dep_kind => {
-        let r = *cdata.dep_kind.lock();
-        r
-    }
+    dep_kind => { cdata.dep_kind }
     module_children => {
         tcx.arena.alloc_from_iter(cdata.get_module_children(def_id.index, tcx.sess))
     }
@@ -357,8 +345,7 @@ provide! { tcx, def_id, other, cdata,
     missing_lang_items => { cdata.get_missing_lang_items(tcx) }
 
     missing_extern_crate_item => {
-        let r = matches!(*cdata.extern_crate.borrow(), Some(extern_crate) if !extern_crate.is_direct());
-        r
+        matches!(cdata.extern_crate, Some(extern_crate) if !extern_crate.is_direct())
     }
 
     used_crate_source => { Lrc::clone(&cdata.source) }
@@ -581,6 +568,19 @@ impl CStore {
     ) -> Span {
         self.get_crate_data(cnum).get_proc_macro_quoted_span(id, sess)
     }
+
+    pub(crate) fn update_extern_crate(&mut self, cnum: CrateNum, extern_crate: ExternCrate) {
+        let cmeta = self.get_crate_data_mut(cnum);
+        if cmeta.update_extern_crate(extern_crate) {
+            // Propagate the extern crate info to dependencies if it was updated.
+            let extern_crate = ExternCrate { dependency_of: cnum, ..extern_crate };
+            let dependencies = std::mem::take(&mut cmeta.dependencies);
+            for &dep_cnum in &dependencies {
+                self.update_extern_crate(dep_cnum, extern_crate);
+            }
+            self.get_crate_data_mut(cnum).dependencies = dependencies;
+        }
+    }
 }
 
 impl CrateStore for CStore {