1565827 - Capturing profiler screenshots with WebRender on Android is really slow on the Moto G5

Status: RESOLVED FIXED

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

Description

[Markus Stange [:mstange]]

Profile: https://perfht.ml/32AFfk6

read_pixels_into_pbo is taking up 150ms per call and looks to be doing pixel format conversions on the CPU.

Comment 1

[Jamie Nicol [:jnicol]]

I would guess this is the inverse problem as bug 1498732. That ReadPixels cannot be done asynchronously because the stride of the data being transferred is not 256 bytes. I think the "convert" in the function names is a red herring, it's probably just that the slow path is the same regardless of whether the format needs conversion?

If I change the width in read_pixels_into_pbo to a multiple of 256 / 4 then read pixels disappears from the profiles.

Comment 2

[Markus Stange [:mstange]]

I see! So could we fix this by just upping the screenshot size from 350 to 384 in ProfilerScreenshots.h?

Comment 3

[Jamie Nicol [:jnicol]]

We could fix it by making sure the screenshot width a multiple of 64, yes. But I don't think setting ScreenshotSize() to 384x384 is quite be enough - currently on my device the size we choose is 213x350, because the width and height are both scaled to fit inside 350x350. If we changed it to 384 it'd choose 233x384 or something like that, and will choose something else on a screen with a different aspect ratio. So the logic to choose the scale needs to be a wee bit smarter.

Comment 4

[Markus Stange [:mstange]]

Oh, I assumed we always read back the entire square and cropped it afterwards. (At least in the non-WebRender implementation, the scale down buffers are all square.)

Comment 5

[Jamie Nicol [:jnicol]]

I've only really skimmed this code but presumably we could round up the width of the final scale-down buffer. (and then we'd have to return both the width and the stride so that the calling code knows how to crop it?)

Comment 6

[Jamie Nicol [:jnicol]]

Been getting up to speed on this this today. The scale down buffers are indeed square, but we ReadPixels the rectangular area to the PBO (which is obviously 1-dimensional, but we tightly pack it and assume that width == stride later on when we map it). This shouldn't be too hard to fix.

Comment 7

[Jamie Nicol [:jnicol]]

Attachment: Bug 1565827 - Ensure glReadPixels for profiler screenshots happens asynchronously. r?kvark

The glReadPixels call for capturing profiler screenshots is very slow
on Adreno devices. Similarly to bug 1498732, this is because the
stride of the data being transferred is not a multiple of 256, so the
driver is taking the synchronous path instead of reading in to a PBO
asynchronously.

This solves this problem by increasing the width of the area we read
so that we hit the fast path. To do this we must ensure that the PBO
and the final scale-down texture are large enough to include the extra
pixels in each row. As the required size of the PBO or texture may now
change, for example after a screen rotation, we now handle deleting
and recreating them when necessary.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d404bde4a6c5
Ensure glReadPixels for profiler screenshots happens asynchronously. r=kvark

Comment 9

[Cosmin Sabou [:CosminS]]

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