[Pitch #4] Actors

Chris_Lattner3 · March 3, 2021, 1:54am

Agreed, this is further great progress!

Allow cross-actor references to actor properties, so long as they are reads (not writes or inout references)

Got it, I assume the challenge here is in promoting setters to async? Do you anticipate that this is difficult to add in the future, or just "non-critical" for now? It would be weird if the final model allows one without the other.

Fantastic, thank you for adding this.

I'm also thrilled to see this get limited further, and have further suggestions below.

All great cleanups.

I am concerned about this direction. As proposed, the actor feature doesn't provide the basic protocol abstraction features that Swift provides for every other nominal type. I don't think that shipping Actors 1.0 without this would be acceptable: it would push swift programmers into a lot of bad patterns and habits, cause a bunch of stuff to be marked async that shouldn't be, and lead to the definition of a lot of protocols that are "wrong" for the general design. I urge you to consider pulling a solution to this problem into the base proposal. Further thoughts on this below.

Here are detailed comments from a fresh read-through of the proposal:

Motivation section:

It's completely a wording/positioning thing, but I'd suggest changing the discussion to be less about isolation/data races (structured concurrency provides the same benefits) and more something that talks about abstraction and design patterns. Here's a rough sketch:

"""Two key pillars of the the Swift concurrency model is to provide a safe programming model that statically detects data races and other common concurrency bugs, and "structured concurrency" (link) which provides a way to define concurrent tasks at a function/closure-level of abstraction. This is a powerful model which is suitable for a number of common design patterns (including things like parallel maps and concurrent callback patterns), but is limited to working with state that is captured by closures. You can say that structure concurrency is optimized for "operation first; data second" use-cases where there is a single primary operation on that data.

While this model is critical and great for simple use cases, we would like to go beyond that: we think it is important to provide a programming model and set of design patterns that allows programmers to explicitly declare the state managed by a concurrency domain, define methods that work with that state, provide a richer interface to clients across concurrency domains than the arguments and results of a closure, and allow abstraction and reuse across common implementations of these.

Actors are our solution to this. Actors allow you as a programmer to declare that a bag of state is held within a concurrency domain and then define multiple async operations that act upon it. You can say that actors provide a "data first; multiple operations allowed" model for concurrent programming. As part of the Swift Concurrency Model, actors provide the same race and memory safety properties as structured concurrency, but provide the familiar abstraction and reuse features that other explicitly declared types in Swift enjoy. """

Super nit, but I'd recommend dropping the private markers from the lead example of actors. They are unnecessary and a bit distracting now, since they syntactically overpower the let/var declarations that matter.

In the cross actor reference discussion at the end of this section it would be good to mention some rationale for the decisions - why aren't cross-actor setters allowed? I think I know, but it would be good to make the proposal self contained and obvious to other readers.

Actor Isolated parameters

I love the addition of this into the proposal, I think it makes the actors design much stronger. Thank you.

I can see where you're going here with the reuse of the word isolated (and it is much better than @sync) but it doesn't really read right and convey the right thing. I don't have a concrete suggestion of a better term though - the presence of one of these on a function means that the "function may only be invoked within the concurrency domain of the actor parameter", not that the function has isolated the parameter.
Regardless of the keyword, are optionals and IUO's allowed here? e.g. func thing(x: isolated BankAccount?) and func thing(x: isolated BankAccount!)
Is it possible to pull the runOnActor method into a protocol extension on AnyActor so it is available everywhere?
The currying example is great
"Therefore, we prohibit the definition of a function with more than one isolated parameter:" --> Why? This can be narrowly useful (e.g. unsafe casts when two actor instances are known to be on the same executor) and there is no reason to ban it. Why exclude a (narrowly useful) valid case and add the compiler complexity? I don't see any safety benefit to this.

Nonisolated declarations

As you know, I am very concerned about adding "nonisolated" proactively as part of the base actor proposal - it is sugar for one specific case and significantly expands the conceptual model of actors. This may or may not be important for actor 1.0, but it confuses and complicates the core actor model (e.g. protocol support) which is critical to get right independent of nonisolated, and makes the proposal more difficult to understand.

About the feature itself, I think it would be much better if you motivate nonisolated without using protocol conformance as the rationale. We need to solve protocol conformance independently of "nonisolated" declarations to allow basic type abstraction etc, because actors contain mutable state (the entire reason we have actors in the first place ;-) and we need "isolated" protocol conformances to be able to touch that mutable state. While it may be important to have nonisolated as part of the model, it is confusing for readers of the proposal that aren't deep into it to think that this is the solution for protocol conformance in actors (since it doesn't cover the whole problem).

My concern here can be addressed in three ways: 1) Change the introduction of nonisolated to be motivated by non-protocol things, 2) pull proper protocol conformance support comes into the base actor proposal. 3) split nonisolated into a completely additive follow-on proposal.

If it stays, then I'd also recommend moving the discussion about nonisolated after the closures/inout section since this is really a different additive language feature, and closures/inout already exist.

Closures

Nit: The closure isn't non-isolated. It is @concurrent or @sendable or whatever:

error: actor-isolated property 'transactions' can not be referenced by non-isolated closure
error: actor-isolated property 'lastUpdateDate' can not be referenced by non-isolated closure

"If the closure is @escaping or is nested within an @escaping closure or a local function, it is non-isolated."

This is still highly unprincipled and unnecessary w.r.t. the type system, but this is a lot less onerous now with the introduction of isolated actor references. I don't understand how it is specifically enforced in the compiler though because the type system features don't compose in an obvious way, and your concerns with escaping values aren't specific to actors at all (also apply to structured concurrency).

However, in practice nearly all @escaping closures in Swift code today are executed concurrently via mechanisms that predate Swift Concurrency.

I don't think this is true, there are lots of reasons to use @escaping closures other than concurrency, including basic callback things, main thread UI things, etc.

Overall I'm curious if this is actually required now that the other changes have gone into the model. The original concern was about GCD and other APIs that bottom out in it, but won't those be marked with Clang attributes as taking @concurrent closures? Also keep in mind that this isn't actually solving the problem as Dispatch.sync and other concurrency APIs don't take escaping closures.

It isn't obvious to me that this is "better" for the model (in terms of catching bugs) or if it is just "weird" -- even in the short term.

inout parameters section

This limitation is written to apply the to properties in actors, but this limitation should really apply to any state accessible from the actor, e.g.:

class C {   var state : Double }
struct Pair { var a, b : Double }
actor A {
  let someC : C
  var somePair : Pair
...
  func foo() async {
     // not ok.
     await modifiesAsynchronously(&someC.state)
     // not ok.
     await modifiesAsynchronously(&somePair.a)
  }

I think this needs to be expanded.

ConcurrentValue section

Wording nit: I'd replace "A separate proposal introduces the ConcurrentValue protocol" with "SE-0302 introduces the ConcurrentValue marker protocol"

Nonisolated in detailed design section

Again, it is weird to me that the first thing introduced after the syntax is something that has nothing to do with the core of the actor model. I'd recommend moving this much later in the section (or out of the proposal entirely) as it is orthogonal to the other mechanics of the actor model.
I would love to see more detailed explaination of what nonisolated(unsafed) property declarations are - where do they get used in practice, why do we need language support for this instead of using existing property wrapper functionality, etc. This seems like a completely additive language proposal that only has a brief mention in the design paper, and the introduction of isolated MyActor means that we can introduce casts (ala the unsafeActorInternalsCast in the whitepaper I wrote) that achieves this with library techniques. This nonisolated(unsafe) feature isn't clearly motivated, and is a syntactic sugar proposal. I'd recommend splitting it off.

I would also love to see nonisolated in general split out from the base actor proposal as an additive thing that follows on the basic model.

Actor Isolation Checking section

Writing suggestion: I think it would be best to rethink the presentation of this section in general. It came from historical roots, but now has a bunch of stuff (incl how protocol conformance works) that have little to do with isolation and relate to the overall design of actors as a nominal type. I'd recommend changing the "detailed design" section to just list out how all the language features work (not as subsections of isolation) and then have actor isolation be a small peer section that describes the limitations imposed by it.

Furthermore, the section is conflating two different things: 1) how actor isolation works in the general model, and 2) various details of the additive and separable "nonisolated" language feature. For example, the "overrides" section is related to the later not the former.

This is part of why I think it is important to split nonisolated into a follow-on proposal. The core actor model is clearly definable without it, and would be much easier to understand the proposal. The follow-on nonisolated proposal can then carry its own motivation etc to cleanly separate out the concerns.

Partial Applications section

The writing of this section evolved and I think this has made it more confusing to understand than it needs to be (just because it is mixing non-isolated,isolated,concurrent, and escaping together as the terms). I would recommend simplifying this by describing the short rules that compose to provide this emergent behavior. Something like:

Partial application of an isolated actor reference to a method produces a normal function type: self.f -> (Int)->Double
Partial application of a non-isolated actor reference to a method produces an async function type: otherA.f -> (Int)->Double async
You can't pass self.g to runDetatched because it isn't a @concurrent function (per rule 1) and SE-0203 doesn't allow passing non-concurrent functions to runDetatched
You can't pass self.g to runLater because (some rules that I don't actually understand w.r.t. escaping).

Isolated Protocol Conformances in future directions

The discussion here is a reasonable framing, but I think you have the sense of protocol conformance wrong. This draft of the proposal is suggesting that:

actor Foo : SomeProtocol means a "non-isolated" conformance
actor Foo : isolated SomeProtocol means an "isolated" conformance

However, non-isolated functionality is much more limited (and no core to the actors design as I argue above) than isolated functionality. I think instead you should have:

actor Foo : SomeProtocol means an "isolated" conformance
actor Foo : nonisolated SomeProtocol means a "nonisolated" conformance

This also allows you to move the "nonisolated" feature in general out to a followup, focusing this proposal on building out the core actor model.

I didn't see it in this proposal, but in previous versions of the draft there was the suggestion of having an AnyActor like protocol that all actors conform to, as an analog to AnyClass. Is this still interesting and inflight or did this get intentionally dropped?

Overall, this is another huge step, thank you for driving this forward!

-Chris