crossmate

A collaborative crossword app for iOS
Log | Files | Refs | LICENSE

GameView.md (8088B)


      1 # Per-keystroke hitches on populated databases
      2 
      3 ## Symptom
      4 
      5 Solving a puzzle on iPhone 14 Pro Max produces visible hitches as letters
      6 appear in the grid. The same build on a freshly-set-up iPhone 13 Mini (new
      7 Apple Account, empty store) is smooth. Disabling iCloud sync in Settings does
      8 not help. Disabling ProMotion (removing `CADisableMinimumFrameDurationOnPhone`
      9 from `Info.plist`) does not help. Running "Reset Database" from the
     10 diagnostics screen restores responsiveness immediately.
     11 
     12 The shared property of the three things that *did* affect responsiveness
     13 (brand-new account, reset database) is **how much data the local Core Data
     14 store contains**, not network or display behaviour.
     15 
     16 ## Diagnosis
     17 
     18 Per-keystroke trace:
     19 
     20 1. The user types a letter. `GameMutator.setLetter` updates the in-memory
     21    `Game` and calls `emitMove`, which schedules
     22    `MoveBuffer.enqueue(...)` on the `MoveBuffer` actor.
     23 2. If the new edit targets a different cell than the previous one,
     24    `enqueue` triggers `performFlush()` for Lamport ordering.
     25 3. `performFlush` calls `persistAndAssignLamports`, which on a background
     26    context inserts a `MoveEntity`, updates the `GameEntity`
     27    (`lamportHighWater`, `updatedAt`), updates a `CellEntity`, and saves.
     28 4. `viewContext.automaticallyMergesChangesFromParent = true`
     29    (`PersistenceController.swift:39`) merges the background save into the
     30    main context.
     31 5. `GameListView` declares
     32    `@FetchRequest<GameEntity>(sortDescriptors: [], animation: .default)`
     33    (`GameListView.swift:11-15`). The merge invalidates the fetch request,
     34    which marks `GameListView` dirty.
     35 6. SwiftUI recomputes `GameListView.body`. The body reads two computed
     36    properties:
     37 
     38    ```swift
     39    private var inProgress: [GameSummary] {
     40        games.compactMap(GameSummary.init(entity:))
     41            .filter { $0.completedAt == nil }
     42            .sorted { ... }
     43    }
     44    private var completed: [GameSummary] {
     45        games.compactMap(GameSummary.init(entity:))
     46            .filter { $0.completedAt != nil }
     47            .sorted { ... }
     48    }
     49    ```
     50 
     51    Each calls `games.compactMap(GameSummary.init(entity:))` independently.
     52 7. `GameSummary.init(entity:)` (`GameStore.swift:34-77`) calls
     53    `XD.parse(source)`, constructs a full `Puzzle`, and walks every
     54    `CellEntity` to build the thumbnail array.
     55 
     56 So per keystroke, on the main thread:
     57 
     58 ```
     59 GameCount × 2 × (XD.parse + Puzzle init + thumbnail build)
     60 ```
     61 
     62 With one game in the store the cost is invisible. With many games it is
     63 proportional to library size and lands on every cell-change keystroke.
     64 
     65 ## Why this matches every observation
     66 
     67 - **Empty DB after reset → fast.** The `@FetchRequest` returns nothing, so
     68   the body has no per-game work to do.
     69 - **iCloud disabled didn't help.** The work is local: a Core Data save in
     70   the view context is enough to invalidate the fetch request. Network
     71   state is irrelevant.
     72 - **ProMotion disabled didn't help.** The cost is real and on the main
     73   thread. Capping the refresh rate only changes whether the dropped frame
     74   is visible, not whether the work happens.
     75 - **iPhone 13 Mini (new account) was fast.** Same code, but the library was
     76   empty, so the FetchRequest had nothing to iterate.
     77 - **Got worse over time.** Every game added to the library multiplies the
     78   per-keystroke cost.
     79 
     80 ## Why `GameListView` is involved while `PuzzleView` is on screen
     81 
     82 `NavigationStack` does **not** unmount the root view when a destination is
     83 pushed; it only covers it visually. The root remains in the view hierarchy,
     84 its property wrappers stay live, and SwiftUI continues to invalidate and
     85 re-evaluate its body whenever a tracked dependency changes. `@FetchRequest`
     86 is a Core Data observer; it fires on context changes regardless of whether
     87 the view that owns it is currently visible. SwiftUI does not skip body
     88 evaluation for "covered" views, because visibility is a render-tree concept
     89 while body evaluation is a data-graph concept — it has to compute the new
     90 body to know whether anything visible changed (toolbar, title, navigation
     91 state, transitions, etc.).
     92 
     93 `animation: .default` on the fetch request also wraps each invalidation in
     94 an animation transaction, which can amplify the perceived hitch.
     95 
     96 ## Secondary issue (not the root cause of the hitch)
     97 
     98 `MoveEntity` rows are only pruned after CloudKit confirms the snapshot save.
     99 `AppServices.swift:71` wraps the entire `afterFlush` callback in
    100 `guard isEnabled` — meaning that with iCloud sync disabled, snapshots are
    101 never created and moves are never pruned. The move table grows unbounded.
    102 This is not what is producing the per-keystroke hitch (the dominant cost is
    103 the FetchRequest + `XD.parse`), but it is a real correctness/space concern
    104 worth fixing separately so users running with sync off don't accumulate
    105 forever.
    106 
    107 ## Verification
    108 
    109 Drop a one-line probe at the top of `GameListView.body`:
    110 
    111 ```swift
    112 let _ = print("GameListView body")
    113 ```
    114 
    115 Run a build against the populated database. If the print fires on every
    116 cell-change keystroke, the diagnosis is correct.
    117 
    118 ## Fixes
    119 
    120 ### Recommended: gate `@FetchRequest` on visibility
    121 
    122 Stop doing the work entirely while the puzzle is on screen. The
    123 `@FetchRequest` only observes Core Data because the view that owns it is
    124 live. Removing the owning view from the hierarchy tears down the underlying
    125 `NSFetchedResultsController`. `NavigationStack` fires `.onDisappear` on the
    126 root when a destination is pushed, which is the signal we need.
    127 
    128 ```swift
    129 struct GameListView: View {
    130     let store: GameStore
    131     let shareController: ShareController
    132     @Binding var navigationPath: NavigationPath
    133 
    134     @State private var isActive = true
    135 
    136     var body: some View {
    137         Group {
    138             if isActive {
    139                 GameListContent(
    140                     store: store,
    141                     shareController: shareController,
    142                     navigationPath: $navigationPath
    143                 )
    144             } else {
    145                 Color.clear
    146             }
    147         }
    148         .onAppear { isActive = true }
    149         .onDisappear { isActive = false }
    150     }
    151 }
    152 
    153 private struct GameListContent: View {
    154     // current GameListView body, including:
    155     @FetchRequest(sortDescriptors: [], animation: .default)
    156     private var games: FetchedResults<GameEntity>
    157     // ...
    158 }
    159 ```
    160 
    161 Push `PuzzleView` → `.onDisappear` → `isActive = false` →
    162 `GameListContent` is removed → `@FetchRequest` is torn down → keystrokes no
    163 longer have a SwiftUI subscriber to invalidate. Pop back → `.onAppear` →
    164 `isActive = true` → fresh fetch, list reflects whatever changed.
    165 
    166 Caveats:
    167 
    168 1. `.onDisappear` also fires when a sheet covers the list (`NewGameSheet`,
    169    `SettingsView`). That's fine — list is not visible then either — but
    170    quick sheet dismissals will flap `isActive`. If that becomes a problem,
    171    gate on debounce or on `navigationPath.count > 0` instead.
    172 2. The first frame after popping back has to fetch and build summaries.
    173    For a small-to-moderate library this should be imperceptible; if it
    174    isn't, render a cached lightweight snapshot in the inactive branch.
    175 3. This addresses the "while typing" case only. Opening the library on a
    176    populated database still triggers the same `XD.parse`-per-game work
    177    during scrolling and resizing. If that becomes a problem, the longer-
    178    term fix is to cache derived summary fields (title, publisher, date,
    179    gridWidth, gridHeight, blockMask) directly on `GameEntity` so the list
    180    path never parses XD.
    181 
    182 ### Alternative: cache derived puzzle data on `GameEntity`
    183 
    184 Pre-compute and store everything `GameSummary` needs (title, publisher,
    185 date, width, height, block mask) on `GameEntity` at creation time. `XD.parse`
    186 disappears from the hot path entirely. This is a bigger change (schema +
    187 migration + initialisation) but fixes both the "while typing" case and the
    188 "opening a populated library" case.
    189 
    190 ### Cheap partial: deduplicate the iteration
    191 
    192 `inProgress` and `completed` both iterate `games.compactMap(...)`. Compute
    193 the summaries once per body and partition. Halves the constant but stays
    194 O(games) per keystroke; not a real fix.