Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix blinking loader in connect bank account page #29260

Merged
merged 4 commits into from
Oct 13, 2023

Conversation

honnamkuan
Copy link
Contributor

Details

fix blinking loading screen when accessing Connect bank account from Workspace Bills page

Fixed Issues

$ #27901
PROPOSAL: #27901 (comment)

Tests

  1. Login as any user with workspace setup for USD currency
  2. Go to Settings > Workspace > click on workspace name > Bills
  3. Click on Connect Bank Account under Unlock online bill payment
  4. Verify that there is a loader indicator followed by Connect bank account page showing up successfully, with no loader screen blinking
  • Verify that no errors appear in the JS console

Offline tests

Not applicable as Add Bank Account requires internet

QA Steps

Same steps as Tests

  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
    • MacOS: Desktop
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
    • I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
      • If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
    • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
    • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
    • I verified the JSDocs style guidelines (in STYLE.md) were followed
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
  • I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
  • I verified that if a function's arguments changed that all usages have also been updated correctly
  • If a new component is created I verified that:
    • A similar component doesn't exist in the codebase
    • All props are defined accurately and each prop has a /** comment above it */
    • The file is named correctly
    • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
    • The only data being stored in the state is data necessary for rendering and nothing else
    • If we are not using the full Onyx data that we loaded, I've added the proper selector in order to ensure the component only re-renders when the data it is using changes
    • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
    • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
    • All JSX used for rendering exists in the render method
    • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
  • If any new file was added I verified that:
    • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
  • I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native
Screen.Recording.2023-10-11.at.3.12.46.PM.mov
Android: mWeb Chrome
Screen.Recording.2023-10-11.at.3.21.40.PM.mov
iOS: Native
Screen.Recording.2023-10-11.at.3.00.15.PM.mov
iOS: mWeb Safari
Screen.Recording.2023-10-11.at.3.19.37.PM.mov
MacOS: Chrome / Safari
Screen.Recording.2023-10-11.at.3.23.20.PM.mov
MacOS: Desktop
Screen.Recording.2023-10-11.at.3.25.13.PM.mov

@honnamkuan honnamkuan requested a review from a team as a code owner October 11, 2023 07:29
@melvin-bot melvin-bot bot requested review from rushatgabhane and removed request for a team October 11, 2023 07:29
@melvin-bot
Copy link

melvin-bot bot commented Oct 11, 2023

@rushatgabhane Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

@honnamkuan
Copy link
Contributor Author

honnamkuan commented Oct 11, 2023

@rushatgabhane can you help to rerun the perf-tests check later when you are review this PR?
Not sure why the GitHub Action job is timing out. Thanks.

I ran the same tests on my machine, and verified that is no slowness due to my new commits compared to latest main branch.

Issue resolved on subsequent run

Report below:

Performance Comparison Report

  • Current: 1e30a02 - 2023-10-11 07:58:48Z
  • Baseline: main (7b52fa5) - 2023-10-11 07:57:40Z

Significant Changes To Render Duration

There are no entries

Meaningless Changes To Render Duration

Show entries
Name Render Duration Render Count
should render multiple selection and select 3 items 46.0 ms → 46.2 ms (+0.1 ms, ±0.0%) 5 → 5
should press a list item 22.8 ms → 22.9 ms (+0.1 ms, ±0.0%) 3 → 3
should render 1 section and a thousand items 12.6 ms → 12.5 ms (-0.0 ms, ±0.0%) 2 → 2
should scroll and click some of the items 48.0 ms → 47.3 ms (-0.7 ms, -1.5%) 52 → 52
should render Sidebar with 500 reports stored 44.1 ms → 43.3 ms (-0.8 ms, -1.9%) 42 → 42
should scroll and select a few items 84.8 ms → 81.1 ms (-3.6 ms, -4.3%) 7 → 7
Show details
Name Render Duration Render Count
should render multiple selection and select 3 items Baseline
Mean: 46.0 ms
Stdev: 1.2 ms (2.7%)
Runs: 47.91585999727249 47.760829389095306 46.90186858177185 46.62997943162918 46.324260115623474 45.37574464082718 45.190720081329346 44.91649341583252 44.819605350494385 44.520145654678345

Current
Mean: 46.2 ms
Stdev: 1.6 ms (3.4%)
Runs: 47.572171092033386 47.504024624824524 47.463968336582184 47.436483323574066 47.39441579580307 46.98191279172897 44.539928674697876 44.36742860078812 44.331240355968475 44.194270610809326
Baseline
Mean: 5
Stdev: 0 (0.0%)
Runs: 5 5 5 5 5 5 5 5 5 5

Current
Mean: 5
Stdev: 0 (0.0%)
Runs: 5 5 5 5 5 5 5 5 5 5
should press a list item Baseline
Mean: 22.8 ms
Stdev: 0.5 ms (2.3%)
Runs: 23.67829656600952 23.516056835651398 23.172451078891754 23.07083350419998 22.90027207136154 22.429281055927277 22.415651619434357 22.345730185508728 22.328465223312378 22.299334704875946

Current
Mean: 22.9 ms
Stdev: 0.7 ms (3.0%)
Runs: 24.14732038974762 23.650656700134277 23.476589500904083 23.257543742656708 22.598704874515533 22.442285001277924 22.407998383045197 22.397879719734192 22.28516709804535 22.23716050386429
Baseline
Mean: 3
Stdev: 0 (0.0%)
Runs: 3 3 3 3 3 3 3 3 3 3

Current
Mean: 3
Stdev: 0 (0.0%)
Runs: 3 3 3 3 3 3 3 3 3 3
should render 1 section and a thousand items Baseline
Mean: 12.6 ms
Stdev: 0.4 ms (2.9%)
Runs: 13.22669368982315 13.013010144233704 12.925998449325562 12.63634318113327 12.456665933132172 12.310878813266754 12.290196776390076 12.273535549640656 12.261791110038757 12.22324800491333

Current
Mean: 12.5 ms
Stdev: 0.5 ms (3.7%)
Runs: 13.544408977031708 12.909254968166351 12.904284656047821 12.650449872016907 12.333028018474579 12.26404321193695 12.223158657550812 12.203664064407349 12.18387633562088 12.109502792358398
Baseline
Mean: 2
Stdev: 0 (0.0%)
Runs: 2 2 2 2 2 2 2 2 2 2

Current
Mean: 2
Stdev: 0 (0.0%)
Runs: 2 2 2 2 2 2 2 2 2 2
should scroll and click some of the items Baseline
Mean: 48.0 ms
Stdev: 0.7 ms (1.5%)
Runs: 49.253879845142365 48.81139427423477 48.18880486488342 48.15634089708328 48.13970363140106 47.99161672592163 47.87733310461044 47.83053773641586 47.13367986679077 46.64378333091736

Current
Mean: 47.3 ms
Stdev: 1.1 ms (2.3%)
Runs: 49.10247355699539 48.85737419128418 48.19044154882431 47.793650448322296 46.78548753261566 46.64716976881027 46.469601929187775 46.41000717878342 46.37772238254547 46.23394191265106
Baseline
Mean: 52
Stdev: 0 (0.0%)
Runs: 52 52 52 52 52 52 52 52 52 52

Current
Mean: 52
Stdev: 0 (0.0%)
Runs: 52 52 52 52 52 52 52 52 52 52
should render Sidebar with 500 reports stored Baseline
Mean: 44.1 ms
Stdev: 0.6 ms (1.2%)
Runs: 45.0555534362793 44.60871207714081 44.55835276842117 44.24979782104492 44.12403094768524 43.95203483104706 43.94583058357239 43.871727764606476 43.583161890506744 43.12204957008362

Current
Mean: 43.3 ms
Stdev: 0.4 ms (0.9%)
Runs: 43.994644463062286 43.99138206243515 43.30838018655777 43.2441765666008 43.19263410568237 43.115213096141815 43.05196863412857 43.04981446266174 43.00964969396591 42.870215594768524
Baseline
Mean: 42
Stdev: 0 (0.0%)
Runs: 42 42 42 42 42 42 42 42 42 42

Current
Mean: 42
Stdev: 0 (0.0%)
Runs: 42 42 42 42 42 42 42 42 42 42
should scroll and select a few items Baseline
Mean: 84.8 ms
Stdev: 2.0 ms (2.4%)
Runs: 86.85326135158539 86.65244090557098 86.63639652729034 86.08130556344986 85.67132616043091 85.52990913391113 83.75226843357086 82.530288875103 82.16537529230118 81.72144341468811

Current
Mean: 81.1 ms
Stdev: 0.8 ms (1.0%)
Runs: 83.24153083562851 81.1864777803421 81.16418159008026 81.13323187828064 81.00080305337906 80.99258303642273 80.92815440893173 80.68531447649002 80.635881960392 80.51495736837387
Baseline
Mean: 7
Stdev: 0 (0.0%)
Runs: 7 7 7 7 7 7 7 7 7 7

Current
Mean: 7
Stdev: 0 (0.0%)
Runs: 7 7 7 7 7 7 7 7 7 7

Changes To Render Count

There are no entries

Added Scenarios

There are no entries

Removed Scenarios

There are no entries

@honnamkuan
Copy link
Contributor Author

@rushatgabhane appreciate your review on the PR

@rushatgabhane
Copy link
Member

rushatgabhane commented Oct 12, 2023

Reviewer Checklist

  • I have verified the author checklist is complete (all boxes are checked off).
  • I verified the correct issue is linked in the ### Fixed Issues section above
  • I verified testing steps are clear and they cover the changes made in this PR
    • I verified the steps for local testing are in the Tests section
    • I verified the steps for Staging and/or Production testing are in the QA steps section
    • I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I checked that screenshots or videos are included for tests on all platforms
  • I included screenshots or videos for tests on all platforms
  • I verified tests pass on all platforms & I tested again on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
    • MacOS: Desktop
  • If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
  • I verified proper code patterns were followed (see Reviewing the code)
    • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
    • I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
    • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
    • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
    • I verified the JSDocs style guidelines (in STYLE.md) were followed
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I verified that this PR follows the guidelines as stated in the Review Guidelines
  • I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
  • I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
  • I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
  • If a new component is created I verified that:
    • A similar component doesn't exist in the codebase
    • All props are defined accurately and each prop has a /** comment above it */
    • The file is named correctly
    • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
    • The only data being stored in the state is data necessary for rendering and nothing else
    • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
    • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
    • All JSX used for rendering exists in the render method
    • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
  • If any new file was added I verified that:
    • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
  • I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web
Screen.Recording.2023-10-13.at.03.12.23.mov
Mobile Web - Chrome
WhatsApp.Video.2023-10-13.at.03.26.58.mp4
Mobile Web - Safari
Screen.Recording.2023-10-13.at.03.20.26.mov
Desktop
Screen.Recording.2023-10-13.at.03.32.27.mov
iOS
Screen.Recording.2023-10-13.at.03.29.00.mov
Android
Screen.Recording.2023-10-13.at.03.33.10.mov

@honnamkuan
Copy link
Contributor Author

@marcochavezf appreciate your review on the PR. Thanks.

@marcochavezf marcochavezf merged commit f80de72 into Expensify:main Oct 13, 2023
13 checks passed
@OSBotify
Copy link
Contributor

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

@github-actions github-actions bot added the DeployBlockerCash This issue or pull request should block deployment label Oct 13, 2023
@github-actions
Copy link
Contributor

Performance Comparison Report 📊

Significant Changes To Duration

Name Duration
App start TTI 1270.621 ms → 1685.258 ms (+414.637 ms, +32.6%) 🔴
App start runJsBundle 868.315 ms → 1182.682 ms (+314.367 ms, +36.2%) 🔴🔴
Show details
Name Duration
App start TTI Baseline
Mean: 1270.621 ms
Stdev: 52.887 ms (4.2%)
Runs: 1123.8128500003368 1154.0506600001827 1163.8020919999108 1177.002465000376 1186.7382129998878 1190.248449999839 1192.0808420004323 1197.6207269998267 1200.2869910001755 1203.6403860002756 1203.6914440002292 1212.6833419995382 1214.4863130003214 1217.0984349995852 1217.465972000733 1218.345958000049 1218.9986629998311 1219.6145449997857 1223.2733309995383 1224.75452900026 1226.8298279996961 1228.0280860001221 1228.035508999601 1233.2981770001352 1233.8734820000827 1234.5537180006504 1235.8311310000718 1237.9585710000247 1238.727651000023 1241.5051179993898 1246.0373149998486 1246.8567789997905 1250.1173900002614 1250.120512000285 1255.4900979995728 1256.902507999912 1260.5428560003638 1262.2982890000567 1263.9035049993545 1264.5310529991984 1265.5298229996115 1267.1888350006193 1267.6833270005882 1274.4131400007755 1276.1035689990968 1276.8499640002847 1277.2116539999843 1278.115772999823 1280.5406719995663 1280.6715229991823 1284.0307860001922 1285.3732650000602 1286.4262530002743 1288.1016969997436 1291.2416350003332 1291.3736860007048 1293.4214590005577 1293.6360780000687 1294.874776000157 1295.5100880004466 1296.1951039992273 1297.8497700002044 1300.3851079996675 1300.4693969991058 1301.819339999929 1303.4295230004936 1305.8286029994488 1306.5987190008163 1310.974000999704 1312.1350500006229 1312.2734409999102 1312.8723150007427 1313.7328650001436 1317.6496099997312 1320.539565000683 1324.6532959993929 1325.174257999286 1325.8633069992065 1331.2502110004425 1332.5773150008172 1336.0513620004058 1339.0822419999167 1340.4935240000486 1342.840196000412 1343.0592650007457 1344.2378550004214 1346.9644120000303 1349.4424019996077 1352.920501999557 1363.776507999748 1411.9781780000776

Current
Mean: 1685.258 ms
Stdev: 69.965 ms (4.2%)
Runs: 1526.576353000477 1534.1513400003314 1549.4403559993953 1553.201508000493 1574.3858129996806 1575.9542590007186 1578.5656610000879 1580.792519999668 1585.7055939994752 1592.9675190001726 1599.1888769995421 1605.8110259994864 1609.312480000779 1612.5354379992932 1612.5740319993347 1615.8435389995575 1620.1803629994392 1620.8314720001072 1623.2737629991025 1627.7365109995008 1631.8083989992738 1634.6590759996325 1635.0285710003227 1637.8406860008836 1638.714488999918 1639.4136350005865 1639.687397999689 1646.1123200003058 1647.2786110006273 1649.3882490005344 1659.823913000524 1662.8068809993565 1667.8463240005076 1672.2151089999825 1672.4101890008897 1672.7636859994382 1674.758293999359 1675.197650000453 1677.8628180008382 1681.3897530008107 1681.5193840004504 1682.7772380001843 1685.4065419994295 1686.4796580001712 1687.4065400008112 1688.2577400002629 1689.7547030001879 1691.7695429995656 1697.12616099976 1698.6107800006866 1703.2009730003774 1703.3419690001756 1704.1441810000688 1708.0410069990903 1708.7703460007906 1709.9984959997237 1711.0351730007678 1711.5770239997655 1711.6261390000582 1714.185523999855 1714.6069879997522 1717.2034540008754 1718.8485989999026 1722.1510690003633 1724.6181860007346 1724.798696000129 1725.6012319996953 1725.6994569990784 1726.8318719994277 1730.1286469995975 1735.095969999209 1735.1063279993832 1737.7480089999735 1741.949026999995 1746.08458600007 1747.6032650005072 1754.4277630001307 1755.9733089990914 1758.1849220003933 1767.2302199993283 1770.5529660005122 1775.6729700006545 1782.4269600007683 1784.3369420003146 1790.8998610004783 1815.4014790002257 1818.3907619994134 1827.2986140009016 1851.1932699996978 1856.1168070007116
App start runJsBundle Baseline
Mean: 868.315 ms
Stdev: 42.099 ms (4.8%)
Runs: 749 778 782 786 787 789 798 802 807 816 817 821 825 828 828 828 829 829 833 835 835 838 838 838 842 843 843 844 845 849 849 850 850 855 856 859 860 862 863 865 865 865 870 872 873 874 874 874 874 876 878 878 880 880 881 882 883 884 885 888 890 890 890 892 897 897 899 899 901 902 903 906 906 906 908 911 914 915 915 915 916 916 921 926 927 927 929 931 933 934 935 947

Current
Mean: 1182.682 ms
Stdev: 33.665 ms (2.8%)
Runs: 1108 1113 1120 1125 1127 1128 1132 1134 1135 1139 1146 1146 1148 1148 1151 1153 1153 1154 1154 1155 1156 1161 1162 1164 1165 1166 1166 1167 1167 1168 1169 1169 1170 1170 1172 1172 1173 1174 1174 1175 1176 1177 1178 1180 1181 1182 1182 1182 1185 1186 1186 1190 1191 1193 1193 1196 1197 1198 1199 1199 1200 1202 1202 1203 1203 1203 1206 1208 1208 1212 1212 1212 1213 1214 1219 1222 1224 1227 1228 1230 1231 1232 1235 1236 1241 1251 1255 1267

Meaningless Changes To Duration

Show entries
Name Duration
Open Search Page TTI 634.722 ms → 651.830 ms (+17.108 ms, +2.7%)
App start nativeLaunch 21.716 ms → 23.079 ms (+1.363 ms, +6.3%)
App start regularAppStart 0.015 ms → 0.017 ms (+0.002 ms, +11.8%)
Show details
Name Duration
Open Search Page TTI Baseline
Mean: 634.722 ms
Stdev: 22.904 ms (3.6%)
Runs: 595.3641769997776 600.4986169990152 602.2925620004535 605.2688400000334 606.5754400007427 606.8468019999564 607.4459230005741 607.9178470000625 607.9328620005399 608.5752770006657 609.1399340014905 610.957397999242 611.0252279993147 611.5796309988946 612.7291259989142 614.1019289996475 614.6954350005835 614.8457030002028 614.9362790007144 615.1014000009745 615.7845060005784 616.2608649991453 617.0222580004483 618.0590420011431 618.1202400010079 619.7965089995414 620.6646740008146 621.472413001582 622.5552570000291 623.45320700109 623.5414230003953 624.0536700002849 624.0565189998597 624.3024899996817 624.3397220000625 624.3625900000334 624.5415039993823 624.5758870001882 624.5763760004193 624.5810959991068 625.2599689997733 625.3117680009454 625.3612879998982 625.953735999763 626.824258999899 630.5808520000428 630.9481610003859 631.593708999455 632.7853599991649 634.0566009990871 635.5652259998024 636.5760099999607 636.7452799994498 637.0241299998015 638.8690590001643 639.3515629991889 641.2904869988561 641.9669190011919 642.5979410000145 645.5974939987063 645.6162519995123 646.394369000569 646.4359539989382 647.832438999787 648.538412000984 648.6386719997972 650.097289999947 650.6033120006323 650.7466640006751 651.1370450016111 652.4992679990828 655.6936440002173 655.8793129995465 656.2000730000436 657.0304370000958 657.8742680009454 659.5204269997776 665.9878339990973 672.9237879998982 674.0673019997776 677.3471280001104 680.0906170010567 682.1678069997579 682.2498779986054 682.6134850010276 682.8996989987791 684.7943530008197 691.4674890004098

Current
Mean: 651.830 ms
Stdev: 30.319 ms (4.7%)
Runs: 599.4789629988372 606.3763429988176 606.5449230000377 608.5225019995123 609.0010580010712 614.0399169996381 614.8445640001446 617.0131430011243 617.0358079988509 618.1250409986824 618.3066809996963 619.3070890009403 623.8194590006024 624.6753339990973 625.4834799990058 626.1121420003474 626.2049559988081 626.3390309996903 626.4787189997733 627.0519619993865 627.7016599997878 628.9667159989476 630.5773120000958 631.1429449990392 631.1790779996663 631.2753100004047 631.6199140008539 631.722494000569 632.1311449985951 633.5682789999992 634.675945000723 635.6357829999179 637.1026610005647 637.4378669988364 639.538616001606 639.8514809999615 639.9198000002652 640.0259199999273 640.9729819986969 641.0517980009317 641.312173999846 642.0243740007281 644.2141529992223 645.5756030008197 645.7041020002216 646.8990480005741 648.7819010000676 648.8508710004389 649.2034499999136 650.2221690006554 652.1462409999222 655.296590000391 655.9838459994644 656.4098710007966 656.7282320000231 657.0420740004629 657.98852599971 658.8575030006468 659.0522870011628 659.5358479991555 663.6946219988167 664.9265139997005 665.1666269991547 665.8066000007093 666.0777589995414 667.2707929983735 668.0254309996963 668.1196699999273 672.3726400006562 672.4025069996715 672.8924560006708 676.3964440003037 677.9863290004432 684.922322999686 689.5440270006657 689.6252449993044 690.2712810002267 695.8678389992565 699.8682049997151 704.4210620000958 704.4748539999127 705.8627519998699 706.4154470004141 714.8110349997878 716.4537760000676 723.6490889992565 725.1658529993147 725.8603930007666
App start nativeLaunch Baseline
Mean: 21.716 ms
Stdev: 2.876 ms (13.2%)
Runs: 18 18 18 19 19 19 19 19 19 19 19 19 19 19 19 19 19 19 19 19 19 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 21 21 21 21 21 21 21 21 21 21 21 21 21 21 21 22 22 22 22 22 22 22 23 23 23 23 23 23 24 24 24 24 24 24 25 25 25 25 26 26 26 26 27 28 28 29 29 29 30

Current
Mean: 23.079 ms
Stdev: 2.745 ms (11.9%)
Runs: 19 19 19 19 19 20 20 20 20 20 21 21 21 21 21 21 21 21 21 21 21 21 21 21 21 21 21 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22 23 23 23 23 23 23 23 23 23 23 23 23 24 24 24 24 24 24 24 24 24 24 25 25 25 25 25 26 26 26 26 27 27 27 28 29 29 29 30 30 30 31
App start regularAppStart Baseline
Mean: 0.015 ms
Stdev: 0.001 ms (7.2%)
Runs: 0.01285799965262413 0.012980000115931034 0.013020999729633331 0.013345999643206596 0.013346001505851746 0.013386998325586319 0.013427999801933765 0.013509000651538372 0.013591000810265541 0.013630999252200127 0.013671999797224998 0.013793000020086765 0.013793999329209328 0.013793999329209328 0.013833999633789062 0.013876000419259071 0.013876000419259071 0.013916000723838806 0.013957001268863678 0.013996999710798264 0.013996999710798264 0.0139979999512434 0.0139979999512434 0.014038000255823135 0.01407800056040287 0.014079000800848007 0.014118999242782593 0.014159999787807465 0.014241999015212059 0.014281999319791794 0.014282000251114368 0.014362999238073826 0.014403999783098698 0.01444500032812357 0.014526000246405602 0.014527000486850739 0.014566998928785324 0.014607001096010208 0.014608001336455345 0.01464799977838993 0.014649000018835068 0.014689000323414803 0.014770999550819397 0.014810999855399132 0.014850998297333717 0.014851999469101429 0.014892000705003738 0.014932999387383461 0.014973999932408333 0.015015000477433205 0.015095999464392662 0.015137000009417534 0.015137000009417534 0.015217998996376991 0.015217999927699566 0.01521800085902214 0.015257999300956726 0.015298999845981598 0.015300000086426735 0.015381000004708767 0.015461999922990799 0.015461999922990799 0.015542999841272831 0.015583999454975128 0.015583999454975128 0.015583999454975128 0.015584999695420265 0.015625 0.015665000304579735 0.015666000545024872 0.015706000849604607 0.01574699953198433 0.016032000072300434 0.01603200100362301 0.016072001308202744 0.016235999763011932 0.01635799929499626 0.016397999599575996 0.0165200000628829 0.016520000994205475 0.016600999981164932 0.01664300076663494 0.01680499967187643 0.016885999590158463 0.0176189998164773 0.01782199926674366

Current
Mean: 0.017 ms
Stdev: 0.001 ms (6.0%)
Runs: 0.01448499970138073 0.014892000705003738 0.014892999082803726 0.014934001490473747 0.01505499891936779 0.015096001327037811 0.015137000009417534 0.015178000554442406 0.015217998996376991 0.015298999845981598 0.015298999845981598 0.015298999845981598 0.015420999377965927 0.015583999454975128 0.015625 0.015665000304579735 0.015665000304579735 0.015666000545024872 0.015828000381588936 0.015869000926613808 0.01590999960899353 0.015949999913573265 0.015949999913573265 0.016071999445557594 0.016072001308202744 0.016112999990582466 0.01615399867296219 0.016154000535607338 0.016154000535607338 0.016154000535607338 0.01619499921798706 0.01619500108063221 0.016235999763011932 0.016235999763011932 0.016316000372171402 0.01631700061261654 0.016356999054551125 0.016397999599575996 0.016439000144600868 0.016439000144600868 0.01647999882698059 0.016519999131560326 0.016600999981164932 0.01660200022161007 0.016683001071214676 0.016683001071214676 0.016683001071214676 0.016764000058174133 0.016804998740553856 0.016804998740553856 0.016805000603199005 0.016805000603199005 0.016845999285578728 0.016846001148223877 0.0168869998306036 0.016927000135183334 0.016927000135183334 0.016967998817563057 0.017048999667167664 0.017048999667167664 0.0170889999717474 0.017171001061797142 0.017211999744176865 0.017211999744176865 0.0172520000487566 0.017253000289201736 0.017253000289201736 0.0174150001257658 0.017456000670790672 0.017456000670790672 0.017537999898195267 0.017537999898195267 0.017617998644709587 0.017618998885154724 0.01782200112938881 0.017904000356793404 0.017985999584197998 0.018066998571157455 0.018067000433802605 0.018067000433802605 0.01822899840772152 0.01826999895274639 0.018635999411344528 0.018758999183773994 0.01879899948835373 0.018879998475313187

@github-actions
Copy link
Contributor

@Expensify/mobile-deployers 📣 Please look into this performance regression as it's a deploy blocker.

@rushatgabhane
Copy link
Member

I highly doubt this PR reduced app start time by 30%

This could have been caused by another PR maybe, so we should still investigate 🔍

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by https://github.com/marcochavezf in version: 1.3.84-0 🚀

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

@honnamkuan
Copy link
Contributor Author

I believe the changes in this PR are all runtime code updates and will not affect start up performance.

Moreover I have ran the perf-tests myself baselined against main branch and post the success results at #29260 (comment)
prior to the PR merge.

Anyway, please let me know if there is something I can / should do.

@OSBotify
Copy link
Contributor

🚀 Deployed to production by https://github.com/francoisl in version: 1.3.84-10 🚀

platform result
🤖 android 🤖 skipped 🚫
🖥 desktop 🖥 skipped 🚫
🍎 iOS 🍎 skipped 🚫
🕸 web 🕸 skipped 🚫

@OSBotify
Copy link
Contributor

🚀 Deployed to production by https://github.com/francoisl in version: 1.3.84-10 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by https://github.com/marcochavezf in version: 1.3.85-0 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to production by https://github.com/francoisl in version: 1.3.85-4 🚀

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DeployBlockerCash This issue or pull request should block deployment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants