[Performance] Add a new method setAllKeys to OnyxCache.ts

[muttmuure]

Problem

In Onyx.js we have getAllKeys method, which upon invocation first try to get the keys from cache, if no keys are in cache then it look whether there is a pending task and finally it gets all the keys from the storage. At this step, we are iterating over the returned keys from the storage and adding them individually to the cache using addKey method from OnyxCache.ts .
Below is how things look as of now:

function getAllKeys() {
    // When we've already read stored keys, resolve right away
    const storedKeys = cache.getAllKeys();
    if (storedKeys.length > 0) {
        return Promise.resolve(storedKeys);
    }

    const taskName = 'getAllKeys';

    // When a value retrieving task for all keys is still running hook to it
    if (cache.hasPendingTask(taskName)) {
        return cache.getTaskPromise(taskName);
    }

    // Otherwise retrieve the keys from storage and capture a promise to aid concurrent usages
    const promise = Storage.getAllKeys().then((keys) => {
        _.each(keys, (key) => cache.addKey(key));
        return keys;
    });

    return cache.captureTask(taskName, promise);
}

Solution

We can improve this by adding a new method setAllKeys to OnyxCache.ts which will accept keys as arguments and set them to storageKeys . Then we can use this method instead of iterating over the keys returned from the storage.

Let’s see how it looks like in action:

// In OnyxCache.js

    setAllKeys(keys: Key[]) {
        this.storageKeys = new Set(keys);
    }

// In Onyx.js

function getAllKeys() {
 .....

    const promise = Storage.getAllKeys().then((keys) => {
        cache.setAllKeys(keys);
        return keys;
    });
....
}

Previously, this Onyx->getAllKeys() took around ~36 ms and with this improvement it is now reduced to ~22 ms. You can find the Before and After Hermes traces screenshots in the PR.

Expensify/react-native-onyx#501

From @hurali97 here

[hurali97]

@muttmuure Please assign this to me 👍

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[mountiny]

@hurali97 I believe this was deployed, is there a way we could measure the impact of this?