Stop the nav-data decode from blocking the main thread

SF50 TOLD/Loaders/NavDataLoader/NavDataLoader.swift

@@ -50,26 +50,43 @@ import SwiftNASR
 ///
 /// ## Progress Tracking
 ///
-/// Poll the ``state`` property to track loading progress:
+/// Iterate ``stateUpdates()`` to follow the loader's progress. The loader pushes
+/// each new ``State`` into the stream:
 ///
 /// ```swift
 /// let loader = NavDataLoader(modelContainer: container)
+/// let updates = await loader.stateUpdates()
 /// Task {
-///     while true {
-///         switch await loader.state {
+///     for await state in updates {
+///         switch state {
 ///         case .downloading(let progress):
 ///             print("Downloading: \(progress ?? 0)")
 ///         case .loading(let progress):
 ///             print("Importing: \(progress ?? 0)")
 ///         default:
 ///             break
 ///         }
-///         try? await Task.sleep(for: .seconds(0.25))
 ///     }
 /// }
 /// let result = try await loader.load()
 /// ```
 ///
+/// ## Executor Constraints
+///
+/// A `@ModelActor`'s serial executor is its `NSManagedObjectContext`'s dispatch
+/// queue, and SwiftData enqueues jobs onto that executor with
+/// `-[NSManagedObjectContext performBlockAndWait:]`. Enqueueing therefore blocks
+/// the *calling* thread until the executor is free, so every caller — the main
+/// actor included — stalls for as long as this actor stays busy. Two rules
+/// follow, and both are load-bearing for main-thread responsiveness:
+///
+/// - No long-running work may occupy the executor without suspending. CPU-bound
+///   work belongs in a `nonisolated` `@concurrent` function the actor `await`s,
+///   and persistence work is split into bounded batches separated by `await`.
+/// - Progress is pushed out through ``stateUpdates()`` rather than pulled by
+///   callers, because yielding into an `AsyncStream` is nonblocking whereas
+///   reading an isolated property is an enqueue.
+///
 /// ## See Also
 ///
 /// - ``NavDataLoaderViewModel``
@@ -89,27 +106,63 @@ actor NavDataLoader {
   /// Pause between batch saves so other store users can interleave.
   private static let interBatchPause: Duration = .milliseconds(50)
 
-  var state: State = .idle
+  /// Smallest change in download progress worth pushing to consumers.
+  ///
+  /// The download reports progress once per 8 KB chunk, which is far finer than
+  /// a progress indicator can show; coarsening it keeps consumers from waking
+  /// hundreds of times a second for changes they cannot render.
+  private static let progressReportingStep: Float = 0.005
 
-  private let decoder = PropertyListDecoder()
+  private(set) var state: State = .idle {
+    didSet { stateContinuation?.yield(state) }
+  }
 
   private let logger = Logger(
     subsystem: "codes.tim.SF50-TOLD",
     category: "NavDataLoader"
   )
 
   private var navaidLookup: [String: SF50_Shared.Navaid] = [:]
+  private var stateContinuation: AsyncStream<State>.Continuation?
 
   private var dataURL: URL {
     URL(string: String(format: Self.dataURLTemplate, "\(Cycle.effective)"))!
   }
 
+  /// Inflates the LZMA payload and decodes it, off this actor's executor.
+  ///
+  /// Both steps are CPU-bound and touch no `modelContext`, so they run on the
+  /// concurrent pool while the actor suspends.
+  @concurrent
+  nonisolated private static func decompress(data: Data) async throws -> AirportDataCodable {
+    // swiftlint:disable:next legacy_objc_type
+    let inflated = try (data as NSData).decompressed(using: .lzma)
+    return try PropertyListDecoder().decode(AirportDataCodable.self, from: inflated as Data)
+  }
+
+  /// A stream of ``State`` values, starting with the loader's current state and
+  /// finishing when ``load()`` returns or throws.
+  ///
+  /// Only one stream is live at a time; a second call finishes the previous one.
+  func stateUpdates() -> AsyncStream<State> {
+    stateContinuation?.finish()
+    let (stream, continuation) = AsyncStream.makeStream(
+      of: State.self,
+      bufferingPolicy: .bufferingNewest(1)
+    )
+    continuation.yield(state)
+    stateContinuation = continuation
+    return stream
+  }
+
   func load() async throws -> LoadResult {
+    defer { stateContinuation?.finish() }
+
     state = .downloading(progress: 0)
-    let data = try await download { self.state = .downloading(progress: $0) }
+    let data = try await download { self.reportDownloadProgress($0) }
 
     state = .extracting(progress: nil)
-    let nasr = try decompress(data: data)
+    let nasr = try await Self.decompress(data: data)
 
     // The replacement data is fully decoded, so the old dataset can go
     try await resetData()
@@ -170,6 +223,12 @@ actor NavDataLoader {
     )
   }
 
+  private func reportDownloadProgress(_ progress: Float) {
+    guard case .downloading(let reported) = state else { return }
+    if let reported, abs(progress - reported) < Self.progressReportingStep { return }
+    state = .downloading(progress: progress)
+  }
+
   private func download(progress: (Float) -> Void) async throws -> Data {
     try await withRetry(logger: logger, label: "nav data") {
       let session = URLSession(configuration: .ephemeral)
@@ -192,12 +251,6 @@ actor NavDataLoader {
     }
   }
 
-  private func decompress(data: Data) throws -> AirportDataCodable {
-    // swiftlint:disable:next legacy_objc_type
-    let data = try (data as NSData).decompressed(using: .lzma)
-    return try decoder.decode(AirportDataCodable.self, from: data as Data)
-  }
-
   private func loadAirports(
     _ airports: [AirportDataCodable.AirportCodable],
     progress: (Int) -> Void
@@ -441,7 +494,7 @@ actor NavDataLoader {
   /// - ``extracting(progress:)``: Decompressing LZMA data
   /// - ``loading(progress:)``: Importing into SwiftData (0.0-1.0)
   /// - ``finished``: Complete
-  enum State {
+  enum State: Sendable {
     case idle
     case downloading(progress: Float?)
     case extracting(progress: Float?)

SF50 TOLD/Loaders/NavDataLoader/NavDataLoaderViewModel.swift

@@ -43,11 +43,32 @@ final class NavDataLoaderViewModel: WithIdentifiableError {
     (noData || needsLoad) && !deferred
   }
 
+  /// Whether a fresh import may start, so a second tap cannot spawn a
+  /// concurrent importer on the same store.
+  private var canStartLoad: Bool {
+    switch state {
+      case .idle, .finished: true
+      default: false
+    }
+  }
+
   init(container: ModelContainer) {
     self.container = container
     setupObservation()
   }
 
+  /// Builds the importer's loader away from the main actor.
+  ///
+  /// Opening the importer's container brings up a second persistent store
+  /// coordinator, which touches the filesystem, so it runs on the concurrent
+  /// pool rather than on the main thread at the moment the user taps Load.
+  @concurrent
+  nonisolated private static func makeImportLoader(
+    matching container: ModelContainer
+  ) async throws -> NavDataLoader {
+    NavDataLoader(modelContainer: try makeImportContainer(matching: container))
+  }
+
   /// Creates a standalone container on the same store for the importer.
   ///
   /// The importer's bulk transactions then queue on their own persistent store
@@ -118,76 +139,87 @@ final class NavDataLoaderViewModel: WithIdentifiableError {
   }
 
   func load() {
-    // A second tap must not spawn a concurrent importer on the same store
-    switch state {
-      case .idle, .finished: break
-      default: return
-    }
+    guard canStartLoad else { return }
 
-    let loader: NavDataLoader
+    // Block re-entry and switch to the progress UI before the actor reports
+    state = .downloading(progress: nil)
+
+    addTask(Task { await runLoad() })
+  }
+
+  func loadLater() {
+    if canSkip { deferred = true }
+  }
+
+  private func runLoad() async {
+    guard let loader = await makeLoader() else { return }
+    let progressTask = await observeProgress(of: loader)
+    addTask(progressTask)
+    await performLoad(with: loader, progressTask: progressTask)
+  }
+
+  private func makeLoader() async -> NavDataLoader? {
     do {
-      loader = NavDataLoader(modelContainer: try Self.makeImportContainer(matching: container))
+      return try await Self.makeImportLoader(matching: container)
     } catch {
       SentrySDK.capture(error: error) { scope in
         scope.setTag(value: "importContainer", key: "navData.operation")
         scope.setFingerprint(["navData", "importContainer"])
       }
       self.error = error
-      return
+
+      // Return to the consent screen so the user can retry
+      state = .idle
+      return nil
     }
+  }
 
-    // Block re-entry and switch to the progress UI before the actor reports
-    state = .downloading(progress: nil)
+  /// Mirrors the loader's pushed state onto the main actor for the progress UI.
+  ///
+  /// The loader yields into an `AsyncStream`, so following its progress never
+  /// enqueues a job onto the loader's executor — an enqueue would block the main
+  /// thread for as long as the import occupies that executor.
+  private func observeProgress(of loader: NavDataLoader) async -> Task<Void, Never> {
+    let updates = await loader.stateUpdates()
+    return Task { [weak self] in
+      for await loaderState in updates where !Task.isCancelled {
+        guard let self else { return }
 
-    let progressTask = Task { [weak self] in
-      while !Task.isCancelled {
-        let loaderState = await loader.state
-        guard !Task.isCancelled else { return }
-        if case .idle = loaderState {
-          // The actor hasn't begun loading; don't regress the UI to consent
-        } else {
-          self?.state = loaderState
-        }
-        try? await Task.sleep(for: .seconds(0.25))
+        // The actor hasn't begun loading; don't regress the UI to consent
+        if case .idle = loaderState { continue }
+        state = loaderState
       }
     }
-    addTask(progressTask)
+  }
 
-    addTask(
-      Task {
-        let transaction = SentrySDK.startTransaction(
-          name: "Nav Data Load",
-          operation: "navData.load"
-        )
-        defer { progressTask.cancel() }
-        do {
-          error = nil
-          try await loader.clearCycles()
-          Defaults[.ourAirportsLastUpdated] = nil
-          let result = try await loader.load()
-          state = await loader.state
-
-          Defaults[.ourAirportsLastUpdated] = result.ourAirportsLastUpdated
-          Defaults[.schemaVersion] = latestSchemaVersion
-          transaction.finish()
-        } catch {
-          transaction.finish(status: .internalError)
-          SentrySDK.capture(error: error) { scope in
-            scope.setTag(value: "load", key: "navData.operation")
-            scope.setFingerprint(["navData", "load"])
-          }
-          self.error = error
-
-          // Return to the consent screen so the user can retry the download
-          progressTask.cancel()
-          state = .idle
-        }
-      }
+  private func performLoad(with loader: NavDataLoader, progressTask: Task<Void, Never>) async {
+    let transaction = SentrySDK.startTransaction(
+      name: "Nav Data Load",
+      operation: "navData.load"
     )
-  }
+    defer { progressTask.cancel() }
+    do {
+      error = nil
+      try await loader.clearCycles()
+      Defaults[.ourAirportsLastUpdated] = nil
+      let result = try await loader.load()
+      state = .finished
 
-  func loadLater() {
-    if canSkip { deferred = true }
+      Defaults[.ourAirportsLastUpdated] = result.ourAirportsLastUpdated
+      Defaults[.schemaVersion] = latestSchemaVersion
+      transaction.finish()
+    } catch {
+      transaction.finish(status: .internalError)
+      SentrySDK.capture(error: error) { scope in
+        scope.setTag(value: "load", key: "navData.operation")
+        scope.setFingerprint(["navData", "load"])
+      }
+      self.error = error
+
+      // Return to the consent screen so the user can retry the download
+      progressTask.cancel()
+      state = .idle
+    }
   }
 
   private func applyState(_ state: NavDataStateHelper.State) {