[IMPROVEMENT] Double linked list used to improve performance of deque…

…ue and remove operations in OperationQueue (#1899)

* [IMPROVEMENT] Double linked list used to improve performance of dequeue and remove operations in OperationQueue

- remove() and dequeue() methods time complexity reduces to O(1) from O(n).
- All arrays, veryHigh, high, normal, low, veryLow, all, converted to DoubleLinkedList.
- New internal class _IndexedOperationLinkedList introduced specifically to work with Operation.
- HashMap used to index, node by it's operation, in the linked list, in order to avoid traversing the list to find a node, which in return reduces the time complexity to O(1).
- Unit Test added to test cancel one operation and cancel operations of a specific queue priority

* Fixed an issue that was failing OperationQueue's unit tests to fail

- In _IndexedOperationLinkedList internal class, when removing an node from Double linked list, the bug was setting the root not the linked list to nil.

* In Operation, private properties, next and previous, introduced to use the operation instance as node in double linked list

Foundation/Operation.swift

@@ -25,6 +25,12 @@ open class Operation : NSObject {
     internal var _depGroup = DispatchGroup()
     internal var _groups = [DispatchGroup]()
 #endif
+    // prioritizedPrevious && prioritizedNext used to traverse in the order in which the operations need to be dequeued
+    fileprivate weak var prioritizedPrevious: Operation?
+    fileprivate var prioritizedNext: Operation?
+    // orderdedPrevious && orderdedNext used to traverse in the order in which the operations added to the queue
+    fileprivate weak var orderdedPrevious: Operation?
+    fileprivate var orderdedNext: Operation?
 
     public override init() {
         super.init()
@@ -244,16 +250,120 @@ extension OperationQueue {
     public static let defaultMaxConcurrentOperationCount: Int = Int.max
 }
 
-internal struct _OperationList {
-    var veryLow = [Operation]()
-    var low = [Operation]()
-    var normal = [Operation]()
-    var high = [Operation]()
-    var veryHigh = [Operation]()
-    var all = [Operation]()
+fileprivate class _IndexedOperationLinkedList {
+    private(set) var root: Operation? = nil
+    private(set) var tail: Operation? = nil
+
+    func append(_ operation: Operation, inOrderList: Bool = false) {
+        if root == nil {
+            root = operation
+            tail = operation
+        } else {
+            if inOrderList {
+                appendOperationInOrderList(operation)
+            } else {
+                appendOperationInPriorityList(operation)
+            }
+        }
+    }
+
+    private func appendOperationInPriorityList(_ operation: Operation) {
+        operation.prioritizedPrevious = tail
+        tail?.prioritizedNext = operation
+        tail = operation
+    }
+
+    private func appendOperationInOrderList(_ operation: Operation) {
+        operation.orderdedPrevious = tail
+        tail?.orderdedNext = operation
+        tail = operation
+    }
+
+    func remove(_ operation: Operation, fromOrderList: Bool) {
+        if fromOrderList {
+            removeOperationFromOrderList(operation)
+        } else {
+            removeOperationFromPriorityList(operation)
+        }
+    }
+
+    private func removeOperationFromPriorityList(_ operation: Operation) {
+        guard let unwrappedRoot = root, let unwrappedTail = tail else {
+            return
+        }
+
+        if operation === unwrappedRoot {
+            let next = operation.prioritizedNext
+            next?.prioritizedPrevious = nil
+            root = next
+            if root == nil {
+                tail = nil
+            }
+        } else if operation === unwrappedTail {
+            tail = operation.prioritizedPrevious
+            tail?.prioritizedNext = nil
+        } else {
+            // Middle Node
+            let previous = operation.prioritizedPrevious
+            let next = operation.prioritizedNext
+            previous?.prioritizedNext = next
+            next?.prioritizedPrevious = previous
+
+            operation.prioritizedNext = nil
+            operation.prioritizedPrevious = nil
+        }
+    }
+
+    private func removeOperationFromOrderList(_ operation: Operation) {
+        guard let unwrappedRoot = root, let unwrappedTail = tail else {
+            return
+        }
+
+        if operation === unwrappedRoot {
+            let next = operation.orderdedNext
+            next?.orderdedPrevious = nil
+            root = next
+            if root == nil {
+                tail = nil
+            }
+        } else if operation === unwrappedTail {
+            tail = operation.orderdedPrevious
+            tail?.orderdedNext = nil
+        } else {
+            // Middle Node
+            let previous = operation.orderdedPrevious
+            let next = operation.orderdedNext
+            previous?.orderdedNext = next
+            next?.orderdedPrevious = previous
+
+            operation.orderdedNext = nil
+            operation.orderdedPrevious = nil
+        }
+    }
+
+    func removeFirstInPriority() -> Operation? {
+        guard let operation = root else {
+            return nil
+        }
+
+        remove(operation, fromOrderList: false)
+        return operation
+    }
+}
+
+fileprivate struct _OperationList {
+    var veryLow = _IndexedOperationLinkedList()
+    var low = _IndexedOperationLinkedList()
+    var normal = _IndexedOperationLinkedList()
+    var high = _IndexedOperationLinkedList()
+    var veryHigh = _IndexedOperationLinkedList()
+    var all =  _IndexedOperationLinkedList()
+
+    var count: Int = 0
 
     mutating func insert(_ operation: Operation) {
-        all.append(operation)
+        all.append(operation, inOrderList: true)
+
         switch operation.queuePriority {
         case .veryLow:
             veryLow.append(operation)
@@ -266,61 +376,53 @@ internal struct _OperationList {
         case .veryHigh:
             veryHigh.append(operation)
         }
+
+        count += 1
     }
 
     mutating func remove(_ operation: Operation) {
-        if let idx = all.firstIndex(of: operation) {
-            all.remove(at: idx)
-        }
+        all.remove(operation, fromOrderList: true)
+
         switch operation.queuePriority {
         case .veryLow:
-            if let idx = veryLow.firstIndex(of: operation) {
-                veryLow.remove(at: idx)
-            }
+            veryLow.remove(operation, fromOrderList: false)
         case .low:
-            if let idx = low.firstIndex(of: operation) {
-                low.remove(at: idx)
-            }
+            low.remove(operation, fromOrderList: false)
         case .normal:
-            if let idx = normal.firstIndex(of: operation) {
-                normal.remove(at: idx)
-            }
+            normal.remove(operation, fromOrderList: false)
         case .high:
-            if let idx = high.firstIndex(of: operation) {
-                high.remove(at: idx)
-            }
+            high.remove(operation, fromOrderList: false)
         case .veryHigh:
-            if let idx = veryHigh.firstIndex(of: operation) {
-                veryHigh.remove(at: idx)
-            }
-        }
-    }
-
-    mutating func dequeue() -> Operation? {
-        if !veryHigh.isEmpty {
-            return veryHigh.remove(at: 0)
-        }
-        if !high.isEmpty {
-            return high.remove(at: 0)
-        }
-        if !normal.isEmpty {
-            return normal.remove(at: 0)
-        }
-        if !low.isEmpty {
-            return low.remove(at: 0)
+            veryHigh.remove(operation, fromOrderList: false)
         }
-        if !veryLow.isEmpty {
-            return veryLow.remove(at: 0)
+
+        count -= 1
+    }
+
+    func dequeue() -> Operation? {
+        if let operation = veryHigh.removeFirstInPriority() {
+            return operation
+        } else if let operation = high.removeFirstInPriority() {
+            return operation
+        } else if let operation = normal.removeFirstInPriority() {
+            return operation
+        } else if let operation = low.removeFirstInPriority() {
+            return operation
+        } else if let operation = veryLow.removeFirstInPriority() {
+            return operation
+        } else {
+            return nil
         }
-        return nil
-    }
-
-    var count: Int {
-        return all.count
     }
 
     func map<T>(_ transform: (Operation) throws -> T) rethrows -> [T] {
-        return try all.map(transform)
+        var result: [T] = []
+        var current = all.root
+        while let node = current {
+            result.append(try transform(node))
+            current = current?.orderdedNext
+        }
+        return result
     }
 }
 
@@ -339,7 +441,7 @@ open class OperationQueue: NSObject {
     var unscheduledWorkItems: [DispatchWorkItem] = []
 #endif
 
-    var _operations = _OperationList()
+    fileprivate var _operations = _OperationList()
 #if DEPLOYMENT_ENABLE_LIBDISPATCH
     internal var _concurrencyGate: DispatchSemaphore? {
         get {

TestFoundation/TestOperationQueue.swift

@@ -17,6 +17,8 @@ class TestOperationQueue : XCTestCase {
             ("test_AsyncOperation", test_AsyncOperation),
             ("test_isExecutingWorks", test_isExecutingWorks),
             ("test_MainQueueGetter", test_MainQueueGetter),
+            ("test_CancelOneOperation", test_CancelOneOperation),
+            ("test_CancelOperationsOfSpecificQueuePriority", test_CancelOperationsOfSpecificQueuePriority),
             ("test_CurrentQueueOnMainQueue", test_CurrentQueueOnMainQueue),
             ("test_CurrentQueueOnBackgroundQueue", test_CurrentQueueOnBackgroundQueue),
             ("test_CurrentQueueOnBackgroundQueueWithSelfCancel", test_CurrentQueueOnBackgroundQueueWithSelfCancel),
@@ -112,6 +114,75 @@ class TestOperationQueue : XCTestCase {
         XCTAssertFalse(OperationQueue.main.isSuspended)
     }
 
+    func test_CancelOneOperation() {
+        var operations = [Operation]()
+        var valueOperations = [Int]()
+        for i in 0..<5 {
+            let operation = BlockOperation {
+                valueOperations.append(i)
+                sleep(2)
+            }
+            operations.append(operation)
+        }
+
+        let queue = OperationQueue()
+        queue.maxConcurrentOperationCount = 1
+        queue.addOperations(operations, waitUntilFinished: false)
+        operations.remove(at: 2).cancel()
+        queue.waitUntilAllOperationsAreFinished()
+        XCTAssertTrue(!valueOperations.contains(2))
+    }
+
+    func test_CancelOperationsOfSpecificQueuePriority() {
+        var operations = [Operation]()
+        var valueOperations = [Int]()
+
+        let operation1 = BlockOperation {
+            valueOperations.append(0)
+            sleep(2)
+        }
+        operation1.queuePriority = .high
+        operations.append(operation1)
+
+        let operation2 = BlockOperation {
+            valueOperations.append(1)
+            sleep(2)
+        }
+        operation2.queuePriority = .high
+        operations.append(operation2)
+
+        let operation3 = BlockOperation {
+            valueOperations.append(2)
+        }
+        operation3.queuePriority = .normal
+        operations.append(operation3)
+
+        let operation4 = BlockOperation {
+            valueOperations.append(3)
+        }
+        operation4.queuePriority = .normal
+        operations.append(operation4)
+
+        let operation5 = BlockOperation {
+            valueOperations.append(4)
+        }
+        operation5.queuePriority = .normal
+        operations.append(operation5)
+
+        let queue = OperationQueue()
+        queue.maxConcurrentOperationCount = 1
+        queue.addOperations(operations, waitUntilFinished: false)
+        for operation in queue.operations {
+            if operation.queuePriority == .normal {
+                operation.cancel()
+            }
+        }
+        queue.waitUntilAllOperationsAreFinished()
+        XCTAssertTrue(valueOperations.count == 2)
+        XCTAssertTrue(valueOperations[0] == 0)
+        XCTAssertTrue(valueOperations[1] == 1)
+    }
+
     func test_CurrentQueueOnMainQueue() {
         XCTAssertTrue(OperationQueue.main === OperationQueue.current)
     }