Merge branch 'main' into fix/improve-onyxConnectionManager

Author: pasyukevich

API-INTERNAL.md

@@ -72,10 +72,6 @@ If the requested key is a collection, it will return an object with all the coll
 <dt><a href="#sendDataToConnection">sendDataToConnection()</a></dt>
 <dd><p>Sends the data obtained from the keys to the connection.</p>
 </dd>
-<dt><a href="#addKeyToRecentlyAccessedIfNeeded">addKeyToRecentlyAccessedIfNeeded()</a></dt>
-<dd><p>We check to see if this key is flagged as safe for eviction and add it to the recentlyAccessedKeys list so that when we
-run out of storage the least recently accessed key can be removed.</p>
-</dd>
 <dt><a href="#getCollectionDataAndSendAsObject">getCollectionDataAndSendAsObject()</a></dt>
 <dd><p>Gets the data for a given an array of matching keys, combines them into an object, and sends the result back to the subscriber.</p>
 </dd>
@@ -326,13 +322,6 @@ keyChanged(key, value, subscriber => subscriber.initWithStoredValues === false)
 ## sendDataToConnection()
 Sends the data obtained from the keys to the connection.
 
-**Kind**: global function  
-<a name="addKeyToRecentlyAccessedIfNeeded"></a>
-
-## addKeyToRecentlyAccessedIfNeeded()
-We check to see if this key is flagged as safe for eviction and add it to the recentlyAccessedKeys list so that when we
-run out of storage the least recently accessed key can be removed.
-
 **Kind**: global function  
 <a name="getCollectionDataAndSendAsObject"></a>
 

README.md

@@ -314,42 +314,24 @@ If a platform needs to use a separate library (like using MMVK for react-native)
 
 [Docs](./API.md)
 
-# Cache Eviction
+# Storage Eviction
 
 Different platforms come with varying storage capacities and Onyx has a way to gracefully fail when those storage limits are encountered. When Onyx fails to set or modify a key the following steps are taken:
-1. Onyx looks at a list of recently accessed keys (access is defined as subscribed to or modified) and locates the key that was least recently accessed
-2. It then deletes this key and retries the original operation
+1. Onyx looks at a list of evictable keys ordered by recent access and locates the least recently accessed one
+2. It then deletes this key from both cache and storage, and retries the original operation
+3. This process repeats up to 5 times until the write succeeds or no more evictable keys are available
 
 By default, Onyx will not evict anything from storage and will presume all keys are "unsafe" to remove unless explicitly told otherwise.
 
-**To flag a key as safe for removal:**
-- Add the key to the `evictableKeys` option in `Onyx.init(options)`
-- Implement `canEvict` in the Onyx config for each component subscribing to a key
-- The key will only be deleted when all subscribers return `true` for `canEvict`
+**To flag a key as safe for removal**, add the key to the `evictableKeys` option in `Onyx.init(options)`:
 
-e.g.
 ```js
 Onyx.init({
     evictableKeys: [ONYXKEYS.COLLECTION.REPORT_ACTIONS],
 });
 ```
 
-```js
-const ReportActionsView = ({reportID, isActiveReport}) => {
-    const [reportActions] = useOnyx(
-        `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}_`,
-        {canEvict: () => !isActiveReport}
-    );
-
-    return (
-        <View>
-            {/* Render with reportActions data */}
-        </View>
-    );
-};
-
-export default ReportActionsView;
-```
+Only individual (non-collection) keys matching the `evictableKeys` patterns will be considered for eviction. Collection keys themselves cannot be evicted — only their individual members can.
 
 # Debug mode
 

lib/Onyx.ts

@@ -34,7 +34,6 @@ function init({
     keys = {},
     initialKeyStates = {},
     evictableKeys = [],
-    maxCachedKeysCount = 1000,
     shouldSyncMultipleInstances = !!global.localStorage,
     enableDevTools = true,
     skippableCollectionMemberIDs = [],
@@ -69,10 +68,6 @@ function init({
         });
     }
 
-    if (maxCachedKeysCount > 0) {
-        cache.setRecentKeysLimit(maxCachedKeysCount);
-    }
-
     OnyxUtils.initStoreValues(keys, initialKeyStates, evictableKeys);
 
     // Initialize all of our keys with data provided then give green light to any pending connections.

lib/OnyxCache.ts

@@ -25,9 +25,6 @@ class OnyxCache {
     /** A list of keys where a nullish value has been fetched from storage before, but the key still exists in cache */
     private nullishStorageKeys: Set<OnyxKey>;
 
-    /** Unique list of keys maintained in access order (most recent at the end) */
-    private recentKeys: Set<OnyxKey>;
-
     /** A map of cached values */
     private storageMap: Record<OnyxKey, OnyxValue<OnyxKey>>;
 
@@ -40,22 +37,15 @@ class OnyxCache {
      */
     private pendingPromises: Map<string, Promise<OnyxValue<OnyxKey> | OnyxKey[]>>;
 
-    /** Maximum size of the keys store din cache */
-    private maxRecentKeysSize = 0;
-
     /** List of keys that are safe to remove when we reach max storage */
     private evictionAllowList: OnyxKey[] = [];
 
-    /** Map of keys and connection arrays whose keys will never be automatically evicted */
-    private evictionBlocklist: Record<OnyxKey, string[] | undefined> = {};
-
     /** List of keys that have been directly subscribed to or recently modified from least to most recent */
     private recentlyAccessedKeys = new Set<OnyxKey>();
 
     constructor() {
         this.storageKeys = new Set();
         this.nullishStorageKeys = new Set();
-        this.recentKeys = new Set();
         this.storageMap = {};
         this.collectionData = {};
         this.pendingPromises = new Map();
@@ -76,12 +66,8 @@ class OnyxCache {
             'hasPendingTask',
             'getTaskPromise',
             'captureTask',
-            'addToAccessedKeys',
-            'removeLeastRecentlyUsedKeys',
-            'setRecentKeysLimit',
             'setAllKeys',
             'setEvictionAllowList',
-            'getEvictionBlocklist',
             'isEvictableKey',
             'removeLastAccessedKey',
             'addLastAccessedKey',
@@ -144,14 +130,8 @@ class OnyxCache {
         return this.storageMap[key] !== undefined || this.hasNullishStorageKey(key);
     }
 
-    /**
-     * Get a cached value from storage
-     * @param [shouldReindexCache] – This is an LRU cache, and by default accessing a value will make it become last in line to be evicted. This flag can be used to skip that and just access the value directly without side-effects.
-     */
-    get(key: OnyxKey, shouldReindexCache = true): OnyxValue<OnyxKey> {
-        if (shouldReindexCache) {
-            this.addToAccessedKeys(key);
-        }
+    /** Get a cached value from storage */
+    get(key: OnyxKey): OnyxValue<OnyxKey> {
         return this.storageMap[key];
     }
 
@@ -161,7 +141,6 @@ class OnyxCache {
      */
     set(key: OnyxKey, value: OnyxValue<OnyxKey>): OnyxValue<OnyxKey> {
         this.addKey(key);
-        this.addToAccessedKeys(key);
 
         // When a key is explicitly set in cache, we can remove it from the list of nullish keys,
         // since it will either be set to a non nullish value or removed from the cache completely.
@@ -207,7 +186,6 @@ class OnyxCache {
         }
 
         this.storageKeys.delete(key);
-        this.recentKeys.delete(key);
         OnyxKeys.deregisterMemberKey(key);
     }
 
@@ -229,7 +207,6 @@ class OnyxCache {
 
         for (const [key, value] of Object.entries(data)) {
             this.addKey(key);
-            this.addToAccessedKeys(key);
 
             const collectionKey = OnyxKeys.getCollectionKey(key);
 
@@ -287,56 +264,9 @@ class OnyxCache {
         return returnPromise;
     }
 
-    /** Adds a key to the top of the recently accessed keys */
-    addToAccessedKeys(key: OnyxKey): void {
-        this.recentKeys.delete(key);
-        this.recentKeys.add(key);
-    }
-
-    /** Remove keys that don't fall into the range of recently used keys */
-    removeLeastRecentlyUsedKeys(): void {
-        const numKeysToRemove = this.recentKeys.size - this.maxRecentKeysSize;
-        if (numKeysToRemove <= 0) {
-            return;
-        }
-
-        const iterator = this.recentKeys.values();
-        const keysToRemove: OnyxKey[] = [];
-
-        const recentKeysArray = Array.from(this.recentKeys);
-        const mostRecentKey = recentKeysArray[recentKeysArray.length - 1];
-
-        let iterResult = iterator.next();
-        while (!iterResult.done) {
-            const key = iterResult.value;
-            // Don't consider the most recently accessed key for eviction
-            // This ensures we don't immediately evict a key we just added
-            if (key !== undefined && key !== mostRecentKey && this.isEvictableKey(key)) {
-                keysToRemove.push(key);
-            }
-            iterResult = iterator.next();
-        }
-
-        for (const key of keysToRemove) {
-            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];
-            }
-            this.recentKeys.delete(key);
-        }
-    }
-
-    /** Set the recent keys list size */
-    setRecentKeysLimit(limit: number): void {
-        this.maxRecentKeysSize = limit;
-    }
-
     /** Check if the value has changed */
     hasValueChanged(key: OnyxKey, value: OnyxValue<OnyxKey>): boolean {
-        const currentValue = this.get(key, false);
+        const currentValue = this.get(key);
         return !deepEqual(currentValue, value);
     }
 
@@ -348,13 +278,6 @@ class OnyxCache {
         this.evictionAllowList = keys;
     }
 
-    /**
-     * Get the eviction block list that prevents keys from being evicted
-     */
-    getEvictionBlocklist(): Record<OnyxKey, string[] | undefined> {
-        return this.evictionBlocklist;
-    }
-
     /**
      * Checks to see if this key has been flagged as safe for removal.
      * @param testKey - Key to check
@@ -408,15 +331,12 @@ class OnyxCache {
     }
 
     /**
-     * Finds a key that can be safely evicted
+     * Finds the least recently accessed key that can be safely evicted from storage.
      */
     getKeyForEviction(): OnyxKey | undefined {
-        for (const key of this.recentlyAccessedKeys) {
-            if (!this.evictionBlocklist[key]) {
-                return key;
-            }
-        }
-        return undefined;
+        // recentlyAccessedKeys is ordered from least to most recently accessed,
+        // so the first element is the best candidate for eviction.
+        return this.recentlyAccessedKeys.values().next().value;
     }
 
     /**

lib/OnyxConnectionManager.ts

@@ -5,7 +5,6 @@ import OnyxUtils from './OnyxUtils';
 import OnyxKeys from './OnyxKeys';
 import * as Str from './Str';
 import type {CollectionConnectCallback, DefaultConnectCallback, DefaultConnectOptions, OnyxKey, OnyxValue} from './types';
-import cache from './OnyxCache';
 import onyxSnapshotCache from './OnyxSnapshotCache';
 
 type ConnectCallback = DefaultConnectCallback<OnyxKey> | CollectionConnectCallback<OnyxKey>;
@@ -106,7 +105,7 @@ class OnyxConnectionManager {
         this.sessionID = Str.guid();
 
         // Binds all public methods to prevent problems with `this`.
-        bindAll(this, 'generateConnectionID', 'fireCallbacks', 'connect', 'disconnect', 'disconnectAll', 'refreshSessionID', 'addToEvictionBlockList', 'removeFromEvictionBlockList');
+        bindAll(this, 'generateConnectionID', 'fireCallbacks', 'connect', 'disconnect', 'disconnectAll', 'refreshSessionID');
     }
 
     /**
@@ -240,7 +239,6 @@ class OnyxConnectionManager {
         // If the connection's callbacks map is empty we can safely unsubscribe from the Onyx key.
         if (connectionMetadata.callbacks.size === 0) {
             OnyxUtils.unsubscribeFromKey(connectionMetadata.subscriptionID);
-            this.removeFromEvictionBlockList(connection);
 
             this.connectionsMap.delete(connection.id);
         }
@@ -250,11 +248,8 @@ class OnyxConnectionManager {
      * Disconnect all subscribers from Onyx.
      */
     disconnectAll(): void {
-        for (const [connectionID, connectionMetadata] of this.connectionsMap.entries()) {
+        for (const connectionMetadata of this.connectionsMap.values()) {
             OnyxUtils.unsubscribeFromKey(connectionMetadata.subscriptionID);
-            for (const callbackID of connectionMetadata.callbacks.keys()) {
-                this.removeFromEvictionBlockList({id: connectionID, callbackID});
-            }
         }
 
         this.connectionsMap.clear();
@@ -272,55 +267,6 @@ class OnyxConnectionManager {
         // Clear snapshot cache when session refreshes to avoid stale cache issues
         onyxSnapshotCache.clear();
     }
-
-    /**
-     * Adds the connection to the eviction block list. Connections added to this list can never be evicted.
-     * */
-    addToEvictionBlockList(connection: Connection): void {
-        if (!connection) {
-            Logger.logInfo(`[ConnectionManager] Attempted to add connection to eviction block list passing an undefined connection object.`);
-            return;
-        }
-
-        const connectionMetadata = this.connectionsMap.get(connection.id);
-        if (!connectionMetadata) {
-            Logger.logInfo(`[ConnectionManager] Attempted to add connection to eviction block list but no connection was found.`);
-            return;
-        }
-
-        const evictionBlocklist = cache.getEvictionBlocklist();
-        if (!evictionBlocklist[connectionMetadata.onyxKey]) {
-            evictionBlocklist[connectionMetadata.onyxKey] = [];
-        }
-
-        evictionBlocklist[connectionMetadata.onyxKey]?.push(`${connection.id}_${connection.callbackID}`);
-    }
-
-    /**
-     * Removes a connection previously added to this list
-     * which will enable it to be evicted again.
-     */
-    removeFromEvictionBlockList(connection: Connection): void {
-        if (!connection) {
-            Logger.logInfo(`[ConnectionManager] Attempted to remove connection from eviction block list passing an undefined connection object.`);
-            return;
-        }
-
-        const connectionMetadata = this.connectionsMap.get(connection.id);
-        if (!connectionMetadata) {
-            Logger.logInfo(`[ConnectionManager] Attempted to remove connection from eviction block list but no connection was found.`);
-            return;
-        }
-
-        const evictionBlocklist = cache.getEvictionBlocklist();
-        evictionBlocklist[connectionMetadata.onyxKey] =
-            evictionBlocklist[connectionMetadata.onyxKey]?.filter((evictionKey) => evictionKey !== `${connection.id}_${connection.callbackID}`) ?? [];
-
-        // Remove the key if there are no more subscribers.
-        if (evictionBlocklist[connectionMetadata.onyxKey]?.length === 0) {
-            delete evictionBlocklist[connectionMetadata.onyxKey];
-        }
-    }
 }
 
 const connectionManager = new OnyxConnectionManager();

lib/OnyxSnapshotCache.ts

@@ -60,7 +60,7 @@ class OnyxSnapshotCache {
      * - `selector`: Different selectors produce different results, so each selector needs its own cache entry
      * - `initWithStoredValues`: This flag changes the initial loading behavior and affects the returned fetch status
      *
-     * Other options like `canEvict` and `reuseConnection` don't affect the data transformation
+     * Other options like `reuseConnection` don't affect the data transformation
      * or timing behavior of getSnapshot, so they're excluded from the cache key for better cache hit rates.
      */
     registerConsumer<TKey extends OnyxKey, TReturnValue>(options: Pick<UseOnyxOptions<TKey, TReturnValue>, 'selector' | 'initWithStoredValues'>): string {