1669567 - The picture cache texture array gets resized too many times on first scroll

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Jamie Nicol [:jnicol]]

On this device (Samsung A5 2017), on the first render we allocate the main picture cache array with 8 layers. We then grow this in increments of 4 layers. With a single fling on mozilla.com, I see it grow to 12, 16, then 20 within the space of a few seconds. Each of these allocations can take around 30ms on this mid-range android phone, which means that the user's first scroll after opening firefox is very stuttery.

This is presumably either a case of not evicting picture cache tiles aggressively enough, or not picking a realistic initial size. I'd lean more towards the former. (Is there much point in holding on to off-screen tiles?) Glenn, how do we decide to evict picture cache allocations from the cache? Is it via the normal LRU mechanism? Should we consider evicting them manually and immediately when they go offscreen instead?

Comment 1

[Glenn Watson [:gw]]

Looking at the specs, so I can emulate locally, is that a 1920 x 1080 device? And is the behavior similar whether running in portrait and landscape?

Comment 2

[Jamie Nicol [:jnicol]]

Yep, 1080x1920. I only tested in portrait mode. I'd expect there to be fewer tiles in landscape, because it still needs 2 columns of 1024-wide tiles, but the number of rows would be smaller. But even if the number of tiles is lower the behaviour could be similar.

Comment 3

[Glenn Watson [:gw]]

Attachment: Bug 1669567 - Use fixed size texture arrays for picture cache slices.

Previously we used a single texture array for a given tile size,
and resized the texture array as more tiles were needed.

However, this results in expensive driver stalls and GPU copy times
when resizing the array.

Instead, use a fixed slice count for each texture array, and support
multiple textures with the same tile size.

This may result in slightly more draw calls during compositing of
picture cache tiles due to batch breaks, but will remain a small
number due to the limited number of picture cache tiles that are
allocated at any one time.

Comment 4

[Glenn Watson [:gw]]

Added a patch with a potential solution to test on device and see if that's reasonable.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fe44655a70ca
Use fixed size texture arrays for picture cache slices. r=jnicol

Comment 6

[Glenn Watson [:gw]]

My comment in riot earlier that we retain tiles for a full screen size either side of the viewport was incorrect - it's only one tile height either side of the viewport that we retain.

I changed that to not retain anything not visible and the eviction seemed to be working correctly - I could count the number of tiles on screen, and it matched the number of live picture cache tiles.

So, if you are seeing many more than that, you might want to locally change [1] to not inflate at all, and then add some debug logs to see how many slices are being created and why those extra tiles exist in your use case.

[1] https://searchfox.org/mozilla-central/source/gfx/wr/webrender/src/picture.rs#2713

Comment 7

[Glenn Watson [:gw]]

Attachment: Bug 1669567 - Increase the default tile size used for scrollbars.

Most platforms have scrollbars > 16 pixels in width. This typically
reduces the number of native layers allocated on most platforms for
scrollbars. This is quite important as the fixed overhead of each
native layer is relatively high on DC and CA.

Comment 8

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/fe44655a70ca

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/18fb8d11fd2d
Increase the default tile size used for scrollbars. r=jnicol

Comment 10

[Cosmin Sabou [:CosminS]]

Backed out for causing talos crashes @unscaled_composite.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=18fb8d11fd2d8b85ff361c3323729a47ee4577ff&searchStr=talos&selectedTaskRun=aCGvYveIQcq3hGx-Zxtj1A.0

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=318319316&repo=autoland&lineNumber=1137

Backout link: https://hg.mozilla.org/integration/autoland/rev/28458ab40734e68e2343c90bf72f9df435aed921

Comment 11

[Glenn Watson [:gw]]

Lee, looks like this failure is a crash inside SWGL - any ideas what might cause it (the patch changes the dimensions of tiles used for scroll bars).

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c4d6ac24a7f6
Increase the default tile size used for scrollbars. r=jnicol

Comment 13

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/c4d6ac24a7f6

Comment 14

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

The leave-open keyword is there and there is no activity for 6 months.
:jnicol, maybe it's time to close this bug?

Comment 15

[Jamie Nicol [:jnicol]]

Yes I believe this is fixed now as we no longer use texture arrays for picture cache tiles