SE-0282: Low-Level Atomic Operations

[Chris_Lattner3 (Chris Lattner)]

@Joe_Groff

When reviewing a proposal, here are some questions to consider:

• What is your evaluation of the proposal?

I love it, I can't wait for Swift to have this functionality. That said, this is a very large proposal and I think it is very important to continue refining some details of the proposal.

In particular, given the design points, I think it would make sense to carefully consider whether it makes sense to make a significant design shift and replace the UnsafeAtomic struct with an AtomicReference final class. This would be safe (just like the eventual Atomic<T> type in the future) and provide the same reference behavior. AtomicReference would still provide access to the low-level unsafe mechanics as static methods for library builders. I expect this to be controversial, so the other feedback below doesn't assume this change.

More detailed thoughts and comments:

• Unifying this under a single generic type is great, but the name UnsafeAtomic is very problematic to me. This type is unsafe, but the name UnsafeAtomic does not imply the reference semantic behavior that this type provides, and I don't believe that "Unsafe" connotes references that at all. I would much rather see a name like UnsafePointerToAtomic or UnsafeAtomicReference.

• Relatedly, I do not believe that the type name or method names need to be have particularly terse names. This is a bundle of sharp knives intended for experts, and being explicit is more important than being terse. The method names LGTM btw, this is just a general opinion.

• Comment on the writing: in the "We want to limit this proposal to constructs that satisfy the following requirements:" section, points 2/3 could be turned into a single point about how the proposal values "predictability over generality". I think this is the principle that underlies much of your approach - and I agree that is super important btw.

• Curious: why is it important to split this off to another module? With the new design, it seems to be a single UnsafeAtomic type, a few protocol and some other stuff that starts with the word Atomic - is there harm in putting that into the standard library? As far as I know, this would be the first thing split off of the stdlib, so this would set new precedent - is there a principle for how we should think about this in the future?

• I would recommend renaming AtomicProtocol -> AtomicValueProtocol (or something else) for clarity. Multiple things in this design relate to atomics, but this protocol is specifically about values that can be atomically accessed.

• I still think that UnsafeAtomicLazyReference is an outlier when compared to the rest of this proposal. The rest of the proposal consists of the base primitives that require access to compiler intrinsics to implement. This is one specific helper type used for higher level applications. I would recommend subsetting this out of the initial proposal, and add it back when the community has more experience with the basic functionality. At that time, we may find that there is a collection of helpers that would be added as a group and designed together as a family (along with double wide atomics and many other things that were intentionally subset out).

• As I mentioned in the pitch phase, I think it is really important to provide access to the low-level operations as static methods on UnsafeAtomic. This allows implementors to build things that are more efficient. For example, compare the AtomicCounter example in the proposal to the version shown in the linked post. I don't see any downside to this either, this is just a matter of exposing the members that already exist on AtomicProtocol but which aren't "public API".

• I would recommend using an initializer instead of a create method, because Swift consistently uses initializers for these things. Instead of: let atomic = UnsafeAtomic<Value>.create(initialValue: 0), it would be better to have: let atomic = UnsafeAtomic<Value>(createInitialValue: 0) or whatever.

• More significantly, I think this whole create/destroy pattern is a potentially significant design smell for this type in general. Did you consider making UnsafeAtomic be a final class instead of a struct? This would define away the destroy method, make the whole system far safer. You're already forcing a separate allocation anyway, the only downside is that you'd get an additional reference count, but that is probably swallowed up by a typical malloc quantum anyway. I think this is a pretty big design hinge, and I think it should be discussed in depth in the Alternatives Considered section.

@Joe_Groff

• Is the problem being addressed significant enough to warrant a change to Swift?
• Does this proposal fit well with the feel and direction of Swift?
• If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

Yes; Yes. I have use the c++ std::atomic and related functionally extensively. This seems like a great step for Swift.

@Joe_Groff

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

I participated in several rounds of iteration in the pitch phase and did a detailed review of this draft of the proposal, but did not read all the other comments above.

Thank you so much to the proposal authors for driving this - this is a really important step in Swift's evolution, and they have put in a tremendous amount of work shaping and iterating this in the pitch phase, I am super excited to see this coming together!

-Chris