From 42ceb2567979cacb97b58c779a76207b24620fa0 Mon Sep 17 00:00:00 2001 From: Askar Safin Date: Sat, 8 Feb 2025 11:55:41 +0300 Subject: [PATCH 01/11] compiler/rustc_data_structures/src/sync.rs: these RwLock methods are never called, so let's remove them --- compiler/rustc_data_structures/src/sync.rs | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/compiler/rustc_data_structures/src/sync.rs b/compiler/rustc_data_structures/src/sync.rs index bea87a6685d8d..779127ce32f75 100644 --- a/compiler/rustc_data_structures/src/sync.rs +++ b/compiler/rustc_data_structures/src/sync.rs @@ -203,12 +203,6 @@ impl RwLock { } } - #[inline(always)] - #[track_caller] - pub fn with_read_lock R, R>(&self, f: F) -> R { - f(&*self.read()) - } - #[inline(always)] pub fn try_write(&self) -> Result, ()> { self.0.try_write().ok_or(()) @@ -223,12 +217,6 @@ impl RwLock { } } - #[inline(always)] - #[track_caller] - pub fn with_write_lock R, R>(&self, f: F) -> R { - f(&mut *self.write()) - } - #[inline(always)] #[track_caller] pub fn borrow(&self) -> ReadGuard<'_, T> { @@ -240,14 +228,6 @@ impl RwLock { 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 From 25b6761c9ed7fa814a0182dbd9a04e1c4909b88e Mon Sep 17 00:00:00 2001 From: Askar Safin Date: Sun, 9 Feb 2025 08:10:08 +0300 Subject: [PATCH 02/11] compiler/rustc_data_structures/src/sync.rs: remove "impl Clone for RwLock" parking_lot::RwLock doesn't have "impl Clone", so we should not have, either --- compiler/rustc_data_structures/src/sync.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/compiler/rustc_data_structures/src/sync.rs b/compiler/rustc_data_structures/src/sync.rs index 779127ce32f75..ab457a639c7a3 100644 --- a/compiler/rustc_data_structures/src/sync.rs +++ b/compiler/rustc_data_structures/src/sync.rs @@ -229,11 +229,3 @@ impl RwLock { self.write() } } - -// FIXME: Probably a bad idea -impl Clone for RwLock { - #[inline] - fn clone(&self) -> Self { - RwLock::new(self.borrow().clone()) - } -} From c9c7b44510425c2d594ee0b6653e68a2aead3b6d Mon Sep 17 00:00:00 2001 From: Askar Safin Date: Tue, 11 Feb 2025 08:14:00 +0300 Subject: [PATCH 03/11] compiler/rustc_data_structures/src/sync.rs: delete MappedLockGuard It seems it is left-over after some refactoring --- compiler/rustc_data_structures/src/sync.rs | 5 ++--- src/doc/rustc-dev-guide/src/parallel-rustc.md | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_data_structures/src/sync.rs b/compiler/rustc_data_structures/src/sync.rs index ab457a639c7a3..6ea7360bcedd3 100644 --- a/compiler/rustc_data_structures/src/sync.rs +++ b/compiler/rustc_data_structures/src/sync.rs @@ -112,9 +112,8 @@ 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; diff --git a/src/doc/rustc-dev-guide/src/parallel-rustc.md b/src/doc/rustc-dev-guide/src/parallel-rustc.md index 44c78a125f4ef..49c52976c9cad 100644 --- a/src/doc/rustc-dev-guide/src/parallel-rustc.md +++ b/src/doc/rustc-dev-guide/src/parallel-rustc.md @@ -58,7 +58,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 From 8f684c9db7092d8a0c924ba63b967db021ab9860 Mon Sep 17 00:00:00 2001 From: Askar Safin Date: Tue, 11 Feb 2025 08:25:50 +0300 Subject: [PATCH 04/11] compiler/rustc_data_structures/src/sync.rs: delete Weak --- compiler/rustc_data_structures/src/sync.rs | 3 +-- src/doc/rustc-dev-guide/src/parallel-rustc.md | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/compiler/rustc_data_structures/src/sync.rs b/compiler/rustc_data_structures/src/sync.rs index 6ea7360bcedd3..37f2e1f096e21 100644 --- a/compiler/rustc_data_structures/src/sync.rs +++ b/compiler/rustc_data_structures/src/sync.rs @@ -18,7 +18,6 @@ //! //! | Type | Serial version | Parallel version | //! | ----------------------- | ------------------- | ------------------------------- | -//! |` Weak` | `rc::Weak` | `sync::Weak` | //! | `LRef<'a, T>` [^2] | `&'a mut T` | `&'a T` | //! | | | | //! | `AtomicBool` | `Cell` | `atomic::AtomicBool` | @@ -104,11 +103,11 @@ 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::{ diff --git a/src/doc/rustc-dev-guide/src/parallel-rustc.md b/src/doc/rustc-dev-guide/src/parallel-rustc.md index 49c52976c9cad..4fb91da682b2d 100644 --- a/src/doc/rustc-dev-guide/src/parallel-rustc.md +++ b/src/doc/rustc-dev-guide/src/parallel-rustc.md @@ -46,7 +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) | | OnceCell | std::sync::OnceLock | std::cell::OnceCell | | Lock\ | (parking_lot::Mutex\) | (std::cell::RefCell) | From 4a2c7f48b52fe9ecb34d3e406b12a99ba8de4042 Mon Sep 17 00:00:00 2001 From: Askar Safin Date: Tue, 11 Feb 2025 08:57:36 +0300 Subject: [PATCH 05/11] compiler/rustc_data_structures/src/sync.rs: remove atomics, but not AtomicU64! --- compiler/rustc_data_structures/src/sync.rs | 6 ------ compiler/rustc_data_structures/src/sync/freeze.rs | 4 ++-- compiler/rustc_query_system/src/dep_graph/graph.rs | 4 ++-- src/doc/rustc-dev-guide/src/parallel-rustc.md | 1 - 4 files changed, 4 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_data_structures/src/sync.rs b/compiler/rustc_data_structures/src/sync.rs index 37f2e1f096e21..e93a0891f0a01 100644 --- a/compiler/rustc_data_structures/src/sync.rs +++ b/compiler/rustc_data_structures/src/sync.rs @@ -20,11 +20,6 @@ //! | ----------------------- | ------------------- | ------------------------------- | //! | `LRef<'a, T>` [^2] | `&'a mut T` | `&'a T` | //! | | | | -//! | `AtomicBool` | `Cell` | `atomic::AtomicBool` | -//! | `AtomicU32` | `Cell` | `atomic::AtomicU32` | -//! | `AtomicU64` | `Cell` | `atomic::AtomicU64` | -//! | `AtomicUsize` | `Cell` | `atomic::AtomicUsize` | -//! | | | | //! | `Lock` | `RefCell` | `RefCell` or | //! | | | `parking_lot::Mutex` | //! | `RwLock` | `RefCell` | `parking_lot::RwLock` | @@ -107,7 +102,6 @@ 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 mode::{is_dyn_thread_safe, set_dyn_thread_safe_mode}; pub use parking_lot::{ diff --git a/compiler/rustc_data_structures/src/sync/freeze.rs b/compiler/rustc_data_structures/src/sync/freeze.rs index 5236c9fe15679..9720b22ea7d1b 100644 --- a/compiler/rustc_data_structures/src/sync/freeze.rs +++ b/compiler/rustc_data_structures/src/sync/freeze.rs @@ -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. diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index 7988afd3e1393..5e6bee1dbd5a1 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -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}; diff --git a/src/doc/rustc-dev-guide/src/parallel-rustc.md b/src/doc/rustc-dev-guide/src/parallel-rustc.md index 4fb91da682b2d..deddf77e34770 100644 --- a/src/doc/rustc-dev-guide/src/parallel-rustc.md +++ b/src/doc/rustc-dev-guide/src/parallel-rustc.md @@ -46,7 +46,6 @@ are implemented differently depending on whether `parallel-compiler` is true. | data structure | parallel | non-parallel | | -------------------------------- | --------------------------------------------------- | ------------ | -| Atomic{Bool}/{Usize}/{U32}/{U64} | std::sync::atomic::Atomic{Bool}/{Usize}/{U32}/{U64} | (std::cell::Cell) | | OnceCell | std::sync::OnceLock | std::cell::OnceCell | | Lock\ | (parking_lot::Mutex\) | (std::cell::RefCell) | | RwLock\ | (parking_lot::RwLock\) | (std::cell::RefCell) | From d79f9ca2573276742f8a946dff72f9a04e842860 Mon Sep 17 00:00:00 2001 From: Askar Safin Date: Tue, 11 Feb 2025 09:15:54 +0300 Subject: [PATCH 06/11] compiler/rustc_data_structures/src/sync.rs: delete Sync and Send --- compiler/rustc_data_structures/src/owned_slice.rs | 15 +++++---------- compiler/rustc_data_structures/src/sync.rs | 1 - 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_data_structures/src/owned_slice.rs b/compiler/rustc_data_structures/src/owned_slice.rs index 17c48aee6fa30..0c00e4f4a4b31 100644 --- a/compiler/rustc_data_structures/src/owned_slice.rs +++ b/compiler/rustc_data_structures/src/owned_slice.rs @@ -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 @@ -34,7 +29,7 @@ pub struct OwnedSlice { // \/ // ⊂(´･◡･⊂ )∘˚˳° (I am the phantom remnant of #97770) #[expect(dead_code)] - owner: Arc, + owner: Arc, } /// Makes an [`OwnedSlice`] out of an `owner` and a `slicer` function. @@ -61,7 +56,7 @@ pub struct OwnedSlice { /// ``` pub fn slice_owned(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() @@ -72,7 +67,7 @@ where /// See [`slice_owned`] for the infallible version. pub fn try_slice_owned(owner: O, slicer: F) -> Result 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. @@ -139,10 +134,10 @@ impl Borrow<[u8]> for OwnedSlice { } // Safety: `OwnedSlice` is conceptually `(&'self.1 [u8], Arc)`, which is `Send` -unsafe impl sync::Send for OwnedSlice {} +unsafe impl Send for OwnedSlice {} // Safety: `OwnedSlice` is conceptually `(&'self.1 [u8], Arc)`, which is `Sync` -unsafe impl sync::Sync for OwnedSlice {} +unsafe impl Sync for OwnedSlice {} #[cfg(test)] mod tests; diff --git a/compiler/rustc_data_structures/src/sync.rs b/compiler/rustc_data_structures/src/sync.rs index e93a0891f0a01..37b54fe38ff74 100644 --- a/compiler/rustc_data_structures/src/sync.rs +++ b/compiler/rustc_data_structures/src/sync.rs @@ -97,7 +97,6 @@ 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")] From afa01c145c290b2fd19f36db7037beb46a7ed0c9 Mon Sep 17 00:00:00 2001 From: Askar Safin Date: Tue, 11 Feb 2025 09:23:54 +0300 Subject: [PATCH 07/11] src/doc/rustc-dev-guide/src/parallel-rustc.md: remove Arc and Rc (it seems they are left-over after my PR) --- src/doc/rustc-dev-guide/src/parallel-rustc.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/doc/rustc-dev-guide/src/parallel-rustc.md b/src/doc/rustc-dev-guide/src/parallel-rustc.md index deddf77e34770..c5b70706a8180 100644 --- a/src/doc/rustc-dev-guide/src/parallel-rustc.md +++ b/src/doc/rustc-dev-guide/src/parallel-rustc.md @@ -170,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 From 851cc4bed408586a4adb599bb953e069692deb8a Mon Sep 17 00:00:00 2001 From: Askar Safin Date: Tue, 11 Feb 2025 09:47:13 +0300 Subject: [PATCH 08/11] compiler/rustc_codegen_gcc/src/back/lto.rs: delete "unsafe impl Sync/Send" --- compiler/rustc_codegen_gcc/src/back/lto.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/compiler/rustc_codegen_gcc/src/back/lto.rs b/compiler/rustc_codegen_gcc/src/back/lto.rs index e419bd1809900..cb4caec8c326c 100644 --- a/compiler/rustc_codegen_gcc/src/back/lto.rs +++ b/compiler/rustc_codegen_gcc/src/back/lto.rs @@ -710,10 +710,6 @@ pub struct ThinBuffer { context: Arc, } -// 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) -> Self { Self { context: Arc::clone(context) } From 51f49d84649d27af91e9fc8f6aac319be100cecd Mon Sep 17 00:00:00 2001 From: Askar Safin Date: Tue, 11 Feb 2025 09:58:53 +0300 Subject: [PATCH 09/11] compiler/rustc_codegen_llvm/src/lib.rs: remove "unsafe impl Send/Sync" --- compiler/rustc_codegen_llvm/src/lib.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index f0d04b2b64476..51380d82943c6 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -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 { Box::new(LlvmCodegenBackend(())) From 02406903b0c26440428580a4bbaf30da973c5b23 Mon Sep 17 00:00:00 2001 From: Askar Safin Date: Tue, 11 Feb 2025 10:21:17 +0300 Subject: [PATCH 10/11] compiler/rustc_data_structures/src/sync/worker_local.rs: delete "unsafe impl Sync" --- .../rustc_data_structures/src/sync/worker_local.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_data_structures/src/sync/worker_local.rs b/compiler/rustc_data_structures/src/sync/worker_local.rs index d75af00985047..402ec9827bbaa 100644 --- a/compiler/rustc_data_structures/src/sync/worker_local.rs +++ b/compiler/rustc_data_structures/src/sync/worker_local.rs @@ -106,12 +106,6 @@ pub struct WorkerLocal { 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 Sync for WorkerLocal {} - impl WorkerLocal { /// Creates a new worker local where the `initial` closure computes the /// value this worker local should take for each thread in the registry. @@ -138,6 +132,11 @@ impl Deref for WorkerLocal { 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 } } } From 8506dd264439fd2db26d73aa1fa504b47c33b8f4 Mon Sep 17 00:00:00 2001 From: Askar Safin Date: Tue, 11 Feb 2025 11:08:48 +0300 Subject: [PATCH 11/11] config.example.toml: remove "parallel-compiler" --- config.example.toml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/config.example.toml b/config.example.toml index 7f6bbf2799288..75ad030231016 100644 --- a/config.example.toml +++ b/config.example.toml @@ -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