From 4bc3552cd871afe32cb45f7c9685492af430ed77 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Thu, 4 Jan 2024 21:09:13 +0300 Subject: [PATCH] cstore: Remove unnecessary locking from `CrateMetadata` --- compiler/rustc_metadata/src/creader.rs | 41 ++++++++++--------- compiler/rustc_metadata/src/rmeta/decoder.rs | 39 +++++++++--------- .../src/rmeta/decoder/cstore_impl.rs | 36 ++++++++-------- 3 files changed, 58 insertions(+), 58 deletions(-) diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index a3da8c14f63a7..bb02a8a1e4743 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/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 { + 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, 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 { let cnum = self.resolve_crate(name, span, CrateDepKind::Explicit)?; - self.update_extern_crate( + self.cstore.update_extern_crate( cnum, ExternCrate { src: ExternCrateSource::Path, diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index 2de29db9e5c84..d13a1664adea4 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/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, + dependencies: Vec, /// How to link (or not link) this crate to the currently compiled crate. - dep_kind: Lock, + dep_kind: CrateDepKind, /// Filesystem location of this crate. source: Lrc, /// 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, @@ -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>, + extern_crate: Option, } /// 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 + '_ { - 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 { diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index bb8f4af8e97ae..912c2f36eb3c8 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/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 {