[HOLD for payment 2024-04-09] [$500] [Performance] Refactor our current Onyx.connect calls to Reports to include waitForCollectionCallback

[muttmuure]

Problem

We have lots of connections to Reports data in Onyx in our utils files and some of them uses waitForCollectionCallback: true and the rest of them don’t. Which adds up to the app startup time. The reason is that when we don’t set waitForCollectionCallback to true in Onyx.js the following code is executed:

// Onyx.js

if (mapping.waitForCollectionCallback) {
    getCollectionDataAndSendAsObject(matchingKeys, mapping);
    return;
}

 // We did not opt into using waitForCollectionCallback mode so the callback is called for every matching key.
for (let i = 0; i < matchingKeys.length; i++) {
    get(matchingKeys[i]).then((val) => sendDataToConnection(mapping, val, matchingKeys[i], true));
}

This loop will be executed for all the keys and will individually inform the subscriber of updates. The get function first checks whether the value for a key is present in cache and if it doesn’t it then gets it from the storage.
We have 6 connections to Reports without the waitForCollectionCallback so this loop will be executed 6 times. Bearing in mind that Reports can be a huge collection, for eg on a heavy account it can get to 15k reports.

Solution

We can consider to refactor our current Onyx.connect calls to Reports to include waitForCollectionCallback and adjust the code accordingly. There’s one place where we won’t be changing it and that’s in ReportUtils.ts where we have to call handleReportChanged on each report.
One more thing to notice is that currently our cache limit is 10k which is easily hit by a heavy account of 15k reports, so this cache limit isn’t helping the app in case of a heavy account. We can increase it to 20k so that it can hold more reports. Hermes traces are attached in the PR description.
To summarise, When launching the app on Android:

We get 32 seconds of TTI ----- on baseline, which is what we currently have 🔴
We get 25 seconds of TTI ----- with improvements by adding waitForCollectionCallback :large_green_circle:
We get 18 seconds of TTI ----- with increased cache limit to 20k and improvements by adding waitForCollectionCallback :large_green_circle: :large_green_circle:

From: @hurali97

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~012ebdb211b4ffdbac
• Upwork Job ID: 1770120318947196928
• Last Price Increase: 2024-03-19

Issue Owner

Current Issue Owner: @isabelastisser

[muttmuure]

#38464

[melvin-bot]

Triggered auto assignment to @isabelastisser (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~012ebdb211b4ffdbac

[melvin-bot]

Current assignee @shubham1206agra is eligible for the External assigner, not assigning anyone new.

[mountiny]

Handled by callstack

[isabelastisser]

Hey @mountiny, can you please clarify if this is fixed and can be closed? Thanks!

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.58-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [NoQA] [Audit][Implementation] - Increase max cache keys count and use waitForCollectionCallback #38464

If no regressions arise, payment will be issued on 2024-04-09. 🎊

For reference, here are some details about the assignees on this issue:

• @shubham1206agra requires payment (Needs manual offer from BZ)

[melvin-bot]

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@shubham1206agra] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
• [@isabelastisser] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

[mountiny]

$500 to @shubham1206agra for testing and review, then we can close, thank you!

[melvin-bot]

Payment Summary

Upwork Job

• ROLE: @shubham1206agra paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@isabelastisser)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1770120318947196928/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[isabelastisser]

Hey @shubham1206agra, sorry for the delay. I hired you in Upwork, please accept it and I will process the payment ASAP. Thanks!

[shubham1206agra]

@isabelastisser I have discussed this internally here. You may close this issue as I am keeping track of payment internally and will ask to pay once the issue is resolved. Just write in the payment summary that I have not been paid yet.

[shubham1206agra]

@isabelastisser You can process payment here now.