SE-0395: Observability

I noticed that this proposal looks as though it will be leveraging AsyncStreams.

One of the current limitations of AsyncStream is that you can't have multiple consumers of a single stream. This is a very common use case when using combine.

For example, let's say you have a ViewModel class that has a State property. Your view controller might want to observe changes to that state property and update appropriately. Depending on how complex the view is, it might make sense to observe that state property multiple times to split up update logic. Some parts of your view might want to debounce changes, whereas others might not. In this case, the easiest solution would be to have separate consumers of those updates.

Will anything be done to address this limitation of AsyncSequences going forward? There is a few existing issues relating to this on the async-algorithms package, but they have been stagnant for quite some time. Multi-consumption of an `AsyncSequence` Ā· Issue #110 Ā· apple/swift-async-algorithms Ā· GitHub for example

4 Likes

This is an interesting design direction to discuss in another thread, but this really seems off-topic for this review.

Just create multiple observations instead of trying to share an AsyncStream.

I agree: a per-property annotation is a requirement for me.

4 Likes

ah so you will be able to create multiple observations on a single observable? Thats good.

I disagree; if AsyncStream is an important part of this proposal’s API surface, then AsyncStream limitations become important limitations of this API as well.

I also echo very strongly the @rballard has expressed about ā€œif anything is observable, everything is observable.ā€ I almost always wind up with internal-use properties on my classes that should not be observed.

If Swift didn’t insist on conflating instance variables and properties and only properties were observable this would be more reasonable, but because Swift has this conflation it actually feels less reasonable now than it would for, say, ObjC specifying that all properties are observable.

I do quite like the way it keeps the simple case simple though. If there was a way to say @Observable(.all) to have all properties observable, or @Observable(.annotated) to have only specially-annotated properties observable, that’d be wonderful. (Pretty much regardless of the exact spelling of how that can be made to happen.)

6 Likes

There has to be a limit here. Many other APIs often form part of the surface of another proposal. If for example you had an idea about how Collection should be changed, this does not mean it's appropriate to raise it on a proposal for a new Collection algorithm, unless the issue with collection is specifically entangled with how that algorithm works – even if your thoughts on how Collection should change are insightful and important. In almost all cases, and I think in this case with the point re AsyncSequence, that isn't the case.

2 Likes

Last minute question: what happens when mixing @Observable with Codable? My supposition is it will prevent automatically generated conformances from happening. Is this handled by the macro?

Overall an impressive proposal. Reviewers thus far have made super insightful comments on need-to-have functionality, so just a question or two and a few scattered comments on more superficial API design points:


Certainly, Swift has avoided making the distinction between stored and computed properties affect end users' code, but is it actually the case that for any final class C { ... }, adding @Observable really has no ABI-breaking effect?


Agree with @tera above about the concern regarding non-observability of superclass properties without @michelf's explicit workaround. While the limitation is understandable, I wonder if there's anything that can be done (even if it requires some ad-hoc internal APIs bolted into macros) to at least detect that there is a superclass with possibly non-observed properties and to emit a silenceable diagnostic warning.


It would be irrelevant to the end user in the simplest case thanks to macros, but have you considered enriching the requirements of Observable and providing default implementations of these methods? They don't really seem to afford much leeway for manual customization, and perhaps such a design would simplify code generation routines for the macro—with accompanying binary size improvements for the end user maybe?

I can understand the impulse for making _$observationRegistrar internal as an "implementation detail" of the observed type, but it doesn't seem like there's any functionality exposed there that a user who can user the observed type shouldn't be able to access which would preclude making this a protocol requirement. Just as property wrappers expose _property and $property, it seems fine to do something like that here.


I agree with @mpangburn regarding the design of ObservationTracking.withTracking: as far as the end user can see, ObservationTracking would only be a namespace (in which case, we've standardized on the uninhabited enum as the expression of a namespace). That there is library-internal functionality which perhaps justifies its struct-ness seems an odd reason to have an externally exposed "rump" type.

Moreover, in its fullest invocation, the API is named: Observation.ObservationTracking.withTracking, a schesis onomaton fitting if straight out of the Department of Redundancy Department. Seems the API could benefit from being simply withObservationTracking, if it can't take a page out of the concurrency design with something like Observation.tracking Ć  la Task.async.

(Whether internal or public, incidentally, ObservationTracking would be more consistent with API naming guidelines for a protocol; a concrete type would be more fittingly something like ObservationTracker.)

6 Likes

If I understand the proposal correctly, then all properties (stored and computed) will be observable by default with the @Observable macro. And further if I understand this correctly:

The generated implementation for the dependencies(of:) requirement creates a list of all of the type's stored properties, and then returns that as the set of dependencies for any computed property.

Then anytime any stored property changes, then all computed properties will be affected. To me this approach seems like it will be wrong most of the time because most computed properties won't depend on all stored properties. I think I would rather see the proposal say that all stored properties are observable by default, and computed properties are observable via an opt-in mechanism. Maybe that mechanism is simply providing an implementation of dependencies(of:).

2 Likes

That would fundamentally break current SwiftUI semantics. It's unfortunate but the proposal is probably right here; in the future it will likely be possible for macros to receive a list of accessed properties from within a scope, allowing the default implementation to more accurately determine dependencies. Ultimately I think computed properties at the root level of an Observable object are going to be rare. More likely the related values are tied together through a model value of some kind that encapsulates the related values. In fact, that might be a workaround to this sort of inefficiency: ensure you model related properties with computed products as a single property. In fact, that's likely easier than having to manually implement dependencies(of:) anyway.

I understand that the review period is up (and overall thumbs up to the proposal and it really make sense as a first class thing to support in the standard library rather than a platform-constrained one), but I have a general question applicable to both this proposal but also to the pitch/proposal templates in general;

There is little discussion on performance related concerns (same questions came up with Regex was in progress, Predicates and surely many more) - it's always hard to judge what sort of performance expectations one can have from these super nice abstractions - I understand it can be hard to know exactly, but it'd be helpful I think to just have some discussion around performance and expectations of the implementation as a separate section? It could be scalability considerations/concerns (how does observability cope with many observers? what are the implications in space/time?), what are expected data rates that could be supported? Coalescing overhead? Etc, etc - I'd expect that many of these would be measured and tested during the implementation and it'd be very helpful to get an indication of expected usage envelope (is this implementation expected to mainly be useful for slower data rates updating UI on a mobile device, or is the intention to handle very high data rate server use cases?)

Basically sharing the thought process and experience there - it's easy to say that we get it to work first and fix performance later, but I think it's useful to at least touch on those things also for review?

Maybe it's just me who's interested in it, but wanted to put it out there - we'll build our own internal benchmarks of course to evaluate if/where we can use it when it drops, but maybe it'd be useful to at least share the thought process of the people proposing changes what their expectations of performance (runtime, binary size implications (especially for macro stuff...) and memory footprint at least).

Or am I barking up the wrong tree?

11 Likes

Just to keep folks in the loop; answering this very simple and honest question appropriately (beyond just the status of what the proposal was offering) ended up opening up a huge can of worms.

Ive been working with @nnnnnnnn to get some updates to the proposal - not just for subclassing but also answering some of the feedback that was posed by @gwendal.roue and many others with regards to how the AsyncSequences work. The preview of our changes is mainly to refocus to the core of how observability works in practice. Additionally we were concerned by some of the feedback that there were potential holes in the logic to move the feature forward after this initial version. As neat as iterating individual properties might be - that ended up posing more constraints on what the evolution of observation could be.

So stay tuned - we should have updates for the proposal shortly.

46 Likes

Thanks, @Philippe_Hausler, I’m looking forward to hearing more. I’m especially keen to hear about the resolution of the sync vs async issues mentioned above.

It would be great to see a suitable user facing API for achieving same actor observation. One that is suitable for synchronising UI events within the current event loop cycle and is composable between components. (i.e. chainable, and not only restricted to the top-level observer as seems to be the case with the current implementation )

1 Like

Thank you for all your input in this review. A second review has now started here.

1 Like