Skip to content

Commit

Permalink
Auto merge of #111713 - Zoxc:lock-switch, r=nnethercote
Browse files Browse the repository at this point in the history 
Use conditional synchronization for Lock

This changes `Lock` to use synchronization only if `mode::is_dyn_thread_safe` could be true. This reduces overhead for the parallel compiler running with 1 thread.

The emitters are changed to use `DynSend` instead of `Send` so they can still use `Lock`.

A Rayon thread pool is not used with 1 thread anymore, as session globals contains `Lock`s which are no longer `Sync`.

Performance improvement with 1 thread and `cfg(parallel_compiler)`:
<table><tr><td rowspan="2">Benchmark</td><td colspan="1"><b>Before</b></th><td colspan="2"><b>After</b></th></tr><tr><td align="right">Time</td><td align="right">Time</td><td align="right">%</th></tr><tr><td>🟣 <b>clap</b>:check</td><td align="right">1.7665s</td><td align="right">1.7336s</td><td align="right">💚  -1.86%</td></tr><tr><td>🟣 <b>hyper</b>:check</td><td align="right">0.2780s</td><td align="right">0.2736s</td><td align="right">💚  -1.61%</td></tr><tr><td>🟣 <b>regex</b>:check</td><td align="right">0.9994s</td><td align="right">0.9824s</td><td align="right">💚  -1.70%</td></tr><tr><td>🟣 <b>syn</b>:check</td><td align="right">1.5875s</td><td align="right">1.5656s</td><td align="right">💚  -1.38%</td></tr><tr><td>🟣 <b>syntex_syntax</b>:check</td><td align="right">6.0682s</td><td align="right">5.9532s</td><td align="right">💚  -1.90%</td></tr><tr><td>Total</td><td align="right">10.6997s</td><td align="right">10.5083s</td><td align="right">💚  -1.79%</td></tr><tr><td>Summary</td><td align="right">1.0000s</td><td align="right">0.9831s</td><td align="right">💚  -1.69%</td></tr></table>

cc `@SparrowLii`
  • Loading branch information
@bors
bors committed Aug 30, 2023
2 parents 7659abc + d35179f commit 61efe9d
Show file tree
Hide file tree
Showing 13 changed files with 368 additions and 139 deletions.
2 changes: 0 additions & 2 deletions compiler/rustc_data_structures/src/marker.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ cfg_if!(
[std::collections::BTreeMap<K, V, A> where K: DynSend, V: DynSend, A: std::alloc::Allocator + Clone + DynSend]
[Vec<T, A> where T: DynSend, A: std::alloc::Allocator + DynSend]
[Box<T, A> where T: ?Sized + DynSend, A: std::alloc::Allocator + DynSend]
[crate::sync::Lock<T> where T: DynSend]
[crate::sync::RwLock<T> where T: DynSend]
[crate::tagged_ptr::CopyTaggedPtr<P, T, CP> where P: Send + crate::tagged_ptr::Pointer, T: Send + crate::tagged_ptr::Tag, const CP: bool]
[rustc_arena::TypedArena<T> where T: DynSend]
Expand Down Expand Up @@ -171,7 +170,6 @@ cfg_if!(
[std::collections::BTreeMap<K, V, A> where K: DynSync, V: DynSync, A: std::alloc::Allocator + Clone + DynSync]
[Vec<T, A> where T: DynSync, A: std::alloc::Allocator + DynSync]
[Box<T, A> where T: ?Sized + DynSync, A: std::alloc::Allocator + DynSync]
[crate::sync::Lock<T> where T: DynSend]
[crate::sync::RwLock<T> where T: DynSend + DynSync]
[crate::sync::OneThread<T> where T]
[crate::sync::WorkerLocal<T> where T: DynSend]
Expand Down
105 changes: 19 additions & 86 deletions compiler/rustc_data_structures/src/sync.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
//! | `AtomicU64` | `Cell<u64>` | `atomic::AtomicU64` |
//! | `AtomicUsize` | `Cell<usize>` | `atomic::AtomicUsize` |
//! | | | |
//! | `Lock<T>` | `RefCell<T>` | `parking_lot::Mutex<T>` |
//! | `Lock<T>` | `RefCell<T>` | `RefCell<T>` or |
//! | | | `parking_lot::Mutex<T>` |
//! | `RwLock<T>` | `RefCell<T>` | `parking_lot::RwLock<T>` |
//! | `MTLock<T>` [^1] | `T` | `Lock<T>` |
//! | `MTLockRef<'a, T>` [^2] | `&'a mut MTLock<T>` | `&'a MTLock<T>` |
Expand All @@ -45,6 +46,9 @@ use std::hash::{BuildHasher, Hash};
use std::ops::{Deref, DerefMut};
use std::panic::{catch_unwind, resume_unwind, AssertUnwindSafe};

mod lock;
pub use lock::{Lock, LockGuard};

mod worker_local;
pub use worker_local::{Registry, WorkerLocal};

Expand Down Expand Up @@ -75,6 +79,13 @@ mod mode {
}
}

// Whether thread safety might be enabled.
#[inline]
#[cfg(parallel_compiler)]
pub fn might_be_dyn_thread_safe() -> bool {
DYN_THREAD_SAFE_MODE.load(Ordering::Relaxed) != DYN_NOT_THREAD_SAFE
}

// Only set by the `-Z threads` compile option
pub fn set_dyn_thread_safe_mode(mode: bool) {
let set: u8 = if mode { DYN_THREAD_SAFE } else { DYN_NOT_THREAD_SAFE };
Expand All @@ -94,14 +105,15 @@ pub use mode::{is_dyn_thread_safe, set_dyn_thread_safe_mode};

cfg_if! {
if #[cfg(not(parallel_compiler))] {
use std::ops::Add;
use std::cell::Cell;

pub unsafe auto trait Send {}
pub unsafe auto trait Sync {}

unsafe impl<T> Send for T {}
unsafe impl<T> Sync for T {}

use std::ops::Add;

/// This is a single threaded variant of `AtomicU64`, `AtomicUsize`, etc.
/// It has explicit ordering arguments and is only intended for use with
/// the native atomic types.
Expand Down Expand Up @@ -255,15 +267,11 @@ cfg_if! {
pub use std::cell::Ref as MappedReadGuard;
pub use std::cell::RefMut as WriteGuard;
pub use std::cell::RefMut as MappedWriteGuard;
pub use std::cell::RefMut as LockGuard;
pub use std::cell::RefMut as MappedLockGuard;

pub use std::cell::OnceCell;

use std::cell::RefCell as InnerRwLock;
use std::cell::RefCell as InnerLock;

use std::cell::Cell;

pub type MTLockRef<'a, T> = &'a mut MTLock<T>;

Expand Down Expand Up @@ -305,6 +313,8 @@ cfg_if! {
}
}
} else {
use parking_lot::Mutex;

pub use std::marker::Send as Send;
pub use std::marker::Sync as Sync;

Expand All @@ -313,7 +323,6 @@ cfg_if! {
pub use parking_lot::RwLockWriteGuard as WriteGuard;
pub use parking_lot::MappedRwLockWriteGuard as MappedWriteGuard;

pub use parking_lot::MutexGuard as LockGuard;
pub use parking_lot::MappedMutexGuard as MappedLockGuard;

pub use std::sync::OnceLock as OnceCell;
Expand Down Expand Up @@ -355,7 +364,6 @@ cfg_if! {
}
}

use parking_lot::Mutex as InnerLock;
use parking_lot::RwLock as InnerRwLock;

use std::thread;
Expand Down Expand Up @@ -441,7 +449,7 @@ cfg_if! {
) {
if mode::is_dyn_thread_safe() {
let for_each = FromDyn::from(for_each);
let panic: Lock<Option<_>> = Lock::new(None);
let panic: Mutex<Option<_>> = Mutex::new(None);
t.into_par_iter().for_each(|i| if let Err(p) = catch_unwind(AssertUnwindSafe(|| for_each(i))) {
let mut l = panic.lock();
if l.is_none() {
Expand Down Expand Up @@ -479,7 +487,7 @@ cfg_if! {
map: impl Fn(I) -> R + DynSync + DynSend
) -> C {
if mode::is_dyn_thread_safe() {
let panic: Lock<Option<_>> = Lock::new(None);
let panic: Mutex<Option<_>> = Mutex::new(None);
let map = FromDyn::from(map);
// We catch panics here ensuring that all the loop iterations execute.
let r = t.into_par_iter().filter_map(|i| {
Expand Down Expand Up @@ -542,81 +550,6 @@ impl<K: Eq + Hash, V: Eq, S: BuildHasher> HashMapExt<K, V> for HashMap<K, V, S>
}
}

#[derive(Debug)]
pub struct Lock<T>(InnerLock<T>);

impl<T> Lock<T> {
#[inline(always)]
pub fn new(inner: T) -> Self {
Lock(InnerLock::new(inner))
}

#[inline(always)]
pub fn into_inner(self) -> T {
self.0.into_inner()
}

#[inline(always)]
pub fn get_mut(&mut self) -> &mut T {
self.0.get_mut()
}

#[cfg(parallel_compiler)]
#[inline(always)]
pub fn try_lock(&self) -> Option<LockGuard<'_, T>> {
self.0.try_lock()
}

#[cfg(not(parallel_compiler))]
#[inline(always)]
pub fn try_lock(&self) -> Option<LockGuard<'_, T>> {
self.0.try_borrow_mut().ok()
}

#[cfg(parallel_compiler)]
#[inline(always)]
#[track_caller]
pub fn lock(&self) -> LockGuard<'_, T> {
if ERROR_CHECKING {
self.0.try_lock().expect("lock was already held")
} else {
self.0.lock()
}
}

#[cfg(not(parallel_compiler))]
#[inline(always)]
#[track_caller]
pub fn lock(&self) -> LockGuard<'_, T> {
self.0.borrow_mut()
}

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

#[inline(always)]
#[track_caller]
pub fn borrow(&self) -> LockGuard<'_, T> {
self.lock()
}

#[inline(always)]
#[track_caller]
pub fn borrow_mut(&self) -> LockGuard<'_, T> {
self.lock()
}
}

impl<T: Default> Default for Lock<T> {
#[inline]
fn default() -> Self {
Lock::new(T::default())
}
}

#[derive(Debug, Default)]
pub struct RwLock<T>(InnerRwLock<T>);

Expand Down

0 comments on commit 61efe9d

Please sign in to comment.