Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Atomic::compare_exchange(_weak) and deprecate Atomic::compare_and_set(_weak) #628

Merged
merged 2 commits into from
Jan 3, 2021

Conversation

taiki-e
Copy link
Member

@taiki-e taiki-e commented Jan 1, 2021

Instead of changing the argument of Atomic::compare_and_set(_weak), add new methods Atomic::compare_exchange(_weak) that always use two orderings.

This PR adds:

  • Atomic::compare_exchange
    Unlike compare_and_set, this always receive two orderings.
  • Atomic::compare_exchange_weak
    Unlike compare_and_set_weak, this always receive two orderings.
  • CompareExchangeError (error returned by Atomic::compare_exchange(_weak))
    The definition is the same as CompareAndSetError, just renamed CompareExchangeError.

This PR deprecates:

Closes #621

@taiki-e
Copy link
Member Author

taiki-e commented Jan 1, 2021

I don't know why this method was originally called compare-and-set, but Wikipedia says a variant of compare-and-swap that returns a boolean indicating whether a substitution was performed is often called compare-and-set. https://en.wikipedia.org/wiki/Compare-and-swap

The result of the operation must indicate whether it performed the substitution; this can be done either with a simple boolean response (this variant is often called compare-and-set), or by returning the value read from the memory location (not the value written to it).

As this method actually returns a value rather than a boolean, I think it's okay to call it compare_exchange. (Given that other similar methods in the standard library and crossbeam are called compare_exchange, it's probably more consistent.)

@jeehoonkang
Copy link
Contributor

std's compare_exchange receives two orderings: one for success and the other for fail cases. But this PR's compare_exchange receives only one that can be rendered into two. Do you think we need to match them?

@taiki-e
Copy link
Member Author

taiki-e commented Jan 3, 2021

But this PR's compare_exchange receives only one that can be rendered into two.

Uh, both methods (compare_exchange(_weak)) added by this PR always receive two orderings, not CompareAndSetOrdering.
I guess what you are mentioning is the behavior of the methods (compare_and_set(_weak)) that will be deprecated by this PR.

pub fn compare_exchange<'g, P>(
&self,
current: Shared<'_, T>,
new: P,
success: Ordering,
failure: Ordering,
_: &'g Guard,
) -> Result<Shared<'g, T>, CompareExchangeError<'g, T, P>>
where
P: Pointer<T>,

pub fn compare_exchange_weak<'g, P>(
&self,
current: Shared<'_, T>,
new: P,
success: Ordering,
failure: Ordering,
_: &'g Guard,
) -> Result<Shared<'g, T>, CompareExchangeError<'g, T, P>>
where
P: Pointer<T>,

@taiki-e
Copy link
Member Author

taiki-e commented Jan 3, 2021

Updated the PR description to explain the changes in more detail.

@jeehoonkang
Copy link
Contributor

bors r+

@bors
Copy link
Contributor

bors bot commented Jan 3, 2021

Build succeeded:

@bors bors bot merged commit 06229f5 into crossbeam-rs:master Jan 3, 2021
@taiki-e taiki-e deleted the compare_and_set branch January 3, 2021 11:09
@taiki-e taiki-e mentioned this pull request Jan 4, 2021
bors bot added a commit that referenced this pull request Feb 20, 2021
659: Prepare for the next release r=taiki-e a=taiki-e

It's been over two months since the previous release. There are some improvements and deprecations in the master branch, and it would be nice to release them. Also, there is no breaking change that needs a major version bump.

Changes:

- crossbeam-epoch 0.9.1 -> 0.9.2
  - Add `Atomic::compare_exchange` and `Atomic::compare_exchange_weak`. (#628)
  - Deprecate `Atomic::compare_and_set` and `Atomic::compare_and_set_weak`. (#628)
  - Make `const_fn` dependency optional. (#611)
  - Add unstable support for `loom`. (#487)
- crossbeam-utils 0.8.1 -> 0.8.2
  - Deprecate `AtomicCell::compare_and_swap`. (#619)
  - Add `Parker::park_deadline`. (#563)
  - Improve implementation of `CachePadded`. (#636)
  - Add unstable support for `loom`. (#487)


Co-authored-by: Taiki Endo <te316e89@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Remove CompareAndSetOrdering trait and change compere_and_set to always use two orderings
2 participants