Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
# master
*Please add new entries at the top.*

1. Addressed the exceptionally high build time. (#495)
1. The `SignalProducer` internals have undergone a significant refactoring, which bootstraps the effort to reduce the overhead of constant producers and producer compositions. (#487, kudos to @andersio)

1. New method ``retry(upTo:interval:on:)``. This delays retrying on failure by `interval` until hitting the `upTo` limitation.

1. Addressed the exceptionally high build time. (#495)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is duplicated in the 2.0.1 notes. 🙈


# 2.0.0
# 2.0.0-rc.3
1. `Lifetime.+=` which ties a `Disposable` to a `Lifetime`, is now part of the public API and is no longer deprecated.
Expand Down
7 changes: 7 additions & 0 deletions Sources/Disposable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ extension UnsafeAtomicState where State == DisposableState {
}
}

internal final class NopDisposable: Disposable {
static let shared = NopDisposable()
var isDisposed = false
func dispose() {}
private init() {}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks the contract of Disposable by not changing isDisposed when you call dispose. It may be fine in practice, but this makes me very wary.

I think we'd be better off bringing back the implementation of SimpleDisposable and using that instead.

Copy link
Member Author

@andersio andersio Aug 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we treat it as disposed then? It still makes sense for its use case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not bring back SimpleDisposable? This still strikes me as a little weird.

Copy link
Member Author

@andersio andersio Aug 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a shared disposable for constant producers that practically would never be disposed of, because they terminate before the disposable is returned.

When I iterate on the design, benchmark shows allocation contributes considerable overhead over NopDisposable.shared (which uses swift_once). I could check again to see if the numbers are still high though.


/// A type-erased disposable that forwards operations to an underlying disposable.
public final class AnyDisposable: Disposable {
private final class ActionDisposable: Disposable {
Expand Down
86 changes: 86 additions & 0 deletions Sources/Event.swift
Original file line number Diff line number Diff line change
Expand Up @@ -178,3 +178,89 @@ extension Signal.Event: EventProtocol {
return self
}
}

extension Signal.Event {
internal static func filter(_ isIncluded: @escaping (Value) -> Bool) -> (@escaping Signal<Value, Error>.Observer.Action) -> (Signal<Value, Error>.Event) -> Void {
return { action in
return { event in
switch event {
case let .value(value):
if isIncluded(value) {
action(.value(value))
}

case .completed:
action(.completed)

case let .failed(error):
action(.failed(error))

case .interrupted:
action(.interrupted)
}
}
}
}

internal static func filterMap<U>(_ transform: @escaping (Value) -> U?) -> (@escaping Signal<U, Error>.Observer.Action) -> (Signal<Value, Error>.Event) -> Void {
return { action in
return { event in
switch event {
case let .value(value):
if let newValue = transform(value) {
action(.value(newValue))
}

case .completed:
action(.completed)

case let .failed(error):
action(.failed(error))

case .interrupted:
action(.interrupted)
}
}
}
}

internal static func map<U>(_ transform: @escaping (Value) -> U) -> (@escaping Signal<U, Error>.Observer.Action) -> (Signal<Value, Error>.Event) -> Void {
return { action in
return { event in
switch event {
case let .value(value):
action(.value(transform(value)))

case .completed:
action(.completed)

case let .failed(error):
action(.failed(error))

case .interrupted:
action(.interrupted)
}
}
}
}

internal static func mapError<E>(_ transform: @escaping (Error) -> E) -> (@escaping Signal<Value, E>.Observer.Action) -> (Signal<Value, Error>.Event) -> Void {
return { action in
return { event in
switch event {
case let .value(value):
action(.value(value))

case .completed:
action(.completed)

case let .failed(error):
action(.failed(transform(error)))

case .interrupted:
action(.interrupted)
}
}
}
}
}
20 changes: 20 additions & 0 deletions Sources/Observer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,24 @@ extension Signal {
/// Whether the observer should send an `interrupted` event as it deinitializes.
private let interruptsOnDeinit: Bool

/// The target observer of `self`.
private let wrapped: AnyObject?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love adding more to Observer. 😕

Copy link
Member Author

@andersio andersio Aug 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly, we have to retain observers marked with interruptsOnDeinit, and retaining it using a thunk adds an overhead to all calls to the observer, as opposed to an 8 byte storage for a reference. :|

// @testEventTransformingCoreMapFilter(): avg 6091 ns; min 5250 ns
self.action = transform(observer.action)
self.wrapped = observer

// @testEventTransformingCoreMapFilter(): avg 6238 ns; min 5402 ns
self.action = transform { observer.action($0) }


/// An initializer that transforms the action of the given observer with the
/// given transform.
///
/// If the given observer would perform side effect on deinitialization, the
/// created observer would retain it.
///
/// - parameters:
/// - observer: The observer to transform.
/// - transform: The transform.
internal init<U, E: Swift.Error>(_ observer: Signal<U, E>.Observer, _ transform: @escaping (@escaping Signal<U, E>.Observer.Action) -> Action) {
self.action = transform(observer.action)
self.wrapped = observer.interruptsOnDeinit ? observer : nil
self.interruptsOnDeinit = false
}

/// An initializer that accepts a closure accepting an event for the
/// observer.
///
Expand All @@ -27,6 +45,7 @@ extension Signal {
/// event as it deinitializes. `false` otherwise.
internal init(action: @escaping Action, interruptsOnDeinit: Bool) {
self.action = action
self.wrapped = nil
self.interruptsOnDeinit = interruptsOnDeinit
}

Expand All @@ -37,6 +56,7 @@ extension Signal {
/// - action: A closure to lift over received event.
public init(_ action: @escaping Action) {
self.action = action
self.wrapped = nil
self.interruptsOnDeinit = false
}

Expand Down
58 changes: 20 additions & 38 deletions Sources/Signal.swift
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,22 @@ extension Signal where Error == NoError {
}

extension Signal {
/// Perform an action upon every event from `self`. The action may generate zero or
/// more events.
///
/// - precondition: The action must be synchronous.
///
/// - parameters:
/// - transform: A closure that creates the said action from the given event
/// closure.
///
/// - returns: A signal that forwards events yielded by the action.
internal func flatMapEvent<U, E>(_ transform: @escaping (@escaping Signal<U, E>.Observer.Action) -> (Event) -> Void) -> Signal<U, E> {
return Signal<U, E> { observer in
return self.observe(.init(observer, transform))
}
}

/// Map each value in the signal to a new value.
///
/// - parameters:
Expand All @@ -535,11 +551,7 @@ extension Signal {
///
/// - returns: A signal that will send new values.
public func map<U>(_ transform: @escaping (Value) -> U) -> Signal<U, Error> {
return Signal<U, Error> { observer in
return self.observe { event in
observer.action(event.map(transform))
}
}
return flatMapEvent(Signal.Event.map(transform))
}

#if swift(>=3.2)
Expand All @@ -562,11 +574,7 @@ extension Signal {
///
/// - returns: A signal that will send new type of errors.
public func mapError<F>(_ transform: @escaping (Error) -> F) -> Signal<Value, F> {
return Signal<Value, F> { observer in
return self.observe { event in
observer.action(event.mapError(transform))
}
}
return flatMapEvent(Signal.Event.mapError(transform))
}

/// Maps each value in the signal to a new value, lazily evaluating the
Expand Down Expand Up @@ -599,18 +607,7 @@ extension Signal {
///
/// - returns: A signal that forwards the values passing the given closure.
public func filter(_ isIncluded: @escaping (Value) -> Bool) -> Signal<Value, Error> {
return Signal { observer in
return self.observe { (event: Event) -> Void in
guard let value = event.value else {
observer.action(event)
return
}

if isIncluded(value) {
observer.send(value: value)
}
}
}
return flatMapEvent(Signal.Event.filter(isIncluded))
}

/// Applies `transform` to values from `signal` and forwards values with non `nil` results unwrapped.
Expand All @@ -620,22 +617,7 @@ extension Signal {
///
/// - returns: A signal that will send new values, that are non `nil` after the transformation.
public func filterMap<U>(_ transform: @escaping (Value) -> U?) -> Signal<U, Error> {
return Signal<U, Error> { observer in
return self.observe { (event: Event) -> Void in
switch event {
case let .value(value):
if let mapped = transform(value) {
observer.send(value: mapped)
}
case let .failed(error):
observer.send(error: error)
case .completed:
observer.sendCompleted()
case .interrupted:
observer.sendInterrupted()
}
}
}
return flatMapEvent(Signal.Event.filterMap(transform))
}
}

Expand Down
Loading