From eeb5b782a613b37c13bfd39588a3e3ae6dcf0ff5 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 30 Mar 2023 10:57:44 +1100 Subject: [PATCH 1/4] Improve the `rustc_data_structures::sync` module doc comment. Also, `MTRef<'a, T>` is a typedef for a reference to a `T`, but in practice it's only used (and useful) in combination with `MTLock`, i.e. `MTRef<'a, MTLock>`. So this commit changes it to be a typedef for a reference to an `MTLock`, and renames it as `MTLockRef`. I think this clarifies things, because I found `MTRef` quite puzzling at first. --- compiler/rustc_data_structures/src/sync.rs | 53 ++++++++++++++------ compiler/rustc_monomorphize/src/collector.rs | 10 ++-- 2 files changed, 44 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_data_structures/src/sync.rs b/compiler/rustc_data_structures/src/sync.rs index 31323c21df009..09765607cbd54 100644 --- a/compiler/rustc_data_structures/src/sync.rs +++ b/compiler/rustc_data_structures/src/sync.rs @@ -1,21 +1,46 @@ -//! This module defines types which are thread safe if cfg!(parallel_compiler) is true. +//! This module defines various operations and types that are implemented in +//! one way for the serial compiler, and another way the parallel compiler. //! -//! `Lrc` is an alias of `Arc` if cfg!(parallel_compiler) is true, `Rc` otherwise. +//! Operations +//! ---------- +//! The parallel versions of operations use Rayon to execute code in parallel, +//! while the serial versions degenerate straightforwardly to serial execution. +//! The operations include `join`, `parallel`, `par_iter`, and `par_for_each`. //! -//! `Lock` is a mutex. -//! It internally uses `parking_lot::Mutex` if cfg!(parallel_compiler) is true, -//! `RefCell` otherwise. +//! `rustc_erase_owner!` erases an `OwningRef` owner into `Erased` for the +//! serial version and `Erased + Send + Sync` for the parallel version. //! -//! `RwLock` is a read-write lock. -//! It internally uses `parking_lot::RwLock` if cfg!(parallel_compiler) is true, -//! `RefCell` otherwise. +//! Types +//! ----- +//! The parallel versions of types provide various kinds of synchronization, +//! while the serial compiler versions do not. //! -//! `MTLock` is a mutex which disappears if cfg!(parallel_compiler) is false. +//! The following table shows how the types are implemented internally. Except +//! where noted otherwise, the type in column one is defined as a +//! newtype around the type from column two or three. //! -//! `MTRef` is an immutable reference if cfg!(parallel_compiler), and a mutable reference otherwise. +//! | Type | Serial version | Parallel version | +//! | ----------------------- | ------------------- | ------------------------------- | +//! | `Lrc` | `rc::Rc` | `sync::Arc` | +//! |` Weak` | `rc::Weak` | `sync::Weak` | +//! | | | | +//! | `AtomicBool` | `Cell` | `atomic::AtomicBool` | +//! | `AtomicU32` | `Cell` | `atomic::AtomicU32` | +//! | `AtomicU64` | `Cell` | `atomic::AtomicU64` | +//! | `AtomicUsize` | `Cell` | `atomic::AtomicUsize` | +//! | | | | +//! | `Lock` | `RefCell` | `parking_lot::Mutex` | +//! | `RwLock` | `RefCell` | `parking_lot::RwLock` | +//! | `MTLock` [^1] | `T` | `Lock` | +//! | `MTLockRef<'a, T>` [^2] | `&'a mut MTLock` | `&'a MTLock` | +//! | | | | +//! | `ParallelIterator` | `Iterator` | `rayon::iter::ParallelIterator` | //! -//! `rustc_erase_owner!` erases an OwningRef owner into Erased or Erased + Send + Sync -//! depending on the value of cfg!(parallel_compiler). +//! [^1] `MTLock` is similar to `Lock`, but the serial version avoids the cost +//! of a `RefCell`. This is appropriate when interior mutability is not +//! required. +//! +//! [^2] `MTLockRef` is a typedef. use crate::owning_ref::{Erased, OwningRef}; use std::collections::HashMap; @@ -209,7 +234,7 @@ cfg_if! { } } - pub type MTRef<'a, T> = &'a mut T; + pub type MTLockRef<'a, T> = &'a mut MTLock; #[derive(Debug, Default)] pub struct MTLock(T); @@ -267,7 +292,7 @@ cfg_if! { pub use std::sync::Arc as Lrc; pub use std::sync::Weak as Weak; - pub type MTRef<'a, T> = &'a T; + pub type MTLockRef<'a, T> = &'a MTLock; #[derive(Debug, Default)] pub struct MTLock(Lock); diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index 98265d58a0a59..e24d31c43d377 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -174,7 +174,7 @@ //! regardless of whether it is actually needed or not. use rustc_data_structures::fx::{FxHashMap, FxHashSet}; -use rustc_data_structures::sync::{par_for_each_in, MTLock, MTRef}; +use rustc_data_structures::sync::{par_for_each_in, MTLock, MTLockRef}; use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::{DefId, DefIdMap, LocalDefId}; @@ -340,8 +340,8 @@ pub fn collect_crate_mono_items( let recursion_limit = tcx.recursion_limit(); { - let visited: MTRef<'_, _> = &mut visited; - let inlining_map: MTRef<'_, _> = &mut inlining_map; + let visited: MTLockRef<'_, _> = &mut visited; + let inlining_map: MTLockRef<'_, _> = &mut inlining_map; tcx.sess.time("monomorphization_collector_graph_walk", || { par_for_each_in(roots, |root| { @@ -406,10 +406,10 @@ fn collect_roots(tcx: TyCtxt<'_>, mode: MonoItemCollectionMode) -> Vec( tcx: TyCtxt<'tcx>, starting_point: Spanned>, - visited: MTRef<'_, MTLock>>>, + visited: MTLockRef<'_, FxHashSet>>, recursion_depths: &mut DefIdMap, recursion_limit: Limit, - inlining_map: MTRef<'_, MTLock>>, + inlining_map: MTLockRef<'_, InliningMap<'tcx>>, ) { if !visited.lock_mut().insert(starting_point.node) { // We've been here already, no need to search again. From 0ccb60096ac8d166c740ff1180ba33b5f7421c8f Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 30 Mar 2023 13:00:28 +1100 Subject: [PATCH 2/4] Remove `RwLock::clone_guard`. It's unused. --- compiler/rustc_data_structures/src/sync.rs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/compiler/rustc_data_structures/src/sync.rs b/compiler/rustc_data_structures/src/sync.rs index 09765607cbd54..4e2126fff7be9 100644 --- a/compiler/rustc_data_structures/src/sync.rs +++ b/compiler/rustc_data_structures/src/sync.rs @@ -578,18 +578,6 @@ impl RwLock { self.write() } - #[cfg(not(parallel_compiler))] - #[inline(always)] - pub fn clone_guard<'a>(rg: &ReadGuard<'a, T>) -> ReadGuard<'a, T> { - ReadGuard::clone(rg) - } - - #[cfg(parallel_compiler)] - #[inline(always)] - pub fn clone_guard<'a>(rg: &ReadGuard<'a, T>) -> ReadGuard<'a, T> { - ReadGuard::rwlock(&rg).read() - } - #[cfg(not(parallel_compiler))] #[inline(always)] pub fn leak(&self) -> &T { From 44bfb6538e553a8da386270c7399076b3a2f725d Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 30 Mar 2023 16:48:03 +1100 Subject: [PATCH 3/4] `CacheAligned` and `Sharded` don't need to derive `Clone`. --- compiler/rustc_data_structures/src/sharded.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/rustc_data_structures/src/sharded.rs b/compiler/rustc_data_structures/src/sharded.rs index 01d292dde8d13..f88c055a9b569 100644 --- a/compiler/rustc_data_structures/src/sharded.rs +++ b/compiler/rustc_data_structures/src/sharded.rs @@ -5,7 +5,7 @@ use std::collections::hash_map::RawEntryMut; use std::hash::{Hash, Hasher}; use std::mem; -#[derive(Clone, Default)] +#[derive(Default)] #[cfg_attr(parallel_compiler, repr(align(64)))] struct CacheAligned(T); @@ -21,7 +21,6 @@ const SHARD_BITS: usize = 0; pub const SHARDS: usize = 1 << SHARD_BITS; /// An array of cache-line aligned inner locked structures with convenience methods. -#[derive(Clone)] pub struct Sharded { shards: [CacheAligned>; SHARDS], } From 08dec8969fe526c4419d066dc01344031a0ddc1f Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 30 Mar 2023 16:53:32 +1100 Subject: [PATCH 4/4] Remove an out-of-date comment on `QueryCache::lookup`. --- compiler/rustc_query_system/src/query/caches.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/compiler/rustc_query_system/src/query/caches.rs b/compiler/rustc_query_system/src/query/caches.rs index 5f554a54deac1..d3efc22a19451 100644 --- a/compiler/rustc_query_system/src/query/caches.rs +++ b/compiler/rustc_query_system/src/query/caches.rs @@ -21,9 +21,6 @@ pub trait QueryCache: Sized { type Value: Copy + Debug; /// Checks if the query is already computed and in the cache. - /// It returns the shard index and a lock guard to the shard, - /// which will be used if the query is not in the cache and we need - /// to compute it. fn lookup(&self, key: &Self::Key) -> Option<(Self::Value, DepNodeIndex)>; fn complete(&self, key: Self::Key, value: Self::Value, index: DepNodeIndex);