Long delay (487ms+) during scrolling to load Pocket content on the homescreen; also slow on cold start

[mcomella]

Steps to reproduce

• Open enough homescreen sections to have Pocket below the fold (on my Moto G5, I just need to have one tab open for the "Jump back in" section to show)
• Scroll down to reveal Pocket section

Expected behavior

Scrolling is smooth & section appears immediately

Actual behavior

Scrolling stops as the main thread is blocked to load the Pocket content and contributes to a feeling of jank. Here's a profile: https://share.firefox.dev/3mKBZxb

According to the profile, the initial dispatchTouchEvent runs for 487ms (a very long time) so the delay is likely to last even longer than that. Here's a screen capture:

pocket-jank.mp4

This delay also affects the cold start experience where it takes a long time to load the homescreen to visual completeness:

device-2021-10-11-113101.mp4

Device information

• Android device: Moto G5
• Fenix version: Nightly 10/10

---

To address both issues raised above, I think we need to consider one or both of:

• optimizing the layout to be fast enough to load 1) during the scroll step and 2) without delaying visual completeness on the homescreen for too long
• synchronously load a lightweight, incomplete layout in place of the full Pocket layout (a "skeleton layout") and asynchronously load the remainder of the content into it, never blocking the main thread for too long

CC @Mugurell

┆Issue is synchronized with this Jira Task

[mcomella]

One quick-to-implement optimization we could try is to split the "Thought-provoking stories" section and the "Stories by topic" section into two separate RecyclerView items. It'll also help determine if the perf problem is in the "Thought-provoking" section or the "Stories by topic" section, or both.

[Mugurell]

Splitting the current composable from PocketStoriesViewHolder into more (we could have even 3 separate composables there: stories, categories and the pocket header) would probably mean some gains (if composeView.setContent is not expensive in it's own).
The other thing to look for is ensuring there are no unneeded recompositions.

Another important aspect I think is how much of an effect does this have on the most used devices.
Compose will be a bit slower than xml (at least for now) but on my Pixel3 (which has a more powerful hw than Moto G5) i'm seeing some glitches in debug but no issue on Nightly.

[mcomella]

Another important aspect I think is how much of an effect does this have on the most used devices.

The perf team picked a high-end and a low-end device that we believe is representative of our users – these are our reference devices. If we see a perf problem on our low-end reference device, we generally assume all low-end devices see similar issues. That said, we last set the reference devices in maybe 2019 so they're due for an upgrade. Those devices were the Pixel 2 (high-end) and the Moto G5 (low-end). I don't think we have time to revisit this before addressing this bug so I'm inclined to go with what I go with what I see on the Moto G5 but I'm open to discussion. fwiw, my intuition is that the Pixel 3 is closer to a high-end device amongst our user base but I haven't looked recently.

Compose will be a bit slower than xml (at least for now)

@Mugurell Just curious, why is compose slower? How did you find this out?

[Mugurell]

Compose will be a bit slower than xml (at least for now)

@Mugurell Just curious, why is compose slower? How did you find this out?

Haven't profiled this myself but it seems that after all these years of being used on Android, time in which probably many improvements were made, XML layouts are really very fast.
This seemed like an interesting read:
https://engineering.premise.com/measuring-render-performance-with-jetpack-compose-c0bf5814933

[mcomella]

Though we labeled this P2, I think it's a P2 we want to track so moving to the appropriate column.

[mcomella]

There is still non-trivial jank when scrolling into the Pocket section (though it's less significant than I remember) on a Moto G5 in Nightly from 2/15: reopening.

[Mugurell]

@mcomella @csadilek
Enabled GPU rendering profile - On screen as bars to be able to easily link frame drops to what's happening on screen an tested the following scenario:

• cold start the app
• scroll to bottom
• open a webpage
• go home
• scroll to bottom

on two versions of the debug app with the only difference being the position of the Pocket section: first at the top, then at the bottom.
Tested on a Samsung S8 with Android 8, one on which there was indeed a visible lag when scrolling on homescreen the (the first time in that fragment).

Pocket at the top	Pocket at the bottom
JankWithPocketAtTheTop.mp4	JankWithPocketAtTheBottom.mp4

Looking at the bar chart we can see the app showing the same heavy processing and framedrops when the Pocket section is at the top and then scrolling to bottom so other Views are inflated (there are also some other composables at the bottom) as when Pocket is at the bottom and the user scrolls to it to be "inflated".

To me this shows that there's isn't an inherent performance issue with the Pocket section but rather with the home screen as a feature that uses RecyclerView + Android Views + Composables, a combination that proves quite heavy so to try and fix these slowdowns we should finish migrating the entire homescreen to Compose.

[mcomella]

The GPU rendering profile show that most of the time is spent in in the three green colors, which we have control over (here's what they represent). That doesn't tell us exactly what to optimize though a profile might get us that info. Here's a profile of scrolling down to Pocket: https://share.firefox.dev/3oU31nV and a view in that profile of the first long blocking operation https://share.firefox.dev/3v8dLTk. I don't see anything that would indicate that switching all of these views to compose will improve the performance but I'm unfamiliar with compose – e.g. maybe it's able to better batch work when it's not split between multiple RecyclerView Views.

If we think that switching to all compose views would improve performance enough, we should prototype it to confirm that hypothesis.

---

fwiw, when using the View framework, I see places where we can optimize. For example, I see we spend ~125ms inflating and populating Top Sites items on start up to homescreen https://share.firefox.dev/3oVcOd9. Instead, we could inflate the parent to a height that is based on the number of top site items (i.e. 1 row or 2?) then inflate and populate the children (the top site items) asynchronously which should speed up the initial inflation.

I'm concerned we won't know how to identify optimizations, or if it's even possible to do them, with compose but again, I'm unfamiliar with it.

[Mugurell]

@mcomella @csadilek
Since there are already a few other homescreen features implemented in compose I used them all in a HomeFragment refactored to show a composable containing only

• A compose.foundation.layout.Row containing the Firefox logo (I added this simple enough composable that we'd need later down the road anyway. Also helps with increasing the layout's height and scroll distance)
• Recent tabs
• Recent bookmarks
• Recent visits
• Pocket

to compare how using just Compose on the homescreen might impact app performance.

For the current implementation baseline I've also refactored HomeFragment to only show (and load data for) the same sections (in the current RecyclerView based implementation):

• The AppBar containing the Firefox logo
• Recent tabs
• Recent bookmarks
• Recent visits
• Pocket

Results:

Current impl with only 5 sections	Compose based impl with only 5 sections
5SectionsHomescreenInRV.trace.zip	5SectionsHomescreenInCompose.trace.zip
5SectionsHomescreenInRV.mp4	5SectionsHomescreenInCompose.mp4

Haven't tested this thoroughly and the refactorings were hasted but I can upload the patches if more tests are needed.
At a quick glance though a compose only implementation is visibly quicker (by just looking at how the app starts and scrolls) and the GPU on-screen tracing clearly shows this plus that there is no layout penalty for when scrolling.
So I'd say refactoring everything on the homescreen to compose should be prioritized before trying other optimizations in the current RecyclerView based implementation.

[csadilek]

Thanks @Mugurell for digging into this. Given the results, and the fact that it structurally also makes more sense to move the refactoring along (compared to tweaking code that will have to be refactored anyway), I agree that this is the right approach. Let's talk offline how we can make time to move this forward.

[mcomella]

Thanks for doing the investigation. It looks significantly smoother when scrolling so that's great. I'm slightly concerned that the time to first frame or time to visual completeness will be longer with Compose but I assume we can optimize that (e.g. we make the initial layout only contain the logo and then add more views).

fwiw, we have some perf tools that can make your investigation, and sharing your investigation, a little easier:

• you can get start up profiles with lower overhead and without modifying the code by using the Firefox Profiler. Here's how to do start up profiling: https://profiler.firefox.com/docs/#/./guide-remote-profiling?id=startup-profiling-fenix
• you can upload .trace files to profiler.firefox.com so you can look at them in the Firefox Profiler (imo it's more usable than Android Studio's) and easily share them with a link. e.g. here's your RecyclerView https://share.firefox.dev/3sdTDxq and Compose https://share.firefox.dev/3BLC9M2 traces
• you can put two videos side-by-side with timestamps using perf-tools/combine-videos-side-by-side.sh. For example, I made this video with it:

combined2.mp4

However, in this case, take the timings in this video with a grain of salt: I don't know if this side-by-side is synchronized correctly because I can't see the action that triggered the build to start (e.g. the finger tap).

[Mugurell]

Removing my assignment since this resulted in a meta tracking more work towards refactoring the homescreen for the future with the hope of also improving performance.

[stale]

See: #17373 This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.