Skip to content

Commit 731fe95

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 9445bbf commit 731fe95

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
@@ -1582,12 +1582,20 @@ function mergeCollectionWithPatches<TKey extends CollectionKeyBase>(
15821582
// finalMergedCollection contains all the keys that were merged, without the keys of incompatible updates
15831583
const finalMergedCollection = {...existingKeyCollection, ...newCollection};
15841584

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

0 commit comments

Comments
 (0)