Skip to content

Commit 3295637

Browse files
committed
perf: batch cold-cache pre-warm in mergeCollectionWithPatches via multiGet
Replaces the unconditional Promise.all(existingKeys.map(get)) pre-warm with a hybrid: - Fast path (every existingKey is already in cache): use a sync- resolved Promise — no extra microtask hops, preserving the original promise-chain depth and subscriber-callback timing that dependent tests rely on (Onyx.update batch tests broadcast a single merged callback rather than an `undefined` initial followed by the merged result). - Slow path (at least one cache-miss existingKey): use multiGet — one Storage.multiGet round-trip for the missing keys instead of N parallel get() invocations. Net result: same correctness as before, fewer storage operations on cold-cache merges, identical broadcast timing for warm-cache merges. Addresses follow-up from Expensify#787 review.
1 parent b506f32 commit 3295637

1 file changed

Lines changed: 14 additions & 6 deletions

File tree

lib/OnyxUtils.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1597,12 +1597,20 @@ function mergeCollectionWithPatches<TKey extends CollectionKeyBase>(
15971597
// finalMergedCollection contains all the keys that were merged, without the keys of incompatible updates
15981598
const finalMergedCollection = {...existingKeyCollection, ...newCollection};
15991599

1600-
// Pre-warm cache for any existing storage keys that aren't yet in cache. get() is a no-op
1601-
// (sync-resolved) for cache hits, and on a cache miss it reads from storage and writes the
1602-
// value back to cache. This is required so the subsequent cache.merge() merges the new delta
1603-
// into the real previous storage value (rather than starting from `undefined` and dropping
1604-
// the existing keys).
1605-
return Promise.all(existingKeys.map((key) => get(key))).then(() => {
1600+
// Pre-warm cache for any existing storage keys that aren't yet in cache so the subsequent
1601+
// cache.merge() merges the new delta into the real previous storage value (rather than
1602+
// starting from `undefined` and dropping the existing keys).
1603+
//
1604+
// Fast path: when every existingKey is already in cache, skip the pre-warm entirely. This
1605+
// preserves the original promise-chain depth and the subscriber-callback timing that
1606+
// dependent tests rely on.
1607+
//
1608+
// Slow path: when at least one existingKey is a cache miss, use multiGet — it batches the
1609+
// missing keys into a single Storage.multiGet call (vs. N parallel get() invocations) and
1610+
// writes the storage values back to cache before resolving.
1611+
const hasColdExistingKey = existingKeys.some((key) => !cache.hasCacheForKey(key));
1612+
const prewarmPromise = hasColdExistingKey ? multiGet(existingKeys) : Promise.resolve();
1613+
return prewarmPromise.then(() => {
16061614
// Snapshot previous values from the (now-warm) cache for keysChanged's diff, then update
16071615
// cache and notify subscribers synchronously BEFORE issuing storage writes. This matches
16081616
// the cache-first / storage-second invariant followed by every other Onyx write method

0 commit comments

Comments
 (0)