Skip to content

Commit

Permalink
Rollup merge of #136858 - safinaskar:parallel-cleanup-2025-02-11-07-5…
Browse files Browse the repository at this point in the history
…4, r=SparrowLii

Parallel-compiler-related cleanup

Parallel-compiler-related cleanup

I carefully split changes into commits. Commit messages are self-explanatory. Squashing is not recommended.

cc "Parallel Rustc Front-end" #113349

r? SparrowLii

``@rustbot`` label: +WG-compiler-parallel
  • Loading branch information
jhpratt authored Feb 13, 2025
2 parents 4ea2610 + 8506dd2 commit 1f669fd
Show file tree
Hide file tree
Showing 9 changed files with 17 additions and 77 deletions.
4 changes: 0 additions & 4 deletions compiler/rustc_codegen_gcc/src/back/lto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -710,10 +710,6 @@ pub struct ThinBuffer {
context: Arc<SyncContext>,
}

// TODO: check if this makes sense to make ThinBuffer Send and Sync.
unsafe impl Send for ThinBuffer {}
unsafe impl Sync for ThinBuffer {}

impl ThinBuffer {
pub(crate) fn new(context: &Arc<SyncContext>) -> Self {
Self { context: Arc::clone(context) }
Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_codegen_llvm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,6 @@ impl WriteBackendMethods for LlvmCodegenBackend {
}
}

unsafe impl Send for LlvmCodegenBackend {} // Llvm is on a per-thread basis
unsafe impl Sync for LlvmCodegenBackend {}

impl LlvmCodegenBackend {
pub fn new() -> Box<dyn CodegenBackend> {
Box::new(LlvmCodegenBackend(()))
Expand Down
15 changes: 5 additions & 10 deletions compiler/rustc_data_structures/src/owned_slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,6 @@ use std::borrow::Borrow;
use std::ops::Deref;
use std::sync::Arc;

// Use our fake Send/Sync traits when on not parallel compiler,
// so that `OwnedSlice` only implements/requires Send/Sync
// for parallel compiler builds.
use crate::sync;

/// An owned slice.
///
/// This is similar to `Arc<[u8]>` but allows slicing and using anything as the
Expand Down Expand Up @@ -34,7 +29,7 @@ pub struct OwnedSlice {
// \/
// ⊂(´・◡・⊂ )∘˚˳° (I am the phantom remnant of #97770)
#[expect(dead_code)]
owner: Arc<dyn sync::Send + sync::Sync>,
owner: Arc<dyn Send + Sync>,
}

/// Makes an [`OwnedSlice`] out of an `owner` and a `slicer` function.
Expand All @@ -61,7 +56,7 @@ pub struct OwnedSlice {
/// ```
pub fn slice_owned<O, F>(owner: O, slicer: F) -> OwnedSlice
where
O: sync::Send + sync::Sync + 'static,
O: Send + Sync + 'static,
F: FnOnce(&O) -> &[u8],
{
try_slice_owned(owner, |x| Ok::<_, !>(slicer(x))).into_ok()
Expand All @@ -72,7 +67,7 @@ where
/// See [`slice_owned`] for the infallible version.
pub fn try_slice_owned<O, F, E>(owner: O, slicer: F) -> Result<OwnedSlice, E>
where
O: sync::Send + sync::Sync + 'static,
O: Send + Sync + 'static,
F: FnOnce(&O) -> Result<&[u8], E>,
{
// We wrap the owner of the bytes in, so it doesn't move.
Expand Down Expand Up @@ -139,10 +134,10 @@ impl Borrow<[u8]> for OwnedSlice {
}

// Safety: `OwnedSlice` is conceptually `(&'self.1 [u8], Arc<dyn Send + Sync>)`, which is `Send`
unsafe impl sync::Send for OwnedSlice {}
unsafe impl Send for OwnedSlice {}

// Safety: `OwnedSlice` is conceptually `(&'self.1 [u8], Arc<dyn Send + Sync>)`, which is `Sync`
unsafe impl sync::Sync for OwnedSlice {}
unsafe impl Sync for OwnedSlice {}

#[cfg(test)]
mod tests;
43 changes: 3 additions & 40 deletions compiler/rustc_data_structures/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,8 @@
//!
//! | Type | Serial version | Parallel version |
//! | ----------------------- | ------------------- | ------------------------------- |
//! |` Weak<T>` | `rc::Weak<T>` | `sync::Weak<T>` |
//! | `LRef<'a, T>` [^2] | `&'a mut T` | `&'a T` |
//! | | | |
//! | `AtomicBool` | `Cell<bool>` | `atomic::AtomicBool` |
//! | `AtomicU32` | `Cell<u32>` | `atomic::AtomicU32` |
//! | `AtomicU64` | `Cell<u64>` | `atomic::AtomicU64` |
//! | `AtomicUsize` | `Cell<usize>` | `atomic::AtomicUsize` |
//! | | | |
//! | `Lock<T>` | `RefCell<T>` | `RefCell<T>` or |
//! | | | `parking_lot::Mutex<T>` |
//! | `RwLock<T>` | `RefCell<T>` | `parking_lot::RwLock<T>` |
Expand Down Expand Up @@ -103,18 +97,15 @@ mod mode {

// FIXME(parallel_compiler): Get rid of these aliases across the compiler.

pub use std::marker::{Send, Sync};
pub use std::sync::OnceLock;
// Use portable AtomicU64 for targets without native 64-bit atomics
#[cfg(target_has_atomic = "64")]
pub use std::sync::atomic::AtomicU64;
pub use std::sync::atomic::{AtomicBool, AtomicU32, AtomicUsize};
pub use std::sync::{OnceLock, Weak};

pub use mode::{is_dyn_thread_safe, set_dyn_thread_safe_mode};
pub use parking_lot::{
MappedMutexGuard as MappedLockGuard, MappedRwLockReadGuard as MappedReadGuard,
MappedRwLockWriteGuard as MappedWriteGuard, RwLockReadGuard as ReadGuard,
RwLockWriteGuard as WriteGuard,
MappedRwLockReadGuard as MappedReadGuard, MappedRwLockWriteGuard as MappedWriteGuard,
RwLockReadGuard as ReadGuard, RwLockWriteGuard as WriteGuard,
};
#[cfg(not(target_has_atomic = "64"))]
pub use portable_atomic::AtomicU64;
Expand Down Expand Up @@ -203,12 +194,6 @@ impl<T> RwLock<T> {
}
}

#[inline(always)]
#[track_caller]
pub fn with_read_lock<F: FnOnce(&T) -> R, R>(&self, f: F) -> R {
f(&*self.read())
}

#[inline(always)]
pub fn try_write(&self) -> Result<WriteGuard<'_, T>, ()> {
self.0.try_write().ok_or(())
Expand All @@ -223,12 +208,6 @@ impl<T> RwLock<T> {
}
}

#[inline(always)]
#[track_caller]
pub fn with_write_lock<F: FnOnce(&mut T) -> R, R>(&self, f: F) -> R {
f(&mut *self.write())
}

#[inline(always)]
#[track_caller]
pub fn borrow(&self) -> ReadGuard<'_, T> {
Expand All @@ -240,20 +219,4 @@ impl<T> RwLock<T> {
pub fn borrow_mut(&self) -> WriteGuard<'_, T> {
self.write()
}

#[inline(always)]
pub fn leak(&self) -> &T {
let guard = self.read();
let ret = unsafe { &*(&raw const *guard) };
std::mem::forget(guard);
ret
}
}

// FIXME: Probably a bad idea
impl<T: Clone> Clone for RwLock<T> {
#[inline]
fn clone(&self) -> Self {
RwLock::new(self.borrow().clone())
}
}
4 changes: 2 additions & 2 deletions compiler/rustc_data_structures/src/sync/freeze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ use std::intrinsics::likely;
use std::marker::PhantomData;
use std::ops::{Deref, DerefMut};
use std::ptr::NonNull;
use std::sync::atomic::Ordering;
use std::sync::atomic::{AtomicBool, Ordering};

use crate::sync::{AtomicBool, DynSend, DynSync, ReadGuard, RwLock, WriteGuard};
use crate::sync::{DynSend, DynSync, ReadGuard, RwLock, WriteGuard};

/// A type which allows mutation using a lock until
/// the value is frozen and can be accessed lock-free.
Expand Down
11 changes: 5 additions & 6 deletions compiler/rustc_data_structures/src/sync/worker_local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,6 @@ pub struct WorkerLocal<T> {
registry: Registry,
}

// This is safe because the `deref` call will return a reference to a `T` unique to each thread
// or it will panic for threads without an associated local. So there isn't a need for `T` to do
// it's own synchronization. The `verify` method on `RegistryId` has an issue where the id
// can be reused, but `WorkerLocal` has a reference to `Registry` which will prevent any reuse.
unsafe impl<T: Send> Sync for WorkerLocal<T> {}

impl<T> WorkerLocal<T> {
/// Creates a new worker local where the `initial` closure computes the
/// value this worker local should take for each thread in the registry.
Expand All @@ -138,6 +132,11 @@ impl<T> Deref for WorkerLocal<T> {
fn deref(&self) -> &T {
// This is safe because `verify` will only return values less than
// `self.registry.thread_limit` which is the size of the `self.locals` array.

// The `deref` call will return a reference to a `T` unique to each thread
// or it will panic for threads without an associated local. So there isn't a need for `T` to do
// it's own synchronization. The `verify` method on `RegistryId` has an issue where the id
// can be reused, but `WorkerLocal` has a reference to `Registry` which will prevent any reuse.
unsafe { &self.locals.get_unchecked(self.registry.id().verify()).0 }
}
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_query_system/src/dep_graph/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ use std::fmt::Debug;
use std::hash::Hash;
use std::marker::PhantomData;
use std::sync::Arc;
use std::sync::atomic::Ordering;
use std::sync::atomic::{AtomicU32, Ordering};

use rustc_data_structures::fingerprint::Fingerprint;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::profiling::{QueryInvocationId, SelfProfilerRef};
use rustc_data_structures::sharded::{self, Sharded};
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_data_structures::sync::{AtomicU32, AtomicU64, Lock};
use rustc_data_structures::sync::{AtomicU64, Lock};
use rustc_data_structures::unord::UnordMap;
use rustc_index::IndexVec;
use rustc_macros::{Decodable, Encodable};
Expand Down
5 changes: 0 additions & 5 deletions config.example.toml
Original file line number Diff line number Diff line change
Expand Up @@ -595,11 +595,6 @@
# Whether to always use incremental compilation when building rustc
#incremental = false

# Build a multi-threaded rustc. This allows users to use parallel rustc
# via the unstable option `-Z threads=n`.
# This option is deprecated and always true.
#parallel-compiler = true

# The default linker that will be hard-coded into the generated
# compiler for targets that don't specify a default linker explicitly
# in their target specifications. Note that this is not the linker
Expand Down
5 changes: 0 additions & 5 deletions src/doc/rustc-dev-guide/src/parallel-rustc.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ are implemented differently depending on whether `parallel-compiler` is true.

| data structure | parallel | non-parallel |
| -------------------------------- | --------------------------------------------------- | ------------ |
| Weak | std::sync::Weak | std::rc::Weak |
| Atomic{Bool}/{Usize}/{U32}/{U64} | std::sync::atomic::Atomic{Bool}/{Usize}/{U32}/{U64} | (std::cell::Cell<bool/usize/u32/u64>) |
| OnceCell | std::sync::OnceLock | std::cell::OnceCell |
| Lock\<T> | (parking_lot::Mutex\<T>) | (std::cell::RefCell) |
| RwLock\<T> | (parking_lot::RwLock\<T>) | (std::cell::RefCell) |
Expand All @@ -58,7 +56,6 @@ are implemented differently depending on whether `parallel-compiler` is true.
| WriteGuard | parking_lot::RwLockWriteGuard | std::cell::RefMut |
| MappedWriteGuard | parking_lot::MappedRwLockWriteGuard | std::cell::RefMut |
| LockGuard | parking_lot::MutexGuard | std::cell::RefMut |
| MappedLockGuard | parking_lot::MappedMutexGuard | std::cell::RefMut |

- These thread-safe data structures are interspersed during compilation which
can cause lock contention resulting in degraded performance as the number of
Expand Down Expand Up @@ -173,12 +170,10 @@ Here are some resources that can be used to learn more:
- [This list of interior mutability in the compiler by nikomatsakis][imlist]

[`rayon`]: https://crates.io/crates/rayon
[Arc]: https://doc.rust-lang.org/std/sync/struct.Arc.html
[imlist]: https://github.com/nikomatsakis/rustc-parallelization/blob/master/interior-mutability-list.md
[irlo0]: https://internals.rust-lang.org/t/parallelizing-rustc-using-rayon/6606
[irlo1]: https://internals.rust-lang.org/t/help-test-parallel-rustc/11503
[monomorphization]: backend/monomorph.md
[parallel-rustdoc]: https://github.com/rust-lang/rust/issues/82741
[Rc]: https://doc.rust-lang.org/std/rc/struct.Rc.html
[rustc-rayon]: https://github.com/rust-lang/rustc-rayon
[tracking]: https://github.com/rust-lang/rust/issues/48685

0 comments on commit 1f669fd

Please sign in to comment.