Improve memoization of LHN by handling the isFocused prop better #28615

kacper-mikolajczak · 2023-10-02T14:14:03Z

Intro

Transition between chats (reports), seems to be quite laggy. In this document, we try to investigate the issue and come up with possible solution.

Steps to reproduce

Load two chats
Navigate from one to another

Recording

expensify_loaded_chat_transition_web_dev_recording.mov

Investigations

It looks like there is a big overhead in form of rendering LHN’s list of OptionRowLHNData components.

In the profiling trace we see that every item in the list is re-rendered. Each re-render takes up to 5ms to complete, which with current list size can significantly throttle rendering.

The re-rendering is caused by change in currentReportID state, which is provided by withCurrentReportID HOC. currentReportID prop is used in OptionRowLHNData in one place, to find out wether component should be focused or not:

const isFocused = !shouldDisableFocusOptions && currentReportID === reportID;

The isFocused is then directly passed down into child.

The change in currentReportID breaks memoization, causing heavy re-render of each item.

Solutions

Wrapper component

As currentReportID is only used once and isFocused is a boolean flag, we can extract this logic into separate wrapper component and refactor OptionRowLHNData to receive primitive isFocused prop, which will not break memoization.

By applying these changes, we were able to shrink down the rendering time of List component to ~100ms, which will be even more significant as number of list items grows. The rest of rendering overhead seems to be related to different components, Report itself etc.

The rendering time should be now only merely related to list size, as it only triggers one heavy re-render.

Wrapper component would look more or less like this:

function OptionRowLHNDataWrapper({currentReportID, shouldDisableFocusOptions, ...props}) {
    const isFocused = !shouldDisableFocusOptions && currentReportID === props.reportID;
    return (
        <OptionRowLHNDataComposed
            {...props}
            isFocused={isFocused}
        />
    );
}

This solution does not add anything beyond a simple wrapper so it is quite easy to do and should not be error-prone.

Use `withCurrentReportID` in list component (Alternative)

Alternatively, we can use withCurrentReportID once at the level of list component instead of in each children. Then we will directly pass isFocused prop into children, so it won’t break memoization of list items.

The solution is more elegant, as it uses currentReportID provider once and does not expose such internals into children components. On the flipside, we need to re-render list itself, which might add some overhead.

In this scenario, it takes about ~120ms to render the list, which is about 15ms slower than previous solution.

Conclusions

While we managed to reduce rendering times, we still need to think on which solution should be implemented. Furthermore, it might be a good idea to investigate rendering of list items, as it is quite simple component but still takes ~5ms to render.

Fixed Issues

$ #28631
PROPOSAL: #28631 (it contains a proposal)

Tests

Verify that no errors/warnings appear in the JS console.

Open app
See if can easily switch between multiple chats
See if focus (opacity) changes according to selected chats

Offline tests

N/A

QA Steps

Open app
See if can easily switch between multiple chats
See if focus (opacity) changes according to selected chats

PR Author Checklist

Screenshots/Videos

Web

Screen.Recording.2023-10-04.at.17.06.42.mov

Mobile Web - Chrome

N/A

Mobile Web - Safari

N/A

Desktop

Screen.Recording.2023-10-04.at.17.15.05.mov

iOS

Mobile recording for reference

Screen.Recording.2023-10-04.at.17.05.00.mov

Android

N/A

kacper-mikolajczak · 2023-10-02T14:15:37Z

Hi @muttmuure, here is a draft of partial improvements that I've done recently. I wanted to share it on Slack but it is quite hard to put such document into Slack message :D Let me know what you think about results and overall shape of such document. Thanks! In the meantime I will work on further improvements 👍

mountiny · 2023-10-02T16:27:33Z

@kacper-mikolajczak Thanks! I think this looks great!

I agree that the second solution leads to more readable code, adding that one wrapper feels a bit odd to me.

@kacper-mikolajczak could you summarize these two options and post it to #newdot-performance room? I think it would be great to get opinions of other agency experts and engineers on this and then we can implement this. Thanks!

kacper-mikolajczak · 2023-10-02T16:31:06Z

@mountiny Yeah definitely! Thanks for the feedback 👍

This reverts commit ffd2418.

hurali97

LGTM ! Just a couple comments.

hurali97 · 2023-10-04T11:49:14Z

src/components/LHNOptionsList/OptionRowLHNData.js

@@ -20,8 +19,8 @@ import CONST from '../../CONST';
 import reportActionPropTypes from '../../pages/home/report/reportActionPropTypes';

 const propTypes = {
-    /** If true will disable ever setting the OptionRowLHN to focused */
-    shouldDisableFocusOptions: PropTypes.bool,
+    /** Wether row should be focused */


Suggested change

/** Wether row should be focused */

/** Whether row should be focused */

hurali97 · 2023-10-04T11:51:11Z

src/components/LHNOptionsList/OptionRowLHNData.js

@@ -91,7 +86,6 @@ function OptionRowLHNData({
    const reportID = propsToForward.reportID;
    // We only want to pass a boolean to the memoized component,
    // instead of a changing number (so we prevent unnecessary re-renders).
-    const isFocused = !shouldDisableFocusOptions && currentReportID === reportID;


We can move this comment to OptionRowLHNDataWithFocus, since we moved the isFocused there as well.

hurali97 · 2023-10-04T11:52:48Z

src/components/LHNOptionsList/OptionRowLHNDataWithFocus.js

+    shouldDisableFocusOptions: false,
+};
+
+function OptionRowLHNDataWithFocus({currentReportID, shouldDisableFocusOptions, ...props}) {


Can we add some comments stating what this OptionRowLHNDataWithFocus does? It would be cool in future to have an understanding of what it does because not everyone will be coming back to the PR to understand.

kacper-mikolajczak · 2023-10-04T14:34:02Z

Thanks for the thorough review @hurali97. ❤️ Let me know if you approve the changes

kacper-mikolajczak · 2023-10-04T15:35:13Z

BTW, on iPhone 13 simulator the difference is huge 🚀

mountiny · 2023-10-04T20:36:40Z

Damn, that looks great!

kacper-mikolajczak · 2023-10-05T07:48:27Z

@mountiny Yeah that does look promising! Wonder what would be a perceived perf gain, as the list is out of the view when the render happens (it is covered by report screen). But hey, at least we reduced the resources consumption a lil bit :D

mountiny · 2023-10-05T07:57:24Z

Correct, it leaves more resources for other stuff which is always great.

Can you test this on super heavy account we provided for callstack? That is more close to real heavy user usage

kacper-mikolajczak · 2023-10-05T08:22:00Z

Sure @mountiny, testing it right now!

kacper-mikolajczak · 2023-10-05T12:01:36Z

Here are the results @mountiny.

Mobile (iOS)

From testing on iOS it seems that the app performance is highly coupled to the amount of chats present. It is no surprise that it is influencing performance but the app should not slow down almost exponentially when amount of chats grows.

My first impression is that there are some heavy calculations based on entire data set going on in each item. I am going to investigate it further in some future, as it is out of the scope of the current RP.

Main branch:

Current branch:

Web

For web, I was not able to load the app at all - no errors or anything particularly wrong. After inspecting devtools it seems that there are a lot of calculations (probably related to what Matt and Monil discussed on Slack).

allroundexperts · 2023-10-05T16:13:41Z

@kacper-mikolajczak This is testing good for me. Are we waiting on something here? If not, let's open the PR!

melvin-bot · 2023-10-05T18:37:09Z

@mountiny 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]

mountiny · 2023-10-05T18:46:11Z

Lets gooo

allroundexperts · 2023-10-05T20:41:57Z

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 / Chrome
- iOS / native
- iOS / 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-08.at.6.36.03.PM.mov

Mobile Web - Chrome

Screen.Recording.2023-10-08.at.6.43.33.PM.mov

Mobile Web - Safari

Screen.Recording.2023-10-08.at.6.45.33.PM.mov

Desktop

Screen.Recording.2023-10-08.at.6.41.36.PM.mov

iOS

Screen.Recording.2023-10-08.at.6.46.01.PM.mov

Android

Screen.Recording.2023-10-08.at.6.44.55.PM.mov

allroundexperts · 2023-10-05T22:03:33Z

I think we can simplify this further without using an additional wrapper component. In here, we can just pass in the isFocused prop to the OptionRowLHNDataWithData function directly. If we don't want to follow this approach, then a HOC component (which isn't tied specifically to the consumer component), would be a better approach.

Creating a consumer specific wrapper component for each of the variable that needs to be memoed is not a good idea in my opinion. Soon, we'll have more cases like this, which will lead to a lot of more component like these popping up in our code. That will ultimately increase the complexity of our code (Not to mention that it will become hard to understand).

PS: This came into my mind while filling in the checklist.

@kacper-mikolajczak

mountiny · 2023-10-05T22:14:19Z

@allroundexperts Thank you! Do you want to raise that here? https://expensify.slack.com/archives/C05LX9D6E07/p1696354046851689 it might be quicker to discuss and more people ot bound the ideas off

kacper-mikolajczak · 2023-10-06T08:59:29Z

Hi @allroundexperts, thanks for your feedback! 👍
I agree with your concern about growing complexity and while I was thinking about the solution you've suggested, I came to the conclusion that it would eventually lead us to adding more abstraction that we might need at the moment.

In the current approach, we simply couple the OptionRowLHNData with the tiny wrapper in the form of OptionRowLHNDataWithFocus (naming itself is another topic - I am not sure it's correct). That's the whole abstraction we introduce.

In the suggested approach, we would first need to create withCurrentReportFocused HOC, but then we would still need to bound it to the OptionRowLHNData, which will lead us to creating OptionRowLHNDataWithFocus for this purpose. We just simply moved part of the logic (as shouldDisableFocusOptions still needs to be handled inside the wrapper component).

I agree that tightly coupling should be less preferred than composition, but the question is how much do we need to have withCurrentReportFocused abstraction as it is potentially just one line of extracted logic that seems kind of specific. Plus, it looks like it's going to be only used in one place and by sticking to YAGNI rule, we can think about leaving it as-is, wdyt?

Having that said, I am not as familiar with the codebase as you are, please let me know if I understood it right. Thanks a lot for the discussion 😄

allroundexperts

As per the Slack discussion, there was an agreement that we should continue using this pattern. As such, the changes are testing good!

OSBotify · 2023-10-08T23:52:56Z

✋ 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 · 2023-10-09T01:06:54Z

Performance Comparison Report 📊

Significant Changes To Duration

Name	Duration
App start TTI	1220.794 ms → 1309.686 ms (+88.893 ms, +7.3%) 🔴
App start runJsBundle	836.244 ms → 904.727 ms (+68.483 ms, +8.2%) 🔴

Show details

Name	Duration
App start TTI	Baseline Mean: 1220.794 ms Stdev: 44.247 ms (3.6%) Runs: 1119.177658997476 1125.358245998621 1136.9372819960117 1137.9093289971352 1146.2260250002146 1149.227544002235 1151.4264940023422 1155.274838000536 1162.2776840031147 1174.1435550004244 1177.6349119991064 1179.2300710007548 1179.9941110014915 1180.9610259979963 1182.1104920059443 1182.3034719973803 1183.8062770068645 1184.626677006483 1184.8348339945078 1186.8984940052032 1187.1419810056686 1190.368762999773 1191.6738940030336 1191.889907002449 1192.4155099987984 1195.7330480068922 1197.1829639971256 1198.6655659973621 1199.497895002365 1199.7667520046234 1200.5958469957113 1201.1280339956284 1201.8858159929514 1202.3756449967623 1204.514293000102 1204.7653740048409 1204.95987200737 1204.9783319979906 1206.2136189937592 1207.0333669930696 1208.660606995225 1208.7127169966698 1209.5161820054054 1214.2909239977598 1221.7991299927235 1222.2478120028973 1224.6236020028591 1225.2381380051374 1225.3146390020847 1226.6693660020828 1227.752379000187 1228.5576570034027 1228.604295000434 1231.5060250014067 1233.1956779956818 1234.4917930066586 1238.3010379970074 1238.736200004816 1239.2425689995289 1240.3032499998808 1242.2212290018797 1246.0409869998693 1246.9233140051365 1247.4301699995995 1247.8416939973831 1248.341481000185 1250.1612769961357 1252.6026629954576 1253.5345010012388 1257.0795859992504 1258.1408900022507 1259.1724299937487 1263.0926019996405 1265.258698001504 1266.496842995286 1266.898578003049 1269.0472449958324 1274.2232169955969 1274.4315429925919 1276.425925001502 1277.5198429971933 1278.5162409991026 1286.0737719982862 1295.2965810000896 1298.1025869995356 1307.7996300011873 1311.0482250005007 1315.9199759960175 1324.0964249968529 Current Mean: 1309.686 ms Stdev: 32.630 ms (2.5%) Runs: 1247.6022489964962 1248.3721839934587 1249.1808999925852 1250.9424609988928 1255.6119710057974 1263.1219229996204 1263.313244998455 1267.7163619995117 1268.912970006466 1270.716734007001 1271.4358350038528 1272.036226004362 1273.3925669938326 1275.7034250050783 1276.0508310049772 1277.886924996972 1279.1947329938412 1280.4693720042706 1280.9358820021152 1281.5023690015078 1281.8386809974909 1282.1716489940882 1282.4831410050392 1283.4265539944172 1285.429886996746 1286.4432779997587 1288.75409899652 1289.0346519947052 1292.8634669929743 1294.9900570064783 1295.3735949993134 1296.02972599864 1298.05912399292 1299.0234780013561 1299.1787459999323 1299.4383919984102 1300.210398003459 1300.245029002428 1300.7385630011559 1303.3879210054874 1305.4789299964905 1307.223445996642 1307.718838006258 1308.2424509972334 1308.8819510042667 1309.442745000124 1310.4626320004463 1310.8291410058737 1311.3615929931402 1312.3812759965658 1313.444843992591 1314.1125089973211 1314.4348790049553 1314.5014480054379 1316.9227499961853 1317.0716679990292 1317.167732000351 1318.2921459972858 1319.9132799953222 1322.01060000062 1322.1207379996777 1323.8906929939985 1324.1969909965992 1324.4032240062952 1326.1872439980507 1326.527501001954 1327.2728410065174 1327.8926839977503 1328.4373490065336 1328.5242799967527 1329.130172997713 1330.5108270049095 1332.5259680002928 1332.7511440068483 1341.93549400568 1342.8910080045462 1343.6347679942846 1345.238919004798 1346.5646959990263 1350.5603169947863 1350.794495999813 1351.0441879928112 1353.4586640000343 1366.2191819995642 1366.4670960009098 1367.227819994092 1372.0842870026827 1373.7375240027905 1376.5557090044022 1387.650270998478 1387.919643998146
App start runJsBundle	Baseline Mean: 836.244 ms Stdev: 38.823 ms (4.6%) Runs: 745 752 752 768 770 772 780 783 783 787 788 793 796 797 798 799 804 804 804 810 810 811 814 815 815 816 816 817 817 819 819 821 822 825 825 826 826 826 830 831 831 832 833 834 834 834 834 835 836 836 837 837 838 844 848 850 851 852 853 855 856 856 856 858 859 862 862 863 865 866 867 871 871 874 874 877 877 878 882 882 884 884 884 891 891 900 904 920 923 935 Current Mean: 904.727 ms Stdev: 25.951 ms (2.9%) Runs: 856 859 861 865 867 869 870 871 871 871 872 874 875 878 878 878 881 881 882 882 884 884 885 887 888 888 890 890 891 893 893 894 895 896 897 897 898 898 898 899 899 900 900 900 900 902 902 903 904 904 906 907 907 908 913 914 916 916 916 917 918 919 919 920 920 921 921 922 925 925 926 928 930 931 935 940 941 941 944 944 945 948 949 949 952 953 955 975

Meaningless Changes To Duration

Show entries

Name	Duration
Open Search Page TTI	622.992 ms → 629.133 ms (+6.141 ms, +1.0%)
App start regularAppStart	0.015 ms → 0.016 ms (+0.001 ms, +8.0%)
App start nativeLaunch	21.736 ms → 21.036 ms (-0.699 ms, -3.2%)

Show details

Name	Duration
Open Search Page TTI	Baseline Mean: 622.992 ms Stdev: 17.323 ms (2.8%) Runs: 587.4924729913473 587.5959879904985 596.9048670083284 597.0810550004244 597.5209560096264 597.5549319982529 597.9144290089607 598.0949710011482 598.3244630098343 600.8076579868793 603.5568450093269 604.1698819994926 606.7221280038357 606.900675997138 607.0341389924288 607.3402509987354 607.6089280098677 607.9577229917049 609.338663995266 610.7142340093851 611.1088059991598 611.6020099967718 612.4393320083618 612.4924719929695 613.3139649927616 613.4117030054331 613.6776120066643 613.9179279953241 613.9278969913721 614.7849529981613 614.8141280114651 615.258625999093 615.6208090037107 615.6561689972878 616.5762940049171 616.6652829945087 617.1433510035276 619.3160810023546 619.3418780118227 619.7668049931526 620.1361500024796 620.1498619914055 620.8793540000916 621.0989589989185 621.2684330046177 621.4018149971962 621.8865569978952 622.1219079941511 622.4796140044928 622.4866950064898 623.0511890053749 623.6676440089941 623.8476160019636 624.1020509898663 624.1337489932775 625.1559659987688 628.2579350024462 629.0837409943342 629.46260599792 629.7924000024796 629.9285079985857 630.174559995532 630.6113689988852 630.6683759987354 630.6733399927616 631.7118330001831 631.968180000782 633.072550997138 633.0872399955988 633.1280930042267 636.1574300080538 637.0090750008821 638.1271570026875 638.3026130050421 638.5028479993343 640.6336669921875 644.5995290130377 644.6091309934855 645.5631920099258 646.0747070014477 646.9649260044098 652.7143560051918 652.9352219998837 654.1121019870043 655.0791829973459 658.4399419873953 663.1572680026293 663.7530930042267 668.6038410067558 Current Mean: 629.133 ms Stdev: 17.889 ms (2.8%) Runs: 590.998250991106 595.3123780041933 603.8872079998255 604.2231450080872 605.2983400076628 605.4444169998169 608.7803139984608 609.0298669934273 609.9243170022964 611.2965500056744 611.4356280118227 611.6780599951744 612.0058999955654 612.0674650073051 613.7341310083866 614.1468919962645 614.8927409946918 614.9069829881191 615.0983889997005 615.1855880022049 615.472289994359 615.9831549972296 616.0111080110073 616.6807049959898 616.8000079989433 617.5666500031948 618.4220380038023 619.0268549919128 619.4364009946585 620.004394993186 620.2181399911642 620.8201089948416 620.8353279978037 620.9945069998503 621.4499920010567 621.5246170014143 621.869995996356 621.9778240025043 622.303874000907 622.3817139863968 622.6389160007238 622.8463129997253 623.2050369977951 624.016194999218 624.5537929981947 628.6675619930029 628.6796880066395 628.885538995266 628.9748950004578 629.5136720091105 629.6946620047092 629.9716399908066 630.8541270047426 630.927124992013 631.0203860104084 631.091065004468 631.1260170042515 631.2567950040102 631.650960996747 632.5253090113401 632.5790209919214 632.647421002388 632.9512130022049 633.0362549871206 634.087035998702 635.7175299972296 636.8015950024128 638.4222819954157 640.0023600012064 640.959066003561 641.6042490005493 642.2810880094767 643.3150230050087 646.3688969910145 648.0344240069389 648.2364509999752 649.1346030086279 649.4801840037107 650.3478199988604 651.7532970011234 653.954549998045 654.540445998311 655.0387369990349 655.8785810023546 669.3324790000916 670.6273600012064 672.2897959947586 672.8958339989185 673.6850589960814 674.7712000012398
App start regularAppStart	Baseline Mean: 0.015 ms Stdev: 0.001 ms (5.7%) Runs: 0.0128990039229393 0.012981005012989044 0.013183996081352234 0.013346001505851746 0.01342800259590149 0.01354999840259552 0.013590000569820404 0.013630986213684082 0.01371300220489502 0.013754010200500488 0.013794004917144775 0.01383499801158905 0.013874992728233337 0.013875007629394531 0.013916000723838806 0.013916000723838806 0.013955995440483093 0.013956993818283081 0.014037996530532837 0.014037996530532837 0.01403801143169403 0.014078006148338318 0.014118999242782593 0.014118999242782593 0.014120012521743774 0.014159992337226868 0.01416100561618805 0.014200001955032349 0.014240995049476624 0.014281995594501495 0.014282003045082092 0.014282003045082092 0.014322996139526367 0.014362990856170654 0.014363005757331848 0.014364004135131836 0.014445006847381592 0.014445006847381592 0.014485999941825867 0.014485999941825867 0.014527007937431335 0.014567002654075623 0.014567002654075623 0.014567002654075623 0.014567002654075623 0.014567002654075623 0.014608010649681091 0.014608010649681091 0.014647990465164185 0.014648005366325378 0.014648005366325378 0.014688998460769653 0.014688998460769653 0.014729991555213928 0.014730006456375122 0.014810994267463684 0.014851987361907959 0.014933004975318909 0.014933004975318909 0.014973998069763184 0.01501399278640747 0.015015006065368652 0.01505500078201294 0.015096008777618408 0.015137001872062683 0.015178009867668152 0.015217989683151245 0.015218004584312439 0.015218004584312439 0.015258997678756714 0.01534000039100647 0.015381008386611938 0.015422001481056213 0.015502989292144775 0.015543997287750244 0.015665993094444275 0.015665993094444275 0.015707001090049744 0.015707001090049744 0.015707001090049744 0.01574699580669403 0.015827998518943787 0.01586899161338806 0.016154006123542786 0.016154006123542786 0.016195014119148254 0.016276001930236816 0.01639801263809204 0.016520008444786072 0.016682997345924377 0.016722992062568665 Current Mean: 0.016 ms Stdev: 0.001 ms (5.7%) Runs: 0.013875991106033325 0.014078989624977112 0.014079004526138306 0.014282003045082092 0.014364004135131836 0.014403998851776123 0.014608010649681091 0.014810994267463684 0.014811009168624878 0.014934003353118896 0.014973998069763184 0.01505500078201294 0.015096008777618408 0.015137001872062683 0.015137001872062683 0.015178009867668152 0.015178009867668152 0.015218004584312439 0.015259012579917908 0.015298992395401001 0.01534000039100647 0.01534000039100647 0.01534000039100647 0.015381008386611938 0.015381008386611938 0.015421003103256226 0.015422001481056213 0.015422001481056213 0.015422001481056213 0.015422001481056213 0.0154619961977005 0.015462011098861694 0.015462994575500488 0.015542998909950256 0.015542998909950256 0.015543997287750244 0.015543997287750244 0.015583992004394531 0.015625 0.015625 0.015665993094444275 0.015706002712249756 0.015707001090049744 0.01574699580669403 0.01574699580669403 0.0157880038022995 0.0157880038022995 0.015827998518943787 0.015828996896743774 0.015829011797904968 0.015870004892349243 0.015949994325637817 0.01595000922679901 0.015950992703437805 0.015950992703437805 0.015992000699043274 0.01603199541568756 0.016071990132331848 0.01607300341129303 0.016112998127937317 0.016153991222381592 0.016154006123542786 0.01619499921798706 0.016234993934631348 0.016275987029075623 0.016276001930236816 0.016317009925842285 0.016519993543624878 0.016520008444786072 0.016520991921424866 0.016561001539230347 0.016601011157035828 0.016804993152618408 0.01684500277042389 0.01692698895931244 0.017048999667167664 0.017049014568328857 0.01704999804496765 0.01713000237941742 0.017131000757217407 0.017212003469467163 0.017416000366210938 0.017455995082855225 0.017497003078460693 0.01765899360179901 0.01765899360179901 0.017781004309654236 0.01782199740409851 0.01802600920200348
App start nativeLaunch	Baseline Mean: 21.736 ms Stdev: 2.863 ms (13.2%) Runs: 18 18 19 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 22 22 22 22 22 22 22 22 22 22 22 22 23 23 23 23 23 23 24 24 24 25 25 25 25 25 25 25 27 28 28 28 28 28 29 29 29 Current Mean: 21.036 ms Stdev: 1.418 ms (6.7%) Runs: 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 20 20 20 20 20 20 20 20 20 20 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 23 23 23 23 23 23 23 23 23 24 24 24 24 24

github-actions · 2023-10-09T01:06:55Z

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

OSBotify · 2023-10-09T23:01:46Z

🚀 Deployed to staging by https://github.com/mountiny in version: 1.3.80-0 🚀

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

OSBotify · 2023-10-11T04:42:02Z

🚀 Deployed to production by https://github.com/jasperhuangg in version: 1.3.80-3 🚀

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

OSBotify · 2023-10-11T05:10:56Z

🚀 Deployed to staging by https://github.com/mountiny in version: 1.3.81-0 🚀

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

OSBotify · 2023-10-13T22:00:02Z

🚀 Deployed to production by https://github.com/jasperhuangg in version: 1.3.83-11 🚀

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

OSBotify · 2023-10-13T22:12:05Z

🚀 Deployed to production by https://github.com/jasperhuangg in version: 1.3.83-11 🚀

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

OSBotify · 2023-10-13T22:28:18Z

🚀 Deployed to production by https://github.com/jasperhuangg in version: 1.3.83-11 🚀

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

OSBotify · 2023-10-13T23:03:42Z

🚀 Deployed to production by https://github.com/jasperhuangg in version: 1.3.83-11 🚀

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

OSBotify · 2023-10-16T21:15:23Z

🚀 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 · 2023-10-16T21:51:43Z

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

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

ORLHND proposal

ffd2418

mountiny assigned kacper-mikolajczak Oct 2, 2023

kacper-mikolajczak changed the title ~~ORLHND performance optimisation proposal~~ Improve memoization of LHN by handling the isFocused prop better Oct 4, 2023

kacper-mikolajczak added 3 commits October 4, 2023 12:49

Revert "ORLHND proposal"

39b4fa3

This reverts commit ffd2418.

add OptionRowLHNDataWithFocus wrapper

8a0abb6

disable props spreading

4bb01bc

hurali97 suggested changes Oct 4, 2023

View reviewed changes

kacper-mikolajczak added 3 commits October 4, 2023 16:28

Fix typo

f758aeb

move explanation comment

fd9446b

add explanation comment to OptionRowLHNDataWithFocus

86ee246

kacper-mikolajczak requested a review from hurali97 October 4, 2023 14:33

Merge branch 'main' into chat-transition-optimisation-proposal

72ee59a

mountiny requested review from allroundexperts and removed request for hurali97 October 4, 2023 20:41

kacper-mikolajczak marked this pull request as ready for review October 5, 2023 18:37

kacper-mikolajczak requested a review from a team as a code owner October 5, 2023 18:37

melvin-bot bot requested review from mountiny and removed request for a team October 5, 2023 18:37

allroundexperts approved these changes Oct 8, 2023

View reviewed changes

mountiny approved these changes Oct 8, 2023

View reviewed changes

mountiny merged commit fabff79 into Expensify:main Oct 8, 2023
14 checks passed

github-actions bot added the DeployBlockerCash This issue or pull request should block deployment label Oct 9, 2023

OSBotify mentioned this pull request Oct 9, 2023

Deploy Checklist: New Expensify 2023-10-09 #29131

Closed

64 tasks

melvin-bot bot mentioned this pull request Oct 11, 2023

[HOLD for payment 2023-10-18] Improve memoization if LHN by handling the isFocused prop better #28631

Closed

OSBotify mentioned this pull request Oct 11, 2023

Deploy Checklist: New Expensify 2023-10-11 #29252

Closed

MelvinBot mentioned this pull request May 7, 2024

[Snyk] Security upgrade expo from 50.0.4 to 51.0.0 #41804

Open

50 tasks

melvin-bot bot mentioned this pull request May 7, 2024

[Snyk] Security upgrade expo from 50.0.4 to 51.0.0 #41805

Closed

pavelbe4solutions mentioned this pull request May 8, 2024

[Snyk] Security upgrade expo from 50.0.4 to 51.0.0 be4solutions/App#7

Open

	/** Wether row should be focused */
	/** Whether row should be focused */

Improve memoization of LHN by handling the isFocused prop better #28615

Improve memoization of LHN by handling the isFocused prop better #28615

Conversation

kacper-mikolajczak commented Oct 2, 2023 • edited Loading

Intro

Steps to reproduce

Recording

Investigations

Solutions

Wrapper component

Use withCurrentReportID in list component (Alternative)

Conclusions

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

kacper-mikolajczak commented Oct 2, 2023 • edited Loading

mountiny commented Oct 2, 2023

kacper-mikolajczak commented Oct 2, 2023

hurali97 left a comment

Choose a reason for hiding this comment

hurali97 Oct 4, 2023

Choose a reason for hiding this comment

kacper-mikolajczak Oct 4, 2023

Choose a reason for hiding this comment

hurali97 Oct 4, 2023

Choose a reason for hiding this comment

kacper-mikolajczak Oct 4, 2023

Choose a reason for hiding this comment

hurali97 Oct 4, 2023

Choose a reason for hiding this comment

kacper-mikolajczak Oct 4, 2023

Choose a reason for hiding this comment

kacper-mikolajczak commented Oct 4, 2023 • edited Loading

kacper-mikolajczak commented Oct 4, 2023 • edited Loading

mountiny commented Oct 4, 2023

kacper-mikolajczak commented Oct 5, 2023

mountiny commented Oct 5, 2023

kacper-mikolajczak commented Oct 5, 2023

kacper-mikolajczak commented Oct 5, 2023 • edited Loading

Mobile (iOS)

Web

allroundexperts commented Oct 5, 2023

melvin-bot bot commented Oct 5, 2023

mountiny commented Oct 5, 2023

allroundexperts commented Oct 5, 2023 • edited Loading

Reviewer Checklist

Screenshots/Videos

allroundexperts commented Oct 5, 2023 • edited Loading

mountiny commented Oct 5, 2023

kacper-mikolajczak commented Oct 6, 2023

allroundexperts left a comment

Choose a reason for hiding this comment

OSBotify commented Oct 8, 2023

github-actions bot commented Oct 9, 2023

Performance Comparison Report 📊

Significant Changes To Duration

Meaningless Changes To Duration

github-actions bot commented Oct 9, 2023

OSBotify commented Oct 9, 2023

OSBotify commented Oct 11, 2023

OSBotify commented Oct 11, 2023

OSBotify commented Oct 13, 2023

OSBotify commented Oct 13, 2023

OSBotify commented Oct 13, 2023

OSBotify commented Oct 13, 2023

OSBotify commented Oct 16, 2023

OSBotify commented Oct 16, 2023

kacper-mikolajczak commented Oct 2, 2023 •

edited

Loading

Use `withCurrentReportID` in list component (Alternative)

kacper-mikolajczak commented Oct 2, 2023 •

edited

Loading

kacper-mikolajczak commented Oct 4, 2023 •

edited

Loading

kacper-mikolajczak commented Oct 4, 2023 •

edited

Loading

kacper-mikolajczak commented Oct 5, 2023 •

edited

Loading

allroundexperts commented Oct 5, 2023 •

edited

Loading

allroundexperts commented Oct 5, 2023 •

edited

Loading