Re: [RFC 2/2] rust: sync: Add atomic support

[Gary Guo <gary@garyguo.net>]

On Fri, 14 Jun 2024 19:39:27 -0700
Boqun Feng <boqun.feng@gmail.com> wrote:

> On Fri, Jun 14, 2024 at 06:28:00PM -0700, John Hubbard wrote:
> > On 6/14/24 6:24 PM, Boqun Feng wrote:  
> > > On Fri, Jun 14, 2024 at 06:03:37PM -0700, John Hubbard wrote:  
> > > > On 6/14/24 2:59 AM, Miguel Ojeda wrote:  
> > > > > On Thu, Jun 13, 2024 at 9:05 PM Boqun Feng <boqun.feng@gmail.com> wrote:  
> > > > > > 
> > > > > > Does this make sense?  
> > > > > 
> > > > > Implementation-wise, if you think it is simpler or more clear/elegant
> > > > > to have the extra lower level layer, then that sounds fine.
> > > > > 
> > > > > However, I was mainly talking about what we would eventually expose to
> > > > > users, i.e. do we want to provide `Atomic<T>` to begin with? If yes,
> > > > > then we could make the lower layer private already.
> > > > > 
> > > > > We can defer that extra layer/work if needed even if we go for
> > > > > `Atomic<T>`, but it would be nice to understand if we have consensus
> > > > > for an eventual user-facing API, or if someone has any other opinion
> > > > > or concerns on one vs. the other.  
> > > > 
> > > > Well, here's one:
> > > > 
> > > > The reason that we have things like atomic64_read() in the C code is
> > > > because C doesn't have generics.
> > > > 
> > > > In Rust, we should simply move directly to Atomic<T>, as there are,
> > > > after all, associated benefits. And it's very easy to see the connection  
> > > 
> > > What are the associated benefits you are referring to? Rust std doesn't
> > > use Atomic<T>, that somewhat proves that we don't need it.  
> > Just the stock things that a generic provides: less duplicated code,  
> 
> It's still a bit handwavy, sorry.
> 
> Admittedly, I haven't looked into too much Rust concurrent code, maybe
> it's even true for C code ;-) So I took a look at the crate that Gary
> mentioned (the one provides generic atomic APIs):
> 
> 	https://crates.io/crates/atomic
> 
> there's a "Dependent" tab where you can see the other crates that
> depends on it. With a quick look, I haven't found any Rust concurrent
> project I'm aware of (no crossbeam, no tokio, no futures). On the other
> hand, there is a non-generic based atomic library:
> 
> 	https://crates.io/crates/portable-atomic
> 
> which has more projects depend on it, and there are some Rust concurrent
> projects that I'm aware of: futures, async-task etc. Note that people
> can get the non-generic based atomic API from Rust std library, and
> the "portable-atomic" crate is only 2-year old, while "atomic" crate is
> 8-year old.
> 
> More interestingly, the same author of "atomic" crate, who is an expert
> in concurrent areas, has another project (there are a lot projects from
> the author, but this is the one I'm mostly aware of) "parking_lot",
> which "provides implementations of Mutex, RwLock, Condvar and Once that
> are smaller, faster and more flexible than those in the Rust standard
> library, as well as a ReentrantMutex type which supports recursive
> locking.", and it doesn't use the "atomic" crate either.

Note that crossbeam's AtomicCell is also generic, and crossbeam is used
by tons of crates. As Miguel mentioned, I think it's very likely that in
the future we want be able to do atomics on new types (e.g. for
seqlocks perhaps). We probably don't need the non-lock-free fallback of
crossbeam's AtomicCell, but the lock-free subset with newtype support
is desirable.

People in general don't use the `atomic` crate because it provides no
additional feature compared to the standard library. But it doesn't
really mean that the standard library's atomic design is good.

People decided to use AtomicT and NonZeroT instead of Atomic<T> or
NonZero<T> long time ago, but many now thinks the decision was bad.
Introduction of NonZero<T> is a good example of it. NonZeroT are now
all type aliases of NonZero<T>.

I also don't see any downside in using generics. We can provide type
aliases so people can use `AtomicI32` and `AtomicI64` when they want
their code to be compatible with userspace Rust can still do so.

`Atomic<i32>` is also just aesthetically better than `AtomicI32` IMO.
When all other types look like `NonZero<i32>`, `Wrapping<i32>`, I don't
think we should have `AtomicI32` just because "it's done this way in
Rust std". Our alloc already deviates a lot from Rust std.

Best,
Gary