diff --git a/lib/Onyx.js b/lib/Onyx.js index 1e69e44a1..b2f8290c9 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -102,19 +102,6 @@ function isCollectionKey(key) { return _.contains(_.values(onyxKeys.COLLECTION), key); } -/** - * Find the collection a collection item belongs to - * or return null if them item is not a part of a collection - * @param {string} key - * @returns {string|null} - */ -function getCollectionKeyForItem(key) { - return _.chain(onyxKeys.COLLECTION) - .values() - .find(name => key.startsWith(name)) - .value(); -} - /** * Checks to see if a given key matches with the * configured key of our connected subscriber @@ -369,14 +356,21 @@ function connect(mapping) { deferredInitTask.promise .then(() => { // Check to see if this key is flagged as a safe eviction key and add it to the recentlyAccessedKeys list - if (mapping.withOnyxInstance && !isCollectionKey(mapping.key) && isSafeEvictionKey(mapping.key)) { - // All React components subscribing to a key flagged as a safe eviction - // key must implement the canEvict property. - if (_.isUndefined(mapping.canEvict)) { - // eslint-disable-next-line max-len - throw new Error(`Cannot subscribe to safe eviction key '${mapping.key}' without providing a canEvict value.`); + if (isSafeEvictionKey(mapping.key)) { + // Try to free some cache whenever we connect to a safe eviction key + cache.removeLeastRecentlyUsedKeys(); + + if (mapping.withOnyxInstance && !isCollectionKey(mapping.key)) { + // All React components subscribing to a key flagged as a safe eviction + // key must implement the canEvict property. + if (_.isUndefined(mapping.canEvict)) { + throw new Error( + `Cannot subscribe to safe eviction key '${mapping.key}' without providing a canEvict value.` + ); + } + + addLastAccessedKey(mapping.key); } - addLastAccessedKey(mapping.key); } }) .then(getAllKeys) @@ -412,48 +406,6 @@ function connect(mapping) { return connectionID; } -/** - * Remove cache items that are no longer connected through Onyx - * @param {string} key - */ -function cleanCache(key) { - // Don't remove default keys from cache, they don't take much memory and are accessed frequently - if (_.has(defaultKeyStates, key)) { - return; - } - - const hasRemainingConnections = _.some(callbackToStateMapping, {key}); - - // When the key is still used in other places don't remove it from cache - if (hasRemainingConnections) { - return; - } - - // When this is a collection - also recursively remove any unused individual items - if (isCollectionKey(key)) { - cache.drop(key); - - getAllKeys().then(cachedKeys => _.chain(cachedKeys) - .filter(name => name.startsWith(key)) - .forEach(cleanCache)); - - return; - } - - // When this is a collection item - check if the collection is still used - const collectionKey = getCollectionKeyForItem(key); - if (collectionKey) { - // When there's an active subscription for a collection don't remove the item - const hasRemainingConnectionsForCollection = _.some(callbackToStateMapping, {key: collectionKey}); - if (hasRemainingConnectionsForCollection) { - return; - } - } - - // Otherwise remove the value from cache - cache.drop(key); -} - /** * Remove the listener for a react component * @@ -471,11 +423,7 @@ function disconnect(connectionID, keyToRemoveFromEvictionBlocklist) { removeFromEvictionBlockList(keyToRemoveFromEvictionBlocklist, connectionID); } - const key = callbackToStateMapping[connectionID].key; delete callbackToStateMapping[connectionID]; - - // When the last subscriber disconnects, drop cache as well - cleanCache(key); } /** @@ -742,13 +690,17 @@ function mergeCollection(collectionKey, collection) { * @param {function} registerStorageEventListener a callback when a storage event happens. * This applies to web platforms where the local storage emits storage events * across all open tabs and allows Onyx to stay in sync across all open tabs. - * @param {Boolean} [options.captureMetrics] + * @param {Number} [options.maxCachedKeysCount=55] Sets how many recent keys should we try to keep in cache + * Setting this to 0 would practically mean no cache + * We try to free cache when we connect to a safe eviction key + * @param {Boolean} [options.captureMetrics] Enables Onyx benchmarking and exposes the get/print/reset functions */ function init({ keys, initialKeyStates, safeEvictionKeys, registerStorageEventListener, + maxCachedKeysCount = 55, captureMetrics = false, }) { if (captureMetrics) { @@ -757,6 +709,10 @@ function init({ applyDecorators(); } + if (maxCachedKeysCount > 0) { + cache.setRecentKeysLimit(maxCachedKeysCount); + } + // Let Onyx know about all of our keys onyxKeys = keys; diff --git a/lib/OnyxCache.js b/lib/OnyxCache.js index f24cd5a71..530e3ec1b 100644 --- a/lib/OnyxCache.js +++ b/lib/OnyxCache.js @@ -17,6 +17,13 @@ class OnyxCache { */ this.storageKeys = new Set(); + /** + * @private + * Unique list of keys maintained in access order (most recent at the end) + * @type {Set} + */ + this.recentKeys = new Set(); + /** * @private * A map of cached values @@ -31,11 +38,12 @@ class OnyxCache { */ this.pendingPromises = {}; - // bind all methods to prevent problems with `this` + // bind all public methods to prevent problems with `this` _.bindAll( this, 'getAllKeys', 'getValue', 'hasCacheForKey', 'addKey', 'set', 'drop', 'merge', - 'hasPendingTask', 'getTaskPromise', 'captureTask', + 'hasPendingTask', 'getTaskPromise', 'captureTask', 'removeLeastRecentlyUsedKeys', + 'setRecentKeysLimit' ); } @@ -53,6 +61,7 @@ class OnyxCache { * @returns {*} */ getValue(key) { + this.addToAccessedKeys(key); return this.storageMap[key]; } @@ -83,6 +92,7 @@ class OnyxCache { */ set(key, value) { this.addKey(key); + this.addToAccessedKeys(key); this.storageMap[key] = value; return value; @@ -106,6 +116,7 @@ class OnyxCache { const storageKeys = this.getAllKeys(); const mergedKeys = _.keys(data); this.storageKeys = new Set([...storageKeys, ...mergedKeys]); + _.each(mergedKeys, key => this.addToAccessedKeys(key)); } /** @@ -144,6 +155,39 @@ class OnyxCache { return this.pendingPromises[taskName]; } + + /** + * @private + * Adds a key to the top of the recently accessed keys + * @param {string} key + */ + addToAccessedKeys(key) { + // Removing and re-adding a key ensures it's at the end of the list + this.recentKeys.delete(key); + this.recentKeys.add(key); + } + + /** + * Remove keys that don't fall into the range of recently used keys + */ + removeLeastRecentlyUsedKeys() { + if (this.recentKeys.size > this.maxRecentKeysSize) { + // Get the last N keys by doing a negative slice + const recentlyAccessed = [...this.recentKeys].slice(-this.maxRecentKeysSize); + const storageKeys = _.keys(this.storageMap); + const keysToRemove = _.difference(storageKeys, recentlyAccessed); + + _.each(keysToRemove, this.drop); + } + } + + /** + * Set the recent keys list size + * @param {number} limit + */ + setRecentKeysLimit(limit) { + this.maxRecentKeysSize = limit; + } } const instance = new OnyxCache(); diff --git a/lib/decorateWithMetrics.js b/lib/decorateWithMetrics.js index 1a91444a2..750c92538 100644 --- a/lib/decorateWithMetrics.js +++ b/lib/decorateWithMetrics.js @@ -48,6 +48,7 @@ function decorateWithMetrics(func, alias = func.name) { methodName: alias, startTime, endTime, + duration: endTime - startTime, args, }); }); @@ -78,8 +79,7 @@ function sum(list, prop) { */ function getMetrics() { const summaries = _.chain(stats) - .map((data, methodName) => { - const calls = _.map(data, call => ({...call, duration: call.endTime - call.startTime})); + .map((calls, methodName) => { const total = sum(calls, 'duration'); const avg = (total / calls.length) || 0; const max = _.max(calls, 'duration').duration || 0; @@ -144,9 +144,10 @@ function toDuration(millis, raw = false) { * @param {'console'|'csv'|'json'|'string'} [options.format=console] The output format of this function * `string` is useful when __DEV__ is set to `false` as writing to the console is disabled, but the result of this * method would still get printed as output + * @param {string[]} [options.methods] Print stats only for these method names * @returns {string|undefined} */ -function printMetrics({raw = false, format = 'console'} = {}) { +function printMetrics({raw = false, format = 'console', methods} = {}) { const {totalTime, summaries, lastCompleteCall} = getMetrics(); const tableSummary = MDTable.factory({ @@ -154,11 +155,12 @@ function printMetrics({raw = false, format = 'console'} = {}) { leftAlignedCols: [0], }); - const methodCallTables = _.chain(summaries) - .filter(method => method.avg > 0) - .sortBy('avg') - .reverse() - .map(({methodName, calls, ...methodStats}) => { + const methodNames = _.isArray(methods) ? methods : _.keys(summaries); + + const methodCallTables = _.chain(methodNames) + .filter(methodName => summaries[methodName] && summaries[methodName].avg > 0) + .map((methodName) => { + const {calls, ...methodStats} = summaries[methodName]; tableSummary.addRow( methodName, toDuration(methodStats.total, raw), diff --git a/tests/unit/onyxCacheTest.js b/tests/unit/onyxCacheTest.js index d505265ba..7891f1bf6 100644 --- a/tests/unit/onyxCacheTest.js +++ b/tests/unit/onyxCacheTest.js @@ -1,5 +1,6 @@ import React from 'react'; import {render} from '@testing-library/react-native'; +import _ from 'underscore'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; import ViewWithText from '../components/ViewWithText'; @@ -406,7 +407,7 @@ describe('Onyx', () => { }, }; - function initOnyx() { + function initOnyx(overrides) { const OnyxModule = require('../../index'); Onyx = OnyxModule.default; withOnyx = OnyxModule.withOnyx; @@ -415,7 +416,10 @@ describe('Onyx', () => { Onyx.init({ keys: ONYX_KEYS, + safeEvictionKeys: [ONYX_KEYS.COLLECTION.MOCK_COLLECTION], registerStorageEventListener: jest.fn(), + maxCachedKeysCount: 10, + ...overrides, }); // Onyx init introduces some side effects e.g. calls the getAllKeys @@ -489,7 +493,47 @@ describe('Onyx', () => { }); }); - it('Expect multiple calls to getItem when no existing component is using a key', () => { + it('Should keep recently accessed items in cache', () => { + // GIVEN Storage with 10 different keys + AsyncStorageMock.getItem.mockResolvedValue('"mockValue"'); + AsyncStorageMock.getAllKeys.mockResolvedValue( + _.range(10).map(number => `${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}${number}`) + ); + let connections; + + // GIVEN Onyx is configured with max 5 keys in cache + return initOnyx({maxCachedKeysCount: 5}) + .then(() => { + // GIVEN 10 connections for different keys + connections = _.range(10).map((number) => { + const key = `${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}${number}`; + return ({ + key, + connectionId: Onyx.connect({key}), + }); + }); + }) + .then(waitForPromisesToResolve) + .then(() => { + // WHEN a new connection for a safe eviction key happens + Onyx.connect({key: `${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}9`}); + }) + .then(() => { + // THEN the most recent 5 keys should remain in cache + _.range(5, 10).forEach((number) => { + const key = connections[number].key; + expect(cache.hasCacheForKey(key)).toBe(true); + }); + + // AND the least recent 5 should be dropped + _.range(0, 5).forEach((number) => { + const key = connections[number].key; + expect(cache.hasCacheForKey(key)).toBe(false); + }); + }); + }); + + it('Expect multiple calls to getItem when value cannot be retrieved from cache', () => { // GIVEN a component connected to Onyx const TestComponentWithOnyx = withOnyx({ text: { @@ -500,16 +544,17 @@ describe('Onyx', () => { // GIVEN some string value for that key exists in storage AsyncStorageMock.getItem.mockResolvedValue('"mockValue"'); AsyncStorageMock.getAllKeys.mockResolvedValue([ONYX_KEYS.TEST_KEY]); - let result; return initOnyx() .then(() => { - // WHEN a component is rendered and unmounted and no longer available - result = render(); + // WHEN a component is rendered + render(); }) .then(waitForPromisesToResolve) - .then(() => result.unmount()) - .then(waitForPromisesToResolve) + .then(() => { + // WHEN the key was removed from cache + cache.drop(ONYX_KEYS.TEST_KEY); + }) // THEN When another component using the same storage key is rendered .then(() => render()) @@ -559,125 +604,42 @@ describe('Onyx', () => { }); }); - it('Expect a single call to getItem when at least one component is still subscribed to a key', () => { - // GIVEN a component connected to Onyx - const TestComponentWithOnyx = withOnyx({ - text: { - key: ONYX_KEYS.TEST_KEY, - }, - })(ViewWithText); - - // GIVEN some string value for that key exists in storage + it('Should clean cache when connections to eviction keys happen', () => { + // GIVEN storage with some data AsyncStorageMock.getItem.mockResolvedValue('"mockValue"'); - AsyncStorageMock.getAllKeys.mockResolvedValue([ONYX_KEYS.TEST_KEY]); - let result2; - let result3; - return initOnyx() - .then(() => { - // WHEN multiple components are rendered - render(); - result2 = render(); - result3 = render(); - }) - .then(waitForPromisesToResolve) - .then(() => { - // WHEN components unmount but at least one remain mounted - result2.unmount(); - result3.unmount(); - }) - .then(waitForPromisesToResolve) + AsyncStorageMock.getAllKeys.mockResolvedValue(_.range(1, 10).map(n => `key${n}`)); - // THEN When another component using the same storage key is rendered - .then(() => render()) - .then(waitForPromisesToResolve) - .then(() => { - // THEN Async storage `getItem` should be called once - expect(AsyncStorageMock.getItem).toHaveBeenCalledTimes(1); - }); - }); + jest.useFakeTimers(); - it('Should remove collection items from cache when collection is disconnected', () => { - // GIVEN a component subscribing to a collection - const TestComponentWithOnyx = withOnyx({ - collections: { - key: ONYX_KEYS.COLLECTION.MOCK_COLLECTION, - }, - })(ViewWithCollections); - - // GIVEN some collection item values exist in storage - const keys = [`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}15`, `${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}16`]; - AsyncStorageMock.setItem(keys[0], JSON.stringify({ID: 15})); - AsyncStorageMock.setItem(keys[1], JSON.stringify({ID: 16})); - AsyncStorageMock.getAllKeys.mockResolvedValue(keys); - let result; - let result2; - return initOnyx() + // GIVEN Onyx with LRU size of 3 + return initOnyx({maxCachedKeysCount: 3}) .then(() => { - // WHEN the collection using components render - result = render(); - result2 = render(); + // WHEN 4 connections for different keys happen + Onyx.connect({key: 'key1'}); + Onyx.connect({key: 'key2'}); + Onyx.connect({key: 'key3'}); + Onyx.connect({key: 'key4'}); }) .then(waitForPromisesToResolve) .then(() => { - // THEN the collection items should be in cache - expect(cache.hasCacheForKey(keys[0])).toBe(true); - expect(cache.hasCacheForKey(keys[1])).toBe(true); - - // WHEN one of the components unmounts - result.unmount(); - return waitForPromisesToResolve(); - }) - .then(() => { - // THEN the collection items should still be in cache - expect(cache.hasCacheForKey(keys[0])).toBe(true); - expect(cache.hasCacheForKey(keys[1])).toBe(true); - - // WHEN the last component using the collection unmounts - result2.unmount(); - return waitForPromisesToResolve(); + // THEN keys 1,2,3,4 should be in cache + expect(cache.hasCacheForKey('key1')).toBe(true); + expect(cache.hasCacheForKey('key2')).toBe(true); + expect(cache.hasCacheForKey('key3')).toBe(true); + expect(cache.hasCacheForKey('key4')).toBe(true); + + // WHEN A connection for safe eviction key happens + Onyx.connect({key: ONYX_KEYS.COLLECTION.MOCK_COLLECTION}); }) + .then(waitForPromisesToResolve) .then(() => { - // THEN the collection items should be removed from cache - expect(cache.hasCacheForKey(keys[0])).toBe(false); - expect(cache.hasCacheForKey(keys[1])).toBe(false); - }); - }); + // THEN key 1 should no longer be in cache + expect(cache.hasCacheForKey('key1')).toBe(false); - it('Should not remove item from cache when it still used in a collection', () => { - // GIVEN component that uses a collection and a component that uses a collection item - const COLLECTION_ITEM_KEY = `${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}10`; - const TestComponentWithOnyx = withOnyx({ - collections: { - key: ONYX_KEYS.COLLECTION.MOCK_COLLECTION, - }, - })(ViewWithCollections); - - const AnotherTestComponentWithOnyx = withOnyx({ - testObject: { - key: COLLECTION_ITEM_KEY, - }, - })(ViewWithCollections); - - // GIVEN some values exist in storage - AsyncStorageMock.setItem(COLLECTION_ITEM_KEY, JSON.stringify({ID: 10})); - AsyncStorageMock.getAllKeys.mockResolvedValue([COLLECTION_ITEM_KEY]); - let result; - - return initOnyx() - .then(() => { - // WHEN both components render - render(); - result = render(); - return waitForPromisesToResolve(); - }) - .then(() => { - // WHEN the component using the individual item unmounts - result.unmount(); - return waitForPromisesToResolve(); - }) - .then(() => { - // THEN The item should not be removed from cache as it's used in a collection - expect(cache.hasCacheForKey(COLLECTION_ITEM_KEY)).toBe(true); + // AND the rest of the keys should be in cache + expect(cache.hasCacheForKey('key2')).toBe(true); + expect(cache.hasCacheForKey('key3')).toBe(true); + expect(cache.hasCacheForKey('key4')).toBe(true); }); }); });