Skip to content

Commit

Permalink
Rollup merge of #119589 - petrochenkov:cdatalock, r=Mark-Simulacrum
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@matthiaskrgr
matthiaskrgr authored Jan 5, 2024
2 parents ad7aabd + 4bc3552 commit 60a2b43
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 58 deletions.
41 changes: 21 additions & 20 deletions compiler/rustc_metadata/src/creader.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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()),
Expand All @@ -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,
Expand Down
39 changes: 19 additions & 20 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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;
Expand Down Expand Up @@ -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>,

Expand All @@ -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.
Expand Down Expand Up @@ -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(),
};
Expand All @@ -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
}
Expand All @@ -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> {
Expand Down
36 changes: 18 additions & 18 deletions compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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 }
Expand All @@ -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 => {
Expand Down Expand Up @@ -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))
}
Expand All @@ -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) }
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 60a2b43

Please sign in to comment.