Skip to content

ACP: Path forward to implementing atomic functionality in terms of Atomic<T> #878

Description

@nahla-nee

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

  1. target_has_atomic_load_store: As the name suggest, the target supports atomic load and store ops. This is the "bare minimum".
  2. target_has_atomic: The target supports swap, CAS, fetch_x, etc.
  3. 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: rust-lang/rust#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

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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    api-change-proposalA proposal to add or alter unstable APIs in the standard libraries

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions