Closed Bug 1629835 Opened 4 months ago Closed 4 months ago

GPU cache updates use slow path on Adreno

Categories

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

Unspecified
Android
defect

Tracking

()

RESOLVED FIXED
mozilla77
Tracking Status
firefox-esr68 --- unaffected
firefox75 --- unaffected
firefox76 --- fixed
firefox77 --- fixed

People

(Reporter: jnicol, Assigned: jnicol)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

In https://phabricator.services.mozilla.com/D65598 we made it so that GPU cache updates are done on as small a region as possible rather than uploading the entire row. This has the unintended consequence of making it so that uploads no longer take the fast path.

Here is a profile scrolling through hacker news comments on a Pixel 2: https://perfht.ml/2K61Ht8

Note that the implementation of TextureUploader::upload() already ensures that the stride, and offset within the PBO are correctly aligned. So I believe the issue is that rect.x is incorrectly aligned. Previously it was always zero because the entire row was uploaded. And for texture cache updates it happens to be aligned because of our allocation algorithm. So until now we haven't required code to ensure the x offset is correct.

Actually, the dest x coordinate doesn't seem to matter at all. The issue is that the stride has to be a multiple of 64 pixels, rather than 256 bytes. So 256 bytes was correct for the RGBA8 texture cache, but not for the RGBAF32 GPU cache.

Summary: GPU cache updates use slow path on Adreno because of destination x offset → GPU cache updates use slow path on Adreno

Previously we were using 256 bytes (which happens to be equal to 64
pixels for RGBA8 textures). A recent change to the GPU Cache uncovered
the fact that the requirement is actually 64 pixels, eg 1024 bytes for
RGBAF32 textures.

Pushed by jnicol@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f5be65b0b22b
Ensure PBO upload strides are a multiple of 64 pixels on adreno r=kvark
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla77

Comment on attachment 9140519 [details]
Bug 1629835 - Ensure PBO upload strides are a multiple of 64 pixels on adreno r?kvark

Beta/Release Uplift Approval Request

  • User impact if declined: Bad performance for webrender on android. Stuttery scrolling, etc
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Very small change, only affects behaviour on Android and uses existing code path.
  • String changes made/needed:
Attachment #9140519 - Flags: approval-mozilla-beta?

Comment on attachment 9140519 [details]
Bug 1629835 - Ensure PBO upload strides are a multiple of 64 pixels on adreno r?kvark

Android-only WR perf improvement. Approved for 76.0b5.

Attachment #9140519 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.