[Pitch #2] Actors

This revision is a huge step forward. I am very excited to see this, and I'm also happy to see that some of the more complicated topics are split out to their own subsequent discussion. This makes it much easier to understand the base proposal and make progress on things in steps. Really this makes a big difference, and the design refinements you are making here are a great improvement.

Some high level comments before detailed ones:

  • I know that many will find it a superficial decision, but I think that actor Foo is much more clear than actor class Foo and will align with how people refer to these in practice, so +1.

  • I know that these proposals are all happening concurrently and there are some race conditions between them (ooh, sorry, I just had to :-), but I think that this proposal logically builds on the ConcurrentValue and @concurrent closure discussion. It is great to make progress on these in parallel, but I think it would be beneficial to nail that one down before actors goes to official review.

  • I appreciate the addition of the actor inheritance discussion in the "Alternatives Considered" section. I still personally disagree with this choice, but I respect your rationale, and am happy to see it included in the proposal in a first class way. Thank you.

  • I still think the @actorLocal and "lets are implicitly accessible across actors" design points are suboptimal and think it is critically important to continue discussing this. I think those discussions will be easier after the @concurrent discussion settles (see below), but I'll add some thoughts on @actorLocal to a follow-on post to this one (just to separate the discussion a bit and provide more space for exploration) when I have time.

Here are some more points from a detailed read through:


Minor writing thing

For actors, the primary mechanism for this protection is by only allowing their stored instance properties to be accessed directly on self

Super nit pick but actor local state can be passed through inout arguments, which means that g touches actor local state even though it doesn't go through self:

func g(_ x : inout Int) { x += 1 }
actor X {
  var a = 42
  func f() {
    g(&self.a)
  }
}

I'm not sure if this is worth pulling into the early part of the writing or would just be confusing.


Async Properties FYI

Synchronous actor functions can be called synchronously on the actor's self , but must be called asynchronously from outside of the actor.

Fantastic, thank you for pulling this in. I think this approach will define away a lot of boilerplate.

However, note that this will/should also directly affect property access, so we'll need to figure out what the semantics of await otherActor.state += 42 means, or explicitly forbid it. There is another proposal exploring effect-ful properties, so that naturally dovetails in.


Class methods and other stuff?

Since actors support inheritance, do they have class-like methods? Are they called actor methods? How about convenience initializers etc? How many of the class-specific attributes (e.g. the core data ones) work on actors?

Please add this to the proposal with a description of their semantics (or the omission).


Intersection of @actorIndependent and @concurrent closures

I don't want to get into whether @actorIndependent is a good idea of not in this post, but I do want to observe that the design point hinges quite heavily on the details of how the @concurrent closure proposal shakes out. In particular, I expect the fallout of it to be that @concurrent closures are actor isolated (since they can run in different concurrency domains) but non-@concurrent ones are not, and you don't need any attributes or additional type system machinery.

Notably, this means that you get this behavior by default, and don't need @actorIndependent at all:

actor SomeThing {
  var x = 42

  func accumulate(arr : [Int]) {
    // Sequence.forEach takes non-concurrent closure, so self reference is perfectly fine.
    // Non-concurrent closures are always guaranteed to be run in the actors concurrency domain.
    arr.forEach { self.x += $0 }

    // Similarly ok.  `sum` is actor local state, even though `self` is not involved, but the closure isn't
    // concurrent, so this is fine.
    var sum = 0
    arr.forEach { sum += $0 }
  }

  func bad(arr : [Int]) {
     // Error: use of x from concurrent closure must be marked await since it is from a different concurrency domain.  "self" is captured by value.
     Task.runDetatched { print(self.x) }

    // Ok because @concurrent closures capture as by value copies.
    var sum = 0
    Task.runDetatched { print(sum) }

    // Error: sum is an immutable let capture because closure is @concurrent.
    Task.runDetatched { sum += 1 }

     // Ok, sum captured by copy.
     arr.pararallelMap { $0 + sum }

     // Error, sum is immutable.
     arr.pararallelMap { sum += 42; return something... }
   }
}

This behavior isn't spelled out in the @concurrent proposal because it is logically underneath the actor proposal in the dependency tree, but I can add some details about this if you think it is helpful. This is one of the major things that shakes out of modeling @concurrent closures correctly, and I think this is really important to make sure that higher order programming works in actor contexts.

Also worth keeping in mind is that @concurrent is the thing that unifies structured concurrency and actors, while @actorIndependent is an actor specific concept. It doesn't really make sense for global functions like runDetatched to know about actors - they should know about one thing that is common to structured concurrency and actors.


@escaping shouldn't imply @concurrent

This is best discussed on the other thread but I think that conflating concurrency with escaping is the wrong way to go, and your example illustrates on strong reason for this: parallelForEach doesn't escape the closure!

It is much cleaner in my opinion to mark runDetatched as an escaping and concurrent function, and parallelForEach to be a concurrent but non-escaping function.

In any case, once the @concurrent thing gets figured out and nailed down, much of the discussion around @actorIndependent will need to be reconsidered. As proposed, it seems like a decl modifier that makes the underlying declaration have @concurrent function type or something.


Actor-isolated stored properties can be passed into synchronous functions via inout parameters, but it is ill-formed to pass them to asynchronous functions via inout parameters.

+1, good call.


Interleaving/reentrancy:

The discussion about interleaving and actor reentrancy is really fantastic. Clear and to the point.

async let shouldBeGood = person.thinkOfGoodIdea() // runs async
async let shouldBeBad = person.thinkOfBadIdea() // runs async

I'd recommend not using the async let syntax in this proposal to keep the proposal dependency graph clean.

Deadlocks with non-reentrant actors can be detected and diagnosed with tools that can identify cyclic call graphs or through various logging/tracing facilities. With such deadlocks, the (asynchronous) call stack, annotated with the actor instances for each actor method, should suffice to debug the problem.

Yes, in the general case, this is pretty equivalent to detecting heap cycles that cause leaks. It is possible that some tools can catch some bugs, but only dynamic tools can reliably diagnose deadlocks that have happened, and no tool (as far as I'm aware) will ever be able to show that deadlocks aren't possible in the general case. It would be good to clarify the writing to avoid giving people false hope.

Therefore, we feel that the approach of automatically cancelling on deadlocks does not fit well with the direction of Swift Concurrency.

I completely agree and would go further: If possible, it would be great for the runtime to do a "best effort" attempt at detecting deadlocks and abort if they happen to "fail fast". I don't think that this can be efficiently implemented in full generality, but detecting this dynamically and failing fast in some cases will be a very useful debugging aid in practice.

As noted previously, we propose that actors be reentrant by default, and provide an attribute ( @reentrant(never) ) to make specific actors or actor-isolated functions non-reentrant.

I completely agree that reentrancy should be the default --- but are you sure we should enable this new thing at all in the base proposal? From my perspective, I have seen a lot of people be concerned about the reentrancy behavior in the discussion, but we as a community have very little experience building large scale code for the Swift actor model -- and we won't until it stabilizes and the APIs around it build out. Would it be reasonable to define this model, say it is available for consideration in future proposals, but intentionally defer it until we get more usage experience?

Rationale:

  1. This attribute introduces a source of deadlocks and other nasty performance issues that we'd like to define away.
  2. This attribute hopes that it reduces other kinds of logic bugs, but we cannot measure the balance between the bugs it introduces and removes today.
  3. No existing language has precedent for await marking, so we can't extrapolate from their experience.
  4. Once this is in the language, we can't take it out, and this clearly adds complexity to the language and runtime.
  5. This attribute is effectively syntactic sugar for moving stuff to sync functions.

Unless there is a really good reason to include this in the base actor proposal, I'd recommend deferring it until we decide we "need" this. Such a need is best shown with evidence that the bugs introduced are less bad than the bugs resolved.


Actors subclassing NSObject

As a special exception described in the complementary proposal Concurrency Interoperability with Objective-C, an actor may inherit from NSObject .

Do they need to inherit from the NSObject class, or should @objc actors be automatically made to subclass NSObject in their ObjC metadata? It seems much cleaner to sweep this special case into the @objc attribute instead of allowing actors to subclass NSObject directly. The different affects member lookup on the Swift side of things, and I don't think we want actors to have self access to all the NSObject stuff.

I think it would also be good to have a description of how @objc works with this proposal. If it is best to handle that as a separable proposal, then I think the discussion of NSObject subclassing should similarly be moved out.


Wording/specificity nitpick

Partial applications of synchronous actor-isolated functions are only well-formed if they are treated as non-concurrent.

I'm not sure what "concurrent" means here, please revise this to be more specific and tie into the type system features defined by and used by this proposal.


Tie into ConcurrentValue

Once the ConcurrentValue discussion converges, it would be great to refer to it from the actor proposal, since it affects can methods and properties can be used across actor boundaries.


Overall

This proposal revision is a huge step forward. At some point I'll write up some thoughts on alternative approaches to the @actorIndependent thing. I see what you're going for here, but there is at least one alternative that achieves the same thing with a different set of tradeoffs that would be good to discuss. Thank you for the hard work on the actor model!

-Chris

10 Likes