Thanks John! This is looking very good overall!
Left some small editorial change proposals in the PR, (method names of existing APIs, maybe adding one more example).
This is very clearly the right thing to do — to enqueue Task { isolated actor in
immediately to that actor, and it’s pretty good that we’re able to get this behavior without having to do a new keyword: I used to think we’d do this with send actor.work()
, but this also a good spelling for it, perhaps more consistent with the language.
Ordering
The tradeoff with optimizing away hops and guaranteeing order being dependent on wether or not the annotation was explicit is probably a good tradeoff. As long as the Task { [isolated thing] in }
is guaranteed to enqueue we’re good for the cases that need to be careful about the order. It not applying when “implicit” works okey for things like just kicking off an unstructured task because “i don’t want to wait” without too much care about order…
It definitely is a “gotcha” though so we should highlight this in the swift book and improve documentation, including Task.init and friend’s API documentation about enqueue behavior. But if we do that, it sounds good to me
Distributed
Nothing to complain about in this proposal, for local ones we’re good — and this actually is very important for making IPC/Network messages be received and enqueued in the order as they arrived. So, I’m very happy we’re able to have correct message order now in recipients without resorting to hacks!
It did made me realize though that we’re still missing a way to send uni-directional to remote actors “in specified order” which is common but this will have to be solved some other way. I was thinking if perhaps our default executor for remote actors being the “crash on enqueue” is bad, and instead we should make up a specific default executor for such actor… then somehow make use of it for the sends…
But this isn’t for this proposal; the proposed semantics of just working ok with the “known local” ones are good. Thank you for including them in the discussion!
Async let
I noticed we’re not discussing async let
in the proposal, probably just missed it?
The behavior should be the same as with a group’s addTask
I think:
actor Worker { func compute() {} }
func go(worker: Worker) async {
async let one = { [isolated worker] in
worker.compute()
}
}
assumeIsolated
As the proposal mentions this is TODO to figure out… So what we’ll want is:
actor Worker { func test() {} }
func test(worker: Worker) {
worker.assumeIsolated { worker.test() }
}
While the current assumeIsolated is forced to take a parameter:
worker.assumeIsolated { isolatedWorker in // current API
isolatedWorker.test()
await worker.test()
}
The way to solve this is somewhat alluded to in the Future Directions: @isolated(parameter)
but also explained that it’s a difficult value-dependent types feature we’d need to introduce here…
Is it worth handling the assumeIsolated
specifically today, until we’re able to generalize the isolated-to-parameter feature?
assumeIsolated + GlobalActor
The lack of this has made the assumeIsolated
APIs unfortunately limited — for example, the API today does exist on MainActor
as a static function. But was not able to be expressed for “any global actor”, because we could not write:
// could not write this before
extension GlobalActor {
static func assumeIsolated<T>(_ operation: !!@Self!! () throws -> T) rethrows ->T
}
But with recent isolation proposals; we’re able to know that GlobalActor.shared
is same as isolating to @GlobalActor
… Technically we could offer an assumeIsolated now that uses the same pattern that the existing assumeIsolated on instance actors, but today it’d still need the isolated parameter…
extension GlobalActor {
static func assumeIsolated(_ operation: (isolated GlobalActor) throws -> T) rethrows -> T { … }
}
but really we’d want to
extension GlobalActor {
static func assumeIsolated(_ operation: @isolated(to: Self.shared) throws -> T) rethrows -> T { … }
}
That’s another quite tricky spelling I think, we’re referring to a property, not even just a parameter of the function, but a property of the global actor.
Long story short — I’m dancing around the question if we should “make assumeIsolated
work as expected” even if we don’t have the general feature of @isolated(to:)
yet?
I’m not sure about the answer, it definitely would be nice to have it work — but on the other hand, I think given workarounds exist for this — just by doing more isolated parameters everywhere now. So that’s an open question I wanted to add to the discussion.
Thanks again for the work on this, it’s really awesome to extract .isolation
out of these functions like that!