Delay report history fetch to give app a chance to initialize #2519

marcaaron · 2021-04-21T20:07:35Z

Details

When a user first signs in we are fetching all of their report history for all reports right away before the app is even interactive. This is causing massive numbers of network requests and data processing that prevent the app from loading fast.

This approximately cuts the time it takes to reach the fully loaded sidebar on Android in half.

Fixed Issues

Fixes https://github.com/Expensify/Expensify/issues/160975
Fixes #2278

Tests

The test I performed was to drop a console.log() here and here:

diff --git a/src/pages/home/sidebar/OptionRow.js b/src/pages/home/sidebar/OptionRow.js
index 88fd1edcf..a973d97a2 100644
--- a/src/pages/home/sidebar/OptionRow.js
+++ b/src/pages/home/sidebar/OptionRow.js
@@ -115,6 +115,7 @@ const OptionRow = ({
         ({displayName, login}) => ({displayName, tooltip: login}),
     );

+    console.log('OptionRow Rendered');
     return (
         <Hoverable>
             {hovered => (

diff --git a/src/pages/signin/PasswordForm.js b/src/pages/signin/PasswordForm.js
index 173d29458..ea05b3f54 100644
--- a/src/pages/signin/PasswordForm.js
+++ b/src/pages/signin/PasswordForm.js
@@ -49,6 +49,7 @@ class PasswordForm extends React.Component {
      * Check that all the form fields are valid, then trigger the submit callback
      */
     validateAndSubmitForm() {
+        console.log('Sign In Pressed');
         if (!this.state.password.trim()
             || (this.props.account.requiresTwoFactorAuth && !this.state.twoFactorAuthCode.trim())
         ) {

Then I measured the time it took to go from pressing the sign in button to seeing the first OptionRow render in the SidebarLinks which tends to correspond with whether or not it is visible.
I then repeated this test against main and go the following results

`main`

[Wed Apr 21 2021 09:28:32.312]  LOG      Sign In Pressed
[Wed Apr 21 2021 09:28:43.683]  LOG      OptionRow rendered

11 seconds

this branch

[Wed Apr 21 2021 09:35:54.446]  LOG      Sign In Pressed
[Wed Apr 21 2021 09:35:59.862]  LOG      OptionRow rendered

5.3 seconds

QA Steps

No QA besides regressions

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

Give personalDetails default to prevent breaking on sign in Reorganize initial API calls a bit fix comment

Julesssss · 2021-04-22T10:03:43Z

src/libs/Navigation/AppNavigator/AuthScreens.js

@@ -119,7 +119,7 @@ class AuthScreens extends React.Component {
        PersonalDetails.fetch();
        User.getUserDetails();
        User.getBetas();
-        fetchAllReports(true, true);
+        fetchAllReports(true, true, 8000);


This number tests well now, but we might need to revisit in the future, perhaps a constant could be used to provide a bit more context?

Yeah, I sort of just hate that this is an arbitrary delay, but not sure what else to do. We could separate the fetching of history from fetchAllReports and then call it on it's own based on some other event.... but still not quite sure what that event would be.

Rather than make a constant I'll bake this into the method and add more context there since I feel like there's a chance this will change in the future and isn't really the best solution - but works for today.

Julesssss

While definitely an improvement, 8 seconds is quite the magic number.

This seems to suggest that the overhead of multiple requests does have an effect of the performance. Doe you think that is the case, or is there more to it? (perhaps rendering is also contributing to the problem)

yuwenmemon · 2021-04-22T17:38:44Z

src/libs/Navigation/AppNavigator/AuthScreens.js

@@ -119,7 +119,7 @@ class AuthScreens extends React.Component {
        PersonalDetails.fetch();
        User.getUserDetails();
        User.getBetas();
-        fetchAllReports(true, true);
+        fetchAllReports(true, true, 8000);


src/libs/actions/Report.js

marcaaron · 2021-04-22T18:12:10Z

This seems to suggest that the overhead of multiple requests does have an effect of the performance. Doe you think that is the case, or is there more to it? (perhaps rendering is also contributing to the problem)

Right, so the real culprit in this case is actually that the SidebarLinks are re-rendering a ton (before they have a chance to even load) since each report that comes back will trigger an update to the report_* collection. Did some console logs and you can see that without fetching the extra actions we end up rendering the links a few times. But once we render them the re-renders are 1:1 with each report that we fetched.

[Thu Apr 22 2021 08:08:06.613]  LOG      SidebarLinks rendered  1  times
[Thu Apr 22 2021 08:08:07.200]  LOG      SidebarLinks rendered  2  times
[Thu Apr 22 2021 08:08:07.131]  LOG      SidebarLinks rendered  3  times
[Thu Apr 22 2021 08:08:07.974]  LOG      SidebarLinks rendered  4  times
[Thu Apr 22 2021 08:08:09.661]  LOG      SidebarLinks rendered  5  times
[Thu Apr 22 2021 08:08:11.533]  LOG      SidebarLinks rendered  6  times
[Thu Apr 22 2021 08:08:13.941]  DEBUG    [info] [Report] Fetching report actions for reports - {"reportIDs":["1","3","6","7","9","10","11","12","14","15","16","17","33","45","46","47","48","49","50","51","52","53","54","55","56","57","58","59","60","61","62","63","64","65","68","77","85","86","87","88","100","101","102","103","104","105","106","107","108","109","110","111","112","113","114","116","117","118","119","129","130","133","134","136","137","138","139","143","145","146","147","153","161","176"]}
[Thu Apr 22 2021 08:08:15.233]  LOG      SidebarLinks rendered  7  times
[Thu Apr 22 2021 08:08:15.382]  LOG      SidebarLinks rendered  8  times
[Thu Apr 22 2021 08:08:15.412]  LOG      SidebarLinks rendered  9  times
[Thu Apr 22 2021 08:08:15.445]  LOG      SidebarLinks rendered  10  times
[Thu Apr 22 2021 08:08:15.640]  LOG      SidebarLinks rendered  11  times
[Thu Apr 22 2021 08:08:15.670]  LOG      SidebarLinks rendered  12  times
[Thu Apr 22 2021 08:08:15.701]  LOG      SidebarLinks rendered  13  times
[Thu Apr 22 2021 08:08:15.999]  LOG      SidebarLinks rendered  14  times
[Thu Apr 22 2021 08:08:16.348]  LOG      SidebarLinks rendered  15  times
[Thu Apr 22 2021 08:08:16.618]  LOG      SidebarLinks rendered  16  times
[Thu Apr 22 2021 08:08:16.707]  LOG      SidebarLinks rendered  17  times
[Thu Apr 22 2021 08:08:16.761]  LOG      SidebarLinks rendered  18  times
[Thu Apr 22 2021 08:08:17.491]  LOG      SidebarLinks rendered  19  times
[Thu Apr 22 2021 08:08:17.541]  LOG      SidebarLinks rendered  20  times
[Thu Apr 22 2021 08:08:17.625]  LOG      SidebarLinks rendered  21  times
[Thu Apr 22 2021 08:08:17.685]  LOG      SidebarLinks rendered  22  times
[Thu Apr 22 2021 08:08:17.770]  LOG      SidebarLinks rendered  23  times
[Thu Apr 22 2021 08:08:17.927]  LOG      SidebarLinks rendered  24  times
[Thu Apr 22 2021 08:08:17.962]  LOG      SidebarLinks rendered  25  times
[Thu Apr 22 2021 08:08:18.320]  LOG      SidebarLinks rendered  26  times
[Thu Apr 22 2021 08:08:18.439]  LOG      SidebarLinks rendered  27  times
[Thu Apr 22 2021 08:08:18.577]  LOG      SidebarLinks rendered  28  times
[Thu Apr 22 2021 08:08:18.628]  LOG      SidebarLinks rendered  29  times
[Thu Apr 22 2021 08:08:18.914]  LOG      SidebarLinks rendered  30  times
[Thu Apr 22 2021 08:08:19.380]  LOG      SidebarLinks rendered  31  times
[Thu Apr 22 2021 08:08:19.740]  LOG      SidebarLinks rendered  32  times
[Thu Apr 22 2021 08:08:19.330]  LOG      SidebarLinks rendered  33  times
[Thu Apr 22 2021 08:08:19.361]  LOG      SidebarLinks rendered  34  times
[Thu Apr 22 2021 08:08:19.598]  LOG      SidebarLinks rendered  35  times
[Thu Apr 22 2021 08:08:19.729]  LOG      SidebarLinks rendered  36  times
[Thu Apr 22 2021 08:08:19.775]  LOG      SidebarLinks rendered  37  times
[Thu Apr 22 2021 08:08:19.850]  LOG      SidebarLinks rendered  38  times
[Thu Apr 22 2021 08:08:20.920]  LOG      SidebarLinks rendered  39  times
[Thu Apr 22 2021 08:08:20.172]  LOG      SidebarLinks rendered  40  times
[Thu Apr 22 2021 08:08:20.219]  LOG      SidebarLinks rendered  41  times
[Thu Apr 22 2021 08:08:20.258]  LOG      SidebarLinks rendered  42  times
[Thu Apr 22 2021 08:08:20.395]  LOG      SidebarLinks rendered  43  times
[Thu Apr 22 2021 08:08:20.516]  LOG      SidebarLinks rendered  44  times
[Thu Apr 22 2021 08:08:20.943]  LOG      SidebarLinks rendered  45  times
[Thu Apr 22 2021 08:08:20.984]  LOG      SidebarLinks rendered  46  times
[Thu Apr 22 2021 08:08:21.373]  LOG      SidebarLinks rendered  47  times
[Thu Apr 22 2021 08:08:21.436]  LOG      SidebarLinks rendered  48  times
[Thu Apr 22 2021 08:08:21.510]  LOG      SidebarLinks rendered  49  times
[Thu Apr 22 2021 08:08:21.561]  LOG      SidebarLinks rendered  50  times
[Thu Apr 22 2021 08:08:21.936]  LOG      SidebarLinks rendered  51  times
[Thu Apr 22 2021 08:08:22.296]  LOG      SidebarLinks rendered  52  times
[Thu Apr 22 2021 08:08:22.374]  LOG      SidebarLinks rendered  53  times
[Thu Apr 22 2021 08:08:22.495]  LOG      SidebarLinks rendered  54  times
[Thu Apr 22 2021 08:08:22.538]  LOG      SidebarLinks rendered  55  times
[Thu Apr 22 2021 08:08:22.791]  LOG      SidebarLinks rendered  56  times
[Thu Apr 22 2021 08:08:22.860]  LOG      SidebarLinks rendered  57  times
[Thu Apr 22 2021 08:08:23.359]  LOG      SidebarLinks rendered  58  times
[Thu Apr 22 2021 08:08:23.415]  LOG      SidebarLinks rendered  59  times
[Thu Apr 22 2021 08:08:23.509]  LOG      SidebarLinks rendered  60  times
[Thu Apr 22 2021 08:08:23.765]  LOG      SidebarLinks rendered  61  times
[Thu Apr 22 2021 08:08:23.806]  LOG      SidebarLinks rendered  62  times
[Thu Apr 22 2021 08:08:23.839]  LOG      SidebarLinks rendered  63  times
[Thu Apr 22 2021 08:08:24.159]  LOG      SidebarLinks rendered  64  times
[Thu Apr 22 2021 08:08:24.199]  LOG      SidebarLinks rendered  65  times
[Thu Apr 22 2021 08:08:24.383]  LOG      SidebarLinks rendered  66  times
[Thu Apr 22 2021 08:08:24.431]  LOG      SidebarLinks rendered  67  times
[Thu Apr 22 2021 08:08:25.207]  LOG      SidebarLinks rendered  68  times
[Thu Apr 22 2021 08:08:25.245]  LOG      SidebarLinks rendered  69  times
[Thu Apr 22 2021 08:08:25.323]  LOG      SidebarLinks rendered  70  times
[Thu Apr 22 2021 08:08:25.363]  LOG      SidebarLinks rendered  71  times
[Thu Apr 22 2021 08:08:25.555]  LOG      SidebarLinks rendered  72  times
[Thu Apr 22 2021 08:08:25.696]  LOG      SidebarLinks rendered  73  times
[Thu Apr 22 2021 08:08:26.120]  LOG      SidebarLinks rendered  74  times
[Thu Apr 22 2021 08:08:26.159]  LOG      SidebarLinks rendered  75  times
[Thu Apr 22 2021 08:08:26.324]  LOG      SidebarLinks rendered  76  times
[Thu Apr 22 2021 08:08:26.489]  LOG      SidebarLinks rendered  77  times
[Thu Apr 22 2021 08:08:26.589]  LOG      SidebarLinks rendered  78  times
[Thu Apr 22 2021 08:08:26.808]  LOG      SidebarLinks rendered  79  times
[Thu Apr 22 2021 08:08:26.972]  LOG      SidebarLinks rendered  80  times

I guess another solution here would be to change this up so that we don't post-process the report history all at once and instead use a mergeCollection(), but I'm hesitant to go that route since we might not have enough space on the device to save all that report history.

Julesssss

Seems the best solution we have for the moment 👍

OSBotify · 2021-04-23T18:22:32Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2021-05-08T01:43:19Z

🚀 Deployed to production in version: 1.0.39-5🚀

platform	result
🤖 android 🤖	failure ❌
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

marcaaron self-assigned this Apr 21, 2021

marcaaron marked this pull request as ready for review April 21, 2021 20:12

marcaaron requested a review from a team as a code owner April 21, 2021 20:12

MelvinBot requested review from yuwenmemon and removed request for a team April 21, 2021 20:12

Move changes to new PR

fb44a6f

Give personalDetails default to prevent breaking on sign in Reorganize initial API calls a bit fix comment

marcaaron force-pushed the marcaaron-delayFetchHistory branch from c501d49 to fb44a6f Compare April 21, 2021 23:40

Make less ugly

67c4a3f

Julesssss reviewed Apr 22, 2021

View reviewed changes

Julesssss previously approved these changes Apr 22, 2021

View reviewed changes

yuwenmemon requested changes Apr 22, 2021

View reviewed changes

Add more comments

bfbdd8a

marcaaron dismissed Julesssss’s stale review via bfbdd8a April 22, 2021 18:02

marcaaron requested review from yuwenmemon and Julesssss April 22, 2021 18:20

Julesssss approved these changes Apr 23, 2021

View reviewed changes

yuwenmemon approved these changes Apr 23, 2021

View reviewed changes

yuwenmemon merged commit 86420de into main Apr 23, 2021

yuwenmemon deleted the marcaaron-delayFetchHistory branch April 23, 2021 18:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delay report history fetch to give app a chance to initialize #2519

Delay report history fetch to give app a chance to initialize #2519

marcaaron commented Apr 21, 2021 •

edited

Loading

Julesssss Apr 22, 2021

yuwenmemon Apr 22, 2021

marcaaron Apr 22, 2021

Julesssss left a comment

yuwenmemon Apr 22, 2021

marcaaron commented Apr 22, 2021 •

edited

Loading

Julesssss left a comment

OSBotify commented Apr 23, 2021

OSBotify commented May 8, 2021

Delay report history fetch to give app a chance to initialize #2519

Delay report history fetch to give app a chance to initialize #2519

Conversation

marcaaron commented Apr 21, 2021 • edited Loading

Details

Fixed Issues

Tests

main

this branch

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

Julesssss Apr 22, 2021

Choose a reason for hiding this comment

yuwenmemon Apr 22, 2021

Choose a reason for hiding this comment

marcaaron Apr 22, 2021

Choose a reason for hiding this comment

Julesssss left a comment

Choose a reason for hiding this comment

yuwenmemon Apr 22, 2021

Choose a reason for hiding this comment

marcaaron commented Apr 22, 2021 • edited Loading

Julesssss left a comment

Choose a reason for hiding this comment

OSBotify commented Apr 23, 2021

OSBotify commented May 8, 2021

marcaaron commented Apr 21, 2021 •

edited

Loading

`main`

marcaaron commented Apr 22, 2021 •

edited

Loading