From 0519c9820eaf088e7c9990ef1f7fa4c23cceae31 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Thu, 20 Jun 2024 14:44:16 +0500 Subject: [PATCH 01/13] perf: use for.in loop, avoid object prototype call and use instanceOf instead --- lib/utils.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/utils.ts b/lib/utils.ts index 502b4da7..ad496722 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -17,7 +17,7 @@ function isEmptyObject(obj: T | EmptyValue): obj is EmptyValue { */ function isMergeableObject(value: unknown): value is Record { const isNonNullObject = value != null ? typeof value === 'object' : false; - return isNonNullObject && Object.prototype.toString.call(value) !== '[object RegExp]' && Object.prototype.toString.call(value) !== '[object Date]' && !Array.isArray(value); + return isNonNullObject && !(value instanceof RegExp) && !(value instanceof Date) && !Array.isArray(value); } /** @@ -37,9 +37,8 @@ function mergeObject>(target: TObject | // If "shouldRemoveNestedNulls" is true, we want to remove null values from the merged object // and therefore we need to omit keys where either the source or target value is null. if (targetObject) { - const targetKeys = Object.keys(targetObject); - for (let i = 0; i < targetKeys.length; ++i) { - const key = targetKeys[i]; + // eslint-disable-next-line no-restricted-syntax, guard-for-in + for (const key in targetObject) { const sourceValue = source?.[key]; const targetValue = targetObject?.[key]; @@ -58,10 +57,9 @@ function mergeObject>(target: TObject | } // After copying over all keys from the target object, we want to merge the source object into the destination object. - const sourceKeys = Object.keys(source); - for (let i = 0; i < sourceKeys.length; ++i) { - const key = sourceKeys[i]; - const sourceValue = source?.[key]; + // eslint-disable-next-line no-restricted-syntax, guard-for-in + for (const key in source) { + const sourceValue = source?.[key] as Record; const targetValue = targetObject?.[key]; // If undefined is passed as the source value for a key, we want to generally ignore it. From cd09aa1cce4ae361aee64040054576a2cafda9a2 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Thu, 20 Jun 2024 14:50:09 +0500 Subject: [PATCH 02/13] perf: avoid Object.keys repeatedly and reduce newKeys filter and reduce operations to one forEach loop --- lib/Onyx.ts | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index bee77736..04ddcee9 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -438,7 +438,8 @@ function mergeCollection(collectionKey: TK // Confirm all the collection keys belong to the same parent let hasCollectionKeyCheckFailed = false; - Object.keys(mergedCollection).forEach((dataKey) => { + const mergedCollectionKeys = Object.keys(mergedCollection); + mergedCollectionKeys.forEach((dataKey) => { if (OnyxUtils.isKeyMatch(collectionKey, dataKey)) { return; } @@ -459,7 +460,7 @@ function mergeCollection(collectionKey: TK return OnyxUtils.getAllKeys() .then((persistedKeys) => { // Split to keys that exist in storage and keys that don't - const keys = Object.keys(mergedCollection).filter((key) => { + const keys = mergedCollectionKeys.filter((key) => { if (mergedCollection[key] === null) { OnyxUtils.remove(key); return false; @@ -471,8 +472,6 @@ function mergeCollection(collectionKey: TK const cachedCollectionForExistingKeys = OnyxUtils.getCachedCollection(collectionKey, existingKeys); - const newKeys = keys.filter((key) => !persistedKeys.has(key)); - const existingKeyCollection = existingKeys.reduce((obj: OnyxInputKeyValueMapping, key) => { const {isCompatible, existingValueType, newValueType} = utils.checkCompatibilityWithExistingValue(mergedCollection[key], cachedCollectionForExistingKeys[key]); if (!isCompatible) { @@ -484,11 +483,13 @@ function mergeCollection(collectionKey: TK return obj; }, {}) as Record>; - const newCollection = newKeys.reduce((obj: OnyxInputKeyValueMapping, key) => { - // eslint-disable-next-line no-param-reassign - obj[key] = mergedCollection[key]; - return obj; - }, {}) as Record>; + const newCollection: Record> = {}; + keys.forEach((key) => { + if (persistedKeys.has(key)) { + return; + } + newCollection[key] = mergedCollection[key]; + }); // When (multi-)merging the values with the existing values in storage, // we don't want to remove nested null values from the data that we pass to the storage layer, From 6ebe8a1f30ed8ce3a6d63164e07fa4d2a4a9b09b Mon Sep 17 00:00:00 2001 From: hurali97 Date: Thu, 20 Jun 2024 14:52:49 +0500 Subject: [PATCH 03/13] perf: use multiGet instead of iteratively getting data from storage --- lib/Onyx.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 04ddcee9..e7372326 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -155,10 +155,11 @@ function connect(connectOptions: ConnectOptions): nu } // We did not opt into using waitForCollectionCallback mode so the callback is called for every matching key. - // eslint-disable-next-line @typescript-eslint/prefer-for-of - for (let i = 0; i < matchingKeys.length; i++) { - OnyxUtils.get(matchingKeys[i]).then((val) => OnyxUtils.sendDataToConnection(mapping, val as OnyxValue, matchingKeys[i] as TKey, true)); - } + Storage.multiGet(matchingKeys).then((values) => { + values.forEach(([key, val]) => { + OnyxUtils.sendDataToConnection(mapping, val as OnyxValue, key as TKey, true); + }); + }); return; } From cf4d2403f6f09add3989219b6d1cf939110b3c14 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Mon, 24 Jun 2024 14:35:25 +0500 Subject: [PATCH 04/13] feat: add multiGet in OnyxUtils --- lib/OnyxUtils.ts | 65 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 13f5fb18..83d31e50 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -29,6 +29,7 @@ import type { } from './types'; import utils from './utils'; import type {WithOnyxState} from './withOnyx/types'; +import type {KeyValuePairList} from './storage/providers/types'; // Method constants const METHOD = { @@ -245,6 +246,69 @@ function get>(key: TKey): P return cache.captureTask(taskName, promise) as Promise; } +// multiGet the data first from the cache and then from the storage for the missing keys. +function multiGet(keys: CollectionKeyBase[]): Promise { + // Keys that are not in the cache + const missingKeys: OnyxKey[] = []; + // Tasks that are pending + const pendingTasks: Array>> = []; + // Keys for the tasks that are pending + const pendingKeys: OnyxKey[] = []; + // Data to be sent back to the invoker + const data: KeyValuePairList = []; + + keys.forEach((key) => { + const cacheValue = cache.get(key) as OnyxValue; + if (cacheValue) { + data.push([key, cacheValue]); + return; + } + + const pendingKey = `get:${key}`; + if (cache.hasPendingTask(pendingKey)) { + pendingTasks.push(cache.getTaskPromise(pendingKey) as Promise>); + pendingKeys.push(key); + } else { + missingKeys.push(key); + } + }); + + return ( + Promise.all(pendingTasks) + // We are going to wait for all the pending tasks to resolve and then add the data to the data object. + .then((values) => { + values.forEach((value, index) => { + data.push([pendingKeys[index], value]); + }); + + return Promise.resolve(); + }) + // We are going to get the missing keys using multiGet from the storage. + .then(() => { + if (missingKeys.length === 0) { + return Promise.resolve(undefined); + } + + return Storage.multiGet(missingKeys); + }) + // We are going to add the data from the missing keys to the data object and also merge it to the cache. + .then((values): Promise => { + if (!values || values.length === 0) { + return Promise.resolve(data); + } + + // temp object is used to merge the missing data into the cache + const temp: OnyxCollection = {}; + values.forEach(([key, value]) => { + data.push([key, value]); + temp[key] = value as OnyxValue; + }); + cache.merge(temp); + return Promise.resolve(data); + }) + ); +} + /** Returns current key names stored in persisted storage */ function getAllKeys(): Promise> { // When we've already read stored keys, resolve right away @@ -1150,6 +1214,7 @@ const OnyxUtils = { applyMerge, initializeWithDefaultKeyStates, getSnapshotKey, + multiGet, }; export default OnyxUtils; From 4eefa3db39448cd3ac535ed203cec3622d3222d7 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Mon, 24 Jun 2024 14:35:53 +0500 Subject: [PATCH 05/13] refactor: use OnyxUtils multiGet instead of Storage --- lib/Onyx.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index e7372326..a1972908 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -155,7 +155,7 @@ function connect(connectOptions: ConnectOptions): nu } // We did not opt into using waitForCollectionCallback mode so the callback is called for every matching key. - Storage.multiGet(matchingKeys).then((values) => { + OnyxUtils.multiGet(matchingKeys).then((values) => { values.forEach(([key, val]) => { OnyxUtils.sendDataToConnection(mapping, val as OnyxValue, key as TKey, true); }); From 442a763414eec8b40498894ceaf699d2a59847e3 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Tue, 25 Jun 2024 15:30:42 +0500 Subject: [PATCH 06/13] refactor: avoid duplicate code --- lib/Onyx.ts | 2 +- lib/OnyxUtils.ts | 96 ++++++++++-------------------------------------- 2 files changed, 20 insertions(+), 78 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index a1972908..b5e11638 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -156,7 +156,7 @@ function connect(connectOptions: ConnectOptions): nu // We did not opt into using waitForCollectionCallback mode so the callback is called for every matching key. OnyxUtils.multiGet(matchingKeys).then((values) => { - values.forEach(([key, val]) => { + values.forEach((val, key) => { OnyxUtils.sendDataToConnection(mapping, val as OnyxValue, key as TKey, true); }); }); diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 83d31e50..eefd4e97 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -29,7 +29,6 @@ import type { } from './types'; import utils from './utils'; import type {WithOnyxState} from './withOnyx/types'; -import type {KeyValuePairList} from './storage/providers/types'; // Method constants const METHOD = { @@ -247,7 +246,7 @@ function get>(key: TKey): P } // multiGet the data first from the cache and then from the storage for the missing keys. -function multiGet(keys: CollectionKeyBase[]): Promise { +function multiGet(keys: CollectionKeyBase[]): Promise>> { // Keys that are not in the cache const missingKeys: OnyxKey[] = []; // Tasks that are pending @@ -255,12 +254,20 @@ function multiGet(keys: CollectionKeyBase[]): Promise>(); + /** + * We are going to iterate over all the matching keys and check if we have the data in the cache. + * If we do then we add it to the data object. If we do not then we check if there is a pending task + * for the key. If there is then we add the promise to the pendingTasks array and the key to the pendingKeys + * array. If there is no pending task then we add the key to the missingKeys array. + * + * These missingKeys will be later to use to multiGet the data from the storage. + */ keys.forEach((key) => { const cacheValue = cache.get(key) as OnyxValue; if (cacheValue) { - data.push([key, cacheValue]); + dataMap.set(key, cacheValue); return; } @@ -278,7 +285,7 @@ function multiGet(keys: CollectionKeyBase[]): Promise { values.forEach((value, index) => { - data.push([pendingKeys[index], value]); + dataMap.set(pendingKeys[index], value); }); return Promise.resolve(); @@ -292,19 +299,19 @@ function multiGet(keys: CollectionKeyBase[]): Promise => { + .then((values) => { if (!values || values.length === 0) { - return Promise.resolve(data); + return dataMap; } // temp object is used to merge the missing data into the cache const temp: OnyxCollection = {}; values.forEach(([key, value]) => { - data.push([key, value]); + dataMap.set(key, value as OnyxValue); temp[key] = value as OnyxValue; }); cache.merge(temp); - return Promise.resolve(data); + return dataMap; }) ); } @@ -907,75 +914,10 @@ function addKeyToRecentlyAccessedIfNeeded(mapping: Mapping * Gets the data for a given an array of matching keys, combines them into an object, and sends the result back to the subscriber. */ function getCollectionDataAndSendAsObject(matchingKeys: CollectionKeyBase[], mapping: Mapping): void { - // Keys that are not in the cache - const missingKeys: OnyxKey[] = []; - // Tasks that are pending - const pendingTasks: Array>> = []; - // Keys for the tasks that are pending - const pendingKeys: OnyxKey[] = []; - - // We are going to combine all the data from the matching keys into a single object - const data: OnyxCollection = {}; - - /** - * We are going to iterate over all the matching keys and check if we have the data in the cache. - * If we do then we add it to the data object. If we do not then we check if there is a pending task - * for the key. If there is then we add the promise to the pendingTasks array and the key to the pendingKeys - * array. If there is no pending task then we add the key to the missingKeys array. - * - * These missingKeys will be later to use to multiGet the data from the storage. - */ - matchingKeys.forEach((key) => { - const cacheValue = cache.get(key) as OnyxValue; - if (cacheValue) { - data[key] = cacheValue; - return; - } - - const pendingKey = `get:${key}`; - if (cache.hasPendingTask(pendingKey)) { - pendingTasks.push(cache.getTaskPromise(pendingKey) as Promise>); - pendingKeys.push(key); - } else { - missingKeys.push(key); - } + multiGet(matchingKeys).then((dataMap) => { + const data = Object.fromEntries(dataMap.entries()) as OnyxValue; + sendDataToConnection(mapping, data, undefined, true); }); - - Promise.all(pendingTasks) - // We are going to wait for all the pending tasks to resolve and then add the data to the data object. - .then((values) => { - values.forEach((value, index) => { - data[pendingKeys[index]] = value; - }); - - return Promise.resolve(); - }) - // We are going to get the missing keys using multiGet from the storage. - .then(() => { - if (missingKeys.length === 0) { - return Promise.resolve(undefined); - } - return Storage.multiGet(missingKeys); - }) - // We are going to add the data from the missing keys to the data object and also merge it to the cache. - .then((values) => { - if (!values || values.length === 0) { - return Promise.resolve(); - } - - // temp object is used to merge the missing data into the cache - const temp: OnyxCollection = {}; - values.forEach(([key, value]) => { - data[key] = value as OnyxValue; - temp[key] = value as OnyxValue; - }); - cache.merge(temp); - return Promise.resolve(); - }) - // We are going to send the data to the subscriber. - .finally(() => { - sendDataToConnection(mapping, data as OnyxValue, undefined, true); - }); } /** From c79f21de2164844bfc42257a8a229f5d50cbb35d Mon Sep 17 00:00:00 2001 From: Muhammad Hur Ali Date: Tue, 25 Jun 2024 16:34:38 +0500 Subject: [PATCH 07/13] update comment Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com> --- lib/OnyxUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index eefd4e97..a62caa1b 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -290,7 +290,7 @@ function multiGet(keys: CollectionKeyBase[]): Promise { if (missingKeys.length === 0) { return Promise.resolve(undefined); From 9fe6ff26316829eb3b0713d705737ac134e3ef96 Mon Sep 17 00:00:00 2001 From: Muhammad Hur Ali Date: Tue, 25 Jun 2024 16:34:48 +0500 Subject: [PATCH 08/13] update comment Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com> --- lib/OnyxUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index a62caa1b..edd60e90 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -282,7 +282,7 @@ function multiGet(keys: CollectionKeyBase[]): Promise { values.forEach((value, index) => { dataMap.set(pendingKeys[index], value); From 4813aed175c3f3522f79555139effd29a58f5a72 Mon Sep 17 00:00:00 2001 From: Muhammad Hur Ali Date: Tue, 25 Jun 2024 16:34:58 +0500 Subject: [PATCH 09/13] update comment Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com> --- lib/OnyxUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index edd60e90..70a8afd1 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -262,7 +262,7 @@ function multiGet(keys: CollectionKeyBase[]): Promise { const cacheValue = cache.get(key) as OnyxValue; From d268df3f9e01720c2103bf60b615373f36d9e217 Mon Sep 17 00:00:00 2001 From: Muhammad Hur Ali Date: Tue, 25 Jun 2024 16:35:07 +0500 Subject: [PATCH 10/13] update comment Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com> --- lib/OnyxUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 70a8afd1..f78b82f1 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -298,7 +298,7 @@ function multiGet(keys: CollectionKeyBase[]): Promise { if (!values || values.length === 0) { return dataMap; From 396eb7f6c94a57f2dc9561612df9853cc7358981 Mon Sep 17 00:00:00 2001 From: Muhammad Hur Ali Date: Tue, 25 Jun 2024 16:35:23 +0500 Subject: [PATCH 11/13] update comment Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com> --- lib/OnyxUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index f78b82f1..cc703115 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -259,7 +259,7 @@ function multiGet(keys: CollectionKeyBase[]): Promise Date: Tue, 25 Jun 2024 16:35:32 +0500 Subject: [PATCH 12/13] update comment Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com> --- lib/OnyxUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index cc703115..eef1360f 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -258,7 +258,7 @@ function multiGet(keys: CollectionKeyBase[]): Promise Date: Tue, 25 Jun 2024 16:35:59 +0500 Subject: [PATCH 13/13] refactor: make code more readable Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com> --- lib/OnyxUtils.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index eef1360f..a1425113 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -249,10 +249,13 @@ function get>(key: TKey): P function multiGet(keys: CollectionKeyBase[]): Promise>> { // Keys that are not in the cache const missingKeys: OnyxKey[] = []; + // Tasks that are pending const pendingTasks: Array>> = []; + // Keys for the tasks that are pending const pendingKeys: OnyxKey[] = []; + // Data to be sent back to the invoker const dataMap = new Map>();