Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New observer abstraction for future backpressure support and decluttered stack traces #799

Merged
merged 3 commits into from
Jan 1, 2021

Conversation

andersio
Copy link
Member

@andersio andersio commented Aug 16, 2020

Preface

A Observer<Value, Error> abstract base class is introduced. It will replace Signal<Value, Error>.Observer as the "currency type" for side effects to be registered with RAS event stream types.

There are four reasons behind the persuation of a new abstraction:

Lay the ground work for backpressure support

Backpressure support requires every operator to have extra state for demand management, and a new "subscribed/started" message. This means at the very least:

  • The current Signal.Observer abstraction, if we stick with it, has to be altered.
  • The signatures of Signal.Event monads have to be changed (e.g. Signal.Event.map) — At the barebone level of closure-as-event-sink (Signal.Event) -> Void, these cannot be extended futher to support the new requirements.

Decluttered stack traces

Nominal operator types incidentially improves also the RAS developer experience — symbols in stack traces now clearly attaches to a nominal Observer type, e.g. Operators.Map, while many compiler inserted reabstraction thunks are now gone with the design change.

Before After
image image

Compiler optimization friendliness to enable untapped performance

The current Signal.Observer forces all values to be boxed by the Signal.Event enum, which is a strict representation of the event grammar in Swift.

While this has served us well for almost half a decade, the enum boxing/unboxing by nature requires copying (especially with tag being embedded in unused bits or "extra inhabitants"). This means the prevalent use of Signal.Event is unfortunately working against some important Swift compiler optimizations, like passing immutable Large Value argument by reference.

Reduce ARC traffic

Some operators like throttle and debounce require states that persist across events. The current design forces them to be declared externally to the (Signal.Event) -> Void event sink, and then be captured by the closure context of the event sink. So ARC traffic has to be incurred on both the closure context and each captured references.

On a related note on Signal.Event, the enum boxing/unboxing can incur defensive ARC traffic, since it is a multi-payload enum potentially packing references.

Changes

  • The new Observer<Value, Error> base class.

    Values are now delivered to Observer.receive(_:), while termination events goes to Observer.terminate(_:). The separation allows large values to be optimized as passing-by-reference by the compiler.

  • Signal.Event.Transformation has an altered signature that is now based on Observer<Value, Error>, instead of (Signal.Event) -> Void.

  • A set of operator implementation has been converted as a pilot to nonimal types under the Operators namespace.

    The pilot includes Operators.Map, Operator.Filter and Operator.CompactMap.

  • Signal.Observer is now a subclass of Observer<Value, Error> as a transitional measure.

Out of scope, subsequent work

  • The rest of the operators should be converted in batches to nominal types under the Operators namespace.
  • Lower SignalProducer to accept Observer<U, E> instead of Signal.Observer<U, E>.

Uncertainty

Signal.Observer is made a subtype of Observer to enable gradual transition in the codebase for the time being. But we might end up designing backpressure support in a way that prefers otherwise, because Signal as a hot event stream cannot and will not support backpressure from any demand-limiting observer.

Checklist

  • Updated CHANGELOG.md.

@andersio andersio marked this pull request as ready for review August 16, 2020 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant