[Optimization] Report Actions View

[kidroca]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

This is a list of possible optimizations in the ReportActionsView component

ReportActionItem.onLayout

https://github.com/Expensify/Expensify.cash/blob/36a336400efc060df2ebc21d74ca1ee19ee5afff/src/pages/home/report/ReportActionsView.js#L377

This is used to measure the time it takes for initial render
A chat might rener 100+ items and they will all trigger this callback at once or very shortly one after the other. As you scroll through the chat it will get called more and more, even when it does nothing it still adds many event listeners that aren't serving a purpose - they still take resources

Or at least this function can be used to cache the measured layout of each item and then provide it to the FlatList through getItemLayout this should boost scenarios where you scroll to the past and then back to recent items. Or even cache measured layout in Onyx so the next app startup can use it - though a user can use different devices web/mobile and this have to be accounted for

Reduce complexity of ReportActionItem

https://github.com/Expensify/Expensify.cash/blob/36a336400efc060df2ebc21d74ca1ee19ee5afff/src/pages/home/report/ReportActionItem.js#L246-L248
Modals, Popovers and such can be extracted out of here
For example aPopoverWithMeasuredContent is rendered for each action item it's content would be measured even when a popup doesn't need to render
Popups that appear onPress can use the event and appear at the correct anchor spot
We can do a test where all popups are removed from ReportActionItem and measure the timing

React cannot statically evaluate a ReportAction item

There's a large portion that cannot get optimized by react as it's recreated for each render
https://github.com/Expensify/Expensify.cash/blob/36a336400efc060df2ebc21d74ca1ee19ee5afff/src/pages/home/report/ReportActionItem.js#L274-L276
Since children are an inline function react needs to compute the Tree inside at (re)render time
Each render declares a new anonymous function

zIndex uses a separate layer for each action item

I think doing a different zIndex for each item is expensive
https://github.com/Expensify/Expensify.cash/blob/36a336400efc060df2ebc21d74ca1ee19ee5afff/src/pages/home/report/ReportActionsView.js#L331-L348
This logic would not be needed if popups are moved out of ReportActionItem

Attachments render in-chat should be squished

Rendering multiple 2-5MB images costs a lot of resources, if it's possible return a lower resolution image from the backend that still looks ok in mobile, show the full size only when a particular attachment is selected
If it's possible use a library like https://github.com/DylanVann/react-native-fast-image

Platform:

Where is this issue occurring?

Web
iOS ✔️
Android ✔️
Desktop App
Mobile Web ✔️

Version Number:
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:

View all open jobs on Upwork

[MelvinBot]

Triggered auto assignment to @zanyrenney (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[MelvinBot]

Triggered auto assignment to @pecanoro (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[quinthar]

All of these are great, but I think we should focus on what it takes to cold boot, and switch to a cold chat. Scrolling backwards is good, and large images are good, but those feel less important than the mainline case of just making the chat itself load fast when you open the app.

[kidroca]

From my experience the chat loads fast but rendering can't cope with the data
rendering attachments might play a bigger role than we think
keep in mind that even though you're on LHN a chat is being pre-rendered underneath, we can experiment and disable that to see the effects on speed and whether the app would be fast enough to populate a chat and slide it in as it is selected
It would definitely be much faster to load just the LHN, but can we make it work past this...

[MelvinBot]

Triggered auto assignment to @jliexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[jboniface]

@jliexpensify it looks like you were accidentally unassigned from this

[marcaaron]

Hey @kidroca, I was thinking we could maybe break these out into separate issues to help evaluate them separately? Seems like some of this may lead to improvements, but probably should not all be done in a single PR.

[marcaaron]

Going to respond to each point then we can decide what to do about each one...

ReportActionItem.onLayout

This is used to measure the time it takes for initial render
A chat might rener 100+ items and they will all trigger this callback at once or very shortly one after the other. As you scroll through the chat it will get called more and more, even when it does nothing it still adds many event listeners that aren't serving a purpose - they still take resources

Using resources itself is not a specific problem. But maybe we can test to show that it improves the time to render a chat in some way? I'm not sure how, but probably would not make this change without some explanation of what experience it improves.

Or at least this function can be used to cache the measured layout of each item and then provide it to the FlatList through getItemLayout this should boost scenarios where you scroll to the past and then back to recent items. Or even cache measured layout in Onyx so the next app startup can use it - though a user can use different devices web/mobile and this have to be accounted for

We don't actually need to "measure" these items so nothing would need to be cached. The onLayout callback is used as a somewhat more reliable way to tell how long it took a chat to render. At least that was the original intention. Obviously not so reliable anymore since the timing logs are currently extra broken due to some other unknown reason 😅

[marcaaron]

Reduce complexity of ReportActionItem

Ok, I think at least some of this might be a possible solution to the Individual chat messages are slow to render problem. Which is likely to have more than one solution as we generically improve the speed at which a report action item loads.

We can do a test where all popups are removed from ReportActionItem and measure the timing

Let's do it!

React cannot statically evaluate a ReportAction item

Makes sense. Can we test this by removing the hoverable temporarily and measure the timing?

zIndex uses a separate layer for each action item
I think doing a different zIndex for each item is expensive
This logic would not be needed if popups are moved out of ReportActionItem

Sounds like icing on the cake and like a theory we don't really need to explicitly prove if removing the inefficient popups makes the rendering faster.

[marcaaron]

Attachments render in-chat should be squished

Rendering multiple 2-5MB images costs a lot of resources, if it's possible return a lower resolution image from the backend that still looks ok in mobile, show the full size only when a particular attachment is selected
If it's possible use a library like https://github.com/DylanVann/react-native-fast-image

We've tested this library locally and didn't see much performance gain from it and ultimately decided not to use it. Performance and rendering issues exist for chats that have no attachments at all. So, while there might be some improvements to make to attachment rendering down the line it doesn't seem like what we should focus on right now.

[marcaaron]

That's all I've got for now. I'm closing this issue for focus since it doesn't propose to solve any specific problem.

It seems like there is a very good chance that reducing the complexity of ReportActionItem could be a solution to this issue. And maybe removing the onLayout callback? But I'd suggest next that we move the conversation there and investigate the scope and impact of those changes further + do some benchmarks.