From be8ee362beacd1fedd2f9fe825d4bcf5c3b43bea Mon Sep 17 00:00:00 2001 From: Daniel Haight Date: Fri, 31 May 2019 16:15:31 +0100 Subject: [PATCH 1/3] Update key generation for adding handlers Previous strategy was failing with Swift 5. It seems that Dictionary.keys is somehow not atomic. I have not checked the diffs to get to the root cause, however this now passes tests and I added a performance test just to check that it does not add any overhead. --- Receiver/Sources/Receiver.swift | 9 ++++---- ReceiverTests/ReceiverTests+Performance.swift | 22 +++++++++++++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/Receiver/Sources/Receiver.swift b/Receiver/Sources/Receiver.swift index 5877afc..15674ca 100644 --- a/Receiver/Sources/Receiver.swift +++ b/Receiver/Sources/Receiver.swift @@ -106,11 +106,10 @@ public class Receiver { /// - returns: A reference to a disposable @discardableResult public func listen(to handle: @escaping (Wave) -> Void) -> Disposable { var _key: Int! - handlers.apply { _handlers in - _key = (_handlers.keys.map { $0.hashValue }.max() ?? -1) + 1 - _handlers[_key] = handle - } - + handlers.apply { _handlers in + _key = _handlers.count + _handlers[_key] = handle + } switch strategy { case .cold: broadcast(elements: Int.max) diff --git a/ReceiverTests/ReceiverTests+Performance.swift b/ReceiverTests/ReceiverTests+Performance.swift index b807fb4..3c2e663 100644 --- a/ReceiverTests/ReceiverTests+Performance.swift +++ b/ReceiverTests/ReceiverTests+Performance.swift @@ -18,4 +18,26 @@ class ReceiverTests_Performance: XCTestCase { } } } + + func test_listen_performance() { + self.measure { + let numberOfListeners = 200000 + let (transmitter, receiver) = Receiver.make() + var called = 0 + + for _ in 1...numberOfListeners { + receiver.listen { wave in + XCTAssertTrue(wave == 1) + called = called + 1 + } + } + + transmitter.broadcast(1) + XCTAssertEqual(called, numberOfListeners) + + transmitter.broadcast(1) + XCTAssertEqual(called, numberOfListeners*2) + } + } + } From 09d2b5dd0f75bf3e50286e8f679a67f05c27f6cc Mon Sep 17 00:00:00 2001 From: Daniel Haight Date: Sat, 1 Jun 2019 12:51:26 +0100 Subject: [PATCH 2/3] Update key generation for listening. Previous solution did not take into account disposing subscriptions. This fixes this, using `Int.random` for key generation. Also added unsubscribing (disposing) to the performance test. --- Receiver/Sources/Receiver.swift | 5 +++- ReceiverTests/ReceiverTests+Performance.swift | 23 ++++++++++++++----- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/Receiver/Sources/Receiver.swift b/Receiver/Sources/Receiver.swift index 15674ca..3cffabd 100644 --- a/Receiver/Sources/Receiver.swift +++ b/Receiver/Sources/Receiver.swift @@ -107,7 +107,10 @@ public class Receiver { @discardableResult public func listen(to handle: @escaping (Wave) -> Void) -> Disposable { var _key: Int! handlers.apply { _handlers in - _key = _handlers.count + _key = Int.random(in: Int.min...Int.max) + while _handlers[_key] != nil { + _key = Int.random(in: Int.min...Int.max) + } _handlers[_key] = handle } switch strategy { diff --git a/ReceiverTests/ReceiverTests+Performance.swift b/ReceiverTests/ReceiverTests+Performance.swift index 3c2e663..89235e3 100644 --- a/ReceiverTests/ReceiverTests+Performance.swift +++ b/ReceiverTests/ReceiverTests+Performance.swift @@ -19,24 +19,35 @@ class ReceiverTests_Performance: XCTestCase { } } - func test_listen_performance() { + func test_listen_and_dispose_performance() { self.measure { - let numberOfListeners = 200000 + let numberOfListeners = 10000 + var numberOfDisposes = 0 let (transmitter, receiver) = Receiver.make() var called = 0 + var disposables = [Disposable]() - for _ in 1...numberOfListeners { - receiver.listen { wave in + for i in 1...numberOfListeners { + + let disposable = receiver.listen { wave in XCTAssertTrue(wave == 1) called = called + 1 } + + disposables.append(disposable) + + if i % 3 == 0 && disposables.count > 0 { + let d = disposables.remove(at: Int.random(in: 0.. Date: Tue, 18 Jun 2019 17:19:08 +0100 Subject: [PATCH 3/3] Update key generation based on ReactiveSwift implementation. --- Receiver/Sources/Receiver.swift | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/Receiver/Sources/Receiver.swift b/Receiver/Sources/Receiver.swift index 3cffabd..4e6c455 100644 --- a/Receiver/Sources/Receiver.swift +++ b/Receiver/Sources/Receiver.swift @@ -68,7 +68,8 @@ public class Receiver { private let values = Atomic<[Wave]>([]) private let strategy: Strategy - private let handlers = Atomic<[Int:Handler]>([:]) + private let handlers = Atomic<[UInt64:Handler]>([:]) + private var nextKey: UInt64 = 0 private init(strategy: Strategy) { self.strategy = strategy @@ -105,13 +106,11 @@ public class Receiver { /// a new value is sent. /// - returns: A reference to a disposable @discardableResult public func listen(to handle: @escaping (Wave) -> Void) -> Disposable { - var _key: Int! + var _key: UInt64! handlers.apply { _handlers in - _key = Int.random(in: Int.min...Int.max) - while _handlers[_key] != nil { - _key = Int.random(in: Int.min...Int.max) - } + _key = nextKey _handlers[_key] = handle + nextKey = nextKey &+ 1 } switch strategy { case .cold: