SE-0304 (4th review): Structured Concurrency

[nikitamounier]

@Chris_Lattner3

avoid temptations to accidentally make global decisions just because they are locally beneficial for the "hot API of the day"

It's not that I like it because it's a supposedly "hot API" – I like it because I strongly believe it's the right API.

[Jumhyn (Frederick Kellison-Linn)]

@Douglas_Gregor

It's not really "new". As I've said, SE-0269 reconsidered when it's useful to require explicit self. and laid out the low-likelihood-of-causing-actual-cycles criteria.

As the author of SE-0269, I will say that I take Chris's point that there is a slight difference-in-kind here.

Pre-SE-0269, the only control an API author had over clients' usage of implicit self was whether the closure capturing self was @escaping or not, and making a closure non-escaping imposes significant enough restrictions that I strongly suspect API authors weren't using escaping-ness as a lever to control the availability of implicit self capture.

The additional cases introduced in SE-0269 to enable the usage of implicit self involve decisions made on the client side of the API, where there's enough context around self to know that a cycle is unlikely to be created, due to either a promise by the client themself (self in capture list) or the nature of the semantics of self (when self is a value type).

Neither of the cases in SE-0269 introduced a new lever for API authors with regards to implicit self. The presence/absence of a capture list is opaque to the API, and the API doesn't have any idea what the type of a captured self is.*

All that said, I have wished (even pre-concurrency) there was some sort of attribute like @_implicitSelfCapture for APIs like DispatchQueue.main.async that cannot reasonably cause a reference cycle via the escaping closure they accept. So I'm sympathetic to the desire to have this pulled out into a small, single-purpose proposal, but even if it doesn't get its own treatment I think it seems like probably the right general direction (and further specifics can be discussed when/if a proposal comes around to un-underscore @_implicitSelfCapture).

*: I suppose you could sort of get a lot of nice implicit self fallout by designing an entire library around value types (a la SwiftUI) but, like escaping-ness, the decision whether a given type should be a reference type or a value type has many more salient implications than "will clients be able to use implicit self?"

[Chris_Lattner3 (Chris Lattner)]

@Douglas_Gregor

It's not really "new". As I've said, SE-0269 reconsidered when it's useful to require explicit self. and laid out the low-likelihood-of-causing-actual-cycles criteria.

@Jumhyn explains this well. The different from SE-0269 is that you're taking a language policy and opening it up to API authors. I'm suggesting that this is a expansion of API design which hasn't had a detailed discussion in the community. These sorts of language extensions have had evolution proposals in the past, so I'm suggesting it would be consistent to do the same thing here.

To be clear, I'm not saying that I think this is a good or bad language extension, I'm just pointing out that we have a protocol for language extensions which isn't being followed.

@Douglas_Gregor

@Chris_Lattner3

Right. You've said this before, so there must be some confusion. I have not seen anyone argue in favor of a public initializer that allows one to create a task, and then launch it separately. We agree with the design approach.

So we're agreeing that the API that creates a task launches the task.

Yes, has that actually been confusing? We're fundamentally talking about how to spell the API, not its behavior.

@Douglas_Gregor

@Chris_Lattner3

We are pointing out that initializers have never been used this way in the Swift standard library (and I gave rationale for that) and pointed out that there are multiple simple alternatives that fit better with the existing language/library design principles.

As @nikitamounier points out, we shouldn't exclude a pattern just because the standard library doesn't already have it.

Agreed, there are always times where it is good to introduce new approaches, and they are typically given strong rationale. On the other hand, this thread has addressed several important reasons the standard library doesn't do this (discussed extensively upthread) which you haven't addressed.

It would be great if you could try to explain why deviating for the norm is better, and your rationale for why the technical points raised upthread aren't a problem. You are consistently waving them away as "you don't think something else is needed" without providing the justification for doing something novel here, nor have you addressed the technical concerns raised upthread.

@Douglas_Gregor

@Chris_Lattner3

Fundamentally, you are arguing for significant deviance from the API design standards in the Swift standard library. I feel like we should have extremely strong justification for such deviance, or we should do the obvious thing and consider other approaches (e.g. discuss their tradeoffs).

One of our principles is "omit needless words." We don't need both "launch" and "task", because we've established that creating and running a task are the same API. I contend that the creation is the more dominant aspect of this relationship, which supports use of the Task initializer.

Right, that is an important goal. However, the top two goals are "Clarity at the point of use" and "Clarity is more important than brevity.". "Omit needless words" doesn't even rank in the top list of "Fundamentals". Furthermore, the entry immediately above "Omit needless words" is "Include all the words needed to avoid ambiguity" - the amount of discussion about this seems to illustrate pretty clearly that there is ambiguity here.

We pretty consistently use active verbs for side-effecting operation like this, object construction for standard library types doesn't have side effects like this, and there are multiple simple and consistent ways to solve this naming problem.

@Douglas_Gregor

That said, I'm surprised that you'd prefer it given your complaints about Task.detached earlier:

@Chris_Lattner3

You also ignored many of my points here: all the static members on Task refer to the current task except this one. In addition to being inconsistent with general API design principles, this method is inconsistent with the rest of the API /in this proposal/.

Either this consistency argument equally dismisses Task.detached and Task.launchDetached / Task.launch , or it's not actually a problem. Personally, I don't consider it to be a problem (as I've stated before) for either API.

Happy to explain that - the difference between Task.detached and Task.launchDetached is that the former sounds like it is detaching the current task. The later makes it clear that the current task is launching a new task which is detached.

The key is that the verb is "launch", not "detach", which makes sense given that the subject is the current task.

-Chris

[Ben_Cohen]

@Chris_Lattner3

Happy to explain that - the difference between Task.detach and Task.launchDetached is that the former sounds like it is detaching the current task.

Quick clarification because it keeps getting misspelled: the proposal is Task.detached not Task.detach.

[Panajev (Goffredo Marocchi)]

Hey Ben,

Thanks for this clarification, this makes Chris point stronger. Task.detach {} gives more of an idea that you are telling the Task class object/thing to detach something which could be understood as launching a block of code as a detached task.

[Task detach: ]; //if we imagine it in the old Obj-C syntax

Task.detached {} is not as clear IMHO. I think I prefer the launchDetached version, it works a bit better clarity at call site wise which is one of the two top points of the language list of priorities.

[drewster99 (Andrew Benson)]

@Chris_Lattner3

Happy to explain that - the difference between Task.detached and Task.launchDetached is that the former sounds like it is detaching the current task. The later makes it clear that the current task is launching a new task which is detached.

The key is that the verb is "launch", not "detach", which makes sense given that the subject is the current task.

Completely agree here.
Clarity is more important.

Clarity for people new to the API. Clarity at the point of use. Clarity for our future selves.

Having the clarifying words in names for API interfaces greatly reduces cognitive load overall when trying to reason about a piece of code and leads to fewer mistakes.

[John_McCall]

@Jumhyn

All that said, I have wished (even pre-concurrency) there was some sort of attribute like @_implicitSelfCapture for APIs like DispatchQueue.main.async that cannot reasonably cause a reference cycle via the escaping closure they accept. So I'm sympathetic to the desire to have this pulled out into a small, single-purpose proposal, but even if it doesn't get its own treatment I think it seems like probably the right general direction (and further specifics can be discussed when/if a proposal comes around to un-underscore @_implicitSelfCapture ).

I think we're fairly clearly inviting that proposal; we just didn't want to hold up this proposal for it, or punish people using the task APIs until we have it.

[JJJ (Jens Jakob Jensen)]

I think Task.detached {...} is pretty clear.

My order of preference:

Task.detached { await stuff() }
Task.runDetached { await stuff() }
Task.startDetached { await stuff() } // NSThread has start()
Task.detachNew { await stuff() } // NSThread has detachNewThread()
Task.launchDetached { await stuff() }

I'm not too fond of "launch" as it sounds like you are setting off something big while it's just a cooperative thread.

[jlukas (Jacob Lukas)]

I would prefer either both forms are initializers or neither are, e.g.:

Task { … }
Task(.detached) { … }

Or (using run as a placeholder):

Task.run { … }
Task.runDetached { … }

[moreindirection]

@jlukas

I would prefer either both forms are initializers or neither are, e.g.:

Task { … }
Task(.detached) { … }

Wow, +1

This is my favorite notation. Simple and consistent. You can tell right away what it does.

[Jon_Shier]

They've explained why that won't work: you can't parameterize the inheritance of the task environment at runtime, it's a compile time setting, so the two must be separate calls.

I agree with both sides on this: something as fundamental as starting a Task should be lightweight, but it does feel weird not to use a verb to describe the whole action. At the same time there's a lot of implicit behavior that shouldn't be exposed to the developer, but it's important, so maybe some indication is necessary? I don't see a solution that feels as light as it should while still retaining the explicitness some feel is necessary.

[jlukas (Jacob Lukas)]

@Jon_Shier

They've explained why that won't work: you can't parameterize the inheritance of the task environment at runtime, it's a compile time setting, so the two must be separate calls.

It could be two different initializers.

[JJJ (Jens Jakob Jensen)]

That's right.

They cannot use the same initializer, because they take different closures: SE-0304 (4th review): Structured Concurrency - #46 by benrimmington

But, yes, they could easily (slightly hackishly) be made two different initializers

[Jon_Shier]

Not really, since it's not possible to disable trailing closure syntax one version instead of the other.

[JJJ (Jens Jakob Jensen)]

enum DetachedMarker { case .detached }

extension Task where Failure == Never {
  @discardableResult
  init(
    priority: TaskPriority? = nil,
    operation: @Sendable @escaping @_inheritActorContext @_implicitSelfCapture () async -> Success
   )

  /// Create a new, detached task that produces a value of type `Success`.
  @discardableResult
  init(
    _ marker: DetachedMarker,
    priority: TaskPriority? = nil,
    operation: @Sendable @escaping () async -> Success
   )
}

[Chris_Lattner3 (Chris Lattner)]

@Douglas_Gregor

@Chris_Lattner3

You're explaining very detailed rationale that the typical user will not know, and without that, this will just be confusing.

As I said, this "can be learned" -- but why does that make it a better API design?

The purpose of defining tasks is to perform some operation concurrently. It is important that we emphasize that you are creating a new entity---the task---because it is a new conceptual task that carries with it priority, task locals, etc.

With respect to the Task {} API, I think I may understand a source of the disconnect. I think it is possible that we are interchangably using the word "task" to mean two different things when we're talking about "creating a task". Let me try to define some specific meanings, then re-explain some points to see if this helps. I'll define:

1. A "task" is a unit of concurrency managed by the runtime.
2. A "Task" is a value in the swift type system, an instance of the Task struct. Instances of this type provide a handle to a "task" managed by the runtime, this was formerly known as Task.Handle.

The difference between these is pretty significant. You've made several arguments that using a Task initializer to create a "task" makes sense because we use initializers to create things. I've argued that a discardableResult initializer is unprecedented and doesn't make sense given how Swift treats values (yes, I realize this behaves like a reference), and argued that the principle side effect of this behavior (however it is spelled) is the side effect of creating a unit of concurrency.

I think the disconnect really comes down to "task" vs "Task". I'll try to explain my position more precisely:

1. The Swift standard library doesn't have any discardableResult initializers, because it doesn't make sense to construct an instance of a type to then throw it away. In Swift it is important and useful to reason about side effects for lots of reasons.
2. I think we both agree that the principle operations in question create a "task" (the runtime concept) and return a Task that the client can use to manipulate the runtime concept if they desire (e.g. cancel it, await it, etc). The question is how to spell/model this.
3. The fact that the Task is discardable is a strong hint that the value produced is not always needed - indeed we've discussed the common "fire and forget" use case which doesn't need the Task. This is why I've argued that this API is principally about creating a "task", not about creating a "Task".
4. The detached form of this has caused confusion - Task.detached as a static method is confusing to me for two different reasons: 1) It is inconsistent with other Task APIs because it creating a new "task", it doesn't detach the current task. 2) In this form, it is using a static method as a sort of "labeled initializer" (presumably because closures swallow keywords) but the primary concern isn't to create a "task" it is to create a "Task".

I think that there is a fairly significant issue here: the Task struct really isn't the underlying "task" - it serves much more like the former name "TaskHandle".

If we renamed Task to TaskHandle (something that I suspect is off the table even if conceptually the right thing) then we would never name these operations TaskHandle.init and TaskHandle.detached.

-Chris

[nikitamounier]

Huge +1 to this. And it shouldn't have any visible effect on the type-checker right? Since they're both structurally different initialisers.

[hborla (Holly Borla)]

@nikitamounier

And it shouldn't have any visible effect on the type-checker right? Since they're both structurally different initialisers.

Introducing another initializer overload would indeed have type checker impact. If the overload is spelled exactly in this way, the impact should be minimal for well-formed calls because one of the overloads has an additional mandatory parameter, and the defaulted parameter in both overloads has a label, so the type checker can conservatively filter one of those out. However, there would be a bigger type checker impact for unapplied references to the initializer that omit labels, as well as invalid calls because both overloads may be explored on the diagnostics path.

[Jon_Shier]

Fair enough. I did not consider an enum that didn't actually do anything. I don't really like that API, but I guess it's a consideration.

[Ben_Cohen]

This general problem – of being able to distinguish an initializer purely with a marker – has come up before in an evolution proposal, though unfortunately I can't recall which one. IIRC we didn't settle it into an idiom at the time, and that the "a single value enum" overload suggestion was rejected (as was the uglier "defaulted bool" i.e. Task(detached: true)).

This potential solution is well known, but types have a cost (they need to be documented, single-case enums are weird at first glance and are kind of a "trick" solution, albeit a mild one compared to other overloading tricks), and I think there needs to be more justification why Task(.detached) { ... } is better than Task.detached { ... }, beyond stylistic preference or claims of better discoverability.