OpenE2K
/
rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Auto merge of #79261 - faern:deprecate-compare-and-swap, r=Amanieu 

Deprecate atomic compare_and_swap method

Finish implementing [RFC 1443](https://github.com/rust-lang/rfcs/blob/master/text/1443-extended-compare-and-swap.md) (https://github.com/rust-lang/rfcs/pull/1443).

It was decided to deprecate `compare_and_swap` [back in Rust 1.12 already](https://github.com/rust-lang/rust/issues/31767#issuecomment-215903038). I can't find any info about that decision being reverted. My understanding is just that it has been forgotten. If there has been a decision on keeping `compare_and_swap` then it's hard to find, and even if this PR does not go through it can act as a place where people can find out about the decision being reverted.

Atomic operations are hard to understand, very hard. And it does not help that there are multiple similar methods to do compare and swap with. They are so similar that for a reader it might be hard to understand the difference. This PR aims to make that simpler by finally deprecating `compare_and_swap` which is essentially just a more limited version of `compare_exchange`. The documentation is also updated (according to the RFC text) to explain the differences a bit better.

Even if we decide to not deprecate `compare_and_swap`. I still think the documentation for the atomic operations should be improved to better describe their differences and similarities. And the documentation can be written nicer than the PR currently proposes, but I wanted to start somewhere. Most of it is just copied from the RFC.

The documentation for `compare_exchange` and `compare_exchange_weak` indeed describe how they work! The problem is that they are more complex and harder to understand than `compare_and_swap`. So for someone who does not fully grasp this they might fall back to using `compare_and_swap`. Making the documentation outline the similarities and differences might build a bridge for people so they can cross over to the more powerful and sometimes more efficient operations.

The conversions I do to avoid the `std` internal deprecation errors are very straight forward `compare_and_swap -> compare_exchange` changes where the orderings are just using the mapping in the new documentation. Only in one place did I use `compare_exchange_weak`. This can probably be improved further. But the goal here was not for those operations to be perfect. Just to not get worse and to allow the deprecation to happen.
This commit is contained in:
bors 2020-12-23 09:32:38 +00:00
parent 28d73a3ee3 454f3ed902
commit 87eecd40e8
17 changed files with 170 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

107
library/core/src/sync/atomic.rs
View File

@ -464,6 +464,23 @@ impl AtomicBool {
/// **Note:** This method is only available on platforms that support atomic
/// operations on `u8`.
///
/// # Migrating to `compare_exchange` and `compare_exchange_weak`
///
/// `compare_and_swap` is equivalent to `compare_exchange` with the following mapping for
/// memory orderings:
///
/// Original | Success | Failure
/// -------- | ------- | -------
/// Relaxed | Relaxed | Relaxed
/// Acquire | Acquire | Acquire
/// Release | Release | Relaxed
/// AcqRel | AcqRel | Acquire
/// SeqCst | SeqCst | SeqCst
///
/// `compare_exchange_weak` is allowed to fail spuriously even when the comparison succeeds,
/// which allows the compiler to generate better assembly code when the compare and swap
/// is used in a loop.
///
/// # Examples
///
/// ```
@ -479,6 +496,10 @@ impl AtomicBool {
/// ```
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_deprecated(
since = "1.50.0",
reason = "Use `compare_exchange` or `compare_exchange_weak` instead"
)]
#[cfg(target_has_atomic = "8")]
pub fn compare_and_swap(&self, current: bool, new: bool, order: Ordering) -> bool {
match self.compare_exchange(current, new, order, strongest_failure_ordering(order)) {
@ -493,9 +514,10 @@ impl AtomicBool {
/// the previous value. On success this value is guaranteed to be equal to `current`.
///
/// `compare_exchange` takes two [`Ordering`] arguments to describe the memory
/// ordering of this operation. The first describes the required ordering if the
/// operation succeeds while the second describes the required ordering when the
/// operation fails. Using [`Acquire`] as success ordering makes the store part
/// ordering of this operation. `success` describes the required ordering for the
/// read-modify-write operation that takes place if the comparison with `current` succeeds.
/// `failure` describes the required ordering for the load operation that takes place when
/// the comparison fails. Using [`Acquire`] as success ordering makes the store part
/// of this operation [`Relaxed`], and using [`Release`] makes the successful load
/// [`Relaxed`]. The failure ordering can only be [`SeqCst`], [`Acquire`] or [`Relaxed`]
/// and must be equivalent to or weaker than the success ordering.
@ -525,6 +547,7 @@ impl AtomicBool {
/// ```
#[inline]
#[stable(feature = "extended_compare_and_swap", since = "1.10.0")]
#[doc(alias = "compare_and_swap")]
#[cfg(target_has_atomic = "8")]
pub fn compare_exchange(
&self,
@ -550,9 +573,10 @@ impl AtomicBool {
/// previous value.
///
/// `compare_exchange_weak` takes two [`Ordering`] arguments to describe the memory
/// ordering of this operation. The first describes the required ordering if the
/// operation succeeds while the second describes the required ordering when the
/// operation fails. Using [`Acquire`] as success ordering makes the store part
/// ordering of this operation. `success` describes the required ordering for the
/// read-modify-write operation that takes place if the comparison with `current` succeeds.
/// `failure` describes the required ordering for the load operation that takes place when
/// the comparison fails. Using [`Acquire`] as success ordering makes the store part
/// of this operation [`Relaxed`], and using [`Release`] makes the successful load
/// [`Relaxed`]. The failure ordering can only be [`SeqCst`], [`Acquire`] or [`Relaxed`]
/// and must be equivalent to or weaker than the success ordering.
@ -578,6 +602,7 @@ impl AtomicBool {
/// ```
#[inline]
#[stable(feature = "extended_compare_and_swap", since = "1.10.0")]
#[doc(alias = "compare_and_swap")]
#[cfg(target_has_atomic = "8")]
pub fn compare_exchange_weak(
&self,
@ -1066,6 +1091,23 @@ impl<T> AtomicPtr<T> {
/// **Note:** This method is only available on platforms that support atomic
/// operations on pointers.
///
/// # Migrating to `compare_exchange` and `compare_exchange_weak`
///
/// `compare_and_swap` is equivalent to `compare_exchange` with the following mapping for
/// memory orderings:
///
/// Original | Success | Failure
/// -------- | ------- | -------
/// Relaxed | Relaxed | Relaxed
/// Acquire | Acquire | Acquire
/// Release | Release | Relaxed
/// AcqRel | AcqRel | Acquire
/// SeqCst | SeqCst | SeqCst
///
/// `compare_exchange_weak` is allowed to fail spuriously even when the comparison succeeds,
/// which allows the compiler to generate better assembly code when the compare and swap
/// is used in a loop.
///
/// # Examples
///
/// ```
@ -1080,6 +1122,10 @@ impl<T> AtomicPtr<T> {
/// ```
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_deprecated(
since = "1.50.0",
reason = "Use `compare_exchange` or `compare_exchange_weak` instead"
)]
#[cfg(target_has_atomic = "ptr")]
pub fn compare_and_swap(&self, current: *mut T, new: *mut T, order: Ordering) -> *mut T {
match self.compare_exchange(current, new, order, strongest_failure_ordering(order)) {
@ -1094,9 +1140,10 @@ impl<T> AtomicPtr<T> {
/// the previous value. On success this value is guaranteed to be equal to `current`.
///
/// `compare_exchange` takes two [`Ordering`] arguments to describe the memory
/// ordering of this operation. The first describes the required ordering if the
/// operation succeeds while the second describes the required ordering when the
/// operation fails. Using [`Acquire`] as success ordering makes the store part
/// ordering of this operation. `success` describes the required ordering for the
/// read-modify-write operation that takes place if the comparison with `current` succeeds.
/// `failure` describes the required ordering for the load operation that takes place when
/// the comparison fails. Using [`Acquire`] as success ordering makes the store part
/// of this operation [`Relaxed`], and using [`Release`] makes the successful load
/// [`Relaxed`]. The failure ordering can only be [`SeqCst`], [`Acquire`] or [`Relaxed`]
/// and must be equivalent to or weaker than the success ordering.
@ -1157,9 +1204,10 @@ impl<T> AtomicPtr<T> {
/// previous value.
///
/// `compare_exchange_weak` takes two [`Ordering`] arguments to describe the memory
/// ordering of this operation. The first describes the required ordering if the
/// operation succeeds while the second describes the required ordering when the
/// operation fails. Using [`Acquire`] as success ordering makes the store part
/// ordering of this operation. `success` describes the required ordering for the
/// read-modify-write operation that takes place if the comparison with `current` succeeds.
/// `failure` describes the required ordering for the load operation that takes place when
/// the comparison fails. Using [`Acquire`] as success ordering makes the store part
/// of this operation [`Relaxed`], and using [`Release`] makes the successful load
/// [`Relaxed`]. The failure ordering can only be [`SeqCst`], [`Acquire`] or [`Relaxed`]
/// and must be equivalent to or weaker than the success ordering.
@ -1604,6 +1652,23 @@ happens, and using [`Release`] makes the load part [`Relaxed`].
**Note**: This method is only available on platforms that support atomic
operations on [`", $s_int_type, "`](", $int_ref, ").
# Migrating to `compare_exchange` and `compare_exchange_weak`
`compare_and_swap` is equivalent to `compare_exchange` with the following mapping for
memory orderings:
Original | Success | Failure
-------- | ------- | -------
Relaxed | Relaxed | Relaxed
Acquire | Acquire | Acquire
Release | Release | Relaxed
AcqRel | AcqRel | Acquire
SeqCst | SeqCst | SeqCst
`compare_exchange_weak` is allowed to fail spuriously even when the comparison succeeds,
which allows the compiler to generate better assembly code when the compare and swap
is used in a loop.
# Examples
```
@ -1619,6 +1684,10 @@ assert_eq!(some_var.load(Ordering::Relaxed), 10);
```"),
#[inline]
#[$stable]
#[rustc_deprecated(
since = "1.50.0",
reason = "Use `compare_exchange` or `compare_exchange_weak` instead")
]
#[$cfg_cas]
pub fn compare_and_swap(&self,
current: $int_type,
@ -1643,9 +1712,10 @@ containing the previous value. On success this value is guaranteed to be equal t
`current`.
`compare_exchange` takes two [`Ordering`] arguments to describe the memory
ordering of this operation. The first describes the required ordering if the
operation succeeds while the second describes the required ordering when the
operation fails. Using [`Acquire`] as success ordering makes the store part
ordering of this operation. `success` describes the required ordering for the
read-modify-write operation that takes place if the comparison with `current` succeeds.
`failure` describes the required ordering for the load operation that takes place when
the comparison fails. Using [`Acquire`] as success ordering makes the store part
of this operation [`Relaxed`], and using [`Release`] makes the successful load
[`Relaxed`]. The failure ordering can only be [`SeqCst`], [`Acquire`] or [`Relaxed`]
and must be equivalent to or weaker than the success ordering.
@ -1695,9 +1765,10 @@ platforms. The return value is a result indicating whether the new value was
written and containing the previous value.
`compare_exchange_weak` takes two [`Ordering`] arguments to describe the memory
ordering of this operation. The first describes the required ordering if the
operation succeeds while the second describes the required ordering when the
operation fails. Using [`Acquire`] as success ordering makes the store part
ordering of this operation. `success` describes the required ordering for the
read-modify-write operation that takes place if the comparison with `current` succeeds.
`failure` describes the required ordering for the load operation that takes place when
the comparison fails. Using [`Acquire`] as success ordering makes the store part
of this operation [`Relaxed`], and using [`Release`] makes the successful load
[`Relaxed`]. The failure ordering can only be [`SeqCst`], [`Acquire`] or [`Relaxed`]
and must be equivalent to or weaker than the success ordering.

6
library/core/tests/atomic.rs
View File

@ -4,11 +4,11 @@ use core::sync::atomic::*;
#[test]
fn bool_() {
let a = AtomicBool::new(false);
assert_eq!(a.compare_and_swap(false, true, SeqCst), false);
assert_eq!(a.compare_and_swap(false, true, SeqCst), true);
assert_eq!(a.compare_exchange(false, true, SeqCst, SeqCst), Ok(false));
assert_eq!(a.compare_exchange(false, true, SeqCst, SeqCst), Err(true));
a.store(false, SeqCst);
assert_eq!(a.compare_and_swap(false, true, SeqCst), false);
assert_eq!(a.compare_exchange(false, true, SeqCst, SeqCst), Ok(false));
}
#[test]

6
library/std/src/sync/mpsc/blocking.rs
View File

@ -36,7 +36,11 @@ pub fn tokens() -> (WaitToken, SignalToken) {
impl SignalToken {
pub fn signal(&self) -> bool {
let wake = !self.inner.woken.compare_and_swap(false, true, Ordering::SeqCst);
let wake = self
.inner
.woken
.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst)
.is_ok();
if wake {
self.inner.thread.unpark();
}

14
library/std/src/sync/mpsc/oneshot.rs
View File

@ -129,7 +129,7 @@ impl<T> Packet<T> {
let ptr = unsafe { signal_token.cast_to_usize() };
// race with senders to enter the blocking state
if self.state.compare_and_swap(EMPTY, ptr, Ordering::SeqCst) == EMPTY {
if self.state.compare_exchange(EMPTY, ptr, Ordering::SeqCst, Ordering::SeqCst).is_ok() {
if let Some(deadline) = deadline {
let timed_out = !wait_token.wait_max_until(deadline);
// Try to reset the state
@ -161,7 +161,12 @@ impl<T> Packet<T> {
// the state changes under our feet we'd rather just see that state
// change.
DATA => {
self.state.compare_and_swap(DATA, EMPTY, Ordering::SeqCst);
let _ = self.state.compare_exchange(
DATA,
EMPTY,
Ordering::SeqCst,
Ordering::SeqCst,
);
match (&mut *self.data.get()).take() {
Some(data) => Ok(data),
None => unreachable!(),
@ -264,7 +269,10 @@ impl<T> Packet<T> {
// If we've got a blocked thread, then use an atomic to gain ownership
// of it (may fail)
ptr => self.state.compare_and_swap(ptr, EMPTY, Ordering::SeqCst),
ptr => self
.state
.compare_exchange(ptr, EMPTY, Ordering::SeqCst, Ordering::SeqCst)
.unwrap_or_else(|x| x),
};
// Now that we've got ownership of our state, figure out what to do

11
library/std/src/sync/mpsc/shared.rs
View File

@ -385,8 +385,15 @@ impl<T> Packet<T> {
self.port_dropped.store(true, Ordering::SeqCst);
let mut steals = unsafe { *self.steals.get() };
while {
let cnt = self.cnt.compare_and_swap(steals, DISCONNECTED, Ordering::SeqCst);
cnt != DISCONNECTED && cnt != steals
match self.cnt.compare_exchange(
steals,
DISCONNECTED,
Ordering::SeqCst,
Ordering::SeqCst,
) {
Ok(_) => false,
Err(old) => old != DISCONNECTED,
}
} {
// See the discussion in 'try_recv' for why we yield
// control of this thread.

9
library/std/src/sync/mpsc/stream.rs
View File

@ -322,12 +322,15 @@ impl<T> Packet<T> {
// (because there is a bounded number of senders).
let mut steals = unsafe { *self.queue.consumer_addition().steals.get() };
while {
let cnt = self.queue.producer_addition().cnt.compare_and_swap(
match self.queue.producer_addition().cnt.compare_exchange(
steals,
DISCONNECTED,
Ordering::SeqCst,
);
cnt != DISCONNECTED && cnt != steals
Ordering::SeqCst,
) {
Ok(_) => false,
Err(old) => old != DISCONNECTED,
}
} {
while self.queue.pop().is_some() {
steals += 1;

18
library/std/src/sync/once.rs
View File

@ -65,7 +65,7 @@
// must do so with Release ordering to make the result available.
// - `wait` inserts `Waiter` nodes as a pointer in `state_and_queue`, and
// needs to make the nodes available with Release ordering. The load in
// its `compare_and_swap` can be Relaxed because it only has to compare
// its `compare_exchange` can be Relaxed because it only has to compare
// the atomic, not to read other data.
// - `WaiterQueue::Drop` must see the `Waiter` nodes, so it must load
// `state_and_queue` with Acquire ordering.
@ -110,7 +110,7 @@ use crate::thread::{self, Thread};
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub struct Once {
// `state_and_queue` is actually an a pointer to a `Waiter` with extra state
// `state_and_queue` is actually a pointer to a `Waiter` with extra state
// bits, so we add the `PhantomData` appropriately.
state_and_queue: AtomicUsize,
_marker: marker::PhantomData<*const Waiter>,
@ -395,12 +395,13 @@ impl Once {
}
POISONED | INCOMPLETE => {
// Try to register this thread as the one RUNNING.
let old = self.state_and_queue.compare_and_swap(
let exchange_result = self.state_and_queue.compare_exchange(
state_and_queue,
RUNNING,
Ordering::Acquire,
Ordering::Acquire,
);
if old != state_and_queue {
if let Err(old) = exchange_result {
state_and_queue = old;
continue;
}
@ -452,8 +453,13 @@ fn wait(state_and_queue: &AtomicUsize, mut current_state: usize) {
// Try to slide in the node at the head of the linked list, making sure
// that another thread didn't just replace the head of the linked list.
let old = state_and_queue.compare_and_swap(current_state, me | RUNNING, Ordering::Release);
if old != current_state {
let exchange_result = state_and_queue.compare_exchange(
current_state,
me | RUNNING,
Ordering::Release,
Ordering::Relaxed,
);
if let Err(old) = exchange_result {
current_state = old;
continue;
}

8
library/std/src/sys/sgx/abi/mod.rs
View File

@ -36,20 +36,20 @@ unsafe extern "C" fn tcs_init(secondary: bool) {
}
// Try to atomically swap UNINIT with BUSY. The returned state can be:
match RELOC_STATE.compare_and_swap(UNINIT, BUSY, Ordering::Acquire) {
match RELOC_STATE.compare_exchange(UNINIT, BUSY, Ordering::Acquire, Ordering::Acquire) {
// This thread just obtained the lock and other threads will observe BUSY
UNINIT => {
Ok(_) => {
reloc::relocate_elf_rela();
RELOC_STATE.store(DONE, Ordering::Release);
}
// We need to wait until the initialization is done.
BUSY => {
Err(BUSY) => {
while RELOC_STATE.load(Ordering::Acquire) == BUSY {
core::hint::spin_loop();
}
}
// Initialization is done.
DONE => {}
Err(DONE) => {}
_ => unreachable!(),
}
}

2
library/std/src/sys/sgx/waitqueue/spin_mutex.rs
View File

@ -42,7 +42,7 @@ impl<T> SpinMutex<T> {
#[inline(always)]
pub fn try_lock(&self) -> Option<SpinMutexGuard<'_, T>> {
if !self.lock.compare_and_swap(false, true, Ordering::Acquire) {
if self.lock.compare_exchange(false, true, Ordering::Acquire, Ordering::Acquire).is_ok() {
Some(SpinMutexGuard { mutex: self })
} else {
None

6
library/std/src/sys/windows/mutex.rs
View File

@ -123,9 +123,9 @@ impl Mutex {
let inner = box Inner { remutex: ReentrantMutex::uninitialized(), held: Cell::new(false) };
inner.remutex.init();
let inner = Box::into_raw(inner);
match self.lock.compare_and_swap(0, inner as usize, Ordering::SeqCst) {
0 => inner,
n => {
match self.lock.compare_exchange(0, inner as usize, Ordering::SeqCst, Ordering::SeqCst) {
Ok(_) => inner,
Err(n) => {
Box::from_raw(inner).remutex.destroy();
n as *const _
}

2
library/std/src/sys/windows/thread_parker.rs
View File

@ -113,7 +113,7 @@ impl Parker {
// Wait for something to happen, assuming it's still set to PARKED.
c::WaitOnAddress(self.ptr(), &PARKED as *const _ as c::LPVOID, 1, c::INFINITE);
// Change NOTIFIED=>EMPTY but leave PARKED alone.
if self.state.compare_and_swap(NOTIFIED, EMPTY, Acquire) == NOTIFIED {
if self.state.compare_exchange(NOTIFIED, EMPTY, Acquire, Acquire).is_ok() {
// Actually woken up by unpark().
return;
} else {

6
library/std/src/sys_common/condvar/check.rs
View File

@ -23,9 +23,9 @@ impl SameMutexCheck {
}
pub fn verify(&self, mutex: &MovableMutex) {
let addr = mutex.raw() as *const mutex_imp::Mutex as usize;
match self.addr.compare_and_swap(0, addr, Ordering::SeqCst) {
0 => {} // Stored the address
n if n == addr => {} // Lost a race to store the same address
match self.addr.compare_exchange(0, addr, Ordering::SeqCst, Ordering::SeqCst) {
Ok(_) => {} // Stored the address
Err(n) if n == addr => {} // Lost a race to store the same address
_ => panic!("attempted to use a condition variable with two mutexes"),
}
}

8
library/std/src/sys_common/thread_local_key.rs
View File

@ -168,7 +168,7 @@ impl StaticKey {
return key;
}
// POSIX allows the key created here to be 0, but the compare_and_swap
// POSIX allows the key created here to be 0, but the compare_exchange
// below relies on using 0 as a sentinel value to check who won the
// race to set the shared TLS key. As far as I know, there is no
// guaranteed value that cannot be returned as a posix_key_create key,
@ -186,11 +186,11 @@ impl StaticKey {
key2
};
rtassert!(key != 0);
match self.key.compare_and_swap(0, key as usize, Ordering::SeqCst) {
match self.key.compare_exchange(0, key as usize, Ordering::SeqCst, Ordering::SeqCst) {
// The CAS succeeded, so we've created the actual key
0 => key as usize,
Ok(_) => key as usize,
// If someone beat us to the punch, use their key instead
n => {
Err(n) => {
imp::destroy(key);
n
}

2
library/std/src/sys_common/thread_parker/futex.rs
View File

@ -49,7 +49,7 @@ impl Parker {
// Wait for something to happen, assuming it's still set to PARKED.
futex_wait(&self.state, PARKED, None);
// Change NOTIFIED=>EMPTY and return in that case.
if self.state.compare_and_swap(NOTIFIED, EMPTY, Acquire) == NOTIFIED {
if self.state.compare_exchange(NOTIFIED, EMPTY, Acquire, Acquire).is_ok() {
return;
} else {
// Spurious wake up. We loop to try again.

7
src/test/ui/array-slice-vec/box-of-array-of-drop-1.rs
View File

@ -17,7 +17,12 @@ impl Drop for D {
fn drop(&mut self) {
println!("Dropping {}", self.0);
let old = LOG.load(Ordering::SeqCst);
LOG.compare_and_swap(old, old << 4 | self.0 as usize, Ordering::SeqCst);
let _ = LOG.compare_exchange(
old,
old << 4 | self.0 as usize,
Ordering::SeqCst,
Ordering::SeqCst
);
}
}

7
src/test/ui/array-slice-vec/box-of-array-of-drop-2.rs
View File

@ -17,7 +17,12 @@ impl Drop for D {
fn drop(&mut self) {
println!("Dropping {}", self.0);
let old = LOG.load(Ordering::SeqCst);
LOG.compare_and_swap(old, old << 4 | self.0 as usize, Ordering::SeqCst);
let _ = LOG.compare_exchange(
old,
old << 4 | self.0 as usize,
Ordering::SeqCst,
Ordering::SeqCst
);
}
}

7
src/test/ui/array-slice-vec/nested-vec-3.rs
View File

@ -18,7 +18,12 @@ impl Drop for D {
fn drop(&mut self) {
println!("Dropping {}", self.0);
let old = LOG.load(Ordering::SeqCst);
LOG.compare_and_swap(old, old << 4 | self.0 as usize, Ordering::SeqCst);
let _ = LOG.compare_exchange(
old,
old << 4 | self.0 as usize,
Ordering::SeqCst,
Ordering::SeqCst,
);
}
}