21

Oh I see, I missed that from the initial writeup. Then indeed assumeIsolated may be your best friend here, thanks for clarifying Christopher.

22

I understand this is a Foundation-only PR but I wonder if SwiftUI's onReceive() can get its variant that would echo this proposal.

Currently, observing notifications in SwiftUI is based on Combine's Publisher. It's wordy and involves a concept that seems outdated in the age of structured concurrency. And of course there's no type safety.

From the user's point of view an ideal implementation would probably look like this:

someContent()
    .onReceive(EventDidOccur.self) { message, isolation in
        //
    }

Although I think isolation should be forced to MainActor in this case (the current implementation issues a run-time debug warning if it's not), so maybe the isolation argument is redundant here.

23

But as the notTheSharedActor is also of type EventActor like declared in the Message (its just a different instance so to say), why would it break the contract? I think the compiler can just check on the concrete type, which is EventActor in both occurences.

1 Like
24

Exactly. The compiler wouldn’t complain because the types match. But the instances are different, and so they are different isolated domains. So I’m still not sure how this would behave :thinking:

That’s a good point. I thought MainActor by default was a bit strange but ultimately I didn’t mind because actors are now non-sticky so any async call would jump outside which is good. But now I realise that the observing closure is non-async so that’s not that easy. That’s a pickle, as much as we want notification delivery to be sync, I think it will be concerning to have all observation code run on the main thread by default. If it was async i would be less concerned but as it stands seems like a trap.

25

Overall, I'm happy to see this being pitched. I'm encouraged to see that the requirement for a synchronous solution to observation has been recognised. Clearly, the approach taken must bridge the old world to the new and therefore it necessities a bunch of boilerplate.

However, it's striking to re-observe that we still don't have a synchronous observation mechanism that is Swift-first in its approach. I remember initial discussions where asynchronous sequences were promoted in this role, but given that an asynchronous sequence is, well, asynchronous, it seems like we've all realised that trying to make them synchronous would be beating a round peg into a square hole. This pitch seems like recognition of this.

With that in mind, if we did have a Swift-first observation mechanism, would the approach taken to bridge the new and the old be slightly different? Or, are we saying that NotificationCenter is now the officially blessed tool for one-to-many synchronous observation?

In this role, with the level of boilerplate required, I think its charm is more limited. Instead, I'd rather see a Swift-first solution, so I hope the realisation of this 'bridging' proposal doesn't delay its arrival.

Drilling down a bit:

addObserver(_:observer:)

What wasn't quite clear to me in the pitch was observation cancellation. On the one hand the pitch says:

[addObserver(_:observer:)] also return[s] a new ObservationToken, which can be used with a new removeObserver() method for faster de-registration of observers

Which suggests that the return value can be ignored, but if that was the case the method should probably be annotated with @discardableResult. And, If the return value can safely be ignored, albeit with an 'slower' de-registration, what exactly does that look like?

ObservationToken

The pitch says that observations can be ended by calling removeObserver(). Is this the only way to end observation, or can observations be removed automatically when an ObservationToken is deinited?

5 Likes
26

Ah. Yes, the compiler wouldn't see the issue with:

struct BasicMessage: NotificationCenter.Message {
    ....
    static var isolation: SomeActor { SomeActor() }
}
...
center.addObserver(BasicMessage.self) { ... }
center.post(BasicMessage())

But the two instances of SomeActor produced above would have two different isolations, which would be caught at runtime by the assumeIsolated check in addObserver mentioned above.

1 Like
27

On the topic of observation tokens, can we consider not having them?

An alternative approach could be this. Developers could define a type whose instances represent (or identify) registrations. This way, clients of NotificationCenter do not have the burden of owning a strong reference to the token but can simply unregister with an instance having the same representation (or identity). For a rough example:

struct Registration: Hashable { }

// register
NotificationCenter.default.addObserver(WillLaunchApplication.self, registration: Registration()) { message in 
    // Do something with message.runningApplication ...
}

// on deinit (or whatever)
NotificationCenter.default.removeObserver(Registration())
28

The token is an integer under the hood so no strong reference needed. It is a token w/ a fast lookup path to avoid de-registration performance bottlenecks. In larger systems this can become a problem when removing hundreds of observers, the direct token gives O(1) unregister whereas other systems is at best O(log N). Often times at deinit phases of large hierarchies (like NSView or similar systems) that becomes O(N) and O(N log N) respectively. We have found that using the token based approach gains a considerable amount of performance.

1 Like
29

Sounds interesting, but it does sound like there’s a bit more manual maintenance required than a reference based approach, i.e. remembering to create removeObserver() calls in a deinit for example. Correct?

With this in mind I would be even more interested in answers to the below:

30

if you ignore the return value the observer will be present forever. The faster part of it is that the token is a quasi index into the table of registered observers (note it is not a direct index offset but it happens to be 64 bits of structure that can look up the registration w/o searching). The other remove methods must search through registered observers.

Calling removeObserver (the new one that takes the token) is the only way to remove that observer from the center.

31

Interesting, thanks for sharing!

So we don't need to maintain a strong reference. But clients still have the burden of "owning" that value (the client remains responsible for the added complexity of managing the storage of that state).

But this ergonomic point is beside your larger performance point. Is there no way for clients sensitive to performant de-registration to provide a token of their own creation for the center to use that does not defeat the fast lookup path?

If not, okay. But since I would suspect most developers don't have those kinds of performance problems, burdening clients with the responsibility of managing the token state is mildly tedious and makes it harder to implement, say, a value type with the responsibility of observing the notifications e.g.:

struct ContentView: View {
    
    @State private var date = Date()
    
    private struct NotificationToken: Hashable { }
    
    var body: some View {
        Text("\(date.formatted())")
            .onAppear(perform: onAppear)
            .onDisappear(perform: onDisappear)
    }
    
    func onAppear() {
        NotificationCenter.default.addObserver(UIApplication.didBecomeActiveNotification.self, registration: NotificationToken()) { 
            self.date = Date()
        }
    }
    
    func onDisappear() {
        NotificationCenter.default.removeObserver(NotificationToken())
    }
}
1 Like
32

In any case, thanks for indulging me, I appreciate the time and effort doing so.

33

Ahhh, I didn't see this. I think that's the answer to my question. No need to reply.

34

Got it. I do wonder if there's some additional affordance/s that could be provided to make this cleanup a little more ergonomic.

Maybe a removeObservers(_ observers: Set<ObservationToken>) to avoid a for...in in the deinit.

I'd also be tempted to bring in Cancellables to Foundation and create a store(in:) method on ObservationToken that could wrap the token in a Cancellable for when the dealloc performance hit isn't a concern.

35

That just pushes the for...in lower, it will still be there.

FWIW that was considered early on, but Swift Concurrency really took an opposing stance to that design (see Task for details).

36

If is is a common pattern in your code, it will be easy enough to write a .onObserving() view modifier that takes care of this, removing the need to store all the tokens in the parent View.

37

I understand that. Purely a convenience thing for ergonomics. I can imagine this being used quite widely short of alternatives, so the deinit for...in dance will get repetitive.

Even in the context of a synchronous, and I expect non-Sendable, utility as a Cancellable type would be in this situation? Is that because of the additional atomics/ARC traffic related to exploiting the deinit?

38

I know its off topic but would really love this "isolation" option on Combine :)

39

Interesting API design. I didn't "believe" it at first, and had to go try it out to convince myself that it actually did what it purports to. Is this actually the way Swift wants to support metaprogramming across global actors? 'cos it feels a bit hacky, at odds with the @GlobalActor syntax (which I guess can now be conceived of as adding a hidden _ isolation: isolated GlobalActor parameter)