Skip to content

Commit 8391cc7

Browse files
committed
keep performance merge logic on native
1 parent ccedbd8 commit 8391cc7

4 files changed

Lines changed: 86 additions & 34 deletions

File tree

lib/Onyx.ts

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
/* eslint-disable no-continue */
21
import _ from 'underscore';
32
import lodashPick from 'lodash/pick';
43
import * as Logger from './Logger';
@@ -35,6 +34,7 @@ import type {Connection} from './OnyxConnectionManager';
3534
import connectionManager from './OnyxConnectionManager';
3635
import * as GlobalSettings from './GlobalSettings';
3736
import decorateWithMetrics from './metrics';
37+
import OnyxMerge from './OnyxMerge.native';
3838

3939
/** Initialize the store with actions and listening for storage events */
4040
function init({
@@ -325,26 +325,7 @@ function merge<TKey extends OnyxKey>(key: TKey, changes: OnyxMergeInput<TKey>):
325325
return Promise.resolve();
326326
}
327327

328-
const {result: mergedValue} = OnyxUtils.mergeChanges(validChanges, existingValue);
329-
330-
// In cache, we don't want to remove the key if it's null to improve performance and speed up the next merge.
331-
const hasChanged = cache.hasValueChanged(key, mergedValue);
332-
333-
// Logging properties only since values could be sensitive things we don't want to log.
334-
Logger.logInfo(`merge called for key: ${key}${_.isObject(mergedValue) ? ` properties: ${_.keys(mergedValue).join(',')}` : ''} hasChanged: ${hasChanged}`);
335-
336-
// This approach prioritizes fast UI changes without waiting for data to be stored in device storage.
337-
const updatePromise = OnyxUtils.broadcastUpdate(key, mergedValue as OnyxValue<TKey>, hasChanged);
338-
339-
// If the value has not changed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead.
340-
if (!hasChanged) {
341-
return updatePromise;
342-
}
343-
344-
return Storage.setItem(key, mergedValue as OnyxValue<TKey>).then(() => {
345-
OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.MERGE, key, changes, mergedValue);
346-
return updatePromise;
347-
});
328+
return OnyxMerge.applyMerge(key, existingValue, validChanges);
348329
} catch (error) {
349330
Logger.logAlert(`An error occurred while applying merge for key: ${key}, Error: ${error}`);
350331
return Promise.resolve();

lib/OnyxMerge.native.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import _ from 'underscore';
2+
import * as Logger from './Logger';
3+
import OnyxUtils from './OnyxUtils';
4+
import type {OnyxKey, OnyxValue} from './types';
5+
import cache from './OnyxCache';
6+
import Storage from './storage';
7+
8+
function applyMerge<TKey extends OnyxKey>(key: TKey, existingValue: OnyxValue<TKey>, validChanges: unknown[]): Promise<void> {
9+
// If any of the changes is null, we need to discard the existing value.
10+
const baseValue = validChanges.includes(null) ? undefined : existingValue;
11+
12+
// We first batch the changes into a single change with object removal marks,
13+
// so that SQLite can merge the changes more efficiently.
14+
const {result: batchedChanges, replaceNullPatches} = OnyxUtils.mergeAndMarkChanges(validChanges);
15+
16+
// We then merge the batched changes with the existing value, because we need to final merged value to broadcast to subscribers.
17+
const {result: mergedValue} = OnyxUtils.mergeChanges([batchedChanges], baseValue);
18+
19+
// In cache, we don't want to remove the key if it's null to improve performance and speed up the next merge.
20+
const hasChanged = cache.hasValueChanged(key, mergedValue);
21+
22+
// Logging properties only since values could be sensitive things we don't want to log.
23+
Logger.logInfo(`merge called for key: ${key}${_.isObject(mergedValue) ? ` properties: ${_.keys(mergedValue).join(',')}` : ''} hasChanged: ${hasChanged}`);
24+
25+
// This approach prioritizes fast UI changes without waiting for data to be stored in device storage.
26+
const updatePromise = OnyxUtils.broadcastUpdate(key, mergedValue as OnyxValue<TKey>, hasChanged);
27+
28+
// If the value has not changed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead.
29+
if (!hasChanged) {
30+
return updatePromise;
31+
}
32+
33+
return Storage.mergeItem(key, batchedChanges as OnyxValue<TKey>, replaceNullPatches).then(() => {
34+
OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.MERGE, key, validChanges, mergedValue);
35+
return updatePromise;
36+
});
37+
}
38+
39+
const OnyxMerge = {
40+
applyMerge,
41+
};
42+
43+
export default OnyxMerge;

lib/OnyxMerge.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import _ from 'underscore';
2+
import * as Logger from './Logger';
3+
import OnyxUtils from './OnyxUtils';
4+
import type {OnyxKey, OnyxValue} from './types';
5+
import cache from './OnyxCache';
6+
import Storage from './storage';
7+
8+
function applyMerge<TKey extends OnyxKey>(key: TKey, existingValue: OnyxValue<TKey>, validChanges: unknown[]): Promise<void> {
9+
const {result: mergedValue} = OnyxUtils.mergeChanges(validChanges, existingValue);
10+
11+
// In cache, we don't want to remove the key if it's null to improve performance and speed up the next merge.
12+
const hasChanged = cache.hasValueChanged(key, mergedValue);
13+
14+
// Logging properties only since values could be sensitive things we don't want to log.
15+
Logger.logInfo(`merge called for key: ${key}${_.isObject(mergedValue) ? ` properties: ${_.keys(mergedValue).join(',')}` : ''} hasChanged: ${hasChanged}`);
16+
17+
// This approach prioritizes fast UI changes without waiting for data to be stored in device storage.
18+
const updatePromise = OnyxUtils.broadcastUpdate(key, mergedValue as OnyxValue<TKey>, hasChanged);
19+
20+
// If the value has not changed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead.
21+
if (!hasChanged) {
22+
return updatePromise;
23+
}
24+
25+
return Storage.setItem(key, mergedValue as OnyxValue<TKey>).then(() => {
26+
OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.MERGE, key, validChanges, mergedValue);
27+
return updatePromise;
28+
});
29+
}
30+
31+
const OnyxMerge = {
32+
applyMerge,
33+
};
34+
35+
export default OnyxMerge;

lib/OnyxUtils.ts

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1236,14 +1236,11 @@ function prepareKeyValuePairsForStorage(
12361236
return pairs;
12371237
}
12381238

1239-
function mergeChanges<TValue extends OnyxInput<OnyxKey> | undefined, TChange extends OnyxInput<OnyxKey> | undefined>(changes: TChange[], existingValue?: TValue): FastMergeResult<TChange> {
1239+
function mergeChanges<TValue extends OnyxInput<OnyxKey> | undefined>(changes: TValue[], existingValue?: TValue): FastMergeResult<TValue> {
12401240
return applyMerge('merge', changes, existingValue);
12411241
}
12421242

1243-
function mergeAndMarkChanges<TValue extends OnyxInput<OnyxKey> | undefined, TChange extends OnyxInput<OnyxKey> | undefined>(
1244-
changes: TChange[],
1245-
existingValue?: TValue,
1246-
): FastMergeResult<TChange> {
1243+
function mergeAndMarkChanges<TValue extends OnyxInput<OnyxKey> | undefined>(changes: TValue[], existingValue?: TValue): FastMergeResult<TValue> {
12471244
return applyMerge('mark', changes, existingValue);
12481245
}
12491246

@@ -1253,11 +1250,7 @@ function mergeAndMarkChanges<TValue extends OnyxInput<OnyxKey> | undefined, TCha
12531250
* @param changes Array of changes that should be merged
12541251
* @param existingValue The existing value that should be merged with the changes
12551252
*/
1256-
function applyMerge<TValue extends OnyxInput<OnyxKey> | undefined, TChange extends OnyxInput<OnyxKey> | undefined>(
1257-
mode: 'merge' | 'mark',
1258-
changes: TChange[],
1259-
existingValue?: TValue,
1260-
): FastMergeResult<TChange> {
1253+
function applyMerge<TValue extends OnyxInput<OnyxKey> | undefined>(mode: 'merge' | 'mark', changes: TValue[], existingValue?: TValue): FastMergeResult<TValue> {
12611254
const lastChange = changes?.at(-1);
12621255

12631256
if (Array.isArray(lastChange)) {
@@ -1266,7 +1259,7 @@ function applyMerge<TValue extends OnyxInput<OnyxKey> | undefined, TChange exten
12661259

12671260
if (changes.some((change) => change && typeof change === 'object')) {
12681261
// Object values are then merged one after the other
1269-
return changes.reduce<FastMergeResult<TChange>>(
1262+
return changes.reduce<FastMergeResult<TValue>>(
12701263
(modifiedData, change) => {
12711264
const options: FastMergeOptions = mode === 'merge' ? {shouldRemoveNestedNulls: true, objectRemovalMode: 'replace'} : {objectRemovalMode: 'mark'};
12721265
const {result, replaceNullPatches} = utils.fastMerge(modifiedData.result, change, options);
@@ -1279,15 +1272,15 @@ function applyMerge<TValue extends OnyxInput<OnyxKey> | undefined, TChange exten
12791272
return modifiedData;
12801273
},
12811274
{
1282-
result: (existingValue ?? {}) as TChange,
1275+
result: (existingValue ?? {}) as TValue,
12831276
replaceNullPatches: [],
12841277
},
12851278
);
12861279
}
12871280

12881281
// If we have anything else we can't merge it so we'll
12891282
// simply return the last value that was queued
1290-
return {result: lastChange as TChange, replaceNullPatches: []};
1283+
return {result: lastChange as TValue, replaceNullPatches: []};
12911284
}
12921285

12931286
/**

0 commit comments

Comments
 (0)