[Pitch] Inherit isolation by default for async functions

FWIW I've also noticed afterwards after re-reading my post that it indeed doesn't make much sense to say nonisolated yet imply that there might be some inherited isolation thorugh nonisolated(inherit) — I was more of trying to piggyback on an existing keyword so that there's not as much keyword explosion and a more straightforward transition option — but as implied, the spelling just came up on a whim :slight_smile:


I could imagine instead having the following set:

isolation(self)
isolation(none)
isolation(inherit)

— or anything along the lines, as long as it unifies the three possible options.

2 Likes

Changing a default is about matching reality to expectation - in this case, as expectations are still being set.

For evolution changes affecting the programmer's model and workflow, it would be nice to have a concrete artifact -- here a documented concurrency story -- so proposals like this could be evaluated in terms of the changes necessary in that story - literally the edits required.

To me in this discussion the most compelling voices speak to the impact on the mental model, but the discussion conveys different perspectives on that model without raising their conflicts.

Usually the normative aspects of the SE process ends with the acceptance of the proposal, and the implementation is delivered in the code, perhaps with notice in the release notes and change-log, and documentation is somewhat downstream. Any consensus is the tacit knowledge of experts.

But concurrency in particular seems like a mind-shift warranting ownership and diligence in specifying and changing the story, particularly if/since Swift emphasizes clarity at point of use over e.g., complications necessary for performance. I think that would help here and might surface other issues users face.

1 Like

I didn't mean to use "unspecified" as an actual Swift keyword, it's how we refer to isolation not being explicitly set on a function (in English). Similar to the concept of "uninitialized" (there's no keyword, it's just how a variable is when we don't explicitly initialize it)

3 Likes

+1 on addressing for the type level. With RBI we have got much wider applicability of nonisolated non-Sendable types with async methods, but they are still pretty limited in what one can do, unless you are ready to go all-in with isolated parameters on each method.

The more I think of this, the more I think that it's not a good idea to not have a keyword for this "unspecified" state that needs a better word (which tbh makes me prefer the proposal itself or some of the other proposed names for this new nonisolated behavior). We'd lose the capability of overriding a class-defined isolation in one of its methods, which could be useful:


class NonsendableClass {
	var value = 1
}

@MainActor
class MyClass {
  init() {
  }

  // not isolated
  nonisolated func nonIsolatedFunc() async {
  }

  // main actor
  func inMainActorFunc() async {
  }

  // oh oops, I don't have a keyword to override MainActor with "inherited/unspecified"
//  isolation(inherited) func inheritedFunc(_ nonsendable: NonsendableClass) async {
//    nonsendableClass += 1
//  }
}

actor SomeActor {
  let nonsendableClass = NonsendableClass()
  
  func someActorFunc() async {
	let obj = MyClass() 
	// runs nonisolated 
	await obj.nonIsolatedFunc()
	
	// runs on the main actor
	await obj.inMainActorFunc()

    // would run on SomeActor and be able to receive a non-sendable but oops we don't have a way of overriding MainActor with "inherited/unspecified"
    // await obj.inheritedFunc(nonsendableClass)
  }
}

One option is to gate this change behind an upcoming feature. The nice thing about this change is it's very easily auto-migratable to preserve the semantics of your code, simply by inserting the explicit annotation for always switching to the generic executor on an function that is nonisolated and async. An upcoming feature for a significant semantic change has downsides, many of which have been echoed throughout this thread, which is why I didn't propose that outright, but I think either way it'll be necessary for this change to come with migration tooling.

I also think it's important to maintain a super convenient way to offload work to the cooperative thread pool, and I think having a more explicit annotation than async has a lot of benefits. It's not immediately clear today from looking at an async function that its implementation is offloaded to the cooperative thread pool because of global actor inference, and if it is offloaded, it's very easy to accidentally violate that invariant by adding a global actor annotation somewhere that ends up getting inferred on your async function.

Yeah, I agree that the implication on async function values is unfortunate, but I don't see a feasible way around it. My suspicion is that solving the problem for non-Sendable function values -- function values that are formed and called within the same isolation domain -- is enough to solve the major pain points that people are facing. @Sendable function values already have different rules for things like isolation inference, and the difference in isolation inference for non-Sendable function values depending on captures already exists today, so I don't think this difference makes the proposal a nonstarter.

2 Likes

I might be off base here, but I kind of like the idea of flipping the logic of this by using a non-negated name instead. The keyword nonisolated could go away entirely and then various isolation(...) keywords would be the defaults in the different circumstances unless you override it by typing it yourself.

By having the nonisolated keyword go away, migration of existing code would have to deal with it at all points with no chance of missing those nonisolated async functions that might matter. (This could be annoying, though - might be a lot.) But the upshot is perhaps the fixits could be conditional so they can try to guide the migration to the correct isolation(...) variant for the specific function depending on if it actually had any awaits in it or whatever else?

(This feels a bit like when Swift went from @escaping functions being the default to @nonescaping and so the "logic" of the keyword we most typically saw in source code flipped around.)

4 Likes

If the existing behavior can get an attribute (@concurrent) when it's still the desired behavior, couldn't the same thing be done instead for functions that should inherit the isolation of the caller? For example isolable async/await isolating:

class NotSendable {
  func performAsync() isolable async { ... }
}

actor MyActor {
  let x: NotSendable

  func call() async {
    await isolating x.performAsync()
  }
}

I think it's unfair to have to match up @concurrent against isolation: isolated (any Actor)? = #isolation. The latter is obviously terrible :pensive:

Should even work for the withResource example (I think?):

public struct Resource {
  internal init() {}
  internal mutating func close() async {}
}

public func withResource<Return>(
  _ body: (inout Resource) isolable async -> Return
) isolable async -> Return {
  var resource = Resource()
  let result = await isolating body(&resource)
  await resource.close()
  return result
}
1 Like

It's how Classic Mac applications worked as well until MacOS 8(or 9?) added opt-in pre-emptive threads.

1 Like

The preamble within SE-0420 - Inheritance of actor isolation aligns with my thoughts:

It is sometimes useful to be able to give a function the same actor isolation as its caller, either to give it access to actor-isolated data or just to avoid unnecessary suspensions. This proposal allows async functions to opt in to this behavior.

I also think it is sometimes (maybe often) useful but am not convinced it should be the default.

I appreciate the behaviour of nonisolated is difficult to understand but I think it's compounded by the lack of sugar syntax for inheriting the current isolation.

I'm optimistic that better spelling can nicely tie these features together (like SE-0335 - Existential Any).

2 Likes

I think some really great, thought-provoking objections have been raised in this thread, which I totally love. Three things stick out to me.

Loss of Local Reasoning
There's been worry that you would no longer be able to look at a function definition and understand how it works. You cannot do this today for synchronous non-isolated functions because they inherit isolation. But I also don't see why things would change. You look at the signature and you see @concurrent or you don't. If you have this concern, can you help me understand where local information would be lost?

Cognitive Load
I think a central point of what @Alejandro_Martinez and others have brought up is, basically, it's nice to not have to think about blocking when calling non-isolated async functions. This goes hand-in-hand with local reasoning too, I think. And I agree, this is nice!

But the second you have involve state, the cognitive load goes way up today, especially if you've used non-Sendable types. It forces you to think, often quite deeply, about how you should (or even can) proceed. I'm surprised to hear others that are making use of non-isolated async functions without running into exactly these same problems.

Syntax
I think @BigZaphod's idea of modifiers on the async keyword was great, and just gets better the more I think about it. It's super-flexible! It could help with this propsoal. It could help if we opt to go a different route and just make inheritance easier. And, on top of all that, it could play well with closure isolation control. Check it out!

func runInBackground async(global) {}

func currentClosureControl(@inheritsIsolation operation: () async -> Void) {}
func newIdea(operation: () async(inherit) -> Void) {}
6 Likes

There's one issue with async(xyz) that I see (especially if this deprecates the plain async): it doesn't play well with the isolated parameter, or just doesn't feel consistent enough to me:

func doSomething(actor: isolated MyActor) async(what?)

Of course, a rule can be made that functions with an isolated parameter use the unadorned async, but this doesn't seem trivial to connect with and teach.

2 Likes

It seems to conflate the idea that the function is asynchronous (has suspension points) with its isolation. Perhaps more could be done with the actor keyword that appears to be a bit underused. The term isolation could potentially be eliminated entirely because isolation is achieved exclusively through the actor model.

2 Likes

To the extent @concurrent behaves similarly to, say, a global actor attribute, ought it be possible to mark synchronous functions as @concurrent? Such functions would be callable synchronously from within the generic executor, but would require a suspension when called from outside the generic executor.

3 Likes

No, because this requires adding a parameter to a function, which is an ABI change. It's critical for async functions to be able to preserve their current semantics without breaking ABI.

This does not solve the problem of nonisolated having different semantics on synchronous versus async functions. @isolated(any) also doesn't have any impact on the inferred isolation of a closure. It only means that you can recover the captured isolation as an (any Actor)? value.

This proposal is indeed a semantic change to the concurrency model, and I believe it's a critical enough problem to make such a change. However, every semantic change inflicts additional work on programmers, and we should not change more than we believe is truly necessary. There might be a better keyword name than nonisolated that we could have used, but I don't have any evidence that nonisolated on synchronous functions has been that difficult for people to understand that it means it can be called from anywhere, and it runs wherever its called. I've definitely seen common misconceptions that nonisolated is not safe, but in fairness, most people have been using minimal concurrency checking where there are many ways to violate data-race safety without compiler warnings. nonisolated is also used on properties to mean that the property is safe to access from any isolation domain. I'm pretty strongly against any direction that renames or removes nonisolated, or any of the other existing isolation annotations, because that means invalidating a lot of existing code that isn't actively problematic.

@Jumhyn just noted this, but the idea to use a variation of the async effect to specify isolation or the fact that a function runs on the global executor cuts off the future direction of applying this annotation to synchronous functions:

Yes.

I agree with this; there are definitely cases where switching to the generic executor helps performance, but there are also cases where it hurts performance due to unnecessary executor switching.

I'm also not convinced that the current default is helping people improve performance by offloading work to the generic executor other than by providing a convenient way to express it after discovering you explicitly need to move expensive computation off the main actor, e.g. through profiling your code.

Finally, I believe the current default optimizes for the advanced programmer who understands parallelism, which hurts progressive disclosure of the concurrency model. The philosophy I think we should follow is that the language should not impose parallelism on programmers by default; that should be a deliberate decision that is explicit in source (while still being convenient to express). There are so many programmers writing asynchronous code just because they need the ability to suspend, e.g. because they need to call some library API that suspends while it waits for other work to complete. In this case, the programmer doesn't actively want parallelism, and they don't need to be confronted with data-race safety errors because they're not trying to pass state across isolation boundaries.

This design decision was made specifically to make the behavior of Task.init consistent across nonisolated synchronous and async functions. We can't change the behavior of unstructured task isolation in nonisolated synchronous functions, because that would require changing the ABI of every unspecified synchronous function. I do not think that's worth it; that's a much larger ABI transition than the one that's currently proposed.

It's also not true that Task.init always inherits isolation in an isolated method. Isolation is only inherited if you capture the isolated parameter in the closure body. The behavior in the proposal more closely matches the existing behavior of task creation in isolated methods.

14 Likes

This would require changing the isolation of the child task created by async let, which this proposal does not do, so you'll still get an error when capturing util in the second child task. Neither async let nor group.addTask will inherit the isolation of the enclosing context. This proposal is nuanced enough, and I'd rather discuss the isolation of the async let child task separately.

4 Likes

Apologies if this is discussed somewhere in proposal (or up thread), but I wonder if there's a way to split the difference here. Would it be possible to have the 'inherit' behavior when there's non-sendable argument types, and do the actor hop when everything is Sendable? I realize this potentially makes the rules even more complex to reason about, but like RBI I think complex rules can be justified when it lets us get closer to a regime where the language can overwhelmingly 'do what I mean'.

2 Likes

Oh, I should have mentioned this in the alternatives considered because I've thought about this a lot, but I think it's fraught because 1. there aren't great answers for conditional conformances to Sendable, and 2. it's common for non-Sendable types to later become Sendable due to incremental migration amongst the ecosystem, which would cause trouble both for API evolution in resilient libraries and/or silently changing where client code is executing when conformances are added. I also still believe that it's not always true that offloading to the generic executor will improve performance, and there's no getting around programmers needing to deliberately make informed decisions about where the code they write runs when they need to leverage parallelism.

Instead of trying to implicitly offload to the generic executor, I'd rather explore avenues for providing guidance where the compiler detects code can be offloaded to the generic executor and prompts people to add the necessary code for doing that. We could also suggest other helpful concurrency-related changes, such as adding sending to the result of an actor-isolated method that returns a value in a disconnected region, adding nonisolated to public static variables that are inferred to be actor isolated but didn't need to be, etc, that will help facilitate less code needing to be serialized on an actor.

13 Likes

Thank you @hborla for providing all these detailed and insightful answers here :pray:

I find myself giddily agreeing with all of it, and I am oh so excited that this topic is tackled with such care. In Swifty Spice we trust!

A thought on this one:

When I think about "general purpose" libraries (things like the stdlib, or server-side packages) the async stuff in there is often littered with isolation: isolated (any Actor)? = #isolation, or worse, things like @_inheritActorContext or @_unsafeInheritExecutor.

With the new default as proposed here, many of these ungodly words will disappear again.
I highly doubt "advanced programmers" will miss them much.

Especially when the "price to pay" is: sometimes you need to add @concurrent.

3 Likes

Oh this sounds really interesting. One of my top concerns with this proposal (along with local reasoning ability) is the uncertainty about how often @concurrent would be used in practice. I wasn't very hopeful. But that changes if it's something the compiler can point out :eyes: