Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: improve native apps startup #563

Merged
28 changes: 15 additions & 13 deletions lib/Onyx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,11 @@ function connect<TKey extends OnyxKey>(connectOptions: ConnectOptions<TKey>): 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<TKey>, matchingKeys[i] as TKey, true));
}
OnyxUtils.multiGet(matchingKeys).then((values) => {
values.forEach((val, key) => {
OnyxUtils.sendDataToConnection(mapping, val as OnyxValue<TKey>, key as TKey, true);
});
});
return;
}

Expand Down Expand Up @@ -438,7 +439,8 @@ function mergeCollection<TKey extends CollectionKeyBase, TMap>(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;
}
Expand All @@ -459,7 +461,7 @@ function mergeCollection<TKey extends CollectionKeyBase, TMap>(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;
Expand All @@ -471,8 +473,6 @@ function mergeCollection<TKey extends CollectionKeyBase, TMap>(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) {
Expand All @@ -484,11 +484,13 @@ function mergeCollection<TKey extends CollectionKeyBase, TMap>(collectionKey: TK
return obj;
}, {}) as Record<OnyxKey, OnyxInput<TKey>>;

const newCollection = newKeys.reduce((obj: OnyxInputKeyValueMapping, key) => {
// eslint-disable-next-line no-param-reassign
obj[key] = mergedCollection[key];
return obj;
}, {}) as Record<OnyxKey, OnyxInput<TKey>>;
const newCollection: Record<OnyxKey, OnyxInput<TKey>> = {};
keys.forEach((key) => {
if (persistedKeys.has(key)) {
return;
}
newCollection[key] = mergedCollection[key];
});
hurali97 marked this conversation as resolved.
Show resolved Hide resolved

// 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,
Expand Down
146 changes: 78 additions & 68 deletions lib/OnyxUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,80 @@ function get<TKey extends OnyxKey, TValue extends OnyxValue<TKey>>(key: TKey): P
return cache.captureTask(taskName, promise) as Promise<TValue>;
}

// multiGet the data first from the cache and then from the storage for the missing keys.
hurali97 marked this conversation as resolved.
Show resolved Hide resolved
function multiGet<TKey extends OnyxKey>(keys: CollectionKeyBase[]): Promise<Map<OnyxKey, OnyxValue<TKey>>> {
// Keys that are not in the cache
const missingKeys: OnyxKey[] = [];

// Tasks that are pending
const pendingTasks: Array<Promise<OnyxValue<TKey>>> = [];

// Keys for the tasks that are pending
const pendingKeys: OnyxKey[] = [];

// Data to be sent back to the invoker
const dataMap = new Map<OnyxKey, OnyxValue<TKey>>();

/**
* 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 have them, then we check if there is a pending task
* for the key. If there is such task, 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 used to multiGet the data from the storage.
*/
keys.forEach((key) => {
const cacheValue = cache.get(key) as OnyxValue<TKey>;
if (cacheValue) {
dataMap.set(key, cacheValue);
return;
}

const pendingKey = `get:${key}`;
if (cache.hasPendingTask(pendingKey)) {
pendingTasks.push(cache.getTaskPromise(pendingKey) as Promise<OnyxValue<TKey>>);
pendingKeys.push(key);
} else {
missingKeys.push(key);
}
});

return (
Promise.all(pendingTasks)
// Wait for all the pending tasks to resolve and then add the data to the data map.
.then((values) => {
values.forEach((value, index) => {
dataMap.set(pendingKeys[index], value);
});

return Promise.resolve();
})
// Get the missing keys using multiGet from the storage.
.then(() => {
if (missingKeys.length === 0) {
return Promise.resolve(undefined);
}

return Storage.multiGet(missingKeys);
})
// Add the data from the missing keys to the data map and also merge it to the cache.
.then((values) => {
if (!values || values.length === 0) {
return dataMap;
}

// temp object is used to merge the missing data into the cache
const temp: OnyxCollection<KeyValueMapping[TKey]> = {};
values.forEach(([key, value]) => {
dataMap.set(key, value as OnyxValue<TKey>);
temp[key] = value as OnyxValue<TKey>;
});
cache.merge(temp);
return dataMap;
})
);
}

/** Returns current key names stored in persisted storage */
function getAllKeys(): Promise<Set<OnyxKey>> {
// When we've already read stored keys, resolve right away
Expand Down Expand Up @@ -843,75 +917,10 @@ function addKeyToRecentlyAccessedIfNeeded<TKey extends OnyxKey>(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<TKey extends OnyxKey>(matchingKeys: CollectionKeyBase[], mapping: Mapping<TKey>): void {
// Keys that are not in the cache
const missingKeys: OnyxKey[] = [];
// Tasks that are pending
const pendingTasks: Array<Promise<OnyxValue<TKey>>> = [];
// 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<KeyValueMapping[TKey]> = {};

/**
* 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<TKey>;
if (cacheValue) {
data[key] = cacheValue;
return;
}

const pendingKey = `get:${key}`;
if (cache.hasPendingTask(pendingKey)) {
pendingTasks.push(cache.getTaskPromise(pendingKey) as Promise<OnyxValue<TKey>>);
pendingKeys.push(key);
} else {
missingKeys.push(key);
}
multiGet(matchingKeys).then((dataMap) => {
const data = Object.fromEntries(dataMap.entries()) as OnyxValue<TKey>;
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<KeyValueMapping[TKey]> = {};
values.forEach(([key, value]) => {
data[key] = value as OnyxValue<TKey>;
temp[key] = value as OnyxValue<TKey>;
});
cache.merge(temp);
return Promise.resolve();
})
// We are going to send the data to the subscriber.
.finally(() => {
sendDataToConnection(mapping, data as OnyxValue<TKey>, undefined, true);
});
}

/**
Expand Down Expand Up @@ -1150,6 +1159,7 @@ const OnyxUtils = {
applyMerge,
initializeWithDefaultKeyStates,
getSnapshotKey,
multiGet,
};

export default OnyxUtils;
14 changes: 6 additions & 8 deletions lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ function isEmptyObject<T>(obj: T | EmptyValue): obj is EmptyValue {
*/
function isMergeableObject(value: unknown): value is Record<string, unknown> {
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);
}

/**
Expand All @@ -37,9 +37,8 @@ function mergeObject<TObject extends Record<string, unknown>>(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];

Expand All @@ -58,10 +57,9 @@ function mergeObject<TObject extends Record<string, unknown>>(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<string, unknown>;
const targetValue = targetObject?.[key];

// If undefined is passed as the source value for a key, we want to generally ignore it.
Expand Down
Loading