Improvements to throttling.

pointfreeco · Jun 21, 2024 · 627f8b8 · 627f8b8
1 parent c0d3ba8
commit 627f8b8
Show file tree

Hide file tree

Showing 4 changed files with 127 additions and 61 deletions.
diff --git a/ComposableArchitecture.xcworkspace/xcshareddata/swiftpm/Package.resolved b/ComposableArchitecture.xcworkspace/xcshareddata/swiftpm/Package.resolved
@@ -14,26 +14,26 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/apple/swift-argument-parser",
       "state" : {
-        "revision" : "0fbc8848e389af3bb55c182bc19ca9d5dc2f255b",
-        "version" : "1.4.0"
+        "revision" : "14c7622293ffbb7a235e896e37adf7eb089884a3",
+        "version" : "0.0.3"
       }
     },
     {
       "identity" : "swift-benchmark",
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/google/swift-benchmark",
       "state" : {
-        "revision" : "8163295f6fe82356b0bcf8e1ab991645de17d096",
-        "version" : "0.1.2"
+        "revision" : "8e0ef8bb7482ab97dcd2cd1d6855bd38921c345d",
+        "version" : "0.1.0"
       }
     },
     {
       "identity" : "swift-case-paths",
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/pointfreeco/swift-case-paths",
       "state" : {
-        "revision" : "b871e5ed11a23e52c2896a92ce2c829982ff8619",
-        "version" : "1.4.2"
+        "revision" : "e593aba2c6222daad7c4f2732a431eed2c09bb07",
+        "version" : "1.3.0"
       }
     },
     {
@@ -77,8 +77,8 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/pointfreeco/swift-dependencies",
       "state" : {
-        "revision" : "00bc30ca03f98881329fab7f1bebef8eba472596",
-        "version" : "1.3.1"
+        "revision" : "d3a5af3038a09add4d7682f66555d6212058a3c0",
+        "version" : "1.2.2"
       }
     },
     {
@@ -113,35 +113,35 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/pointfreeco/swift-macro-testing",
       "state" : {
-        "revision" : "851c8b6bde2000d8051dc9aca1efee04dcc37411",
-        "version" : "0.4.1"
+        "revision" : "90e38eec4bf661ec0da1bbfd3ec507d0f0c05310",
+        "version" : "0.3.0"
       }
     },
     {
       "identity" : "swift-perception",
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/pointfreeco/swift-perception",
       "state" : {
-        "revision" : "d8340521e532cffdf75a64468ff9362de8bd2bb9",
-        "version" : "1.2.3"
+        "revision" : "d3ab98dc2887d1cc3bed676f6fa354da4cb22b3c",
+        "version" : "1.2.4"
       }
     },
     {
       "identity" : "swift-snapshot-testing",
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/pointfreeco/swift-snapshot-testing.git",
       "state" : {
-        "revision" : "8ddd519780452729c6634ad6bd0d2595938e9ea3",
-        "version" : "1.16.1"
+        "revision" : "5b0c434778f2c1a4c9b5ebdb8682b28e84dd69bd",
+        "version" : "1.15.4"
       }
     },
     {
       "identity" : "swift-syntax",
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/apple/swift-syntax",
       "state" : {
-        "revision" : "303e5c5c36d6a558407d364878df131c3546fad8",
-        "version" : "510.0.2"
+        "revision" : "08a2f0a9a30e0f705f79c9cfaca1f68b71bdc775",
+        "version" : "510.0.0"
       }
     },
     {
@@ -158,17 +158,17 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/pointfreeco/swiftui-navigation.git",
       "state" : {
-        "revision" : "7ab04c6e2e6a73d34d5a762970ef88bf0aedb084",
-        "version" : "1.4.0"
+        "revision" : "d9e72f3083c08375794afa216fb2f89c0114f303",
+        "version" : "1.2.1"
       }
     },
     {
       "identity" : "xctest-dynamic-overlay",
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/pointfreeco/xctest-dynamic-overlay",
       "state" : {
-        "revision" : "6f30bdba373bbd7fbfe241dddd732651f2fbd1e2",
-        "version" : "1.1.2"
+        "revision" : "b13b1d1a8e787a5ffc71ac19dcaf52183ab27ba2",
+        "version" : "1.1.1"
       }
     }
   ],

diff --git a/Package.resolved b/Package.resolved
@@ -104,8 +104,8 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/pointfreeco/swift-identified-collections",
       "state" : {
-        "revision" : "2481e39ea43e14556ca9628259fa6b377427730c",
-        "version" : "1.0.1"
+        "revision" : "2f5ab6e091dd032b63dacbda052405756010dc3b",
+        "version" : "1.1.0"
       }
     },
     {

diff --git a/Sources/ComposableArchitecture/SharedState/PersistenceKey/FileStorageKey.swift b/Sources/ComposableArchitecture/SharedState/PersistenceKey/FileStorageKey.swift
@@ -20,8 +20,14 @@ public final class FileStorageKey<Value: Codable & Sendable>: PersistenceKey, Se
   private let storage: FileStorage
   private let isSetting = LockIsolated(false)
   private let url: URL
-  private let value = LockIsolated<Value?>(nil)
-  private let workItem = LockIsolated<DispatchWorkItem?>(nil)
+  fileprivate let state = LockIsolated(State())
+//  private let value = LockIsolated<Value?>(nil)
+//  private let workItem = LockIsolated<DispatchWorkItem?>(nil)
+
+  fileprivate struct State {
+    var value: Value?
+    var workItem: DispatchWorkItem?
+  }
 
   public var id: AnyHashable {
     FileStorageKeyID(url: self.url, storage: self.storage)
@@ -42,28 +48,32 @@ public final class FileStorageKey<Value: Codable & Sendable>: PersistenceKey, Se
   }
 
   public func save(_ value: Value) {
-    if self.workItem.value == nil {
-      self.isSetting.setValue(true)
-      try? self.storage.save(JSONEncoder().encode(value), self.url)
-      let workItem = DispatchWorkItem { [weak self] in
-        guard let self else { return }
-        defer {
-          self.value.setValue(nil)
-          self.workItem.setValue(nil)
-        }
-        guard let value = self.value.value
-        else { return }
+    self.state.withValue { state in
+      if state.workItem == nil {
         self.isSetting.setValue(true)
         try? self.storage.save(JSONEncoder().encode(value), self.url)
-      }
-      self.workItem.setValue(workItem)
-      if canListenForResignActive {
-        self.storage.asyncAfter(.seconds(1), workItem)
+        let workItem = DispatchWorkItem { [weak self] in
+          guard let self else { return }
+          self.state.withValue { state in
+            defer {
+              state.value = nil
+              state.workItem = nil
+            }
+            guard let value = state.value
+            else { return }
+            self.isSetting.setValue(true)
+            try? self.storage.save(JSONEncoder().encode(value), self.url)
+          }
+        }
+        state.workItem = workItem
+        if canListenForResignActive {
+          self.storage.asyncAfter(.seconds(1), workItem)
+        } else {
+          self.storage.async(workItem)
+        }
       } else {
-        self.storage.async(workItem)
+        state.value = value
       }
-    } else {
-      self.value.setValue(value)
     }
   }
 
@@ -82,17 +92,21 @@ public final class FileStorageKey<Value: Codable & Sendable>: PersistenceKey, Se
           try? self.storage.save(Data(), self.url)
         }
         let writeCancellable = self.storage.fileSystemSource(self.url, [.write]) {
-          if self.isSetting.value == true {
-            self.isSetting.setValue(false)
-          } else {
-            self.workItem.withValue {
-              $0?.cancel()
-              $0 = nil
+          self.state.withValue { state in
+            if self.isSetting.value == true {
+              self.isSetting.setValue(false)
+            } else {
+              state.workItem?.cancel()
+              state.workItem = nil
+              didSet(self.load(initialValue: initialValue))
             }
-            didSet(self.load(initialValue: initialValue))
           }
         }
         let deleteCancellable = self.storage.fileSystemSource(self.url, [.delete, .rename]) {
+          self.state.withValue { state in
+            state.workItem?.cancel()
+            state.workItem = nil
+          }
           `didSet`(self.load(initialValue: initialValue))
           setUpSources()
         }
@@ -143,17 +157,19 @@ public final class FileStorageKey<Value: Codable & Sendable>: PersistenceKey, Se
   }
 
   private func performImmediately() {
-    guard let workItem = self.workItem.value
-    else { return }
-    self.storage.async(workItem)
-    self.storage.async(
-      DispatchWorkItem {
-        self.workItem.withValue {
-          $0?.cancel()
-          $0 = nil
+    self.state.withValue { state in
+      guard let workItem = state.workItem
+      else { return }
+      self.storage.async(workItem)
+      self.storage.async(
+        DispatchWorkItem {
+          self.state.withValue { state in
+            state.workItem?.cancel()
+            state.workItem = nil
+          }
         }
-      }
-    )
+      )
+    }
   }
 }
 
@@ -270,7 +286,9 @@ public struct FileStorage: Hashable, Sendable {
       asyncAfter: { scheduler.schedule(after: scheduler.now.advanced(by: .init($0)), $1.perform) },
       createDirectory: { _, _ in },
       fileExists: { fileSystem.keys.contains($0) },
-      fileSystemSource: { url, _, handler in
+      fileSystemSource: { url, event, handler in
+        guard event.contains(.write)
+        else { return AnyCancellable {} }
         let handler = Handler(operation: handler)
         sourceHandlers.withValue { _ = $0[url, default: []].insert(handler) }
         return AnyCancellable {

diff --git a/Tests/ComposableArchitectureTests/FileStorageTests.swift b/Tests/ComposableArchitectureTests/FileStorageTests.swift
@@ -221,7 +221,7 @@ final class FileStorageTests: XCTestCase {
   }
 
   @MainActor
-  func testWriteFileWhileDebouncing() throws {
+  func testWriteFileWhileThrottling() throws {
     let fileSystem = LockIsolated<[URL: Data]>([:])
     let scheduler = DispatchQueue.test
     let fileStorage = FileStorage.inMemory(
@@ -235,6 +235,8 @@ final class FileStorageTests: XCTestCase {
       @Shared(.fileStorage(.fileURL)) var users = [User]()
 
       users.append(.blob)
+      try XCTAssertNoDifference(fileSystem.value.users(for: .fileURL), [.blob])
+
       try fileStorage.save(Data(), .fileURL)
       scheduler.run()
       XCTAssertNoDifference(users, [])
@@ -386,6 +388,52 @@ final class FileStorageTests: XCTestCase {
     XCTAssertEqual(shared1.wrappedValue.name, "Blob Jr")
     XCTAssertEqual(shared2.wrappedValue.name, "Blob Sr")
   }
+
+  func testCancelThrottleWhenFileIsDeleted() async throws {
+    try await withMainSerialExecutor {
+      try? FileManager.default.removeItem(at: .fileURL)
+
+      try await withDependencies {
+        $0.defaultFileStorage = .fileSystem
+      } operation: {
+        @Shared(.fileStorage(.fileURL)) var users = [User.blob]
+        await Task.yield()
+        XCTAssertNoDifference(users, [.blob])
+
+        $users.withLock { $0 = [.blobJr] }  // NB: Saved immediately
+        $users.withLock { $0 = [.blobSr] }  // NB: Throttled for 1 second
+        try FileManager.default.removeItem(at: .fileURL)
+        try await Task.sleep(nanoseconds: 1_200_000_000)
+        XCTAssertNoDifference(users, [.blob])
+        try XCTAssertEqual(Data(contentsOf: .fileURL), Data())
+      }
+    }
+  }
+
+  func testWritesFromManyThreads() async {
+    let fileSystem = LockIsolated<[URL: Data]>([:])
+    let fileStorage = FileStorage.inMemory(
+      fileSystem: fileSystem,
+      scheduler: DispatchQueue.main.eraseToAnyScheduler()
+    )
+
+    await withDependencies {
+      $0.defaultFileStorage = fileStorage
+    } operation: {
+      @Shared(.fileStorage(.fileURL)) var count = 0
+      let max = 10_000
+      await withTaskGroup(of: Void.self) { group in
+        for index in (1...max) {
+          group.addTask { [count = $count] in
+            try? await Task.sleep(for: .milliseconds(Int.random(in: 200...3_000)))
+            await count.withLock { $0 += index }
+          }
+        }
+      }
+
+      XCTAssertEqual(count, max * (max + 1) / 2)
+    }
+  }
 }
 
 extension URL {