[Pitch] Transactional observation of values

I am quite emphatically arguing that we should have no such 'terminate upon nil' behavior for this initializer. :grin:

8 Likes

There are two choices as a consequence to that; either a) the closure cannot return the element and we have to replicate the behavior of optional in a lesser featured type, or b) Observed AsyncSequences will just hang around forever and never terminate.

Neither of those options really seem reasonable to me; the language is missing a mechanism to avoid the flattening of nil.

There is potentially another option where that behavior is somehow configurable. But I fear there is no real way to express that appropriately and satisfy all constraints.

1 Like

If both of these are viewed as 'hard' constraints then IMO the best proposed alternative from @j-f1 up-thread was the one which had the weakification folded into the Observed initializer itself:

Observed(weak: person) { $0.maybeName }

The objection that there might be nested weak references as part of the observed expression is not all that compelling to me. While I think it's clear that terminating the sequence above is 'correct' when person gets deinitialized (we can know that there will be no more values produced by this object), it's not at all obvious to me that this is the case for something like:

Observed { obj.someWeakRef?.property }

Even after someWeakRef gets deinitialized, something else with a reference to obj could put it back! It's not a strong signal that the value stream is done.

I think forcing clients to declare up front 'these are the objects upon which observation lifetime depends' is quite a reasonable compromise. I really don't see a good way to intuit it from the totally abstracted value stream itselfβ€”nil alone is not a good enough signal, IMO.

7 Likes

Can you elaborate on this? Would like to understand better.

  • Is there something unique to Observed in this regard? I understood infinite AsyncSequences to be common for observation today, e.g. notifications(named:object:)
  • Does deallocation of the Observed instance itself result in termination?

That is a non-starter, the weak reference may buried deep within the graph of calls and may be more than one; what about if the access is person.friends.first?.parent?.name etc.

Here is what I would think is a way of approaching this; but my view is that the implications of which are not nearly as ergonomic.

public struct Observed<Element: Sendable, Failure: Error>: AsyncSequence, Sendable {
  public enum Emission: Sendable {
    case next(Element)
    case finish
  }
   public init(
      @_inheritActorContext _ emit: @escaping @isolated(any) @Sendable () throws(Failure) -> Emission
  )
}

That would mean that a simple accessor or for that matter complex accessor would need to add boilerplate to usage to specify the termination intent.

  let names = Observed<String?, Never> {
    if let friendName = person.friends.first?.name {
      return .next(friendName)
    } else {
      return .next(nil)
    }
  }

Granted in that case it is pretty small it curtails some of the existing behaviors of optional and forces a new type to be used in its place. This hampers also the type inference - hence why I had to write it out as the full signature of the Observed<String?, Never> (this is because the type inference picks up on the .next(friendName) as inferring String as the Element first.

Not in particular for the AsyncSequence part, but instead it is due to the weak reference of an @Observable type - if the composition of the closure may never change in the future if we don't have some sort of terminal case it may just hang around forever without ever resuming forcing an explicit cancellation of the containing task (which still works).

The structural definition of the Obseved type is more so a recipe for how to observe things, so that isn't the active component. Instead it is the iterator, that should never deallocate while iterating it so the tldr of that is no-ish. And that is on purpose since this type by its nature is observing the willSet's of the closure so it is intrinsically a shared operation; unlike notifications(name:object:).

2 Likes

In the pitch as currently written, would this concern apply for observation of any non-Optional property?

let names = Observed { person.name }

and how about for a closure which does capture weakly, but wants to offer a fallback rather than terminate?

let names = Observed { [weak person] in
    person?.name ?? "other"
}
1 Like

It's much less clear to me that the right way to respond to a deep weak reference going away is to tear down the entire sequence. It strikes me as a fairly bold assumption that arbitrary weak references in the access graph will become nil once and never be replaced.

E.g., why shouldn't a Person whose parent has been deinitialized be able to get assigned a new parent? For the purpose of our... firstFriendsParentsName sequence it certainly doesn't seem obvious to me that we care about the identity of the specific parent.

More concretely, perhaps consider setting up Observed { person.parent?.name } for the purpose of tying to a "Parent's Name" label on a contact card for person. If the original parent goes away and is later replaced it seems like we would definitely want the value to propagate thru this sequence, but the proposed semantics would have the sequence terminate!

5 Likes

digressing a bit from the termination/optionality questions, i wanted to re-highlight some issues with the offered initial implementation. specifically:

  1. if the observed sequence hasn't yet changed, only one iterator will see the initial value (noted here)
  2. if any of the Observable dependencies in the closure change in between a read of the closure and an iterator's next suspension awaiting a subsequent 'will set' trigger, the sequence functionally breaks as it no longer produces any new values (pointed out here)

are these 'just' bugs, or do they impinge upon the design considerations in a more meaningful way?

and, a few other thoughts/questions inspired by the implementation PR:

  1. currently the behavior to produce new values uses a 'pull' model, in the sense that when a 'will set' event occurs, each iterator effectively performs a fetch of a new value from the 'source' isolation. thus, N pending iterators means N invocations of the Observed closure. this raises both performance questions, and questions about whether the stated aim of the proposal that 'multiple iterations of the same sequence from multiple tasks will emit the same values at the same iteration points' is actually achievable with this model. i.e. if multiple iterators must invoke the closure separately, and additional changes to the input values could interleave between them, they will not necessarily 'see' the same value when they actually perform the read.
  2. if the closure and iterator are both non-isolated[1], it seems likely to undermine the type's goal of providing 'atomic' views into the derived sequence of values. in particular it seems like you essentially lose the 'enqueue on the source isolation' strategy of defending against 'tearing'. how are developers to deal with this? wrap the entire closure invocation in a lock? is supporting this use case important/valuable?
  3. what if the closure has some conditional control flow in which no Observable value is read? e.g.
    var nonObservableCondition = true
    Observed { 
        if nonObservableCondition {
          return "sequence over?"
        } else {
          return person.name
        }
    }
    
    today it seems like the sequence will just stop producing values, but not actually end. can such cases be detected and surfaced to developers?

  1. which i think implies the Observable stuff in the closure must (or at least should) be Sendable β†©οΈŽ

1 Like

Thank you Philippe for the reply. I think your option (a) is the right direction, and it not at all suffers from the downsides you suggest; let me illustrate what I mean by some counterexamples to yours!

Firstly:

Let me stop here. Yes, this is quite right the API signature I and, I think, Nickolas and Jumhyn were wishing for. It differs from your originally pitched proposal with the only major difference that the emit closure body needs to explicitly wrap its result in the .next case.

I'm sorry, but that is just not true. Given that person.friends.first?.name has type String?, all of the following forms type check alike and have the same type of Observed<String?, Never>:

let names1 = Observed<String?, Never> {
  .next(person.friends.first?.name
}
let names2 = Observed<String?, _> {
  .next(person.friends.first?.name
}
let names3 = Observed {
  .next(person.friends.first?.name
}

The only "boilerplate" here compared to the originally pitched nil-terminating API is having to explicitly spell out .next(_) around the next emission. And that's the whole point!


When it comes to Observed { .finished }, yes, that can't possibly type check alone because there is no context for .finished. It's the same with Observed { nil } in the originally pitched API though. Here are some options that do type check:

let a = Observed<Foo, _> { .finish }
let a = Observed<Foo, Never> { .finish }
let c: Observed<Foo, _> = Observed { .finish }
let d = Observed { .finish as Observed<Foo, _>.Emission.finish }

Let's consider another simple example from Nickolas' original motivation. The reason we want termination to be expressed with a case of a new Emission type is so that remote changes can't change the temporal behaviour of an Observed expression without the type checker failing. With the nil-terminating API, we might have:

let names = Observed {
  suspect.name
}
Task {
  for name in names {
    print(name)
  }
}

If a refactoring allowed suspect.name to have the type of String?, this API would still treat names as Observed<String, Never> and just silently stop printing when emitting nil.

OTOH, with the Emission variant, we would have:

let names = Observed {
  .next(suspect.name)
}

The refactoring would just turn names from Observed<String, Never> to Observed<String?, Never>, allowing us to deal with the diagnostic where names is used, namely at print(name):

Expression implicitly coerced from 'String?' to 'Any'


If we wanted, we could also introduce the @_disfavoredOverload variant returning just Element, allowing omitting the .next wrapping when finishing early isn't wanted:

extension Observed {
  @_disfavoredOverload
  public init(
    @_inheritActorContext _ emit: @escaping @isolated(any) @Sendable () throws(Failure) -> Element
  ) {
    self.init { () throws(Failure) in  // Edit: doesn't quite compile, can't forward `@isolated(any) emit` like this.
      .next(try emit())
    }
  }
}

I don't see any immediate problems with that additional convenience, other than I think it doesn't gain much.

3 Likes

This is news to me. Is this documented somewhere?

That particular case is an edge case for SwiftUI, generally speaking you don't really need to do that.

Indeed, types are not enough to resolve overloads. Then let's use names to disambiguate:

// 1
Observed { person.maybeName }

// 2
Observed.emitting { [weak person] in
    guard let person else { return .finish }
    return .next(person.maybeName)
}

struct Observed<Element: Sendable, Failure: Error> {
    enum Emission {
        case next(Element)
        case finish
    }
 
    // 1
    public init(_ emit: @isolated(any) @Sendable @escaping () throws(Failure) -> Element) {
        self.init(elementType: Element.self) { .next(emit()) }
    }

    // 2
    public static func emitting(_ emit: @isolated(any) @Sendable @escaping () throws(Failure) -> Emission) -> Self {
        return Self(elementType: Element.self, emit: emit)
    }

    internal init(elementType dummy: Element.Type, emit: @isolated(any) @Sendable @escaping () throws(Failure) -> Emission { ... }
}

I didn't actually try to compile it, maybe public/internal is sufficient and elementType is not needed. @_disfavoredOverload could be used too. In any case, this is an implementation detail.

1 Like

Out of everything proposed so far; this seems like the nicest syntactically for all use cases. I am not opposed to this as a solution if folks feel this is a decent way to express it. It keeps the syntax still pretty tight for the common use cases and handles the more advanced use case with a more advanced syntax. Not perfect but probably as close as we can get.

I don't wanna look like we are loosing this feedback, to address some of the issues here:

Neither of those issues are going to change the design meaningfully, but the one caveat is that some of the initial behavior may be un-avoidable across concurrency domains; e.g. if you are producing at one rate and consuming on another without a shared isolation between the two. I think the second one is addressable but the first has some consequence that may be inescapable (but thankfully is limited to only particular cases).

Async/await by nature is pull. N fetches is expected given N uses. Buffering or other such mechanisms would be the performance onus of the users of Observed.

the @isolated(any) in conjunction with the closure being marked as @Sendable are doing the heavy lifting there - if the context is non-isolated then the captures MUST be Sendable by rules of concurrency. The expected behavior for making any type Sendable is to ensure that accesses to properties and such don't tear in the first place - e.g. if Person was needed to be Sendable then it would require the developer writing that (not just for Observed usage but ALL Sendable consumption) that the type does not tear across properties and those would need to be mutated in concert just like any other type.

That is pretty much a case of bad input yielding bad results. This is the case today with SwiftUI. If you don't provide a mechanism to surface the observation then it cannot just magically detect any mutation. In that case it would just return the same thing until the property is changed AND some observable thing gets triggered. In your case we could emit a diagnostic saying no observable thing was accessed in the scope of the closure.... but that is future enhancement that can be applied later (and is more generally an Observation feature not per se an Observed feature

So I updated the proposal to add a static method similar to @Nickolas_Pohilets and others have suggested.

The tl;dr is that the initializer will no longer be optional for the element (meaning that construction creates a sequence that does not terminate implicitly on nil) and a new static method is added called untilFinished along with a new enumeration to indicate finishing.

That looks like in practice like this:

let hosts = Observed.untilFinished { [weak person] in
  if let person {
    .next(person.homePage?.host)
  } else {
    .finished
  }
}
18 Likes

thanks for the responses – some additional thoughts:

sorry i'm having trouble following what you're referring to at various points in this comment. what is the 'initial behavior' that may be unavoidable in some circumstances? and what is the potentially 'inescapable consequence'?

i see what you're saying in a sense, but in the current implementation, each iterator waits for their 'will set' continuation to be resumed, and when it is, they each invoke the closure independently. this seems like it will inherently leave room for race conditions between multiple iterators and subsequent changes to the data source – and this seems possible even if everything involved is taking place on the same actor, because there's a potential dynamic suspension when invoking the closure. maybe that is fine, but if it is the expected behavior, then the implication that multiple iterators will see the same sequence of values seems somewhat misleading.

i haven't thought this through entirely, so not sure it would be free of undesirable tradeoffs, but you can imagine an implementation sketch where we could have a single (internal) entity responsible for the 'read loop' to fetch the closure values and listen for changes from the source. then the iterators would wait to be resumed from that type with an actual value, rather than having to produce it independently.

that's a reasonable position, but the current failure mode is pretty problematic IMO, since all iterators end up never again responding to any future changes in the data source, and there's essentially no feedback as to what's gone wrong. it sounds like you're implying this behavior is unexpected though (or will be addressed in some manner) – is that right? ideally, if such an important expectation is violated at runtime, clients should be clearly informed somehow rather than having things silently stop functioning.

If you can provide an example that does that it may indicate a hole in the isolations; even for example holes in the other APIs used. It is worth noting that if there are multiple isolations involved then it is expected the types must be Sendable. Furthermore the tracking itself is always executed by virtue of the @isolated(any) on the same isolation as the original fetch - this means that the original isolation must be in a suspended state; any mutations inside that suspension must be either already finished (on non sendable types) or done safely (for sendable types). This means that upon suspension the values will always aim to consistency - the only cases where it may not be is where there are producer/consumer issues; which still eventually reaches a same-value.

Yes; if a withObservationTracking is invoked and nothing is accessed inside that is observable that could be contrived as a potential failure. However that is something that should be investigated on how to handle on its own - this API would be a consumer of a strict interpretation of that. SwiftUI might be a bit permissive about it. I think it is a valid area of investigation/improvement to avoid missteps, but is only related and not per-se blocking. I do however agree that this proposal increases the importance of that as a focus for future enhancements/fixes.

Just for the avoidance of confusion, this pitch has been brought to review here: SE-0475: Transactional Observation of Values

Further conversation should continue in the review thread!