[Performance] Onyx: Hydrate cache with all finite size keys before init

[kidroca]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

What performance issue do we need to solve?

• storage read times
• React native bridge concerns
• faster app init

What is the impact of this on end-users?

• improved boot time
• improved interactions that involve storage reads, primarily chat switches

List any benchmarks that show the severity of the issue

TBD

Proposed solution (if any)

Onyx keeps track of keys with infinite size - the "Safe Eviction Keys". The data stored under such keys can grow as long as there is storage for it.
ATM the list is solely the Report Action history

Using the "safe eviction keys" and "all keys" we can derive the "finite keys"
These are small size keys that are no larger than a kB each
They contain frequently used information like session, network, currentUrl, and general report information
The typical size all these keys take is around 20-30 kB of data
The only thing that makes the size vary is the general report information, each report would add 1kB of information

Problem

Currently the entire "finite" information is requested and read during init, it just happens in chunks as each components subscribes to different data. This doesn't happen all at once, some components are mounted after other components and they will request the storage data with a delay (at the time they mount)

Solution

Identify all "finite" keys and load them into cache with one multiGet request
Then initialise the app
Components will receive data as soon as they mount

Additional Consideration

Since general report information (typically needed to display reports in the LHN) is stored in separate keys and each one is about 1kB it might not be desired to fetch these in one multiGet
Perhaps these keys should be flagged as safe eviction as well e.g. what if you have 1000 such keys
Correct me if I'm wrong but I think currently we read all such keys to fill the LHN list of conversations
IMO these keys should be flagged as safe eviction as well and at some point a paging solution introduced so that we don't have to read 1000 keys, but perhaps read them incrementally as the users scrolls the LHN

List any benchmarks after implementing the changes to show impacts of the proposed solution (if any)

Note: These should be the same as the benchmarks collected before any changes.
TBD

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.0.82-5
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:

View all open jobs on Upwork

The idea original idea comes from this conversation: Expensify/react-native-onyx#84 (comment)

[MelvinBot]

Triggered auto assignment to @aldo-expensify (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[marcaaron]

Since general report information (typically needed to display reports in the LHN) is stored in separate keys and each one is about 1kB it might not be desired to fetch these in one multiGet
at some point a paging solution introduced so that we don't have to read 1000 keys, but perhaps read them incrementally as the users scrolls the LHN

I agree we need to look at the reports situation. We front load a ton of storage reads when the app inits and that will not scale well in the future when reports can number in the thousands.

But if the app is already slow because these reads are all happening in a short time frame - I'm confused about why batching would help? Would this maybe just delay init further since we are giving priority to "finite" keys instead of priority to things that need to render as quickly as possible?

Why will loading the keys all at once and then initing the app make things faster?

[marcaaron]

also had a random idea... I'm curious if it's possible to have the app init with the "finite" data already in memory without having to ask the native layer for it? e.g. could we hydrate the Onyx cache via the native layer so that the cache is pre-filled by the time the first Onyx.connect() happens? I'm unsure if this would help, but noticed it's possible to pass properties on init.

[kidroca]

But if the app is already slow because these reads are all happening in a short time frame - I'm confused about why batching would help?

1. Reading many keys with separate parallel gets is slower, compared to reading all those keys with one multi get
2. Reading keys separately triggers separate requests over the bridge
3. We're already make a multiGet that retrieves 4 keys before we let connections happen. We can just fetch the finite keys there - reading 4 or 100 keys should pretty much resolve for the same duration

About the last point I've made standalone storage tests

• requesting 50 keys of 9kb each with one multiGet took 100ms on average
• requesting 50 keys of 149kb each with one multiGet took 308ms on average

Would this maybe just delay init further since we are giving priority to "finite" keys instead of priority to things that need to render as quickly as possible?

I don't expect this to cause any delay, as pointed out above getting 7MB (50x149kB) information in one go from storage takes ~300ms (363ms for the worst case, Android)
The problem might be the getAllKeys request if you happen to have 1000 unique keys in storage - but we've already committed doing it

I'm curious if it's possible to have the app init with the "finite" data already in memory without having to ask the native layer for it?

Doing this would require to write some native iOS/Android code that reads storage and set those properties
The main benefit I see is this can happen before JS code inits, but how would the native layer know which are the "finite" keys
Typically JS will start, call Onyx.init which provides Onyx with that information

This might be something to explore if the current proposal provides some benefit, but we see that we need the keys hydrated even earlier

[marcaaron]

requesting 50 keys of 149kb each with one multiGet took 308ms on average

Nice, I'm still really curious to see the impacts on launch time in this case. In reality, we might not need all of that data we are reading on init and should be cautious about reading more than necessary. I'm up for trying this, but we should benchmark the time it takes for the app to become interactive and make sure front loading keys isn't moving the needle in the wrong direction.

[kidroca]

we should benchmark the time it takes for the app to become interactive and make sure front loading keys isn't moving the needle in the wrong direction.

I haven't found something definitive to measure time to interactive.
For example when I benchmark Onyx I will see a lot of calls happening after everything visible is rendered.
Another thing I've tried is the component - if we wrap the with it we can capture the last time something rendered. I've found the ReportActionView looks loaded but actually keeps rendering items for 5-10sec. more

[marcaaron]

I usually track how long it takes the sidebar to load in a release build. And measure time from here to here. I don't think we really have a perfect metric, but it's good to have some idea that things are improving, getting worse, or inconclusive.

[kidroca]

I still think it's worth investigating the effects of this proposal, and I'm planning to spent some time on it when I'm done with other tasks on my list

[MelvinBot]

@kidroca, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.