SE-0392: Custom Actor Executors

Yes, that's one of the use cases. Dispatch queues, Swift NIO, or "specific thread" are the examples that come to mind as immediate use-cases.

There is more work to be done that is outside the governance of the Swift evolution to make dispatch queues efficiently implement serial executors, but it definitely is one of the reasons to standardize custom executors to begin.

You can also see the "complex" equality part of the proposal is specifically tailored for dispatch queues in order for queues targeting the same "root queue" to be able to return "yes, we're on the same context".

5 Likes

For some reason, I didn't even consider the option of creating a PR. I will do that next time.

3 Likes

Yes I agree with that and I understand its purpose. My issue was with the naming of the function that ends with "executor" but it accepts an "actor" type. Reading it again assertOnActorExecutor I guess it makes sense if I read it as "on the actor**'S** executor" and treat the parameter as the subject.

Makes sense, I was just confused on first read.

There you go, I missed those docs. Thanks for clarifying <3

3 Likes

+1 From me. I've been using custom executors for a while and they have proved to be useful. Seeing as the executor protocols are already back ported it would be ideal if an official way of running jobs was back ported. Without this it would create an awkward situation where we would have to add availability annotations to types that are "technically" available on older platforms but cannot actually be used by actors.

2 Likes

The “run” function on UnownedJob we can backdeploy easily I think (since it calls into existing runtime function), agreed.

Some of the other things like the new job type are not as simple but will give it a look cause by case.

3 Likes

Something I think the proposal should clarify is how custom executors for a distributed actor will work. In particular, for this new protocol requirement:

public protocol DistributedActor: AnyActor {
  /// ...
  nonisolated var unownedExecutor: UnownedSerialExecutor { get }
}

what happens when the instance is remote? There isn't really an executor for the "proxy" instances. Since it's nonisolated, we can't make a call to the remote instance to ask about the executor. And even if we could, these executors aren't really Codable.

One idea is to make the type of unownedExecutor for a distributed actor return an optional. You'd get back nil only in the remote case. A downside is that I forsee this optionality being abused as a non-async form of whenLocal to query whether the instance is local.

2 Likes

An anonymous contributor has a naming suggestion here: It seems like the actor portion of the function name could be downplayed by relying on the name of the passed-in value, since assertOnExecutor(of: MainActor.shared) reads rather naturally.

15 Likes

That's a great naming idea, thank you anonymous :slight_smile:

I'll follow up on Kavon's excellent question today, sorry for the delay here, was wrangling with this exact implementation bit just now the last days to form a better opinion on it. :pray:

3 Likes

Sorry for the slow reply here @kavon, and thanks for raising a good topic to discuss here!

I was actively looking into related things for the last few days, so wanted to focus there and be able to provide a good discussion here, so let's dive in.

So in normal usage the executor would never be used in practice, because:

  • calls to distributed func on remote actors are effectively calls to nonisolated thunks that notice the reference is remote, and instead of calling the distributed func hello() on the distributed actor, redirect the call to an invocation to its ActorSystem's remoteCall.

So in that sense, an executor never "matters" for remote distributed function calls -- we never use it, because the code-path for remote actors is always nonisolated for all distributed calls. And non-distributed calls are not allowed externally. If we are "inside" a distributed actor (i.e. have an isolated DistributedActor), we are guaranteed that it is local, so all the usual Actor semantics about the executor hold.

Now, my assumption when proposing this API shape was that there's two ways to implement an unownedExecutor for a distributed actor:

  1. as reference to some global shared executor
    • nonisolated var unownedExecutor: UnownedSerialExecutor { MainActor.sharedUnownedExecutor }
  2. the same way we configure other things about distributed actors, by carrying the information in the ID which is the only stored property the user has a lot of control over (though as usual it depends how much control the specific actor system allows you here),
    • we do this a lot actually in practice, so if this seems weird I assure you it's not as weird as it seems :sweat_smile:
    • this boils down to having an ActorID have e.g. id.preferredExecutor: UnownedExecutor? that you'd use this way to implement the ID (The ActorID can have all kinds of "preferences", and I do think this is a good pattern to stick to, like "which region to prefer" for lookups etc. etc.):
    • I (wrongly! so thank you for reminding me to look into this deeper) assumed we'll then implement the unownedExecutor as id.preferredExecutor ?? buildDefaultActorExecutorRef(self)
    • I was wrong here though, because we don't expose the Builtin.buildDefaultActorExecutorRef(self) and probably shouldn't :exclamation:

So yes, I agree with your proposal to change the user-facing part here to ? and at the same time I don't think we should make the exposed interface and how distributed actors interact with the rest of the compiler different... So I think we can achieve both goals bo doing the following: make the user-facing API more explicit that "this only matters for local actor" but also keep the synthesized unownedExecutor the same way as default actors to it today:

public protocol DistributedActor: AnyActor {
  nonisolated var localUnownedExecutor: UnownedSerialExecutor? { get }
}

distributed actor Worker {
  nonisolated var localUnownedExecutor: UnownedSerialExecutor? { ... }
  
  // KEEP the synthesized implementation, but make it do this:
  // nonisolated var localUnownedExecutor: UnownedSerialExecutor { // do not mark as default actor though
  //   localUnownedExecutor ?? Builtin.buildDefaultActorExecutorRef(self)
  // }
}

This way we don't have to special case a lot of places in the compiler but keep the deciding "is it a default actor" as well as accessess by the compiler when it wants to make hop_to_executor still the same as they are today. I think this way we can nicely keep the "devirtualize" logic for existing default distributed actors as well... :thinking:

I'm going through implementing / validating this idea though in order to make sure all the layout questions work out if we did this. Maybe hanging the hop_to_executor logic won't be too bad... I'll give this a shot too, though I would like to not have to change all places that touch the unownedExecutor but just keep them the same code-paths basically. As far as assert/precondition/assume APIs go, this should be working out fine still, but I'll also verify.

Thanks @kavon for mentioning this bit!

Looking deeper into it I think maybe the ?? default is not a great idea; yes, it allows us to always give an answer, but then again, what would that answer be useful for.

So perhaps changing the synthesized impl to do if __isLocalActor(self) { Builtin.default... } else { nil } might be simpler and more consitent to understand.

However, developers may implement this function however they like, even returning an executor for a remote reference if they wanted to. It won't cause "trouble" because we never use it for anything, nor is there ANY API that would allow abusing this - i.e. the assumeOnLocalExecutor(of: someDistributedActor) also already prevents abuse here. So it is a weird hack to check "is remote" on a default actor, but should not be relied on...

We would have to require that returning nil for a local distributed actor as its executor be illegal; which my previous idea did work around because it just meant "yeah, still default" but that just muddied the waters. Perhaps indeed, let's have it clean cut here and just say returning nil when local is illegal.

I'll investigate this angle as well, thanks again @kavon -- in general I agree we need the ? here, but just figuring out what exactly that means for the implementation.

Excited to see what comes of this in the NIO space specifically. I don’t have anything of substance to say about the specific design. I try to avoid commenting on naming usually, especially for APIs I won’t use directly. But….

I feel that “Job” is an unfortunate name for what is expected to be a very niche API in the standard library. My understanding from this proposal is that Job is expected to be used only by implementations of custom executors and that relatively few people/libraries are expected to write their own custom executors.

Two concrete concerns:

  1. The name sounds as high-level as Task, leading to wasted time and confusion when learning about concurrency. This might be avoided by careful documentation.
  2. The name is widely used in programs. It’s true that declarations in the current module will shadow those in the standard library, but I think it’s generally understood to be bad style to make your own thing which shadows the standard library especially when that thing is totally different from the standard library’s version (e.g. Result/Result shadowing is OK-ish, but making your own Task that has nothing to do with concurrency is probably a bad idea).

To expand on that second point: I have several server-side Swift programs which act as “workers” — they pull a job from a queue, do the work, and output the results somewhere. In those worker programs, the very natural declaration is:

struct Job: Codable {
   var id: String
   var customJobParameter: Int
   // etc
}

I think that having a Job in the standard library makes code more confusing to a new programmer in my codebase. This would be fine if Job were widely used like Task, but I think if it’s meant to be niche, the name should be somewhat niche as well.

I’d recommend ExecutorJob, but any old qualifier would do IMO.

6 Likes

You can track our experiments here.

9 Likes

Could we make the assertOnExecutor function a method, e.g. in a protocol extension of Actor? That would have the clear advantage that we could make it a static method on global actors, so that you could just write something like:

MainActor.assertOnExecutor()

or

MainActor.assertIsolated()

instead of

assertOnExecutor(of: MainActor.shared)

Also, I think I really like the assertIsolated name.

6 Likes

Yeah that would work. Non isolated functions like that should work fine. We’d likely want to keep the executor versions tho, move them onto SerialExecutor then?

Sure. They could even have the same names.

1 Like

I guess that makes sense. It is actually right to say a serial executor "isolates" some work after all, huh. Thanks for the input!

I'll go ahead and prepare those renames/moves, seems like a good one to do, while we wait for complete review summary of this proposal :slight_smile:

4 Likes

Does it tho? I’ve always thought about isolation in terms of having access to shared state which applies to actors but not to executors? I’m afraid that using isolation to talk about executors muddles the waters and makes people think about threads and queues again.

Serial executors have a meaningful concept of isolation just from the inability to run multiple things at once, which is why they can be used to implement actors. I suppose the word "isolated" makes more sense if you're talking about isolating data, but it's not like we refuse to allow actors to have no stored properties because it would make "isolated" meaningless.

2 Likes

I guess I should review this given that I've been using the functionality added in this proposal for a while and find it useful and important.

We have some places that need assumeOnActorExecutor(), and our current implementation is along the lines of the following:

private extension Actor {
    func check() -> (@Sendable () -> Void) {
        let fn: () -> Void = { _ = self }
        return unsafeBitCast(fn, to: (@Sendable () -> Void).self)
    }
}

private struct ActorExecutorHelper<A: Actor> {
    let actor: A
    let check: @Sendable () -> Void
    init(_ actor: A) async {
        self.actor = actor
        self.check = await actor.checker()
    }

    func assumeOnActorExeuctor(_ operation: (isolated A) throws -> T) rethrows -> T {
        check()
        try withoutActuallyEscaping(operation) { fn in
            try unsafeBitCast(fn, to: ((A) throws -> T).self)(actor)
        }
    }
}

This is obviously gross code that should not be encouraged, and the precondition check relies on -enable-actor-data-race-checks and so doesn't work with SPM.

I like the actor.assertIsolated() name better than the one originally proposed.

I've found custom executors to be basically mandatory for testing code using swift concurrency. There's just no other way to test all of the various sequencings of tasks. Currently doing this requires doing naughty things because not enough is publicly exposed, so it'll be good to have a public version. I don't really have any comments on the executor API beyond that it works for the things I've tried to do so far.


The bits which aren't "this proposal is exactly the thing I want":

Ideally we'd like try await Realm() to produce a Realm instance confined to the current actor or executor similar to how try Realm() produces one confined to the current thread. I think with this proposal this is possible†, but awkward and (probably) suboptimal. I'm under the impression that not providing a way to get the current actor or executor is an unstated explicit design goal, but having it be possible but require jumping through hoops is a bad place to be.

This is probably just not related to custom executors at all, but the major awkward bit I keep running into is wanting to take a non-Sendable argument to an async function, and then copy it or convert it to a Sendable form before the actual async part. Currently this requires @_unsafeInheritExecutor, so, well, that's getting slathered all over the place.

† The rough idea I have is to provide a custom executor which runs jobs on a dispatch queue and stashes a reference to the actor in the dispatch-local state. This would be somewhat awkward for users to use (they'd have to explicitly make their actors use this executor) and presumably would perform worse than the built-in executors.

1 Like

Thanks for the feedback, everyone! The Language Workgroup has decided to return this proposal for some small revisions.

3 Likes