[Observation] Initial implementation of Observed for transactional tracked values over time

[phausler]

This is an implementation for the feature swiftlang/swift-evolution#2726

[phausler]

@swift-ci please test

[phausler]

@swift-ci please smoke test

[jamieQ]

maybe not of interest for an initial implementation, but it occurred to me that we could perhaps reduce the number of lock acquisitions by merging the tracking logic and id generation. we could maybe even even get away with dropping the separate storage field for the tracking flag too if we reserved a sentinel id value to mean 'not yet tracking'.

[phausler]

That might be possible but it would have to be refactored very carefully because it does need a unique id before the suspension due to cancellation. The other option would be to use a custom token that passes in a pointer to the current stack location... which I would sincerely hope is unique, and since it does not overstep the asynchronous function we don't have any sort of risk of it being used beyond the frame, plus there is no indirection, just identity.

That optimization is future work but perhaps something worth looking into. However the importance of which is perhaps not huge since the lock acquire should be rather un-contenteded and when it is has a VERY short execution time.

[jamieQ]

i think this also needs to get an appropriate isolated parameter, probably analogous to trackEmission(). otherwise it will presumably suspend when called from next() and that can (will?) break the iteration logic.

[phausler]

I do not believe so, that not having the isolation is actually behaving exactly as intended - else it wouldn't enqueue at the right edge of the scheduling (using the isolation here would practically speaking potentially skip an actual change).

[jamieQ]

since this is awaited from next(), which executes on the iterator's isolation, then under the current language semantics, since it is nonisolated and async, won't it generally suspend so that this runs off the calling actor (if present)? why would we want this code to run on the global executor? if the observation tracking fires before we form and register the continuation, don't we just end up in a broken state?

---

edit: upon reflection, i see my earlier comment was perhaps ambiguous & confusing regarding the suggestion to change things to be like trackEmission(), since there are 2 methods with that name. i meant that this function should probably have an isolated parameter to ensure it runs on the iterator's isolation (not the observed source isolation).

[phausler]

the current isolations and calls should be a guarantee for that; but I could see that adding that might future proof it so that if the careful balance isn't maintained in the future then it would retain the same correct behavior; that is a decent refinement

[jamieQ]

the current isolations and calls should be a guarantee for that

i tested the prototype out, and it appears they are not. if you update the prototype's implementation of next() to something like this:

    public mutating func next(isolation iterationIsolation: isolated (any Actor)? = #isolation) async throws(Failure) -> Element? {
      guard let state else { return nil }
      let id = State.generation(state)
      do {
        if State.startTracking(state) {
          return try await trackEmission(isolation: iterationIsolation, state: state, id: id)
        } else {
          // alias isolation to demonstrate the issue with actor hopping
          let isolationAlias = iterationIsolation
          await withTaskCancellationHandler {
            // N.B. this closure is currently nonisolated since it does not capture the isolated parameter
            // isolationAlias?.assertIsolated("not isolated in next()")

            // and even if we capture isolation in this closure, the call to `State.willChange()` will hop to the global executor
            // iterationIsolation?.assertIsolated("now we're isolated")
            await State.willChange(state, id: id)
          } onCancel: {
            State.cancel(state, id: id)
          }
          return try await trackEmission(isolation: iterationIsolation, state: state, id: id)
        }
      } catch {
        return try terminate(throwing: error, id: id)
      }
    }
  }

then, assuming you're iterating on an actor, if you comment out the first isolation assertion (via the alias), it will crash. if you comment out the second one, it will pass (as the isolated parameter is then directly captured via the closure), but the call to State.willChange() will then cross an isolation boundary (you can confirm by iterating from the main actor and adding an isolation assertion into that method).

the output of this example in godbolt (read & un-comment some of the commented-out parts) can be used to see the issue. the example also highlights the more general concern with how changes to tracked properties occurring between a read of the Observed closure's value and the installation of the next 'will change' continuation can cause the sequence to effectively break.

[phausler]

@swift-ci please smoke test

[phausler]

@swift-ci please smoke test

[phausler]

@swift-ci please smoke test macOS

[phausler]

@swift-ci please smoke test