Investigate flushing Onyx cache to disk on idle

[marcaaron]

We have observed that IndexedDB can be super slow with writes and cause major lag and jank in the app when there is a huge amount of data to store on App init.

Our solution for now has been to just completely cease storing certain keys entirely. But, another potentially viable solution is to keep the data in memory for some time and then persist it to disk when the user is "idling".

cc @hannojg @kidroca curious if you guys have considered this one yet.

h/t @quinthar for the idea

[kidroca]

Hi @marcaaron,

Technically, it's not so much that IndexedDB is inherently slow, but more so that it's implemented inefficiently by localforage when subjected to frequent write operations.

To illustrate, as noted in this thread: https://expensify.slack.com/archives/C01GTK53T8Q/p1689267792230089?thread_ts=1689036681.307159&cid=C01GTK53T8Q:

"Reviewing the localForage source code for setItem, we can observe that a new transaction is indeed opened each time the function is invoked. This could be a significant contributing factor to the noticeable slowdown experienced when executing multiSet or multiMerge operations. Currently, both these operations use setItem to process each item individually, which could potentially lead to high resource contention and decreased performance."

In light of this, I propose that we consider implementing a storage provider that circumvents the use of localforage. Instead, this provider would interface directly with the IndexedDB API, enabling it to more effectively strategize and optimize transactions. For instance, it could perform a single transaction to update multiple keys as opposed to sequential transactions. This would decrease the likelihood of lock contention and should, in theory, enhance speed significantly.
(The change should also let us drop the SyncQueue as we should be able to write in one go, not having to schedule writes one by one)

Alternatively, we could explore other storage driver libraries that might be a better fit for our needs. There exist libraries that streamline the IndexedDB interface, offering ease of use without forfeiting granular control.

I like the idea of batching a few changes and applying them at the best time. It appears that the app does not require immediate data writes. In the worst-case scenario, we might "skip of a beat," and some information might need to be fetched remotely due to delayed persistence.

[marcaaron]

Great thoughts @kidroca. So it seems like our options are:

• Investigate writing our own IndexedDB implementation (Feels potentially high complexity, but I lack experience working with the API directly so not sure)
• Investigate an existing API wrapper around IndexedDB besides localforage (Does it exist?)
• Keep using localforage, but defer writes until idle

I do like this 3rd option since it does not seem to difficult to implement something like:

let timer;
const idleTime = 2000; // or whatever we want to use for this

function resetTimer() {
  pauseSetItemQueue();
  clearTimeout(timer);
  timer = setTimeout(processSetItemQueue, idleTime);
}

document.addEventListener('mousemove', resetTimer);
document.addEventListener('keypress', resetTimer);

[kidroca]

@marcaaron I think this library might work better for our case: https://www.npmjs.com/package/idb-keyval the following methods are noteworthy:

• setMany: Set many keyval pairs at once. This is faster than calling set multiple times.
• update: gives access to the prev value similar to how setState works in react

Regarding deferring until idle, there's this API available to browsers as well as react-native: https://caniuse.com/requestidlecallback - requestIdleCallback

It accepts callback function that will be executed when the browser becomes idle
There's also a numeric timeout parameter in case we want to set a condition like "execute when idle, but no later than {n} milliseconds from now"
Requests are otherwise served in a first in first out basis

Safari lacks support for requestIdleCallback, but there's a polyfill to cover the missing API

[marcaaron]

requestIdleCallback looks pretty neat.

[hannojg]

Hey, I somehow overlooked this ticket and was doing a performance audit on a guides account and came to the exact same issue of the massive amount of data we are persisting at app init.
Here are my thoughts so far (some of them are maybe out of the box, watch out 🧙‍♂️):

1. As @kidroca says, indexedDB itself can be pretty fast. The approach I think would be the most valuable to explore is to use indexedDB directly (or a neater layer if that really exists), and optimise the usage for how we need it. I believe this can bring the most performance improvements (and I have a strong itch to start writing a PoC 😄). Why do I believe it can bring the most performance improvements? I just measured it and for my guide account there are over 100.000 (yes) calls to storage.setItem. This also completely spams the microtask queue.
   Batching all of that together (or at least in a few big transactions) should get most of the performance back I believe.
2. Move onyx to another thread using web workers. See my thought process about it here. The main idea is that it won't matter to congest the micro task queue if it happens on a different thread (however, this has the downside of the persistence still taking super long and we might loose data if the user closes the browser window prematurely.)
3. When it comes to running task only when the browser gets idle I think there are better solutions. Check out this code example, which utilises isInputPending()

async function saveSettings () {
  // A task queue of functions
  // Imagine this being tasks e.g. inside onyx where we we execute database operations.
  const tasks = [
    validateForm,
    showSpinner,
    saveToDatabase,
    updateUI,
    sendAnalytics
  ];
  
  // The value of 50 is important here.
  // Any task that takes longer than 50ms is considered
  // too long and blocking. So we don't want our tasks to
  // take longer than that (so the browser has a chance to
  // squeeze in other tasks e.g. handling user interactions)
  let deadline = performance.now() + 50;

  while (tasks.length > 0) {
    // Optional chaining operator used here helps to avoid
    // errors in browsers that don't support `isInputPending`:
    if (navigator.scheduling?.isInputPending() || performance.now() >= deadline) {
      // There's a pending user input, or the
      // deadline has been reached. Yield here:
      await yieldToMain();

      // Extend the deadline:
      deadline = performance.now() + 50;

      // Stop the execution of the current loop and
      // move onto the next iteration:
      continue;
    }

    // Shift the task out of the queue:
    const task = tasks.shift();

    // Run the task:
    task();
  }
}```

[fedirjh]

I agree with @kidroca about using the https://www.npmjs.com/package/idb-keyval as a new provider. I have tested this package before, and it was significantly faster compared to localforage. I wrote this POC based on the current approach of queuing the tasks. Although I'm not certain if this is the ideal solution, the results were quite promising.

import {
    get, set, createStore, clear, keys as getKeys, del, getMany, setMany,
} from 'idb-keyval';
import _ from 'underscore';
import fastMerge from '../../fastMerge';
import SyncQueue from '../../SyncQueue';

const dbName = 'OnyxDB';
const storeName = 'keyvaluepairs';

const OnyxStore = createStore(dbName, storeName);

const syncQueue = new SyncQueue(({keyVal, command}) => {
    switch (command) {
        case 'multiSet': {
            const label = `multiSet ${keyVal.length}`;
            // eslint-disable-next-line no-console
            console.time(label);
            return setMany(keyVal, OnyxStore).then(() => {
                // eslint-disable-next-line no-console
                console.timeEnd(label);
                return Promise.resolve();
            });
        }
        case 'multiMerge': {
            const label = `multiMerge ${keyVal.length}`;
            // eslint-disable-next-line no-console
            console.time(label);
            const keys = _.map(keyVal, ([key]) => key);

            // We use getMany to get all existing data in one transaction
            // then we merge the existing data with new one
            // finally we use setMany to update new data in one transaction
            return getMany(keys, OnyxStore)
                .then(values => _.object(_.map(keys, (key, i) => [key, values[i]])))
                .then((existingPairsObject) => {
                    const newPairs = _.map(
                        keyVal,
                        ([key, value]) => {
                            // if value is not an object or an array then overwrite the value stored
                            if (!_.isObject(value) && !_.isArray(value)) {
                                return [key, value];
                            }
                            const existingValue = existingPairsObject[key];
                            const newValue = _.isObject(existingValue) ? ({...fastMerge(existingValue, value)}) : value;
                            return ([key, newValue]);
                        },
                    );
                    return setMany(newPairs, OnyxStore).then(() => {
                        // eslint-disable-next-line no-console
                        console.timeEnd(label);
                        return Promise.resolve();
                    });
                });
        }
        case 'setItem': return set(keyVal[0], keyVal[1], OnyxStore);
        default: return set(keyVal[0], keyVal[1], OnyxStore);
    }
});

const provider = {
    getItem: key => get(key, OnyxStore),
    setItem: (key, value) => syncQueue.push({keyVal: [key, value], command: 'setItem'}),
    clear: () => clear(OnyxStore),
    getAllKeys: () => getKeys(OnyxStore),
    removeItem: key => del(key, OnyxStore),
    multiGet: keys => getMany(keys, OnyxStore).then(values => _.map(keys, (key, i) => [key, values[i]])),
    multiSet: pairs => syncQueue.push({keyVal: pairs, command: 'multiSet'}),
    multiMerge: pairs => syncQueue.push({keyVal: pairs, command: 'multiMerge'}),
};

export default provider;

[marcaaron]

Move onyx to another thread using web workers

Curious to hear more about this. So, would we just defer writes to the worker thread by posting data to the worker and then have those messages get written to IndexedDB? Does it work the same when reading data?

[marcaaron]

this has the downside of the persistence still taking super long and we might loose data if the user closes the browser window prematurely

I am not sure how to get around that. Either have to save the data faster or expect to lose some of it 😄

Losing it might not be much of a concern depending on what is lost, when it would be refreshed again, and what bad things could happen if the data is not persisted (maybe nothing).

[marcaaron]

Maybe as a next step we can try to test idb-keyval with setMany() on one of these inundated accounts to gauge real-world web improvements? Let's also test mobile web?

That would probably give us an idea of what kind of performance improvements we might see from getting off localforage (which I think is kind of a separate issue).

Then we can explore throttling storage after?

FWIW, I think web workers are pretty cool, but most hesitant about this idea since it will add some build complexity.

[hannojg]

Hey 👋 I'd like to take the initiative to test with idb-keyval as we are currently hired to improve the performance, and this would just be my next step 😊

Also I'd still would like to test a OMT performance PoC.

Then we can explore throttling storage after?

Yes! (You are right this is a bit of a separate issue we are talking about atm, but I think that should come first)

So, would we just defer writes to the worker thread by posting data to the worker and then have those messages get written to IndexedDB? Does it work the same when reading data?

Yes, the worker thread would be concerned with writing the data. I also thought an about it reading the data. It might adds a tiny little overhead but then we are keeping more work away from the main thread which is the whole idea of the change

[hannojg]

Okay, here are the first result of a really quick test (will do more tomorrow):

Onyx with localforage

Onyx with idb-keyval/IndexedDB

[hannojg]

Results for 1.000 items:

Onyx right now: 6500ms
Onyx with new lib: 143ms

[marcaaron]

Wow. What are we waiting for??? 😄

Seems clearly much better. Next thought would be...

Can we use the same DB? Or do we need some migration to move from localforage to idb-keyval?

Looks very promising though so I think we should go for it.

[hannojg]

I think no migration is needed. I will write a PR today, and check the results for when used in actual product 😊

// Edit: result shared here https://expensify.slack.com/archives/C035J5C9FAP/p1691063111819169