I’m not trying to sell any “fancy” features here, only things based on functionality that we’ve already discussed and has some foundation of implementation in progress already. I don’t think “there’s already too much stuff in the standard library” is a great rationale for not trying to avoid slack in new functionality. And if there are indeed a fixed, well-known set of nullable atomic types, that suggests to me that same type constraints are not “conformance gymnastics” but the natural way to model this, because protocols are for open ended polymorphism.
FWIW, I too would like to get rid of NullableAtomic, and if it's "just" an extra layout-constraint / pseudo-protocol _HasExtraInhabitants and an additional stdlib/runtime API, that miiight be worth it. I do see that it's somewhat opaque to users but it's a concept we might want anyway.
2 Likes
That's fair. But I'm really rather worried about discoverability and that we won't be able to shoehorn double-wide atomics into an implementation that is based on a single-word layout constraint.
How do we implement class ListNode<Payload>: NullableAtomic if optional atomics relies on a layout constraints? Is the implementation going to have an if ladder for specific AtomicStorage types? But then why would we constrain the layout of the original type at all?
Or is this going to need multiple conformances? AFAICT, those aren't even listed as desirable in the Generic Manifesto.
Delaying optional atomics to a future release also assumes that we'll have versioned (and/or back deployable) protocol conformances. Note how this isn't an issue with NullableAtomics.
Is it realistic to expect all these language extensions in the near future? Are these really the language features we need to work on right now? Just to eliminate a protocol? (And I don't even find it obvious that we'd want to do that even if these features were currently shipping today.)
If the goal is to make atomics work better, there are so many more beneficial features we could choose to work on instead. For example, any and all work towards getting addressable ivars, public constexprs, or move-only/nonmovable types could have orders of magnitude bigger impact for atomics users.
If we wanted the type layout constraint approach to work with arbitrary storage types, I think we'd still need a duplicated set of requirements for handling extra inhabitants, like what NullableAtomic requires in the proposal. So maybe it wouldn't really get us out of an extra protocol.
I'm mostly concerned about the permanent API commitment we're making. If NullableAtomic is the best way we know of now to get Optional for some atomic types working today, but we have plausible ideas for how to get the same user API without it later, deploying it as a package without long-term ABI constraints in the short term seems more prudent to me.
1 Like
I suspect that's the case. I know it's frustrating to talk about atomic strong references without being able to look at a concrete prototype for it -- I'll see if I can quickly set up a proof of concept to validate the design I have in mind for it.
Well, I would never want to belittle the effort needed to maintain API/ABI, but it seems to me NullableAtomic won't add any maintenance work we wouldn't already need to do for AtomicProtocol anyway.
I don't foresee we would need to touch these protocols much, beyond maybe adding an operation or two. But even that seems more likely to happen for AtomicInteger than the core atomics protocols.
Thank you everyone for your thoughtful input; this has been a hugely productive discussion! Especially big thanks to @Joe_Groff for wisely pushing back against NullableAtomics -- his hints led me to a radically simpler replacement that provides the same features without polluting the public API, while also cleaning up the regular atomic implementation.
I think this is getting ready to move beyond the pitch phase!
Version 2020-04-10/1 of the document is now available on GitHub, along with the associated update to the implementation. Summary of the latest changes:
- Major design overhaul of atomics internals.
AtomicProtocolnow consists of an associated type implementing storage, and a handful of conversion methods to/from the storage representation. The actual primitive atomic operations are now all implemented by the atomic storage type. - Declare the low-level implementation details as non-public API; underscore them and completely remove them from the proposal text.
- Replace
NullableAtomicwith an internal constraint on the Wrapped type's atomic storage type, simply requiring a dedicated representation for thenilvalue. - Add an example implementation for a simple lock-free single-consumer stack data structure, demonstrating optional atomics.
- Extend the "Alternatives Considered" section with entries on why
UnsafeAtomicis named like that, and why we don't provide default orderings.
8 Likes
This looks like a great revision! Pushing the underlying mechanics of AtomicProtocol down a level definitely makes the API surface a lot more focused.
1 Like
Is there any specific reason that Float and Double are not in the scope?
Yes -- the proposal already takes far too much time investment to properly read and review. Hashing out the fine details of what floating point operations to add and defining how compare-exchange interacts with negative zeroes and NaNs is better left to a separate follow-up proposal.
I like the revision!
What is the right way to initialize an atomic separately from its allocation in this new pattern?
I have:
let p = UnsafeMutablePointer<UnsafeAtomic<Int>.Storage>.allocate(capacity: 1)
p.initialize(to: 42)
The error message is: Cannot convert value of type 'Int' to expected argument type 'UnsafeAtomic<Int>.Storage'
The fix currently needed:
let p = UnsafeMutablePointer<UnsafeAtomic<Int>.Storage>.allocate(capacity: 1)
p.initialize(to: .init(42)) // who will figure that one out?
This can be improved by defining the following (hopefully in the submodule):
extension UnsafeMutablePointer {
public func initialize<Value: AtomicProtocol>(to value: Value)
where UnsafeAtomic<Value>.Storage == Pointee {
initialize(to: UnsafeAtomic.Storage(value))
}
}
...and then the obvious first attempt works.
Replying to myself: this initialization pattern got better for RawRepresentable types, but got worse for same-representation types such as Int.
I'm extremely worried about people omitting the dispose() call. Forcing developers to figure out how to initialize storage is going to be annoying, but it will direct attention to the Storage documentation, which will warn about the need to dispose of the storage instance.
E.g., the code below reads like it followed pointer handling conventions, but it neglects to call dispose(), so it may leak memory when T contains a strong reference. (Such as in the case of UnsafeAtomicLazyReference.
func foo<T: AtomicProtocol>(_ initialValue: T, body: (UnsafeAtomic<T>) -> Void) {
typealias Storage = UnsafeAtomic<T>.Storage
let p = UnsafeMutablePointer<Storage>.allocate(capacity: 1)
p.initialize(to: someValue)
body(UnsafeAtomic(at: p))
p.deinitialize(count: 1)
p.deallocate()
}
Yep; in theory this allows atomic representation of integers to differ from their regular layout. (There were some platforms where regular integer types didn't have proper alignment for atomic operations.) This is a minor concern though -- the real benefit (for my purposes) is that all atomic types fit the same API pattern.
(Currently AtomicInteger requires Self to be used as atomic storage, but this is an arbitrary implementation detail I'll likely eliminate before this ships.)
1 Like
Minor suggestion: rename dispose to deinitialize, to match UnsafePointer.
Although…what's the point of dispose at all? You can't put anything non-trivial in an atomic representation.
dispose operates on a regular value, so it must leave the storage location in an initialized state.
Disposal as a separate operation will become important for atomic strong references, which map to trivial storage that contains an unbalanced +1 (or, more likely, +n) retain that needs to be undone before the storage is destroyed.
(C.f. https://github.com/apple/swift/pull/30553#discussion_r399722585)
In the current proposal, this is demonstrated by UnsafeAtomicLazyReference.
Hm. You've brought this up before and I keep forgetting it because it implies that there'll eventually be a strong refcount atomic primitive that's compatible with UnsafeAtomic. That seems tricky to roll out but I suppose it ought to work. That said, dispose on a strong reference can't possibly leave the storage in an initialized state, and dispose on an optional strong reference would always just be "swap with nil". Given that it absolutely will get left out of people's generic algorithms (and that they shouldn't be forced to write it for an atomic integer), maybe we shouldn't add a dedicated operation for it at all?
1 Like
The storage type won't contain a strong reference -- but it will still represent one.
I understand that, but either it's going to be [implemented in terms of] a new non-trivial, non-nullable primitive type (Builtin.AtomizedStrongReference), or it's going to be a trivial type where deinitialization does not actually leave the storage in an invalid state. (We already promise you can assign trivial types directly to uninitialized memory.)
I suppose there's a third option, which is a non-trivial nullable primitive type, but that seems like a lot more trouble to implement.
Deinitialization still logically marks the underlying memory as uninitialized, which is inappropriate to do within class ivars or compiler-controlled local/temporary variables. All this complexity is to allow use of these types in ManagedBuffer headers, where storage initialization/deinitialization is rigidly controlled by the library.
(And people who aren't doing that sort of thing ought to use the provided create/destroy convenience methods that do the right thing for you.)