Closed Bug 1531142 Opened 6 years ago Closed 5 years ago

Long pinch actions seem to continuously increase CPU (compositor) time

Categories

(Core :: Graphics: WebRender, defect, P1)

Other Branch
defect

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox69 --- fixed

People

(Reporter: kats, Assigned: jnicol)

References

(Blocks 1 open bug)

Details

(Whiteboard: [gfx-noted][wr-amvp][wr-q2][wr-april])

Attachments

(1 file, 1 obsolete file)

On Android, if you zoom in and out slightly for a long time with the webrender profiler HUD on, the CPU (compositor) graph shows the time increasing per frame as long as you continue the pinching action. Seems kind of odd, like we're accumulating some state or not freeing stuff until the user lets go of the pinch.

http://bit.ly/2SvxGoD is an unsymbolicated profile. Not super useful but it shows the memory increasing throughout which is probably related.

These profiles doen't seem to have Renderer thread. Also, do you know why they're unsymbolicated?

I forgot to add the Renderer thread. I knew I was forgetting one :) As for symbolication, not sure exactly but it was a local build with a mozconfig I had lying around so maybe didn't have the right options.

Attachment #9047844 - Attachment description: Bug 1531142 - Use nsDisplayAsyncZoom items to insert zooming animations in WR. r?jrmuizel,botond → Bug 1511740 - Use nsDisplayAsyncZoom items to insert zooming animations in WR. r?jrmuizel,botond
Attachment #9047844 - Attachment is obsolete: true

https://perfht.ml/2SLH4EM is more symbolicated and has the Renderer thread. Clearly shows increasing composite times.

Now with Bobby's patches from bug 1532810: https://perfht.ml/2XHg5h9

copy_image_sub_data seems to be the winner

At least some of the problem seems to be the default eviction algorithm. We might need to tune it more for Android. Reducing the MAX_MEMORY_PRESSURE_BYTES seemed to improve the performance a bit.

The problem seems to be that pinch zooming fills up the texture cache incredibly quickly. I assume because we cache glyphs/images for every slight change in zoom level, compared to on desktop when we zoom at discrete increments.

So it grows very large, and whenever it is resized we need to perform a copy of the entire thing.

FWIW I also see this on my OnePlus 6, though not nearly as serverely.

Tweaking with the eviction algorithm definitely seems like it would help. We could also resize the cache in larger increments so that we don't have to copy as frequently. Also if we can delay resizing until we know exactly how large the cache needs to be that would mean worst case one copy per frame, but I don't know how feasible that is.

Actually, it looks like we already do that last thing.

We should also probably not be trying to cache glyphs for every slight change in zoom level.

Yeah I think that would help significantly.

Just for a sense of scale, a few zooms in and out of planet.mozilla.org grows my texture cache to >200 layers

Priority: -- → P1
Assignee: nobody → jnicol

Yeah, it seems like the issue here is less about our eviction algorithms being miscalibrated for android and more that we need to handle continuous scaling downstream of the texture cache.

Also, we should make sure we're not kicking off lots of CPU glyph rasterization tasks for all the different zoom levels.

I have a WIP patch doing just that. It seems like the eviction policy might also need tweaked, but it's a huge improvement.

Whiteboard: [gfx-noted] → [gfx-noted][wr-amvp][wr-q2]
Whiteboard: [gfx-noted][wr-amvp][wr-q2] → [gfx-noted][wr-amvp][wr-q2][wr-april]

When pinch zooming webrender would re-rasterize glyphs for each tiny
difference in zoom level. This takes time in itself, but also causes
the texture cache to grow incredibly large, to the point where
resizing it to make room for more glyphs takes far too much time.

This patch avoids this by rounding the size at which glyphs are
rasterized whilst pinch zooming. To do this we add a FrameMsg which
APZ uses to tell webrender whether a spatial node is being pinch
zoomed. Then during frame building if a spatial node is being pinch
zoomed we override the raster space of its corresponding picture.

The chosen raster space is the current zoom level rounded up to the
nearest power of two, but not exceeding 8x. This seems to be a good
balance between quality and performance, though at high zoom levels
the cache still does grow very large due to the size of the glyphs.

Pushed by jnicol@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/582dab79ef68
Avoid excessive glyph rasterization when pinch zooming. r=gw,kats
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
No longer blocks: wr-android-mvp
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: