App: [Performance] localforage.setItem() is causing blocking behavior on init for web clients

Creating this in E/App repo from this issue in the E/react-native-onyx repo, in case an external contributor can work on it.

Problem

When the app inits and many keys need to be set things get really slow on web. It currently takes me about 10-12 seconds to switch to a new chat when the app inits.

We’ve narrowed down the source of the problem to:

Not stringifying IndexedDB values. For whatever reason IDB takes a while to store structured JSON.
Calling setItem() too many times in a row

Addressing either one should work. Both seem to be causing similar rates of pain.

Solution

Start stringifying values - complicated because some of the JSON is not serializable i.e. stuff with attachments.
Defer the writing to IndexedDB until we are idle or just slow it down or something at least. We are using a memory cache already so I can’t think of a reason why our writes must happen immediately on page load.

More ideas here: https://rxdb.info/slow-indexeddb.html, but the general suggestion is to use fewer transactions. That is not possible with localforage so we might want to look elsewhere - maybe the plugin mentioned here can help https://github.com/localForage/localForage/issues/315

I did a test here where I just made all the writes synchronous (probably not the solution we want), but it improves things immensely

https://github.com/Expensify/react-native-onyx/pull/118

cc @marcaaron

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 21 (14 by maintainers)

Most upvoted comments

Possibly related, I’d like to try to test after this issue is fixed to see if can reproduce

@dylanexpensify , I posted the job since I was back in here…

https://www.upwork.com/jobs/~01d8bffdd4e6f23ff8

mallenexpensify on Mar 2, 2022

a fix should be on staging soon.

marcaaron on Mar 23, 2022

Linked PRs to fix this are not merged yet so gonna reopen this and assign myself.

marcaaron on Mar 14, 2022

How/when do we determine to batch which requests?

For this specific storage provider (WebStorage) any Onyx write operation (set, merge or mergeCollection) is applied through setItem

We’ve decided to batch

write requests
when a write is already running, capture incoming write requests and apply them as the next batch
we target only writes because there are no problem with many read transactions running in parallel

What happens if a change depends on an existing value in the current batch that is also being updated? Do we just have them in different batches?

Whenever we write something with Onyx we first write the change to cache App is free to read anything anytime and then use it to write something new App would read (though Onyx) the latest data from cache - this should cover the case where a new value depends on existing value

Batches would apply changes in FIFO, so if a batch captures multiple updates for the same key, the last update would overwrite previous. (We can further optimize this to just use the latest key if necessary)

Though if we were to requeue keys we’d be held on that before being able to commit another batch right?

The way we requeue the failed keys is that we insert them in the begging of the next batch and then start the next batch. This would ensure they get overwritten if there are newer changes for these keys So we shouldn’t be blocked

It might turn out that the SyncQueue implemented in @marcaaron PR is completely satisfying performance and there’s no need for all of this

kidroca on Mar 10, 2022

Alright, I tried to wrap my head around this but I just have a lot of (basic) questions which I’ll post later. @thienlnam please take over this issue. I’m just gonna watch and learn, and ofc ask questions 😃

cc: @parasharrajat / @Santhosh-Sellavel if you’re interested in reviewing this.

rushatgabhane on Mar 10, 2022

Reassigning C+ to @rushatgabhane , @Santhosh-Sellavel is starting this/next week @kidroca , can you please look into? For a fix or to help with a fix

mallenexpensify on Mar 2, 2022