Closed Bug 1536121 Opened 6 years ago Closed 6 years ago

cleanup display item representation

Categories

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

task

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox67 --- wontfix
firefox68 --- fixed

People

(Reporter: Gankra, Assigned: Gankra)

References

Details

(Whiteboard: [wr-q2][wr-april])

Attachments

(2 files, 1 obsolete file)

as described in https://github.com/servo/webrender/issues/3334

(push the enum to the top so less items have junk fields)

Assignee: nobody → a.beingessner
  • make all enums repr(u8) (compiler bug blocking this long fixed)
  • enable feature(nll) to remove hack, and catch more unused_mut
  • remove cache markers (abandonned design)
  • don't always push empty SetFilters before PushStackingContext
  • remove dead pub methods

Depends on D25844

Whiteboard: [wr-q2][wr-april]

try push (clean!): https://treeherder.mozilla.org/#/jobs?repo=try&revision=ea85debaa128383037f8a9906f3cfc5df01fb23e&selectedJob=240063589

talos: https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&newProject=try&newRevision=ea85debaa128383037f8a9906f3cfc5df01fb23e&selectedTimeRange=172800

talos analysis:

Looks like a wash, which I consider acceptable since this removes a couple performance hacks to create cleaner/better code that we can optimize more in the future. There are two seemingly concerning linux64-shippable-qr regressions, but we don't see those regressions on linux64-qr, so it's either noise or something that's resolved in nightly.

disclaimer: this isn't an amazing cleanup, but more of a major step that
unlocks the ability to do more minor cleanups and refinements. There's some
messy things and inconsistencies here and there, but we can hopefully iron
them out over time.

  1. The primary change here is to move from
    struct { common_fields, enum(specific_fields) }
    to
    enum (maybe_common_fields, specific_fields)

most notably this drops the common fields from a ton of things
that don't need them PopXXX, SetXXX, ClipChain, etc.

  1. Additionally some types have had some redundant states shaved off,
    for instance, rect no longer has both bounds and a clip_rect, as
    the intersection of the two can be used. This was done a bit conservatively
    as some adjustments will need to be done to the backend to fully eliminate
    some states, and this can be done more incrementally.

2.5. As a minor side-effect of 2, we now early-reject some primitives whose
bounds and clip_rect are disjoint.

  1. A HitTest display item has been added, which is just a Rect without
    color. In addition to the minor space wins from this, this makes it much
    easier to debug display lists

  2. Adds a bunch of comments to the display list, making it easier to understand
    things.

The end result of all these changes is a significantly smaller and easier to
understand display list. Especially on pages like gmail which have so many
clip chains. However this ultimately just makes text an even greater percentage
of pages (often 70-80%).

Attachment #9055276 - Attachment is obsolete: true

scratch that talos analysis, i misunderstood what the "shippable" build meant. That's the most important and most optimized one. My change appears to be hindering PGO from doing some magic enhancement.

So after some discussion I'm going to go ahead and OK the linux64 scrolling regressions. This is the first step in some major cleanups, and it's worth the hit in this one place. I expect future work that builds off this will reclaim the lost performance.

Keywords: checkin-needed

pausing checking-needed while I hash out a detail with aosmond

Keywords: checkin-needed

ok we're comfortable with moving forward

Keywords: checkin-needed

:Gankro , the patch failed to land with message:

Details: We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. (255, 'applying /tmp/tmpEmkblI\npatching file gfx/wr/webrender_api/src/display_list.rs\nHunk #2 FAILED at 37\n1 out of 13 hunks FAILED -- saving rejects to file gfx/wr/webrender_api/src/display_list.rs.rej\nabort: patch failed to apply', '')

Flags: needinfo?(a.beingessner)

rebased

Flags: needinfo?(a.beingessner)
Keywords: checkin-needed

On Fri, April 19, 2019, 3:10 AM GMT+3, by apavel@mozilla.com.
Revisions: D25845 diff 92706 ← D27439 diff 92707
Details: We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. (255, 'applying /tmp/tmpwKqppc\npatching file gfx/wr/webrender/src/display_list_flattener.rs\nHunk #4 FAILED at 594\nHunk #5 succeeded at 687 with fuzz 2 (offset 58 lines).\nHunk #8 succeeded at 834 with fuzz 1 (offset 58 lines).\nHunk #9 FAILED at 818\nHunk #12 FAILED at 983\n3 out of 14 hunks FAILED -- saving rejects to file gfx/wr/webrender/src/display_list_flattener.rs.rej\nabort: patch failed to apply', '') 0 AM GMT+3, by apavel@mozilla.com.
Revisions: D25845 diff 92706 ← D27439 diff 92707
Details: We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. (255, 'applying /tmp/tmpwKqppc\npatching file gfx/wr/webrender/src/display_list_flattener.rs\nHunk #4 FAILED at 594\nHunk #5 succeeded at 687 with fuzz 2 (offset 58 lines).\nHunk #8 succeeded at 834 with fuzz 1 (offset 58 lines).\nHunk #9 FAILED at 818\nHunk #12 FAILED at 983\n3 out of 14 hunks FAILED -- saving rejects to file gfx/wr/webrender/src/display_list_flattener.rs.rej\nabort: patch failed to apply', '')

Flags: needinfo?(a.beingessner)
Keywords: checkin-needed

first time using lando myself

please work!!!

Flags: needinfo?(a.beingessner)
Pushed by abeingessner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/430032511561 cleanup display list code a little bit to prep for refactor. r=gw https://hg.mozilla.org/integration/autoland/rev/0fc395a2ac71 rearchitect the webrender display-list. r=gw
Pushed by abeingessner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/63ab1f265266 cleanup display list code a little bit to prep for refactor. r=gw https://hg.mozilla.org/integration/autoland/rev/9550416c06ab rearchitect the webrender display-list. r=gw

was a simple rebase error in flatten_iframe

Flags: needinfo?(a.beingessner)
Type: enhancement → task
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

Noticed at least one small perf improvement:

== Change summary for alert #20834 (as of Tue, 07 May 2019 03:11:42 GMT) ==

Improvements:

2% displaylist_mutate linux64-shippable-qr opt e10s stylo 3,971.27 -> 3,901.32

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=20834

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: