Skip to content

WIP: Perf test using a RwLock unconditionally #115557

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.

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion compiler/rustc_interface/src/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ impl<'tcx> Queries<'tcx> {
self.codegen_backend().metadata_loader(),
stable_crate_id,
)) as _);
let definitions = RwLock::new(Definitions::new(stable_crate_id));
let definitions = std::sync::RwLock::new(Definitions::new(stable_crate_id));
let source_span = AppendOnlyIndexVec::new();
let _id = source_span.push(krate.spans.inner_span);
debug_assert_eq!(_id, CRATE_DEF_ID);
Expand Down
30 changes: 20 additions & 10 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -878,7 +878,12 @@ impl<'tcx> TyCtxt<'tcx> {
// If this is a DefPathHash from the local crate, we can look up the
// DefId in the tcx's `Definitions`.
if stable_crate_id == self.stable_crate_id(LOCAL_CRATE) {
self.untracked.definitions.read().local_def_path_hash_to_def_id(hash, err).to_def_id()
self.untracked
.definitions
.read()
.unwrap()
.local_def_path_hash_to_def_id(hash, err)
.to_def_id()
} else {
// If this is a DefPathHash from an upstream crate, let the CrateStore map
// it to a DefId.
Expand Down Expand Up @@ -938,7 +943,7 @@ impl<'tcx> TyCtxtAt<'tcx> {
// This is fine because:
// - those queries are `eval_always` so we won't miss their result changing;
// - this write will have happened before these queries are called.
let key = self.untracked.definitions.write().create_def(parent, data);
let key = self.untracked.definitions.write().unwrap().create_def(parent, data);

let feed = TyCtxtFeed { tcx: self.tcx, key };
feed.def_span(self.span);
Expand All @@ -958,14 +963,14 @@ impl<'tcx> TyCtxt<'tcx> {

// Recompute the number of definitions each time, because our caller may be creating
// new ones.
while i < { definitions.read().num_definitions() } {
while i < { definitions.read().unwrap().num_definitions() } {
let local_def_index = rustc_span::def_id::DefIndex::from_usize(i);
yield LocalDefId { local_def_index };
i += 1;
}

// Leak a read lock once we finish iterating on definitions, to prevent adding new ones.
definitions.leak();
std::mem::forget(definitions.read());
})
}

Expand All @@ -976,8 +981,9 @@ impl<'tcx> TyCtxt<'tcx> {

// Leak a read lock once we start iterating on definitions, to prevent adding new ones
// while iterating. If some query needs to add definitions, it should be `ensure`d above.
let definitions = self.untracked.definitions.leak();
definitions.def_path_table()
std::mem::forget(self.untracked.definitions.read());
let definitions = self.untracked.definitions.read().unwrap();
unsafe { &*(definitions.def_path_table() as *const rustc_hir::definitions::DefPathTable) }
}

pub fn def_path_hash_to_def_index_map(
Expand All @@ -988,8 +994,12 @@ impl<'tcx> TyCtxt<'tcx> {
self.ensure().hir_crate(());
// Leak a read lock once we start iterating on definitions, to prevent adding new ones
// while iterating. If some query needs to add definitions, it should be `ensure`d above.
let definitions = self.untracked.definitions.leak();
definitions.def_path_hash_to_def_index_map()
std::mem::forget(self.untracked.definitions.read());
let definitions = self.untracked.definitions.read().unwrap();
unsafe {
&*(definitions.def_path_hash_to_def_index_map()
as *const rustc_hir::def_path_hash_map::DefPathHashMap)
}
}

/// Note that this is *untracked* and should only be used within the query
Expand All @@ -1006,8 +1016,8 @@ impl<'tcx> TyCtxt<'tcx> {
/// Note that this is *untracked* and should only be used within the query
/// system if the result is otherwise tracked through queries
#[inline]
pub fn definitions_untracked(self) -> ReadGuard<'tcx, Definitions> {
self.untracked.definitions.read()
pub fn definitions_untracked(self) -> std::sync::RwLockReadGuard<'tcx, Definitions> {
self.untracked.definitions.read().unwrap()
}

/// Note that this is *untracked* and should only be used within the query
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_query_system/src/ich/hcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ impl<'a> StableHashingContext<'a> {

#[inline]
pub fn local_def_path_hash(&self, def_id: LocalDefId) -> DefPathHash {
self.untracked.definitions.read().def_path_hash(def_id)
self.untracked.definitions.read().unwrap().def_path_hash(def_id)
}

#[inline]
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1207,7 +1207,7 @@ impl<'tcx> Resolver<'_, 'tcx> {
);

// FIXME: remove `def_span` body, pass in the right spans here and call `tcx.at().create_def()`
let def_id = self.tcx.untracked().definitions.write().create_def(parent, data);
let def_id = self.tcx.untracked().definitions.write().unwrap().create_def(parent, data);

// Create the definition.
if expn_id != ExpnId::root() {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_session/src/cstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,5 +261,5 @@ pub struct Untracked {
pub cstore: RwLock<Box<CrateStoreDyn>>,
/// Reference span for definitions.
pub source_span: AppendOnlyIndexVec<LocalDefId, Span>,
pub definitions: RwLock<Definitions>,
pub definitions: std::sync::RwLock<Definitions>,
}