Skip to content

Commit

Permalink
Get rid of Bag which proved not to be as efficient
Browse files Browse the repository at this point in the history
  • Loading branch information
@kean
kean committed Dec 23, 2017
1 parent 524444f commit f98f10a
Show file tree
Hide file tree
Showing 6 changed files with 5 additions and 150 deletions.
8 changes: 0 additions & 8 deletions Nuke.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */; };
Expand Down Expand Up @@ -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 = "<group>"; };
0C3A18841FE85B56006111E2 /* BagTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BagTests.swift; sourceTree = "<group>"; };
0C4AF1E91FE8551D002F86CB /* LinkedListTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LinkedListTest.swift; sourceTree = "<group>"; };
0C4B6A341E36630E00E86B21 /* Nuke 5 Migration Guide.md */ = {isa = PBXFileReference; lastKnownFileType = net.daringfireball.markdown; path = "Nuke 5 Migration Guide.md"; sourceTree = "<group>"; };
0C5D81871D46A385000ECCB6 /* ThreadSafetyTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ThreadSafetyTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -390,7 +386,6 @@
0C1E623A1D6FA0FB00AD5CF5 /* CancellationTokenTests.swift */,
0C7E86141DA59F12002EAD8D /* RateLimiterTests.swift */,
0C4AF1E91FE8551D002F86CB /* LinkedListTest.swift */,
0C3A18841FE85B56006111E2 /* BagTests.swift */,
0C7101D51FC70860000CE3C9 /* TaskQueueTests.swift */,
0C8D74201D9D6EEB0036349E /* PerformanceTests.swift */,
0C8D7BD11D9DBF1600D12EB7 /* Host */,
Expand Down Expand Up @@ -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 */,
Expand Down Expand Up @@ -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 */,
Expand Down Expand Up @@ -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 */,
Expand Down
6 changes: 3 additions & 3 deletions Sources/CancellationToken.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
Expand All @@ -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
}

Expand All @@ -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
Expand Down
44 changes: 0 additions & 44 deletions Sources/Internal.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,47 +235,3 @@ internal final class LinkedList<Element> {
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<Element>: 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<Element>?

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<Element>() } // 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)
}
}
6 changes: 2 additions & 4 deletions Sources/Loader.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Expand Down Expand Up @@ -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<Handler>()
var handlers = ContiguousArray<Handler>()
var retainCount = 0 // number of non-cancelled handlers

init(request: Request, key: AnyHashable) {
Expand Down
29 changes: 0 additions & 29 deletions Tests/BagTests.swift
View file

This file was deleted.

62 changes: 0 additions & 62 deletions Tests/PerformanceTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,68 +113,6 @@ class CachePerformanceTests: XCTestCase {
}
}

class BagPerformanceTests: XCTestCase {
func testTestInsertTwoInts() {
measure {
for _ in 0..<200_000 {
var bag = Bag<Int>()
bag.insert(1)
bag.insert(2)
}
}
}

func testTestInsertThreeInts() {
measure {
for _ in 0..<200_000 {
var bag = Bag<Int>()
bag.insert(1)
bag.insert(2)
bag.insert(3)
}
}
}

func testInsertLotsOfInts() {
measure {
var bag = Bag<Int>()
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))")! }
Expand Down

0 comments on commit f98f10a

Please sign in to comment.