SE-0304 (3rd review): Structured Concurrency

Evolution Proposal Reviews
21

This is an "obviously critical" proposal for the Swift Concurrency direction, but I feel that this version of the proposal is "one step forward, one step back" vs the previous proposal, which I was pretty enthusiastic about. I have put a lot of time and energy into this and neighboring proposals.

While I agree that these are related, the async let proposal is a syntactic sugar proposal for a narrow part of what this proposal covers. It still has serious issues that need to gel, so I recommend that we get the foundation right without over indexing on a sugar proposal that needs more iteration. Thank you for mentioning this though, because one of the threads of discussion from that should absolutely be pulled in here (below).

Here are detailed thoughts below:

The move to embracing unstructured tasks, support for the creation of tasks in sync modifiers, are all really great. That said, I'm concerned with this direction:

let dinnerHandle = Task {
  try await makeDinner()
}
let dinner = try await dinnerHandle.value

This is a subtle but really problematic conflation of two different ideas: a task is an independent process/thread/task that does some computation. While we often specify these as functions and functions have a return value, these often yield many values during their computation and have side effects. This is why we have things like AsyncSequence in proposal, while generators are a thing in other languages, etc.

The problem with this API direction is that it is incorrectly conflating Tasks with Futures by using Task as a standin for a future/asynchronously-completed value. Outside the simple cases, Tasks can produce streams of values, and can return multiple asynchronously completed values as independent futures. This becomes important when you start composing async tasks out of multiple other async tasks which may be detached - and detached tasks emerge extremely quickly when you branch out to IPC and RPC situations.

Furthermore, Actors need to interact with all the same sort of functionality, so it feels that this is a feature best modeled as a new library feature that is orthogonal to structured concurrency and actors, not something that is "part of" the structured concurrency proposal.

The only rationale I see for this is in the changelog, which says:

collapse Task.Handle<Success, Failure> into Task<Success, Failure>. This is the most-used type in the Task API and should have the shortest name.

To be clear, I super endorse giving Task.Handle a better name. I am arguing that it shouldn't be conflated with Task. I am suggesting that we should introduce a new top-level word like Future, since this concept will crosscut structured concurrency and actors. This issue was also raised on the "async let" proposal thread.

The biggest change in this proposal is moving from the unifying word spawn (an active verb) to the word async (an adjective) when creating new tasks. While the /semantics/ of this operation are good (and I think pretty well nailed down by this point) this /naming/ move is a big step backward, for several reasons. The most important of which is:

  1. Async isn't a technically correct word for this operation. The fundamental concept of a async {} child computation or a group.async {} child computation is happens independently and typically in parallel with the current task. However, the word async in the Swift language means "potentially suspends". It does NOT mean "happens concurrently". Conflating these two is a huge problem to me, and I think this will make it much more difficult to teach and learn Swift concurrency.

    This issue is also raised in the async let thread discussion, observing that async is our second effect and that we should learn from precedent of our first effect (throws):

    There are strong reasons why error handling has multiple "words in the lexicon": throws for the effect, try for the marker, Result for the "handle" type when erasing to an uneffectful function, and do/catch when introducing a new catch-processing region. I think that all these things are substantially different and are worth different "words" to clarify them.

    In the case of Structure Concurrency, the former version this proposal had a stronger design: it used the word async for the effect, it used await for the marker, it uses "TBD" for the future abstraction (this is the juicy center of the async let proposal that we haven't gotten to yet) , and it used spawn as the equivalent for do/while that introduces a new independent concurrent region.

  2. The group.async {} and top level async {} operations create a new Task and start it executing in parallel. However, the word async is an adjective, not an active/imperative verb. This directly contradicts the guidance in the published Swift API Design Guidelines, which says we should use an imperative verb here.

  3. The rename create weird APIs that don't make sense: if you aren't a "Swift Concurrency Expert", what would you expect asyncUnlessCancelled to do?

  4. This proposal fractures the "attached" and "detached" world. Where it proposes the spelling async {..} for attached tasks, it proposes Task.detached {..} for detached tasks. We want people to use attached tests where possible, but we shouldn't fragment the API this way. If we go with the term spawn {..} then the natural term is spawnDetached {..} which would pull these things together into a unifying framework, make the different clear, and slightly nudge programmers towards attached tasks.

  5. Version 3 of the proposal continues to use this verb pervasively to explain itself, e.g.:

    group.async spawns a child task in the task group to execute the given operation function concurrently.

    As well as sections like "Spawning TaskGroup child tasks". If people will continue to think about this operation as "spawning" something, then we should just embrace that, particularly without rationale for a change.

  6. Beyond the problems with renaming this operation to async there is no motivation for doing so - spawn was discussed extensively in revision #1 of the proposal and we agreed that it had a lot of prior art and is an active verb that successfully conveys "creating a new thing" concisely.

To recap: the move to the non-verb "async" for this operation is a big step back.
We don't need to stick with the word spawn, but if there is a problem with it, it would be better to air that problem so we can solve it. Moving to overloading an adjective effect modifier isn't a step forward.

The proposal suggest different spelling for the TaskGroup case vs the global case: group.async {...} vs Task {...}, which both inherit metadata. The family also includes Task.detatched {} and asyncUnlessCancelled {}.

I think it would be much more uniform to go with spawn {}, group.spawn { }, spawnDetached {}, and spawnUnlessCancelled {} as discussed in the previous round of the proposal.

Per the above point, this modeling:

struct Task<Success: Sendable, Failure: Error>

Seems wrong. In generality, a task can return multiple different results that have different lifetimes (consider a Task talking to a name server and a computation server independently) and tying their lifetime together seem unnecessary and limiting. It seems better to decouple "spawning" the task from "constructing the object", which allows providing more expressive APIs without sacrificing ease of use.

The move to change the withTaskCancellationHandler is a great move. Changing the onCancel member to be second will lead to more consistent and fluent APIs. A+

Agreeing with the discussion upthread, the design of the sleep API seems like it would benefit from further discussion, crosscuts actors, seems like it could be split out to a subsequent library discussion. This proposal would be easier to read if it were focused on the mechanics of spawning and interoperating with tasks, independent of the values those tasks create (AsyncSequence, futures, etc) and the things they may want to do (sleep, open files, etc).

-Chris

15 Likes