1870350 - Scrolling Firefox View's Open Tabs uses a lot of CPU and checkerboards with virtual list enabled

Status: NEW

(Firefox :: Firefox View, defect, P3)

Description

[Jeff Muizelaar [:jrmuizel]]

Profile: https://share.firefox.dev/3Nv5h11
The time is being spent in the same kinds of places as it was during view switch before the virtual-list.
i.e. Creating the elements and doing layout on them is still expensive, but instead of doing it all at once we pay the cost spread out during scrolling.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:jsudiaman, could you have a look please?

For more information, please visit BugBot documentation.

Comment 2

[Sam Foster [:sfoster] (he/him)]

:kcochrane, do you have any thoughts on next steps for this? AIUI we shouldn't be creating any elements after the intial rendering - we should just be populating pre-created elements.

Arguably shouldn't be doing anything during active scrolling - its a bad time to be mutating the DOM. The user should probably just see some skeleton UI for not-yet-populated rows, and we wait for scrolling to pause before populating the visible rows.

I'm tentatively tagging this as P2, though if it really is severity S2 we should likely bump this to P1.

Comment 3

[Kelly Cochrane [:kcochrane]]

Taking a look now. Jeff could you provide some more info on about how many open tabs/windows you had when you were seeing this?

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Likely 1 window 3000+ tabs

Comment 5

[Sarah Clements [:sclements]]

What is a "checkerboard effect"? A screenshot or video would help here to avoid any confusion.

But I'm not seeing any issues when scrolling the Open Tabs section with a profile of 1000 open tabs (FYI Kelly, you can find that profile here). There were several bugs filed around the same time pertaining to scrolling the virtual list, namely bug 1870141 and bug 1869094, the former of which has since been fixed. I think this can be closed out unless someone else can reproduce.

Comment 6

[Jeff Muizelaar [:jrmuizel]]

(In reply to Sarah Clements [:sclements] | PTO until 1/2 from comment #5)

What is a "checkerboard effect"? A screenshot or video would help here to avoid any confusion.

Sorry, this is where the background of the page is painted instead of the actual content of the page because painting can't keep up with scrolling rate.

In this profile you can see that there's main thread jank of 50-60ms which is a lot higher then what's needed for 60fps (16ms)

Comment 7

[Jeff Muizelaar [:jrmuizel]]

Attachment: scrolling-480.mov

Comment 8

[Sam Foster [:sfoster] (he/him)]

Kelly and I spent some more time looking into this. I've got a profile here where I'm leaning on the page down button, scrolling though the entire list of 1000 tabs as fast as possible. It shows the CPU and layout time associated with trying to render each new chunk of list as we scroll past it.

The way this is implemented, each chunk (approx the number of rows that would fit in the viewport) is a sub-list. And as we scroll through, an IntersectionObserver callback trips a property on those sublists as they come in or out of view. The sublists are only populated and rendered as they come into view. At a high level, that seems like a reasonable approach - the IntersectionObserver is already throttled to only fire once per frame (vs. scroll events which can fire 100s of times). Looking at the profile, we only see layout and CPU as a new sublist becomes visible.

It might be possible to schedule and defer (debounce) that callback so we aren't trying to populate and render a list which the user is immediately scrolling past.

Kelly prototyped using a placeholder/skeleton-row kind of treatment where the sublist rows would initially show solid grey blocks rather than attempt to populate and render the row in real time. We agreed that seems more visually distracting though and not really a net improvement.

Another possibility would be to keep a pool of sublists and re-use them and their rows as you scroll. That might let lit.js' selective-rendering algorithm work for us a bit better and might help reduce the GC we see at the end of scrolling.

I'm downgrading the priority and severity here. Its a real thing and we should fix this, but you have to try pretty hard to get into this situation, and when you do the UI ultimately remains functional. We have other planned optimizations (like bug 1865106) which I think we should prioritize first as they will have more impact and might actually end up improving things here as well; if we can improve list rendering, we'll also improve list scrolling.

Comment 9

[Sarah Clements [:sclements]]

Sam's going to file investigative bug(s) for his ideas in the last comment so we can continue to improve upon the virtual list.

Comment 10

[Sam Foster [:sfoster] (he/him)]

I filed bug 1879669 which I think is probably our best bet for significant performance improvement here.