Closed Bug 1766017 Opened 3 years ago Closed 3 years ago

Bad performances on grid layout [many mix-blend operations on many small cells with off-screen surfaces]

Categories

(Core :: Graphics: WebRender, defect)

Firefox 99
defect

Tracking

()

RESOLVED FIXED
101 Branch
Tracking Status
firefox-esr91 --- unaffected
firefox99 --- wontfix
firefox100 --- wontfix
firefox101 --- fixed

People

(Reporter: gnury, Assigned: gw)

References

(Regressed 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(3 files)

Steps to reproduce:

Open a page with a CSS grid layout.

Actual results:

After build in [8e9c1ffd, b7f88e5c], Firefox is really slow and almost unresponsive.
Mozregression shows: "Bug 1749380 - Improve how WR handles bounding rects for off-screen surfaces r=gfx-reviewers,kvark" and "Differential Revision: https://phabricator.services.mozilla.com/D137569"

Expected results:

Same performance as in january

Comment on attachment 9273445 [details]
HTML that cause the issue

I don't know what part of the HTML/CSS causes the performance regression.

The Bugbug bot thinks this bug should belong to the 'Core::Layout: Grid' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Component: Untriaged → Layout: Grid
Product: Firefox → Core

Thanks for filing and getting the regression range and the test-case, it's super-useful!

Profile: https://share.firefox.dev/3L7ayZu

This is not due to layout, as mozregression and the profile shows. This is WebRender going wild uploading textures it seems? Glenn, can you take a look when you have the time?

Status: UNCONFIRMED → NEW
Component: Layout: Grid → Graphics: WebRender
Ever confirmed: true
Flags: needinfo?(gwatson)
Regressed by: 1749380

Set release status flags based on info from the regressing bug 1749380

Assignee: nobody → gwatson
Flags: needinfo?(gwatson)

The page content itself appears to have a lot more complexity than is immediately apparent. Each of the grid cells ends up with one or two mix-blend-modes that causes a framebuffer readback, and each of these have a border-radius on the stacking context which causes a child surface. Each child cell also contains a fixed background-attachment.

Nonetheless, the root of the bug seems to be that each of those child cell surfaces is allocated to be a full-screen size - so we end up with hundreds of render target allocations, each a full size screen, and then in addition to that try to do mix-blend-modes on each of those as they're drawn in to the parent surface.

I reduced the test case down to a single cell that ends up allocating a full-screen sized surface:

<html>
  <head>
    <style>

    .userLocation.mod-off,
    .dataGrid-row.mod-columnDefinition .dataGrid-row-cell.mod-offFullColumn:after {
      background-image: linear-gradient(135deg, #e9ebf5 16.67%, #ffffff 16.67%, #ffffff 50%, #e9ebf5 50%, #e9ebf5 66.67%, #ffffff 66.67%, #ffffff 100%);
      background-attachment: fixed;
    }

    .dataGrid {
      display: grid;
      grid-template-columns: var(--header-column-min-width);
      grid-auto-rows: -webkit-max-content;
      grid-auto-rows: max-content;
      grid-auto-columns: -webkit-max-content;
      grid-auto-columns: max-content;
    }

    .userLocation {
      padding: .25rem 2px .25rem .5rem;
      border-radius: 4px;
      border: 1px solid #ebf0fa;
    }

  </style>
</head>

<body>
  <fl-grid class="dataGrid">
    <fl-schedule-user-line class="dataGrid-row">
      <fl-grid-cell class="mod-userLocation dataGrid-row-cell" style="--datagrid-row-cell-start:20;">
        <div class="userLocation mod-off"></div>
      </fl-grid-cell>
    </fl-schedule-user-line>
  </fl-grid>
</body></html>

Hopefully fixing the root cause of that so that each of the cells only allocates an appropriately sized render target will then mean that the rest of the page complexity (mix-blends etc) run fast enough that the performance of this page is acceptable.

The issue is that the background-image has a very large bounding rect, which means that the bounding rect being used for the surface allocation due to the border-radius is very large.

I have a local patch which includes the picture clipping rect in the surface allocation, and this fixes the test case above. However, once mix-blend-mode is re-enabled, the same problem occurs. Next step is to work out why mix-blend on that element doesn't also result in the clip-rect restricting the size of the surface allocation.

Summary: Bad performances on grid layout → Bad performances on grid layout [many mix-blend operations on many small cells with off-screen surfaces]

The attached patch fixes the performance issue on the test page (allocated render targets drops from ~100 down to 2). The patch passes all wrench tests locally, but doing a try run now to see if it breaks anything else in gecko tests.

Has Regression Range: --- → yes
Pushed by gwatson@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e24ed89246a4 Do a better job of clipping off-screen surface allocation sizes r=gfx-reviewers,lsalzman

Backed out for causing reftest failures quirks-decorations.html

  • Backout link
  • Push with failures
  • Failure Log
  • Failure line: REFTEST TEST-UNEXPECTED-FAIL | layout/reftests/text-overflow/quirks-decorations.html == layout/reftests/text-overflow/quirks-decorations-ref.html | image comparison, max difference: 154, number of differing pixels: 142
Flags: needinfo?(gwatson)

Updated fuzziness, and trying to land again - hoping to get this in nightly for a day or two and still have time to request a beta uplift. Not sure if that will be feasible though.

Flags: needinfo?(gwatson)
Pushed by gwatson@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5ccaf8100f79 Do a better job of clipping off-screen surface allocation sizes r=gfx-reviewers,lsalzman
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 101 Branch
Regressions: 1767175
Regressions: 1768179
Regressions: 1771057
Regressions: 1793398
Regressions: 1825450
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: