SE-0304 (2nd review): Structured Concurrency

Indeed. But the language should be consistent, no? Either with functions need an argument for specifying return type, and that’s a change that should be made across the board, or they don’t. Or is there something that makes this case radically different from the other with functions in the language?

1 Like

I got the impression that this would be the canonical way to create subtasks, and other ways would desugar to this?

It’s the most low level way to create child task. The actual way that is useful more often is “spawn let” which will get its own proposal — it has been removed from this proposal as people said it was distracting.

That proposal could go up right away after we conclude this.

You can find references to it in the structured concurrency pitch threads — and I’ll be preparing a revised version in a matter of days.

We also cannot exclude other future keywords or functions that might create specific kinds of child tasks. Task groups are simply the “most powerful” api to work with them, because of the in completion order retrieval of values, which won’t be possible with other APIs.

1 Like

I don’t see how this is an argument against claiming a broad name now. Future keywords and functions can always be added. :man_shrugging: Do you have some specific feature in mind where a name like withConcurrentSubtasks would be a better name?

My issue with the name withTaskGroup is that it doesn’t give me as a reader enough to go on — crucially it doesn’t call out the one thing that is most important to the reader, namely that subtasks are executed concurrently — and withConcurrentSubtasks is my understanding of what it actually does. If nothing else, it would help my understanding of the proposal to hear why a name like withConcurrentSubtasks is better saved for some future feature.

As I see it, the closure is not handed "subtasks". It's more handed an empty "subtask list". To me "task group" makes sense, as a specific named object that has certain features like being able to spawn subtasks.

How about something like "subtask scope"?

I suggested TaskGroup(of:) because task groups are AsyncSequences, you interact with them by looping over and the type that each child task returns is the type of the element of the sequence.


Is it true? Isn't concurrency dependent on the executor that is handling the parent task? If the executor is sequential there won't be concurrent child task.

1 Like

While we're bike-shedding, I find "spawning a child (task)" to sound pretty weird, almost like an impedance mismatch.

3 Likes

Like I said, I’m still not sure I understand the proposal fully. Can you know ahead of time whether a function will run on a serial or concurrent executor? Don’t you have assume that any task group could potentially execute subtasks concurrently?

If we’re going to have a low level syntax and a “lightweight” syntax it would be helpful to have an example of the type of pattern you see as impossible within the design space of the lightweight syntax. Spawn let looks great, and I wonder if the basic idea of spawning within a scope could cover all the bases with some design exploration.

You can use this handy list of proposals: Swift Concurrency Proposals · GitHub

To locate: https://github.com/jckarter/swift-evolution/blob/async-let/proposals/mmmm-async-let.md which was split out from the structured concurrency proposal some time ago because people found it distracting. That version is a bit outdated but the general idea stands.

As I've just completed some changes in implementation of those and we have now a better understanding of the feature in general, the proposal will get a refresh and more details very soon -- and then it's own review.

It's a bit catch-22, as we moved the async let proposal out of this proposal based on feedback; and now feedback is that we need to discuss the async let etc. at the same time :wink: So... yes, there will be some form of spawn let how we intend to call it, and very likely other things as well, so it is just just pure speculation on my part about future keywords but anticipation of where we're going.

1 Like

It's the same as spawning a thread; I don't think we're at this point willing to bike shed the word spawn any longer; the core team has debated this for a long time and decided we're going with spawn.

2 Likes

Some quick feedback on the proposal, I apologize for being after the official review cycle:

  • I love the improvements from v1 of the proposal. Moving things out of Task is much more clear.
  • I agree with other comments that the priority names should be generalized, even if they remain a small number of integers. This proposal seems reasonable to me.
  • I think that standardizing around the term spawn is a good way to go, but I'd recommend going further and pulling it into the detatch method. Instead of:
let dinnerHandle = detach {
  try await makeDinner()
}

I think it would be more fluent and would avoid introducing another top-level verb if we used:

let dinnerHandle = spawnDetached {
  try await makeDinner()
}

It seems ok to make the unstructured concurrency API more verbose since we prefer structured concurrency. This also maintains alignment around the spawn verb. Another alternative is detatchTask, but I think that aligning with spawn is a better way to go. You could go really crazy and use spawnDetatchedTask I suppose.

  • With the introduction of effectful properties, should get() in let dinner = try await dinnerHandle.get() be an async get-only property, e.g. let dinner = try await dinnerHandle.value? The corresponding getResult() would just be .result. This aligns with the general cocoa naming convention which doesn't like "get" methods.

  • On withTaskCancellationHandler I agree that putting the cancelation handler second and using something like onCancel seems most consistent with other frameworks and with do/catch syntax.

Overall, this is really great work, I'm excited to see it coming into Swift!

-Chris

12 Likes

I’m not an expert in async tasks, but I’ve been using Swift for a long time and one thing I’m pretty sure of is that Swift’s idiom for creating a new instance is to call an initializer.

In this case, we are creating a new task, so the spelling most in line with Swift’s existing standards is:

let dinnerTask = Task {
  try await makeDinner()
}

We are creating a Task, so we should call an initializer on a type named Task.

I understand that the current design calls this type TaskHandle, but that does not align with Swift’s conventions.

When you create a string, you get a String not a StringHandle. When you create an array, you get an Array not an ArrayHandle. And when you create a task, you should get a Task not a TaskHandle.

The type which the proposal calls Task, based on how I see it used in the proposal, would be better spelled CurrentTask.

11 Likes

Nice! That would be awesome.

It would be nice to change Result.get() too, then :+1: But that's of course a bigger change.

After looking at the Task-Local Value proposal, I also believe that UnsafeCurrentTask doesn't really benefit anyone outside of the standard library. It may have more functionalities than the Task object, but we've been adding them as the needs arise. So an outsider would just use other (safe) APIs within the standard library. Unless the reason is performance, which I don't think is the case.


:smirk:

It's indeed quite apparent that there's a blatant gap in spawning a group of heterogeneous child tasks, especially in the first example of Task groups and child tasks. Not that I particularly mind.


Maybe "spawn a subtask"? I feel like the term child task only works well with nursery terminologies, from which we've since moved away. We still have "parent task" though, :thinking:. Oh well, let bikeshed be bikeshed.

2 Likes

I prefer detach because I assume that this function may be called pretty often.

I truly hope we don't start adding language complexity to start working around existing problems with the language (e.g. lack of variadic generics, and lack of trivial escape checking). I don't see any reason why the makeDinner example couldn't be sugared with /library features/. While it would be best with variadic generics, it would also be perfectly possible to have a bunch of overloads that would allow it to be expressed as something like this:

func makeDinner() async -> Meal {
  // Could also declare these separately and use destructuring instead of initialization.
  // var veggies: [Vegetable], meat: Meat,  oven: Oven

  // Run three closures in parallel using a library feature, returning when all return.
  var (veggies, meat, oven) = try await runInParallel({
      try await chopVegetables()
    }, {
      await marinateMeat()
    }, {
      await .oven(preheatOven(temperature: 350))
    })

  // No need for force unwraps anymore.
  let dish = Dish(ingredients: [veggies, meat])
  return try await oven.cook(dish, duration: .hours(3))
}

Such APIs can be built on top of the basic task group API. Such APIs could also have forms that return TaskHandles, and would be more akin to the proposed async let thing. There is a wide range of convenience APIs for this, just as there are things like parallelForEach/parallelMap that should be part of the standard library, even though they aren't part of the primitive Task APIs.

The only utility that async let sugar provides is more safety around "value does not escape" checking. Such checking would be just as useful for many many other values in Swift (including the group argument to the closure in withTaskGroup, the argument to withUnsafeCurrentTask, withUnsafeBufferPointer, .....etc). Paving this out as part of the general language would be far more valuable than one tiny part of the language/library, and would not require introducing full ownership to get a lot of incremental safety.

-Chris

10 Likes

Speaking personally, I'd very much prefer to stick with detach than to add needless additional words. I think salting a legitimate and useful function like this is unnecessary (and doing so because "we prefer structured concurrency" is excessive moralizing).

Additionally, the whole win with "spawn" is its link with parent/child relationships. But detached tasks are not children, so the term "spawn" is inappropriate, rather than synergistic.

8 Likes

I can see where you are coming from, but I see it differently.

The point of “spawn” was to align with concurrency terminology, where its use is pervasive. That prior art isn’t generally “structured”, so using it with detach seems very reasonable to me. Furthermore, even Ignoring history, I don’t see a “structured” connotation to the verb spawn.

My argument against detach as a top level function/word/command isn’t about moralizing, it is about simplifying and standardizing the lexicon around concurrency. Neither “spawn” or “detached” is a needless word. Detach without a qualifier is too ambiguous, and “detach” is not a concurrency term of art like spawn is.

-Chris

5 Likes

Apologies for being way after the official review deadline.

I followed the 1st pitch, but not later pitches and the 1st review.

In my opinion, structured concurrency is perhaps the most straightforward piece of the concurrency effort.

My feedback focuses on API back-shedding, and echos some posts up-thread.


Although the term "nursery" is much disliked by most, I find it convey better than "task group" the sense that it's where children are kept alive. Perhaps withTaskGroup can be renamed as withChildTaskGroup? This way it conveys to the user directly that it's a group of child tasks, not just some arbitrary tasks. Also it is more consistent with the function's generic type parameter ChildTaskResult.


What about .successfulResult instead of .value? .value is much better than .get(), but it still feels not very obvious what the return is.