Skip to content

Commit ec005f6

Browse files
committed
fix: optimize collection snapshot rebuild with indexed lookup and reference stability
1 parent 73444b3 commit ec005f6

2 files changed

Lines changed: 87 additions & 13 deletions

File tree

lib/OnyxCache.ts

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,11 @@ class OnyxCache {
130130
*/
131131
setAllKeys(keys: OnyxKey[]) {
132132
this.storageKeys = new Set(keys);
133+
134+
// Register all keys in the member→collection forward index
135+
for (const key of keys) {
136+
OnyxKeys.registerMemberKey(key);
137+
}
133138
}
134139

135140
/** Saves a key in the storage keys list
@@ -486,33 +491,62 @@ class OnyxCache {
486491

487492
/**
488493
* Rebuilds the frozen collection snapshot from current storageMap references.
489-
* Derives membership from the previous snapshot (existing members) + additionalKeys (newly added members).
490-
* Removed members are naturally excluded because they're gone from storageMap.
491-
* The snapshot uses structural sharing: unchanged members keep their original references.
494+
* Uses the indexed collection→members map for O(collectionMembers) instead of O(totalKeys).
495+
* Returns the previous snapshot reference when all member references are identical,
496+
* preventing unnecessary re-renders in useSyncExternalStore.
492497
*
493498
* @param collectionKey - The collection key to rebuild
494-
* @param additionalKeys - New member keys not yet in the previous snapshot (single key or set of keys)
495499
*/
496500
private rebuildCollectionSnapshot(collectionKey: OnyxKey): void {
497501
const existing = this.collectionSnapshots.get(collectionKey);
498-
const newVersion = (existing?.version ?? 0) + 1;
502+
const oldSnapshot = existing?.snapshot;
499503

500504
const members: NonUndefined<OnyxCollection<KeyValueMapping[OnyxKey]>> = {};
505+
let hasChanges = false;
506+
let newMemberCount = 0;
501507

502-
// Discover all current members by scanning storageKeys with prefix matching
503-
for (const key of this.storageKeys) {
504-
if (OnyxKeys.isCollectionMemberKey(collectionKey, key)) {
505-
const val = this.storageMap[key];
506-
if (val !== undefined && val !== null) {
507-
members[key] = val;
508+
// Use the indexed forward lookup for O(collectionMembers) iteration.
509+
// Falls back to scanning all storageKeys if the index isn't populated yet.
510+
const memberKeys = OnyxKeys.getMembersOfCollection(collectionKey);
511+
const keysToScan = memberKeys ?? this.storageKeys;
512+
const needsPrefixCheck = !memberKeys;
513+
514+
for (const key of keysToScan) {
515+
if (needsPrefixCheck && !OnyxKeys.isCollectionMemberKey(collectionKey, key)) {
516+
continue;
517+
}
518+
const val = this.storageMap[key];
519+
if (val !== undefined && val !== null) {
520+
members[key] = val;
521+
newMemberCount++;
522+
523+
// Check if this member's reference changed from the old snapshot
524+
if (!hasChanges && (!oldSnapshot || oldSnapshot[key] !== val)) {
525+
hasChanges = true;
508526
}
509527
}
510528
}
511529

530+
// Check if any members were removed (old snapshot had more keys)
531+
if (!hasChanges && oldSnapshot) {
532+
const oldMemberCount = Object.keys(oldSnapshot).length;
533+
if (oldMemberCount !== newMemberCount) {
534+
hasChanges = true;
535+
}
536+
}
537+
538+
// If nothing actually changed, reuse the old snapshot reference.
539+
// This is critical: useSyncExternalStore uses === to detect changes,
540+
// so returning the same reference prevents unnecessary re-renders.
541+
if (!hasChanges && oldSnapshot) {
542+
// Don't even bump the version — nothing changed
543+
return;
544+
}
545+
512546
Object.freeze(members);
513547

514548
this.collectionSnapshots.set(collectionKey, {
515-
version: newVersion,
549+
version: (existing?.version ?? 0) + 1,
516550
snapshot: members,
517551
});
518552
}

lib/OnyxKeys.ts

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ let collectionKeySet = new Set<OnyxKey>();
66
/** Reverse lookup: member key → collection key for O(1) access */
77
const memberToCollectionKeyMap = new Map<OnyxKey, OnyxKey>();
88

9+
/** Forward lookup: collection key → set of member keys for O(collectionMembers) iteration */
10+
const collectionToMembersMap = new Map<OnyxKey, Set<OnyxKey>>();
11+
912
/** Set of keys that should only be stored in RAM, not persisted to storage */
1013
let ramOnlyKeySet = new Set<OnyxKey>();
1114

@@ -99,12 +102,30 @@ function getCollectionKey(key: CollectionKey | OnyxKey): string | undefined {
99102
* Called from OnyxCache.addKey() to ensure the Map stays populated.
100103
*/
101104
function registerMemberKey(key: OnyxKey): void {
102-
if (memberToCollectionKeyMap.has(key)) {
105+
const existingCollectionKey = memberToCollectionKeyMap.get(key);
106+
if (existingCollectionKey !== undefined) {
107+
// Already in reverse map — ensure forward map is also populated.
108+
// getCollectionKey() can populate memberToCollectionKeyMap without
109+
// updating collectionToMembersMap, so we must sync here.
110+
let members = collectionToMembersMap.get(existingCollectionKey);
111+
if (!members) {
112+
members = new Set();
113+
collectionToMembersMap.set(existingCollectionKey, members);
114+
}
115+
members.add(key);
103116
return;
104117
}
105118
for (const collectionKey of collectionKeySet) {
106119
if (isCollectionMemberKey(collectionKey, key)) {
107120
memberToCollectionKeyMap.set(key, collectionKey);
121+
122+
// Also register in the forward lookup (collection → members)
123+
let members = collectionToMembersMap.get(collectionKey);
124+
if (!members) {
125+
members = new Set();
126+
collectionToMembersMap.set(collectionKey, members);
127+
}
128+
members.add(key);
108129
return;
109130
}
110131
}
@@ -115,9 +136,27 @@ function registerMemberKey(key: OnyxKey): void {
115136
* Called when a key is dropped from cache.
116137
*/
117138
function deregisterMemberKey(key: OnyxKey): void {
139+
const collectionKey = memberToCollectionKeyMap.get(key);
140+
if (collectionKey) {
141+
const members = collectionToMembersMap.get(collectionKey);
142+
if (members) {
143+
members.delete(key);
144+
if (members.size === 0) {
145+
collectionToMembersMap.delete(collectionKey);
146+
}
147+
}
148+
}
118149
memberToCollectionKeyMap.delete(key);
119150
}
120151

152+
/**
153+
* Returns the set of member keys for a given collection key.
154+
* O(1) lookup using the forward index.
155+
*/
156+
function getMembersOfCollection(collectionKey: OnyxKey): Set<OnyxKey> | undefined {
157+
return collectionToMembersMap.get(collectionKey);
158+
}
159+
121160
/**
122161
* Splits a collection member key into the collection key part and the ID part.
123162
*
@@ -180,6 +219,7 @@ export default {
180219
getCollectionKey,
181220
registerMemberKey,
182221
deregisterMemberKey,
222+
getMembersOfCollection,
183223
splitCollectionMemberKey,
184224
setRamOnlyKeys,
185225
isRamOnlyKey,

0 commit comments

Comments
 (0)