chore: Use NSLock instead of DispatchQueue to protect Atomics (#868)

Author: palpatim

.gitallowed

@@ -2,3 +2,4 @@ case slave = "den"
 amazon-textract-code-samples/master/src-csharp
 aws-amplify/amplify-ci-support', branch: 'master'
 branch: master
+build-support/codecov.sh

Amplify/Core/Support/AtomicDictionary.swift

@@ -8,7 +8,7 @@
 import Foundation
 
 final class AtomicDictionary<Key: Hashable, Value> {
-    let queue = DispatchQueue(label: "com.amazonaws.AtomicDictionary", target: DispatchQueue.global())
+    let lock = NSLock()
 
     private var value: [Key: Value]
 
@@ -17,48 +17,62 @@ final class AtomicDictionary<Key: Hashable, Value> {
     }
 
     var count: Int {
-        queue.sync {
-            value.count
+        lock.lock()
+        defer {
+            lock.unlock()
         }
+        return value.count
     }
 
     var keys: [Key] {
-        queue.sync {
-            Array(value.keys)
+        lock.lock()
+        defer {
+            lock.unlock()
         }
+        return Array(value.keys)
     }
 
     var values: [Value] {
-        return queue.sync {
-            Array(value.values)
+        lock.lock()
+        defer {
+            lock.unlock()
         }
+        return Array(value.values)
     }
 
     // MARK: - Functions
 
     func getValue(forKey key: Key) -> Value? {
-        queue.sync {
-            value[key]
+        lock.lock()
+        defer {
+            lock.unlock()
         }
+        return value[key]
     }
 
     func removeAll() {
-        queue.sync {
-            value = [:]
+        lock.lock()
+        defer {
+            lock.unlock()
         }
+        value = [:]
     }
 
     @discardableResult
     func removeValue(forKey key: Key) -> Value? {
-        queue.sync {
-            value.removeValue(forKey: key)
+        lock.lock()
+        defer {
+            lock.unlock()
         }
+        return value.removeValue(forKey: key)
     }
 
     func set(value: Value, forKey key: Key) {
-        queue.sync {
-            self.value[key] = value
+        lock.lock()
+        defer {
+            lock.unlock()
         }
+        self.value[key] = value
     }
 
 }

Amplify/Core/Support/AtomicValue+Bool.swift

@@ -16,10 +16,12 @@ extension AtomicValue where Value == Bool {
     /// print(atomicBool.get()) // prints "false"
     /// ```
     public func getAndToggle() -> Value {
-        queue.sync {
-            let oldValue = value
-            value.toggle()
-            return oldValue
+        lock.lock()
+        defer {
+            lock.unlock()
         }
+        let oldValue = value
+        value.toggle()
+        return oldValue
     }
 }

Amplify/Core/Support/AtomicValue+Numeric.swift

@@ -8,17 +8,21 @@
 extension AtomicValue where Value: Numeric {
     /// Increments the current value by `amount` and returns the incremented value
     public func increment(by amount: Value = 1) -> Value {
-        return queue.sync {
-            value += amount
-            return value
+        lock.lock()
+        defer {
+            lock.unlock()
         }
+        value += amount
+        return value
     }
 
     /// Decrements the current value by `amount` and returns the decremented value
     public func decrement(by amount: Value = 1) -> Value {
-        return queue.sync {
-            value -= amount
-            return value
+        lock.lock()
+        defer {
+            lock.unlock()
         }
+        value -= amount
+        return value
     }
 }

Amplify/Core/Support/AtomicValue+RangeReplaceableCollection.swift

@@ -7,26 +7,34 @@
 
 extension AtomicValue where Value: RangeReplaceableCollection {
     public func append(_ newElement: Value.Element) {
-        queue.sync {
-            value.append(newElement)
+        lock.lock()
+        defer {
+            lock.unlock()
         }
+        value.append(newElement)
     }
 
     public func append<S>(contentsOf sequence: S) where S: Sequence, S.Element == Value.Element {
-        queue.sync {
-            value.append(contentsOf: sequence)
+        lock.lock()
+        defer {
+            lock.unlock()
         }
+        value.append(contentsOf: sequence)
     }
 
     public func removeFirst() -> Value.Element {
-        queue.sync {
-            value.removeFirst()
+        lock.lock()
+        defer {
+            lock.unlock()
         }
+        return value.removeFirst()
     }
 
     public subscript(_ key: Value.Index) -> Value.Element {
-        queue.sync {
-            value[key]
+        lock.lock()
+        defer {
+            lock.unlock()
         }
+        return value[key]
     }
 }

Amplify/Core/Support/AtomicValue.swift

@@ -8,7 +8,7 @@
 import Foundation
 
 public final class AtomicValue<Value> {
-    let queue = DispatchQueue(label: "com.amazonaws.AtomicValue", target: DispatchQueue.global())
+    let lock = NSLock()
 
     var value: Value
 
@@ -17,35 +17,49 @@ public final class AtomicValue<Value> {
     }
 
     public func get() -> Value {
-        queue.sync { value }
+        lock.lock()
+        defer {
+            lock.unlock()
+        }
+        return value
     }
 
     public func set(_ newValue: Value) {
-        queue.sync { value = newValue }
+        lock.lock()
+        defer {
+            lock.unlock()
+        }
+        value = newValue
     }
 
     /// Sets AtomicValue to `newValue` and returns the old value
     public func getAndSet(_ newValue: Value) -> Value {
-        return queue.sync {
-            let oldValue = value
-            value = newValue
-            return oldValue
+        lock.lock()
+        defer {
+            lock.unlock()
         }
+        let oldValue = value
+        value = newValue
+        return oldValue
     }
 
     /// Performs `block` with the current value, preventing other access until the block exits.
     public func atomicallyPerform(block: (Value) -> Void) {
-        queue.sync {
-            block(value)
+        lock.lock()
+        defer {
+            lock.unlock()
         }
+        block(value)
     }
 
     /// Performs `block` with an `inout` value, preventing other access until the block exits,
     /// and enabling the block to mutate the value
     public func with(block: (inout Value) -> Void) {
-        queue.sync {
-            block(&value)
+        lock.lock()
+        defer {
+            lock.unlock()
         }
+        block(&value)
     }
 
 }

AmplifyTests/CoreTests/AtomicDictionaryTests.swift

@@ -10,6 +10,15 @@ import XCTest
 
 class AtomicDictionaryTests: XCTestCase {
 
+    func testPerformance() {
+        let atomicDictionary = AtomicDictionary(initialValue: [Int: Int]())
+        measure {
+            DispatchQueue.concurrentPerform(iterations: 10_000) { iteration in
+                atomicDictionary.set(value: iteration, forKey: iteration)
+            }
+        }
+    }
+
     /// Given: An AtomicDictionary
     /// When:
     /// - I `get` a nonexistent key

AmplifyTests/CoreTests/AtomicValueTests.swift

@@ -11,11 +11,20 @@ import XCTest
 // These tests must be run with ThreadSanitizer enabled
 class AtomicValueTests: XCTestCase {
 
+    func testPerformance() {
+        let atomicInt = AtomicValue(initialValue: 0)
+        measure {
+            DispatchQueue.concurrentPerform(iterations: 10_000) { _ in
+                _ = atomicInt.increment()
+            }
+        }
+    }
+
     func testSimpleSet() {
         let atomicInt = AtomicValue(initialValue: -1)
 
         DispatchQueue.concurrentPerform(iterations: 10_000) { iteration in
-            _ = atomicInt.set(iteration)
+            atomicInt.set(iteration)
         }
 
         XCTAssertNotEqual(atomicInt.get(), -1)