We are merging too many personal details during OpenReport

[muttmuure]

Problem

Right now openReport is “dumb” and gets all personal details for everyone in the room every time. It doesn’t know what you already have or if you’ve ever opened this report before

This contributes to slow chat switching speeds, if we download all personal details every time we switch to a room, even if we already have them all.

Solution

[kacper-mikolajczak]

Hi @muttmuure I am Callstack engineer and I would like to investigate this issue 🔧

[kacper-mikolajczak]

Hi @muttmuure! I did some digging and it turns out that the original issue was partially addressed, as from yesterday, we are not passing the entire personalDetails object every time to the LHN items, nor fetch it at every entrance (it is being cached after first opening). The change mentioned above definitely should yield some positive outcome, but the ReportScreen still seems to have issues with performant rendering. In order to investigate its issues:

1. I profiled the loading of a report in HT account without children components. At 4x CPU slowdown It took 39 commits, from which two of them exceeded the 16ms threshold:
   • first commit above the threshold took about 50ms, caused mostly (25ms) on withOnyx(ReportScreen call
   • second commit above the threshold took about 270ms, which most of it was spent at the LHNList component
     • Source of list items issue is optionItem property of OptionRowLHNData that triggers SidebarUtils.getOptionData method, which can take up to 10ms to complete
     • personalDetails prop passed to OptionRowLHNData does not have stable reference, which causes the list items to be re-rendered
       • The personalDetails are being filtered in LHNOptionsList component at 144 line. It is filtered in order to avoid passing heavy props to list item (Tomek’s change I’ve mentioned before)
     • ReportScreen takes about 20ms to render, which is called up to three times at once due to cached Report pages (see recent navigation changes)

Right now I am in the middle of analyzing commits of fully rendered ReportScreen, so I will get back to you if I stumble upon anything worth sharing :)

PS

For the openReport overhead, it looks like there is no significant difference with and without the merge process of personalDetails in an openReport method. That being said, I only measured the synchronous actions so it might be the case that we still get a negative impact from processes undertaken in the background - I will try to investigate it further!

[muttmuure]

Thanks @kacper-mikolajczak!

[melvin-bot]

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

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