rust-lang / rust-lang/libs-team
ACP: Path forward to implementing atomic functionality in terms of Atomic<T>
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 178
- Forks
- 28
- Avg merge
- 15m
- Merged PRs (30d)
- 1
Description
Proposal
Problem statement
Currently, the various AtomicT types in std::sync::atomic each have their own macro-produces implementation for atomic functions despite being more or less the same. An ACP for unifying these various implementations under a generic Atomic<T> implementation already exists and has been approved by the library team (#443) and is a part of the steps for the associated tracking issue in the rust-lang repo (#130539).
Solution sketch
Current implementations annotate individual functions with one of 3 possible target requirements, those being
- target_has_atomic_load_store: As the name suggest, the target supports atomic load and store ops. This is the "bare minimum".
- target_has_atomic: The target supports swap, CAS, fetch_x, etc.
- target_has_atomic_primitive_alignment: The alignment of the primitive is the same as that of the atomic type.
This lends itself to being broken down through using traits that correspond to each target support config. This allows the Atomic<T> to have a generic implementation in terms of types T: trait, which allows any T to opt into the generic implementation by implementing the trait. This still leaves room for custom implementation of atomic methods when an edge case arises (such as atomic bool emulation).
This approach does have some downsides to the current implementation. For example, documentation can't be made specific each atomic type as it already is. Additionally, stability annotations would be have to be clobbered when they conflict amongst each other, as some atomic functionality has existed for some types way longer than it has for others. Also, since unstable trait implementations aren't a thing, as the original ACP says, this does mean that 128 bit atomics would either be wrapped in an unstable type or we keep using macros for them.
// No longer requires Copy because AtomicLoadStore::OpType would require that we can
// freely transmute between it and Self which lets us implement atomics for non-copy types
pub impl(self) unsafe trait AtomicPrimitive: Sized {
type Storage: Sized;
}
pub impl(self) unsafe trait AtomicLoadStore: AtomicPrimitive
{
// Gives room for implementing atomics for non primitives that are the size of atomics.
// e.g. Box<T> (where T != [U]). Not necessary but adds future proofing. Also useful
// for atomic bools where bool has to be cast to u8.
type OpType: Sized + Copy;
}
pub impl(self) unsafe trait AtomicCas: AtomicLoadStore {}
pub impl(self) unsafe trait AtomicAlignedPrimitive: AtomicPrimitive {}
pub impl(self) unsafe trait AtomicInteger: AtomicCas {
/// Whether the integer type is signed or not
const IS_SIGNED: bool;
}
pub impl(self) unsafe trait AtomicBitwise: AtomicCas {}
impl<T: AtomicLoadStore> Atomic<T> {
pub const fn new(v: T) -> Self { ... }
pub const fn into_inner(self) -> T { ... }
pub const fn as_ptr(&self) -> *mut T { ... }
pub const unsafe fn from_ptr<'a>(ptr: *mut T) -> &'a Atomic<T> { ... }
pub const fn from_ptr_raw(ptr: *mut T) -> *const Self { ... }
pub const fn get_mut(&mut self) -> &mut T { ... }
pub const fn get_mut_slice(this: &mut [Self]) -> &mut [T] { ... }
pub const fn load(&self, order: Ordering) -> T { ... }
pub const unsafe fn load_volatile(self: *const Self, order: Ordering) -> T { ... }
pub const unsafe fn store_volatile(self: *const Self, val: T, order: Ordering) { ... }
}
impl<T: AtomicCas> Atomic<T> {
pub const fn swap(&self, v: T, order: Ordering) -> T { ... }
pub fn compare_and_swap(&self, current: T, new: T, order: Ordering) -> T { ... }
pub const fn compare_exchange(
&self,
current: T,
new: T,
success: Ordering,
failure: Ordering,
) -> Result<T, T> { ... }
pub const fn compare_exchange_weak(
&self,
current: T,
new: T,
success: Ordering,
failure: Ordering,
) -> Result<T, T> { ... }
}
// These can't be implemented without copy since f consumes self.
// Since all existing atomics are Copy this retains backwards compatibility.
// Future non-copy atomics would need a different API if any is to be provided.
impl<T: AtomicCas + Copy> {
pub fn fetch_update<F>(
&self,
set_order: Ordering,
fetch_order: Ordering,
f: impl FnMut(T) -> Option<T>
) -> Result<T, T>
{ ... }
pub fn try_update(
&self,
set_order: Ordering,
fetch_order: Ordering,
mut f: impl FnMut(T) -> Option<T>,
) -> Result<T, T> { ... }
pub fn update(
&self,
set_order: Ordering,
fetch_order: Ordering,
mut f: impl FnMut(T) -> T,
) -> T { ... }
}
impl<T: AtomicAlignedPrimitive> Atomic<T> {
pub const fn from_mut(v: &mut T) -> &mut Self { ... }
pub const fn from_mut_slice(v: &mut [T]) -> &mut [Self] { ... }
}
impl<T: AtomicInteger> Atomic<T> {
pub const fn fetch_add(&self, val: T, order: Ordering) -> T { ... }
pub const fn fetch_sub(&self, val: T, order: Ordering) -> T { ... }
pub const fn fetch_max(&self, val: T, order: Ordering) -> T { ... }
pub const fn fetch_min(&self, val: T, order: Ordering) -> T { ... }
}
impl<T: AtomicBitwise> Atomic<T> {
pub const fn fetch_nand(&self, val: T, order: Ordering) -> T { ... }
pub const fn fetch_and(&self, val: T, order: Ordering) -> T { ... }
pub const fn fetch_or(&self, val: T, order: Ordering) -> T { ... }
pub const fn fetch_xor(&self, val: T, order: Ordering) -> T { ... }
}
impl<T: AtomicLoadStore + Default> Default for Atomic<T> {
fn default() -> Self { ... }
}
impl<T: AtomicLoadStore> From<T> for Atomic<T> {
fn from(value: T) -> Self { ... }
}
A full implementation of this actually already exists as I was a bit too eager it seems and didn't realize this proposal was needed first. The full implementation can be viewed here.
https://github.com/nahla-nee/rust/blob/generic_atomic_impls/library/core/src/sync/atomic.rs
Note that AtomicPtr doesn't have an implementation derived for it, not due to technical limitation. Reasoning for various "quirks" such as that can be found in the now-closed PR here: https://github.com/rust-lang/rust/pull/162167
Alternatives
Some 3rd party crates do provide similar functionality with the aim of a unified atomic API, however this results in code fragmentation and highlights dissatisfaction with the existing fragmented API and its limitations.
As mentioned in the original ACP, providing a generic implementation provides an opportunity to expand the API to include currently unsupported primitives (*const T) and primitive-sized non-primitives (NonNull<T>, Box<T>, Option<Box<T>>, etc.) without increasing code complexity.
Links and related work
- Existing and approved proposal for implementing a limited subset of atomic functions: https://github.com/rust-lang/libs-team/issues/443
- Existing PR for said implementation: https://github.com/rust-lang/rust/pull/153407
- A now closed PR of the full implementation linked above: https://github.com/rust-lang/rust/pull/162167
- The
radiumcrate which provides a unified atomic API: https://docs.rs/radium/latest/radium/types/struct.Atom.html - Tokio's custom implementation of the equivalent of
Atomic<Option<Box<T>>>: https://github.com/tokio-rs/tokio/blob/master/tokio/src/util/atomic_cell.rs
What happens now?
This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.
Possible responses
The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):
- We think this problem seems worth solving, and the standard library might be the right place to solve it.
- We think that this probably doesn't belong in the standard library.
Second, if there's a concrete solution:
- We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
- We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the proposal and compare library/core/src/sync/atomic.rs in the linked implementation with the existing standard-library atomic API. Read ACP #443 and the closed PR #162167 for the library team's prior reasoning and unresolved tradeoffs. This issue is done when the libs-api team reaches a decision on the design and next implementation steps.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- api, backend-api-design
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100