Good to see this coming about.
Non-functional fences
I think I mentioned this in an earlier thread, but I'm still not thrilled with the ability to write atomicMemoryFence(ordering: .relaxed) which is effectively a no-op. I don't think it's a big deal, but it is a slightly sharp edge that could confuse newbies (understanding the C++ memory ordering model is hard enough as it is).
Documentation
It'd be great to see a more human-readable version of the explanation on memory ordering as part of adding this to the standard library, in the documentation (perhaps the Swift Reference Guide?). Not only is it preferable if folks learning about atomics can do so without referring to another language's documentation, it'd be great if Swift's version of that documentation could be the best out there.
Atomic weak references
The inclusion of AtomicLazyReference is excellent. It's one of my most common use-cases for atomics, other than atomic integers.
It could perhaps benefit from a throwing store method, so that callers can more easily write preconditions (e.g. value.store(x)!) or otherwise fail (e.g. try value.store(x)).
However, what about the variant case where you want to be able to write the value multiple times? e.g. for a 'most recent value' cache (of sorts). I don't need this often but it has come up occasionally (and might be more frequently used with a nice high level abstraction, rather than having to drop into C).
Before I read to that part of the proposal, I actually spent a bunch of time trying to figure out how to derive this from the earlier primitives (e.g. Atomic<Unmanaged<T>>) but ran into challenges exactly as the proposal mentions regarding reference counting. It makes me wonder whether the inclusion of Unmanaged (perhaps among others?) in the atomic world should be made private, given how difficult it is to use correctly.
Unsupported architectures
Will the compiler specifically diagnose unsupported-only-on-the-target-architecture atomic types? e.g. a simple "UInt64 does not support atomic operations on i386" rather than whatever vaguer message it would if left to the default e.g. some "does not conform to protocol" diagnostic?
Actual instructions for memory fences
If I understand correctly, then with atomicMemoryFence the various arguments map as:
| AtomicUpdateOrdering |
Instruction |
Unintended side-effects |
| relaxed |
None |
No actual function. |
| acquiring |
dmb ishld |
Prevents earlier reads crossing the barrier. |
| releasing |
dmb ish |
Prevents later reads & writes crossing the barrier. |
| acquiringAndReleasing |
dmb ish |
|
| sequentiallyConsistent |
dmb ish |
|
Is that correct? I'm basing it on Table B2-1 in the ARMv8 ARM.
I'm assuming use of the Inner Shareable domain because that seems to me like the most commonly applicable one for user-space code, because per the ARMv8 ARM:
The Inner Shareable domain is expected to be the set of PEs controlled by a single hypervisor or operating system.
The unintended side-effects appear to be because ARMv8 doesn't provide more nuanced modes for dmb…?
How do I get a dmb ishst instruction?
Beyond basic memory barriers
I suspect the answer is that it's beyond the chosen scope (which is fine), but has consideration been given to other types of barriers? e.g. speculation barriers (sb / csdb / ssdb / pssbb), sync barriers (dsb), instruction barriers (isb), etc. I'm not all that familiar with most of them, but as I understand it they are intended for userspace code (instruction barriers for runtime codegen, for example, and speculation barriers for cryptographic or otherwise security-sensitive code).
Acquire-release pairing (etc)
A lot of the time the nature of accesses is intrinsic in the variable, e.g. it's always relaxed, or you always read acquiring and store releasing, or it's always sequentially consistent. Should that be codified in the types themselves, e.g. Atomic<T> (meaning .relaxed), OrderedAtomic<T>, TotallyOrderedAtomic<T>, or somesuch?
It'd be nicer to use, then, because you wouldn't:
- Have to specify the ordering on every access (and therefore could use standard operators etc), and
- Couldn't accidentally screw it up by using the wrong ordering in an individual access (e.g. Xcode auto-completed
.relaxed when you meant .releasing, and you didn't notice).
Maybe this is what is alluded to when the proposal says it's expected that higher-level data types are expected to be built atop these primitives?
At the end of the proposal, it does discuss this, although some of the arguments aren't persuasive to me.
- This design puts the ordering specification far away from the actual operations -- obfuscating their meaning.
Well, only insofar as it does for putting the variable's type, or whether a variable is 'weak' (vs strong), etc. I don't think the ordering method needs to be mentioned at every single use any more than the variable's type needs to be. Like the type of the variable, or its reference semantics, it's (usually) more tied to the nature of the variable than specific uses of the variable.
- It makes it a lot more difficult to use custom orderings for specific operations…
Only if it's the only way to do it. I think it's acceptable to have an optional ordering argument in this scenario, so you can override the default behaviour on a case-specific basis. It does introduce a risk of confusion, between the type declaration and its usage, but IMO it's worth it. But if that is a real bother to folks, then there could always be two layers - one of primitive "Atomic" and one above that. It does make the API surface nominally much bigger but conceptually it's only slightly more complicated.
Alternatively.:
let foo: Atomic<UInt64>(0, defaultOrdering: .acquireAndRelease)
The trick with the above is you'd want the corresponding ordering parameters to be omitted (or become optional) only if a default ordering was specified at the declaration site. To my knowledge Swift (the language) doesn't have a way to express that, currently.
Ordering views
FWIW I like that proposed alternative, counter.relaxed.wrappingAdd(1) etc.
I think the resulting code reads well, and it might also make it possible to address the prior point by allowing users to stash the view in a variable instead of the base atomic (assuming the view retains its base object) - which'd also allow a user to choose to enforce a specific ordering by only storing that ordering's view, and throwing out all accessible references to the base atomic value.
Re. some of the other counter-arguments listed in the proposal:
- Composability. Such ordering views are unwieldy for the variant of
compareExchange that takes separate success/failure orderings. Ordering views don't nest very well at all:
counter.acquiringAndReleasing.butAcquiringOnFailure.compareExchange(...)
I disagree, even with that specific example. It reads easily and clearly.
- API surface area and complexity.…
Granted it's easy for me to say as not the implementor of this functionality, but the API explosion seems worth the benefits. We're talking about the Swift standard library here - it's really widely used and so the trade-off between implementation challenge and user experience should be weighted way towards user experience.
The documentation explosion is a minor annoyance - it's true it'd nominally multiply the volume of documentation, but it'd have little impact on the actual conceptual complexity of the API - folks will pretty quickly realise the API for the views is the same; they're not likely to mistakenly read the redundant copies.
- Unintuitive syntax. While the syntax is indeed superficially attractive, it feels backward to put the memory ordering before the actual operation. While memory orderings are important, I suspect most people would consider them secondary to the operations themselves.
I don't think it's unintuitive. Unconventional, perhaps.
And if you consider my prior point about the ability to save the view rather than the base object, this approach actually gives you a way to abstract the ordering away from individual use sites, if you want.
- Limited Reuse. Implementing ordering views takes a rather large amount of (error-prone) boilerplate-heavy code that is not directly reusable. Every new atomic type would need to implement a new set of ordering views, tailor-fit to its own use-case.
Every additional type or set of equivalent types? I would have thought you could have a single view [for each ordering] for e.g. all atomic integers, genericised over the specific integer type… no?
English sucks
compareExchange doesn't make sense (for the intended meaning) in English, strictly speaking. It parses as "compare Exchange", whatever the noun "Exchange" means. Any particular reason to omit the And, or to not phrase it conditionallyExchange or somesuch?
weakCompareExchange
It's not clear [to me] from the method's documentation what this really means; why I might want this over the [regular] compareExchange. The docs say only:
(In this weak form, transient conditions may cause the original == expected check to sometimes return false when the two values are in fact the same.)
What's missing is an explanation of why those false negatives might occur, which might be important for the caller to understand at least for performance (e.g. if there's deterministic factors, could this cause an infinite loop?).
The proposal itself does provide more insight:
The weakCompareExchange form may sometimes return false even when the original and expected values are equal. (Such failures may happen when some transient condition prevents the underlying operation from succeeding -- such as an incoming interrupt during a load-link/store-conditional instruction sequence.) This variant is designed to be called in a loop that only exits when the exchange is successful; in such loops, using weakCompareExchange may lead to a performance improvement by eliminating a nested loop in the regular, "strong", compareExchange variants.
That should be in the actual "headerdoc".
Missing type information in API listings
e.g. logicalAnd(with:ordering:) is ambiguous because it's not apparent from context what type the first parameter is…?
var, inout, consuming…
The proposal notes that use of these with atomic types is basically wrong and will cause problems, but it doesn't seem to be proposing that the compiler prevent it…?
For full implementation details…
For the full API definition, please refer to the [implementation][implementation].
i.e. typo; missing link.
Atomic Strong References
Note that you can probably implement these in a single pointer-sized value, if there's at least a bit still free in the pointer representation. That bit can be made to mean "I'm retaining or releasing this", and with compare-and-exchange you can use that essentially as a spinlock on the pointer.
If you have two bits available then you can make a more efficient version, I believe.
But of course you need to wrap that pointer in a non-copyable type (same as for the double-word variant the proposal alludes to).
It'd be less efficient than a "non-atomic" reference, of course, due to the extra locking (essentially), but presumably it'd be worth it in some cases. And honestly, it's probably an insignificant inefficiency to the vast majority of users.
Default ordering
I concur that having a default - which would basically have to be .sequentiallyConsistent to be safe - is a bad idea. It's not just that it defeats the point somewhat (re. performance) but that making callers explicitly choose an ordering helps ensure they understand its ramifications (or at the very least, that the 'ordering' concept exists).
More seriously though, the semantics of the current memory orderings best compile down to what you've written and anything besides those are outside of C++'s memory ordering model (which we're now using for our atomics). x86_64 for instance doesn't issue any instruction for fences besides sequentially consistent.
I can however update the proposal to be more explicit here though.