Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cstore: Remove unnecessary locking from CrateMetadata #119589

Merged
merged 1 commit into from
Jan 5, 2024
Merged

cstore: Remove unnecessary locking from CrateMetadata #119589

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 21 additions & 20 deletions compiler/rustc_metadata/src/creader.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading