From f98f10a4c4f89621be05eb1516257e093d28e59e Mon Sep 17 00:00:00 2001 From: kean Date: Sat, 23 Dec 2017 19:38:13 +0300 Subject: [PATCH] Get rid of Bag which proved not to be as efficient --- Nuke.xcodeproj/project.pbxproj | 8 ----- Sources/CancellationToken.swift | 6 ++-- Sources/Internal.swift | 44 ----------------------- Sources/Loader.swift | 6 ++-- Tests/BagTests.swift | 29 --------------- Tests/PerformanceTests.swift | 62 --------------------------------- 6 files changed, 5 insertions(+), 150 deletions(-) delete mode 100644 Tests/BagTests.swift diff --git a/Nuke.xcodeproj/project.pbxproj b/Nuke.xcodeproj/project.pbxproj index fa99d4ac0..70d48f0db 100644 --- a/Nuke.xcodeproj/project.pbxproj +++ b/Nuke.xcodeproj/project.pbxproj @@ -80,9 +80,6 @@ 0C34BF681D4D15B6006B8A57 /* CancellationToken.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0C34BF661D4D15B6006B8A57 /* CancellationToken.swift */; }; 0C34BF691D4D15B6006B8A57 /* CancellationToken.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0C34BF661D4D15B6006B8A57 /* CancellationToken.swift */; }; 0C34BF6A1D4D15B6006B8A57 /* CancellationToken.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0C34BF661D4D15B6006B8A57 /* CancellationToken.swift */; }; - 0C3A18851FE85B56006111E2 /* BagTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0C3A18841FE85B56006111E2 /* BagTests.swift */; }; - 0C3A18861FE85B56006111E2 /* BagTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0C3A18841FE85B56006111E2 /* BagTests.swift */; }; - 0C3A18871FE85B56006111E2 /* BagTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0C3A18841FE85B56006111E2 /* BagTests.swift */; }; 0C4AF1EB1FE85539002F86CB /* LinkedListTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0C4AF1E91FE8551D002F86CB /* LinkedListTest.swift */; }; 0C4AF1EC1FE85539002F86CB /* LinkedListTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0C4AF1E91FE8551D002F86CB /* LinkedListTest.swift */; }; 0C4AF1ED1FE8553A002F86CB /* LinkedListTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0C4AF1E91FE8551D002F86CB /* LinkedListTest.swift */; }; @@ -222,7 +219,6 @@ 0C22DA2C1BCAA220006E1D3B /* Nuke.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Nuke.framework; sourceTree = BUILT_PRODUCTS_DIR; }; 0C22DA381BCAA22A006E1D3B /* Nuke macOS Tests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = "Nuke macOS Tests.xctest"; sourceTree = BUILT_PRODUCTS_DIR; }; 0C34BF661D4D15B6006B8A57 /* CancellationToken.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CancellationToken.swift; sourceTree = ""; }; - 0C3A18841FE85B56006111E2 /* BagTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BagTests.swift; sourceTree = ""; }; 0C4AF1E91FE8551D002F86CB /* LinkedListTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LinkedListTest.swift; sourceTree = ""; }; 0C4B6A341E36630E00E86B21 /* Nuke 5 Migration Guide.md */ = {isa = PBXFileReference; lastKnownFileType = net.daringfireball.markdown; path = "Nuke 5 Migration Guide.md"; sourceTree = ""; }; 0C5D81871D46A385000ECCB6 /* ThreadSafetyTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ThreadSafetyTests.swift; sourceTree = ""; }; @@ -390,7 +386,6 @@ 0C1E623A1D6FA0FB00AD5CF5 /* CancellationTokenTests.swift */, 0C7E86141DA59F12002EAD8D /* RateLimiterTests.swift */, 0C4AF1E91FE8551D002F86CB /* LinkedListTest.swift */, - 0C3A18841FE85B56006111E2 /* BagTests.swift */, 0C7101D51FC70860000CE3C9 /* TaskQueueTests.swift */, 0C8D74201D9D6EEB0036349E /* PerformanceTests.swift */, 0C8D7BD11D9DBF1600D12EB7 /* Host */, @@ -912,7 +907,6 @@ 0C1ECA431D526461009063A9 /* CacheTests.swift in Sources */, 0C22DA591BCAAA3A006E1D3B /* MockImageProcessor.swift in Sources */, 0C1E623C1D6FA0FB00AD5CF5 /* CancellationTokenTests.swift in Sources */, - 0C3A18861FE85B56006111E2 /* BagTests.swift in Sources */, 0C4AF1EC1FE85539002F86CB /* LinkedListTest.swift in Sources */, 0C22DA5B1BCAAA3A006E1D3B /* MockDataLoader.swift in Sources */, 0C1ECA511D52811D009063A9 /* ManagerTests.swift in Sources */, @@ -940,7 +934,6 @@ 0C1ECA421D526461009063A9 /* CacheTests.swift in Sources */, 0C7C06981BCA888800089D7F /* Tests.swift in Sources */, 0C1E623B1D6FA0FB00AD5CF5 /* CancellationTokenTests.swift in Sources */, - 0C3A18851FE85B56006111E2 /* BagTests.swift in Sources */, 0C4AF1EB1FE85539002F86CB /* LinkedListTest.swift in Sources */, 0C7C06971BCA888800089D7F /* MockDataLoader.swift in Sources */, 0C1ECA501D52811D009063A9 /* ManagerTests.swift in Sources */, @@ -1030,7 +1023,6 @@ 0C1ECA441D526462009063A9 /* CacheTests.swift in Sources */, 0CC345C51BD96E1D005C2DD8 /* Tests.swift in Sources */, 0C1E623D1D6FA0FB00AD5CF5 /* CancellationTokenTests.swift in Sources */, - 0C3A18871FE85B56006111E2 /* BagTests.swift in Sources */, 0C4AF1ED1FE8553A002F86CB /* LinkedListTest.swift in Sources */, 0CC345C01BD96E16005C2DD8 /* MockCache.swift in Sources */, 0C1ECA521D52811D009063A9 /* ManagerTests.swift in Sources */, diff --git a/Sources/CancellationToken.swift b/Sources/CancellationToken.swift index a8501cee3..81b947408 100644 --- a/Sources/CancellationToken.swift +++ b/Sources/CancellationToken.swift @@ -16,7 +16,7 @@ public final class CancellationTokenSource { /// Creates a new token associated with the source. public var token: CancellationToken { return CancellationToken(source: self) } - private var _observers: Bag<() -> Void>? = Bag<() -> Void>() + private var _observers: ContiguousArray<() -> Void>? = [] /// Initializes the `CancellationTokenSource` instance. public init() {} @@ -29,7 +29,7 @@ public final class CancellationTokenSource { private func _register(_ closure: @escaping () -> Void) -> Bool { _lock.lock(); defer { _lock.unlock() } - _observers?.insert(closure) + _observers?.append(closure) return _observers != nil } @@ -40,7 +40,7 @@ public final class CancellationTokenSource { } } - private func _cancel() -> Bag<() -> Void>? { + private func _cancel() -> ContiguousArray<() -> Void>? { _lock.lock(); defer { _lock.unlock() } let observers = _observers _observers = nil // transition to `isCancelling` state diff --git a/Sources/Internal.swift b/Sources/Internal.swift index d57be0c2f..05138b605 100644 --- a/Sources/Internal.swift +++ b/Sources/Internal.swift @@ -235,47 +235,3 @@ internal final class LinkedList { init(value: Element) { self.value = value } } } - -// MARK: - Bag - -/// Lightweight unordered data structure for storing a small number of elements. -/// The idea is that it doesn't allocate any space on heap during initialization -/// and it inlines first couple of elements and only then falls back to array -/// (`ContiguousArray`) backed storage. -/// -/// The name `Bag` (`Multiset`) is actually taken and it means something -/// different than what this type does. But since it's an internal type it -/// should work for now. -internal struct Bag: Sequence { - private var first: Element? // inline first couple of elements - private var second: Element? - // ~20% faster than Array based on perf tests - private var remaining: ContiguousArray? - - mutating func insert(_ value: Element) { - guard first != nil else { first = value; return } // inline first - guard second != nil else { second = value; return } // inline second - if remaining == nil { remaining = ContiguousArray() } // create lazily - remaining!.append(value) - } - - // MARK: Sequence - - internal struct BagIterator: IteratorProtocol { - private var index = 0 - private var bag: Bag - init(_ bag: Bag) { self.bag = bag } - - mutating func next() -> Element? { - defer { index += 1 } - if index == 0 { return bag.first } - if index == 1 { return bag.second } - guard let remaining = bag.remaining, index - 2 < remaining.endIndex else { return nil } - return remaining[index - 2] - } - } - - func makeIterator() -> BagIterator { - return BagIterator(self) - } -} diff --git a/Sources/Loader.swift b/Sources/Loader.swift index 78049822e..1f6735c59 100644 --- a/Sources/Loader.swift +++ b/Sources/Loader.swift @@ -133,7 +133,7 @@ public final class Loader: Loading { // The request only gets cancelled when all the underlying requests are. task.retainCount += 1 let handler = DeduplicatedTask.Handler(progress: request.progress, completion: completion) - task.handlers.insert(handler) + task.handlers.append(handler) token?.register { [weak self, weak task] in if let task = task { self?._cancel(task) } @@ -196,9 +196,7 @@ public final class Loader: Loading { // CTS optimizations by only registering twice. let cts = CancellationTokenSource() - // In majority of use cases the `handlers` count is going to stay - // below 2 which takes full advantage of `Bag` optimizations. - var handlers = Bag() + var handlers = ContiguousArray() var retainCount = 0 // number of non-cancelled handlers init(request: Request, key: AnyHashable) { diff --git a/Tests/BagTests.swift b/Tests/BagTests.swift deleted file mode 100644 index 873eb8e27..000000000 --- a/Tests/BagTests.swift +++ /dev/null @@ -1,29 +0,0 @@ -// The MIT License (MIT) -// -// Copyright (c) 2017 Alexander Grebenyuk (github.com/kean). - -import XCTest -@testable import Nuke - - -class BagTests: XCTestCase { - var bag = Bag() - - // MARK: `insert(_:)` - - func testInsert() { - XCTAssertEqual(Set(bag), []) - - bag.insert(1) - XCTAssertEqual(Set(bag), [1]) - - bag.insert(2) - XCTAssertEqual(Set(bag), [1, 2]) - - bag.insert(3) - XCTAssertEqual(Set(bag), [1, 2, 3]) - - bag.insert(4) - XCTAssertEqual(Set(bag), [1, 2, 3, 4]) - } -} diff --git a/Tests/PerformanceTests.swift b/Tests/PerformanceTests.swift index 73dc6cad9..c67ec4d79 100644 --- a/Tests/PerformanceTests.swift +++ b/Tests/PerformanceTests.swift @@ -113,68 +113,6 @@ class CachePerformanceTests: XCTestCase { } } -class BagPerformanceTests: XCTestCase { - func testTestInsertTwoInts() { - measure { - for _ in 0..<200_000 { - var bag = Bag() - bag.insert(1) - bag.insert(2) - } - } - } - - func testTestInsertThreeInts() { - measure { - for _ in 0..<200_000 { - var bag = Bag() - bag.insert(1) - bag.insert(2) - bag.insert(3) - } - } - } - - func testInsertLotsOfInts() { - measure { - var bag = Bag() - for _ in 0..<200_000 { // also makes sure that we don't stack overflow - bag.insert(1) - } - } - } - - func testTestInsertTwoClosures() { - measure { - for _ in 0..<200_000 { - var bag = Bag<() -> Void>() - bag.insert({ print(1) }) - bag.insert({ print(2) }) - } - } - } - - func testTestInsertThreeClosures() { - measure { - for _ in 0..<200_000 { - var bag = Bag<() -> Void>() - bag.insert({ print(1) }) - bag.insert({ print(2) }) - bag.insert({ print(3) }) - } - } - } - - func testInsertLotsOfClosures() { - measure { - var bag = Bag<() -> Void>() - for _ in 0..<200_000 { - bag.insert({ print(1) }) - } - } - } -} - class RequestPerformanceTests: XCTestCase { func testStoringRequestInCollections() { let urls = (0..<200_000).map { _ in return URL(string: "http://test.com/\(rnd(200))")! }