[Second Review] SE-0410: Atomics

Region-based isolation is not an attempt to back away from the Sendable design. If anything, it is an attempt to make the Sendable design more palatable by eliminating a class of sendability errors that do not actually lead to data races.

The "mutable caterpillar, immutable butterfly" pattern is well-known, and there's plenty of room in the language to directly support it in the future if we want. I believe the same set of features would be useful for formalizing the reference types used to implement copy-on-write — it basically just involves statically propagating the knowledge that a reference is currently unique. Midori's compiler supported that kind of analysis, if I remember that paper right. In the meantime, library authors with such types will have to decide whether they want to make them Sendable. I don't think it's unconscionable to decide to make them unsafely Sendable, then stress in the documentation that users are responsible for respecting the idiom of staged mutation.

8 Likes

I think that’s my conclusion, but my path to get there is different:

  1. My interpretation of the goal of Swift concurrency is to make it so concurrency APIs (including classic threading and task queues as well as actors) cannot introduce data races if you stay entirely within the model; that is, if you don’t call out to C APIs that will go behind your back, or use @unsafe Sendable to introduce a data race yourself. That is, extending the ā€œsafeā€ on the https://swift.org home page to include concurrency.

  2. ClientBootstrap and similar pre-concurrency library types are (in retrospect) ill-adapted to modern swift concurrency.

  3. It’s not worth compromising on the ā€œsafetyā€ goal of (1); the compiler must remain conservative to satisfy the guarantee, and it should be obvious when code is opting out of that model. If the model isn’t working, we could keep preconcurrency mode around, or we could find a different model, but a model that doesn’t provide safety guarantees, even conditional ones, isn’t worth the trouble.

What I would do for ClientBootstrap specifically

I’m not particularly familiar with this type, but from its description it could have worked as a builder type producing a Sendable ā€œspawnerā€ (there’s probably a better name for this). This can be retrofitted with a Sendable wrapper type that only supports connections, ensuring that the proper synchronization is done (there might not be any). This is similar to a copy-on-write value type wrapping its unsafely-shared buffer, something else we’d like to make better, and so maybe there’s more we can do in the space of ā€œadaptersā€.

5 Likes

i don’t think we disagree on the basic facts here, so far i’ve been emphasizing the resistance of ClientBootstrap to change, but it’s not crazy to make an executive decision here to prioritize something else, like a longer-term data race safety goal. unstoppable forces and immovable objects and whatnot.

regarding

i would like to know more about what it would actually mean to ā€œopt outā€ of that model. i’m not convinced of the retrofitting story for types like ClientBootstrap - designing adapter types is a lot of ceremony for a problem that crops up a lot, and @preconcurrency is a bit too blunt an instrument.

one idea i have, is if we had an UncheckedAtomic<ClientBootstrap> available specifically for this situation.

2 Likes

Since there doesn’t seem to be a clear winner yet, I’ll re-up my vote for AtomicRepresentable and AtomicOptionalRepresentable (replacing AtomicValue and AtomicOptionalWrappable, respectively). I mentioned during the first review: the proposal even uses the phrase ā€œcustom ā€˜atomic-representable’ typesā€ to describe the types that conform to AtomicValue.

6 Likes

I just want to quickly address the comments around NIO's ClientBootstrap here. This type predates Swift Concurrency and we are well aware that its semantics are not great. The same can be said about all of our bootstrap and we are already thinking about how a next iteration of them could look like. One of the problems we want to solve with the next iteration is the mutability and sendability problem. This problem is not NIO specific but there are a lot of types across various libraries that were just not made for the latest Swift Concurrency features and those types might unevolvable; hence, they will be deprecated and replaces with a more suitable construct over time.

Now taking a step back, I agree with @lorentey that using @unchecked Sendable is the correct spelling here since there are low level memory orderings that developers have to carefully think about when using Atomics. I do think good documentation goes a long way here and we should encourage developers to isolate the usage of @unchecked Sendable to a minimum. At best abstracting it into a internal types with safe interfaces.

4 Likes

Practical question as I'm finally getting a chance to play with the new implementation. I have a nightly build running under Xcode. What flags do I need to be able to

import Synchronization

I'll go ahead and echo a lot of what other reviewers have already mentioned and say I think AtomicRepresentable is an ok name for this protocol if folks really think AtomicValue is not the "best" name for this thing. I'm fine with other names that have been thrown out too like Atomicable or Atomizable. We could bike shed forever :see_no_evil:

I think the naming of this module should be the biggest point of discussion. The name we use here will determine future capabilities of the module as well as how it will interact with current module names today. Naming this thing Atomics is highly disruptive to the existing package and users depending on it or even depending on another package who depends on swift-atomics. The language doesn't handle colliding module names at all nor does it provide tools to help mitigate the issue. Even if the language did have some way of preventing this issue and allowed dependents of the package to "just work", the name doesn't really allow for any future evolution. Sure, there are some other things we can/should propose as followup work for the atomics including atomic references, but other things could include volatile load/stores or things like futex support for wait/notify. However, at some point we can't propose other synchronization primitives that aren't atomics like mutual exclusion locks, semaphores, conditional variables, read/write locks, once support, etc. There are numerous of these primitives that the standard library _could_ expose as API one day (whether or not all of those belong in the standard library is a separate discussion). Synchronization feels like a natural name of the naming of the module for atomics and all of these potential future additions to live as they are _synchronization_ primitives. A shortened name like Sync could work as well, or even Concurrency since the implicit module is named _Concurrency :see_no_evil: I'd love to hear other ideas about this module name if others come up with something better than Synchronization (I'd also love to hear why folks dislike this name), but this name feels very natural to me at least in terms of future evolution of the module.

Atomics haven't been merged to main yet, so nightly builds won't include it. I spun off a toolchain build here: [stdlib] Atomics by Azoy Ā· Pull Request #68857 Ā· apple/swift Ā· GitHub and will update this thread when it's done building.

5 Likes

macOS toolchain: https://ci.swift.org/job/swift-PR-toolchain-macos/997/artifact/branch-main/swift-PR-68857-997-osx.tar.gz

Linux toolchain: https://store-030.blobstore.apple.com/swift-oss/tmp/pull-request/68857/684/ubuntu2004/PR-ubuntu2004.tar.gz

1 Like

here’s a thought experiment you might consider when choosing between AtomicRepresentable and AtomicValue:

should Double conform to this protocol?

in the current swift-atomics package, Double does not conform to AtomicValue, a decision with a number of justifications that all boil down to ā€œfloating point operations don’t mix with atomicsā€.

i personally found that decision unhelpful - most of the time i don’t care about incrementing atomic Doubles or checking them for floating point equality - all i am interested in is publishing some Double value to some concurrent observer, and the library forces me to encode and decode them through their raw bit patterns. but if you’ve named the protocol AtomicValue, you can kinda see why someone might want to keep Double away from that protocol.

but if you’ve got AtomicRepresentable instead, the name sort of evokes RawRepresentable and maybe it makes a lot more sense for Double to have an ā€œraw atomic valueā€ of say, UInt64.

3 Likes

Note that with SE-0412 you can instead apply nonisolated(unsafe) to the atomic stored property and still use a checked Sendable conformance.

9 Likes

Thanks for participating in this second round of review! The Language Steering Group has decided to accept the proposal with modifications.

3 Likes