From aace5a95bddcdbc771a2c0c6dbccd985bc8f6c41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 19 Mar 2026 16:59:31 +0000 Subject: [PATCH 1/3] perf: frozen collection snapshots with lazy rebuild and per-key merge Replace mutable collectionData with frozen collectionSnapshots and dirty tracking. Collection snapshots are lazily rebuilt on read, returning stable frozen references for structural sharing. The merge() method now operates per-key instead of spreading the entire storageMap, and hasValueChanged() uses reference equality as a fast path. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/OnyxCache.ts | 204 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 153 insertions(+), 51 deletions(-) diff --git a/lib/OnyxCache.ts b/lib/OnyxCache.ts index b7526a1c6..7fdb3a64c 100644 --- a/lib/OnyxCache.ts +++ b/lib/OnyxCache.ts @@ -2,9 +2,19 @@ import {deepEqual} from 'fast-equals'; import bindAll from 'lodash/bindAll'; import type {ValueOf} from 'type-fest'; import utils from './utils'; -import type {OnyxKey, OnyxValue} from './types'; +import type {CollectionKeyBase, KeyValueMapping, NonUndefined, OnyxCollection, OnyxKey, OnyxValue} from './types'; import OnyxKeys from './OnyxKeys'; +/** Frozen object containing all collection members — safe to return by reference */ +type CollectionSnapshot = Readonly>>; + +/** + * Stable frozen empty object used as the canonical value for empty collections. + * Returning the same reference avoids unnecessary re-renders in useSyncExternalStore, + * which relies on === equality to detect changes. + */ +const FROZEN_EMPTY_COLLECTION: Readonly>> = Object.freeze({}); + // Task constants const TASK = { GET: 'get', @@ -31,9 +41,6 @@ class OnyxCache { /** A map of cached values */ private storageMap: Record>; - /** Cache of complete collection data objects for O(1) retrieval */ - private collectionData: Record>>; - /** * Captured pending tasks for already running storage methods * Using a map yields better performance on operations such a delete @@ -52,13 +59,20 @@ class OnyxCache { /** List of keys that have been directly subscribed to or recently modified from least to most recent */ private recentlyAccessedKeys = new Set(); + /** Frozen collection snapshots for structural sharing */ + private collectionSnapshots: Map; + + /** Collections whose snapshots need rebuilding (lazy — rebuilt on next read) */ + private dirtyCollections: Set; + constructor() { this.storageKeys = new Set(); this.nullishStorageKeys = new Set(); this.recentKeys = new Set(); this.storageMap = {}; - this.collectionData = {}; this.pendingPromises = new Map(); + this.collectionSnapshots = new Map(); + this.dirtyCollections = new Set(); // bind all public methods to prevent problems with `this` bindAll( @@ -88,8 +102,8 @@ class OnyxCache { 'addEvictableKeysToRecentlyAccessedList', 'getKeyForEviction', 'setCollectionKeys', - 'getCollectionData', 'hasValueChanged', + 'getCollectionData', ); } @@ -168,24 +182,21 @@ class OnyxCache { this.nullishStorageKeys.delete(key); const collectionKey = OnyxKeys.getCollectionKey(key); + const oldValue = this.storageMap[key]; + if (value === null || value === undefined) { delete this.storageMap[key]; - // Remove from collection data cache if it's a collection member - if (collectionKey && this.collectionData[collectionKey]) { - delete this.collectionData[collectionKey][key]; + if (collectionKey && oldValue !== undefined) { + this.dirtyCollections.add(collectionKey); } return undefined; } this.storageMap[key] = value; - // Update collection data cache if this is a collection member - if (collectionKey) { - if (!this.collectionData[collectionKey]) { - this.collectionData[collectionKey] = {}; - } - this.collectionData[collectionKey][key] = value; + if (collectionKey && oldValue !== value) { + this.dirtyCollections.add(collectionKey); } return value; @@ -195,15 +206,14 @@ class OnyxCache { drop(key: OnyxKey): void { delete this.storageMap[key]; - // Remove from collection data cache if this is a collection member const collectionKey = OnyxKeys.getCollectionKey(key); - if (collectionKey && this.collectionData[collectionKey]) { - delete this.collectionData[collectionKey][key]; + if (collectionKey) { + this.dirtyCollections.add(collectionKey); } - // If this is a collection key, clear its data + // If this is a collection key, clear its snapshot if (OnyxKeys.isCollectionKey(key)) { - delete this.collectionData[key]; + this.collectionSnapshots.delete(key); } this.storageKeys.delete(key); @@ -220,12 +230,7 @@ class OnyxCache { throw new Error('data passed to cache.merge() must be an Object of onyx key/value pairs'); } - this.storageMap = { - ...utils.fastMerge(this.storageMap, data, { - shouldRemoveNestedNulls: true, - objectRemovalMode: 'replace', - }).result, - }; + const affectedCollections = new Set(); for (const [key, value] of Object.entries(data)) { this.addKey(key); @@ -233,25 +238,47 @@ class OnyxCache { const collectionKey = OnyxKeys.getCollectionKey(key); - if (value === null || value === undefined) { + if (value === undefined) { this.addNullishStorageKey(key); + // undefined means "no change" — skip storageMap modification + continue; + } + + if (value === null) { + this.addNullishStorageKey(key); + delete this.storageMap[key]; - // Remove from collection data cache if it's a collection member - if (collectionKey && this.collectionData[collectionKey]) { - delete this.collectionData[collectionKey][key]; + if (collectionKey) { + affectedCollections.add(collectionKey); } } else { this.nullishStorageKeys.delete(key); - // Update collection data cache if this is a collection member + // Per-key merge instead of spreading the entire storageMap + const existing = this.storageMap[key]; + const merged = utils.fastMerge(existing, value, { + shouldRemoveNestedNulls: true, + objectRemovalMode: 'replace', + }).result; + + // fastMerge is reference-stable: returns the original target when + // nothing changed, so a simple === check detects no-ops. + if (merged === existing) { + continue; + } + + this.storageMap[key] = merged; + if (collectionKey) { - if (!this.collectionData[collectionKey]) { - this.collectionData[collectionKey] = {}; - } - this.collectionData[collectionKey][key] = this.storageMap[key]; + affectedCollections.add(collectionKey); } } } + + // Mark affected collections as dirty — snapshots will be lazily rebuilt on next read + for (const collectionKey of affectedCollections) { + this.dirtyCollections.add(collectionKey); + } } /** @@ -317,16 +344,22 @@ class OnyxCache { iterResult = iterator.next(); } + const affectedCollections = new Set(); + for (const key of keysToRemove) { delete this.storageMap[key]; - // Remove from collection data cache if this is a collection member + // Track affected collections for snapshot rebuild const collectionKey = OnyxKeys.getCollectionKey(key); - if (collectionKey && this.collectionData[collectionKey]) { - delete this.collectionData[collectionKey][key]; + if (collectionKey) { + affectedCollections.add(collectionKey); } this.recentKeys.delete(key); } + + for (const collectionKey of affectedCollections) { + this.dirtyCollections.add(collectionKey); + } } /** Set the recent keys list size */ @@ -334,9 +367,12 @@ class OnyxCache { this.maxRecentKeysSize = limit; } - /** Check if the value has changed */ + /** Check if the value has changed. Uses reference equality as a fast path, falls back to deep equality. */ hasValueChanged(key: OnyxKey, value: OnyxValue): boolean { - const currentValue = this.get(key, false); + const currentValue = this.storageMap[key]; + if (currentValue === value) { + return false; + } return !deepEqual(currentValue, value); } @@ -425,33 +461,99 @@ class OnyxCache { setCollectionKeys(collectionKeys: Set): void { OnyxKeys.setCollectionKeys(collectionKeys); - // Initialize collection data for existing collection keys + // Initialize frozen snapshots for collection keys for (const collectionKey of collectionKeys) { - if (this.collectionData[collectionKey]) { - continue; + if (!this.collectionSnapshots.has(collectionKey)) { + this.collectionSnapshots.set(collectionKey, Object.freeze({})); } - this.collectionData[collectionKey] = {}; } - // Register existing storageKeys with OnyxKeys + // Pre-populate the reverse lookup map for any existing keys for (const key of this.storageKeys) { OnyxKeys.registerMemberKey(key); } } /** - * Get all data for a collection key + * Rebuilds the frozen collection snapshot from current storageMap references. + * Uses the indexed collection->members map for O(collectionMembers) instead of O(totalKeys). + * Returns the previous snapshot reference when all member references are identical, + * preventing unnecessary re-renders in useSyncExternalStore. + * + * @param collectionKey - The collection key to rebuild + */ + private rebuildCollectionSnapshot(collectionKey: OnyxKey): void { + const oldSnapshot = this.collectionSnapshots.get(collectionKey); + + const members: NonUndefined> = {}; + let hasChanges = false; + let newMemberCount = 0; + + // Use the indexed forward lookup for O(collectionMembers) iteration. + // Falls back to scanning all storageKeys if the index isn't populated yet. + const memberKeys = OnyxKeys.getMembersOfCollection(collectionKey); + const keysToScan = memberKeys ?? this.storageKeys; + const needsPrefixCheck = !memberKeys; + + for (const key of keysToScan) { + if (needsPrefixCheck && !OnyxKeys.isCollectionMemberKey(collectionKey, key)) { + continue; + } + const val = this.storageMap[key]; + if (val !== undefined && val !== null) { + members[key] = val; + newMemberCount++; + + // Check if this member's reference changed from the old snapshot + if (!hasChanges && (!oldSnapshot || oldSnapshot[key] !== val)) { + hasChanges = true; + } + } + } + + // Check if any members were removed (old snapshot had more keys) + if (!hasChanges && oldSnapshot) { + const oldMemberCount = Object.keys(oldSnapshot).length; + if (oldMemberCount !== newMemberCount) { + hasChanges = true; + } + } + + // If nothing actually changed, reuse the old snapshot reference. + // This is critical: useSyncExternalStore uses === to detect changes, + // so returning the same reference prevents unnecessary re-renders. + if (!hasChanges && oldSnapshot) { + return; + } + + Object.freeze(members); + + this.collectionSnapshots.set(collectionKey, members); + } + + /** + * Get all data for a collection key. + * Returns a frozen snapshot with structural sharing — safe to return by reference. + * Lazily rebuilds the snapshot if the collection was modified since the last read. */ getCollectionData(collectionKey: OnyxKey): Record> | undefined { - const cachedCollection = this.collectionData[collectionKey]; - if (!cachedCollection || Object.keys(cachedCollection).length === 0) { + if (this.dirtyCollections.has(collectionKey)) { + this.rebuildCollectionSnapshot(collectionKey); + this.dirtyCollections.delete(collectionKey); + } + + const snapshot = this.collectionSnapshots.get(collectionKey); + if (!snapshot || Object.keys(snapshot).length === 0) { + // If we know we have storage keys loaded, return a stable empty reference + // to avoid new {} allocations that break useSyncExternalStore === equality. + if (this.storageKeys.size > 0) { + return FROZEN_EMPTY_COLLECTION; + } return undefined; } - // Return a shallow copy to ensure React detects changes when items are added/removed - return {...cachedCollection}; + return snapshot; } - } const instance = new OnyxCache(); From 7f580cee2d578c9c8707c194a0198992b00b6040 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 3 Apr 2026 16:05:33 +0100 Subject: [PATCH 2/3] Add unit tests --- tests/unit/onyxCacheTest.tsx | 153 +++++++++++++++++++++++++++++++++++ 1 file changed, 153 insertions(+) diff --git a/tests/unit/onyxCacheTest.tsx b/tests/unit/onyxCacheTest.tsx index fdaed1d51..70a227a0b 100644 --- a/tests/unit/onyxCacheTest.tsx +++ b/tests/unit/onyxCacheTest.tsx @@ -754,6 +754,159 @@ describe('Onyx', () => { }); }); + describe('getCollectionData', () => { + it('should return a frozen object', async () => { + await initOnyx(); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 1}); + + const result = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + expect(result).toBeDefined(); + expect(Object.isFrozen(result)).toBe(true); + }); + + it('should return the same reference when nothing changed', async () => { + await initOnyx(); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 1}); + + const first = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + const second = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + expect(first).toBe(second); + }); + + it('should return a new reference after a member is updated', async () => { + await initOnyx(); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 1}); + + const before = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 2}); + const after = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + + expect(before).not.toBe(after); + expect(after).toEqual({[`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`]: {id: 2}}); + }); + + it('should return a new reference after a member is added', async () => { + await initOnyx(); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 1}); + + const before = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}2`, {id: 2}); + const after = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + + expect(before).not.toBe(after); + expect(after).toEqual({ + [`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`]: {id: 1}, + [`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}2`]: {id: 2}, + }); + }); + + it('should return a stable empty reference for empty collections when keys are loaded', async () => { + await initOnyx(); + // Set a key so storageKeys is non-empty, but not a member of MOCK_COLLECTION + await Onyx.set(ONYX_KEYS.TEST_KEY, 'value'); + + const first = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + const second = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + + expect(first).toBeDefined(); + expect(first).toBe(second); + expect(Object.keys(first!)).toHaveLength(0); + }); + + it('should return undefined for empty collections when no keys are loaded', async () => { + await initOnyx(); + + const result = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + expect(result).toBeUndefined(); + }); + + it('should preserve unchanged member references when a sibling is updated', async () => { + await initOnyx(); + const member1Value = {id: 1, name: 'unchanged'}; + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, member1Value); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}2`, {id: 2}); + + const before = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}2`, {id: 3}); + const after = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + + // Snapshot reference changed (sibling updated) + expect(before).not.toBe(after); + // But unchanged member keeps the same reference + expect(after?.[`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`]).toBe(member1Value); + }); + }); + + describe('hasValueChanged', () => { + it('should return false for the same reference (fast path)', async () => { + await initOnyx(); + const value = {id: 1, name: 'test'}; + cache.set('test', value); + + expect(cache.hasValueChanged('test', value)).toBe(false); + }); + + it('should return false for deep-equal but different reference', async () => { + await initOnyx(); + cache.set('test', {id: 1, name: 'test'}); + + expect(cache.hasValueChanged('test', {id: 1, name: 'test'})).toBe(false); + }); + + it('should return true when value differs', async () => { + await initOnyx(); + cache.set('test', {id: 1}); + + expect(cache.hasValueChanged('test', {id: 2})).toBe(true); + }); + }); + + describe('merge', () => { + it('should not mark collection dirty when merged value is unchanged', async () => { + await initOnyx(); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 1, name: 'test'}); + + const before = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + + // Merge with identical values — fastMerge returns same reference, so no-op + cache.merge({[`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`]: {id: 1, name: 'test'}}); + + const after = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + expect(before).toBe(after); + }); + + it('should mark collection dirty when a member value changes', async () => { + await initOnyx(); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 1}); + + const before = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + + cache.merge({[`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`]: {id: 2}}); + + const after = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + expect(before).not.toBe(after); + expect(after![`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`]).toEqual({id: 2}); + }); + + it('should handle null values by removing the key from storageMap', async () => { + await initOnyx(); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 1}); + + cache.merge({[`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`]: null}); + + expect(cache.get(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`)).toBeUndefined(); + }); + + it('should skip undefined values without modifying storageMap', async () => { + await initOnyx(); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 1}); + + cache.merge({[`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`]: undefined}); + + expect(cache.get(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`)).toEqual({id: 1}); + }); + }); + it('should save RAM-only keys', () => { const testKeys = { ...ONYX_KEYS, From eac67740531ab1c5ebd796ef05d948db2f6d1599 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 3 Apr 2026 17:34:22 +0100 Subject: [PATCH 3/3] fix: use longest-prefix matching in rebuildCollectionSnapshot fallback scan --- lib/OnyxCache.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/OnyxCache.ts b/lib/OnyxCache.ts index 881fd91b9..063ea5eb3 100644 --- a/lib/OnyxCache.ts +++ b/lib/OnyxCache.ts @@ -491,7 +491,7 @@ class OnyxCache { const needsPrefixCheck = !memberKeys; for (const key of keysToScan) { - if (needsPrefixCheck && !OnyxKeys.isCollectionMemberKey(collectionKey, key)) { + if (needsPrefixCheck && OnyxKeys.getCollectionKey(key) !== collectionKey) { continue; } const val = this.storageMap[key];