Skip to content

Commit b0eb40b

Browse files
committed
Remove macro/micro tasks during subscriber update
1 parent 079a43d commit b0eb40b

File tree

5 files changed

+20
-52
lines changed

5 files changed

+20
-52
lines changed

lib/Onyx.ts

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -249,9 +249,8 @@ function merge<TKey extends OnyxKey>(key: TKey, changes: OnyxMergeInput<TKey>):
249249
return Promise.resolve();
250250
}
251251

252-
return OnyxMerge.applyMerge(key, existingValue, validChanges).then(({mergedValue, updatePromise}) => {
252+
return OnyxMerge.applyMerge(key, existingValue, validChanges).then(({mergedValue}) => {
253253
OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.MERGE, key, changes, mergedValue);
254-
return updatePromise;
255254
});
256255
} catch (error) {
257256
Logger.logAlert(`An error occurred while applying merge for key: ${key}, Error: ${error}`);
@@ -361,16 +360,6 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise<void> {
361360
keysToBeClearedFromStorage.push(key);
362361
}
363362

364-
const updatePromises: Array<Promise<void>> = [];
365-
366-
// Notify the subscribers for each key/value group so they can receive the new values
367-
for (const [key, value] of Object.entries(keyValuesToResetIndividually)) {
368-
updatePromises.push(OnyxUtils.scheduleSubscriberUpdate(key, value));
369-
}
370-
for (const [key, value] of Object.entries(keyValuesToResetAsCollection)) {
371-
updatePromises.push(OnyxUtils.scheduleNotifyCollectionSubscribers(key, value));
372-
}
373-
374363
const defaultKeyValuePairs = Object.entries(
375364
Object.keys(defaultKeyStates)
376365
.filter((key) => !keysToPreserve.includes(key))
@@ -388,7 +377,13 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise<void> {
388377
.then(() => Storage.multiSet(defaultKeyValuePairs))
389378
.then(() => {
390379
DevTools.clearState(keysToPreserve);
391-
return Promise.all(updatePromises);
380+
// Notify the subscribers for each key/value group so they can receive the new values
381+
for (const [key, value] of Object.entries(keyValuesToResetIndividually)) {
382+
OnyxUtils.scheduleSubscriberUpdate(key, value);
383+
}
384+
for (const [key, value] of Object.entries(keyValuesToResetAsCollection)) {
385+
OnyxUtils.scheduleNotifyCollectionSubscribers(key, value);
386+
}
392387
});
393388
})
394389
.then(() => undefined);

lib/OnyxMerge/index.native.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,18 +26,17 @@ const applyMerge: ApplyMerge = <TKey extends OnyxKey, TValue extends OnyxInput<T
2626
OnyxUtils.logKeyChanged(OnyxUtils.METHOD.MERGE, key, mergedValue, hasChanged);
2727

2828
// This approach prioritizes fast UI changes without waiting for data to be stored in device storage.
29-
const updatePromise = OnyxUtils.broadcastUpdate(key, mergedValue as OnyxValue<TKey>, hasChanged);
29+
OnyxUtils.broadcastUpdate(key, mergedValue as OnyxValue<TKey>, hasChanged);
3030

3131
// If the value has not changed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead.
3232
if (!hasChanged) {
33-
return Promise.resolve({mergedValue, updatePromise});
33+
return Promise.resolve({mergedValue});
3434
}
3535

3636
// For native platforms we use `mergeItem` that will take advantage of JSON_PATCH and JSON_REPLACE SQL operations to
3737
// merge the object in a performant way.
3838
return Storage.mergeItem(key, batchedChanges as OnyxValue<TKey>, replaceNullPatches).then(() => ({
3939
mergedValue,
40-
updatePromise,
4140
}));
4241
};
4342

lib/OnyxMerge/types.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import type {OnyxInput, OnyxKey} from '../types';
22

33
type ApplyMergeResult<TValue> = {
44
mergedValue: TValue;
5-
updatePromise: Promise<void>;
65
};
76

87
type ApplyMerge = <TKey extends OnyxKey, TValue extends OnyxInput<OnyxKey> | undefined, TChange extends OnyxInput<OnyxKey> | null>(

lib/OnyxUtils.ts

Lines changed: 8 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,6 @@ type OnyxMethod = ValueOf<typeof METHOD>;
7575
let mergeQueue: Record<OnyxKey, Array<OnyxValue<OnyxKey>>> = {};
7676
let mergeQueuePromise: Record<OnyxKey, Promise<void>> = {};
7777

78-
// Used to schedule subscriber update to the macro task queue
79-
let nextMacrotaskPromise: Promise<void> | null = null;
80-
8178
// Holds a mapping of all the React components that want their state subscribed to a store key
8279
let callbackToStateMapping: Record<string, CallbackToStateMapping<OnyxKey>> = {};
8380

@@ -803,23 +800,6 @@ function getCollectionDataAndSendAsObject<TKey extends OnyxKey>(matchingKeys: Co
803800
});
804801
}
805802

806-
/**
807-
* Delays promise resolution until the next macrotask to prevent race condition if the key subscription is in progress.
808-
*
809-
* @param callback The keyChanged/keysChanged callback
810-
* */
811-
function prepareSubscriberUpdate(callback: () => void): Promise<void> {
812-
if (!nextMacrotaskPromise) {
813-
nextMacrotaskPromise = new Promise<void>((resolve) => {
814-
setTimeout(() => {
815-
nextMacrotaskPromise = null;
816-
resolve();
817-
}, 0);
818-
});
819-
}
820-
return Promise.all([nextMacrotaskPromise, Promise.resolve().then(callback)]).then();
821-
}
822-
823803
/**
824804
* Schedules an update that will be appended to the macro task queue (so it doesn't update the subscribers immediately).
825805
*
@@ -831,21 +811,17 @@ function scheduleSubscriberUpdate<TKey extends OnyxKey>(
831811
value: OnyxValue<TKey>,
832812
canUpdateSubscriber: (subscriber?: CallbackToStateMapping<OnyxKey>) => boolean = () => true,
833813
isProcessingCollectionUpdate = false,
834-
): Promise<void> {
835-
return prepareSubscriberUpdate(() => keyChanged(key, value, canUpdateSubscriber, isProcessingCollectionUpdate));
814+
): void {
815+
keyChanged(key, value, canUpdateSubscriber, isProcessingCollectionUpdate);
836816
}
837817

838818
/**
839819
* This method is similar to scheduleSubscriberUpdate but it is built for working specifically with collections
840820
* so that keysChanged() is triggered for the collection and not keyChanged(). If this was not done, then the
841821
* subscriber callbacks receive the data in a different format than they normally expect and it breaks code.
842822
*/
843-
function scheduleNotifyCollectionSubscribers<TKey extends OnyxKey>(
844-
key: TKey,
845-
value: OnyxCollection<KeyValueMapping[TKey]>,
846-
previousValue?: OnyxCollection<KeyValueMapping[TKey]>,
847-
): Promise<void> {
848-
return prepareSubscriberUpdate(() => keysChanged(key, value, previousValue));
823+
function scheduleNotifyCollectionSubscribers<TKey extends OnyxKey>(key: TKey, value: OnyxCollection<KeyValueMapping[TKey]>, previousValue?: OnyxCollection<KeyValueMapping[TKey]>): void {
824+
keysChanged(key, value, previousValue);
849825
}
850826

851827
/**
@@ -919,7 +895,7 @@ function retryOperation<TMethod extends RetriableOnyxOperation>(error: Error, on
919895
/**
920896
* Notifies subscribers and writes current value to cache
921897
*/
922-
function broadcastUpdate<TKey extends OnyxKey>(key: TKey, value: OnyxValue<TKey>, hasChanged?: boolean): Promise<void> {
898+
function broadcastUpdate<TKey extends OnyxKey>(key: TKey, value: OnyxValue<TKey>, hasChanged?: boolean): void {
923899
// Update subscribers if the cached value has changed, or when the subscriber specifically requires
924900
// all updates regardless of value changes (indicated by initWithStoredValues set to false).
925901
if (hasChanged) {
@@ -928,7 +904,7 @@ function broadcastUpdate<TKey extends OnyxKey>(key: TKey, value: OnyxValue<TKey>
928904
cache.addToAccessedKeys(key);
929905
}
930906

931-
return scheduleSubscriberUpdate(key, value, (subscriber) => hasChanged || subscriber?.initWithStoredValues === false).then(() => undefined);
907+
scheduleSubscriberUpdate(key, value, (subscriber) => hasChanged || subscriber?.initWithStoredValues === false);
932908
}
933909

934910
function hasPendingMergeForKey(key: OnyxKey): boolean {
@@ -1314,18 +1290,17 @@ function setWithRetry<TKey extends OnyxKey>({key, value, options}: SetParams<TKe
13141290
OnyxUtils.logKeyChanged(OnyxUtils.METHOD.SET, key, value, hasChanged);
13151291

13161292
// This approach prioritizes fast UI changes without waiting for data to be stored in device storage.
1317-
const updatePromise = OnyxUtils.broadcastUpdate(key, valueWithoutNestedNullValues, hasChanged);
1293+
OnyxUtils.broadcastUpdate(key, valueWithoutNestedNullValues, hasChanged);
13181294

13191295
// If the value has not changed and this isn't a retry attempt, calling Storage.setItem() would be redundant and a waste of performance, so return early instead.
13201296
if (!hasChanged && !retryAttempt) {
1321-
return updatePromise;
1297+
return Promise.resolve();
13221298
}
13231299

13241300
return Storage.setItem(key, valueWithoutNestedNullValues)
13251301
.catch((error) => OnyxUtils.retryOperation(error, setWithRetry, {key, value: valueWithoutNestedNullValues, options}, retryAttempt))
13261302
.then(() => {
13271303
OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET, key, valueWithoutNestedNullValues);
1328-
return updatePromise;
13291304
});
13301305
}
13311306

tests/perf-test/OnyxUtils.perf-test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,7 @@ describe('OnyxUtils', () => {
452452
Object.entries(mockedReportActionsMap).map(([k, v]) => [k, createRandomReportAction(Number(v.reportActionID))] as const),
453453
) as GenericCollection;
454454

455-
await measureAsyncFunction(() => OnyxUtils.scheduleNotifyCollectionSubscribers(collectionKey, changedReportActions, mockedReportActionsMap), {
455+
await measureFunction(() => OnyxUtils.scheduleNotifyCollectionSubscribers(collectionKey, changedReportActions, mockedReportActionsMap), {
456456
beforeEach: async () => {
457457
await Onyx.multiSet(mockedReportActionsMap);
458458
for (const key of mockedReportActionsKeys) {
@@ -517,7 +517,7 @@ describe('OnyxUtils', () => {
517517
const reportAction = mockedReportActionsMap[`${collectionKey}0`];
518518
const changedReportAction = createRandomReportAction(Number(reportAction.reportActionID));
519519

520-
await measureAsyncFunction(() => OnyxUtils.broadcastUpdate(key, changedReportAction, true), {
520+
await measureFunction(() => OnyxUtils.broadcastUpdate(key, changedReportAction, true), {
521521
beforeEach: async () => {
522522
await Onyx.set(key, reportAction);
523523
},

0 commit comments

Comments
 (0)