cleanup display item representation
Categories
(Core :: Graphics: WebRender, task, P2)
Tracking
()
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)
Updated•6 years ago
|
Assignee | ||
Comment 1•6 years ago
|
||
Assignee | ||
Comment 2•6 years ago
|
||
- 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
Updated•6 years ago
|
Assignee | ||
Comment 3•6 years ago
|
||
try push (clean!): https://treeherder.mozilla.org/#/jobs?repo=try&revision=ea85debaa128383037f8a9906f3cfc5df01fb23e&selectedJob=240063589
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.
Assignee | ||
Comment 4•6 years ago
|
||
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.
- 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.
- 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.
-
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 -
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%).
Updated•6 years ago
|
Assignee | ||
Comment 5•6 years ago
|
||
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.
Assignee | ||
Comment 6•6 years ago
|
||
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.
Assignee | ||
Comment 7•6 years ago
|
||
pausing checking-needed while I hash out a detail with aosmond
Comment 9•6 years ago
|
||
: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', '')
Updated•6 years ago
|
Comment 11•6 years ago
|
||
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', '')
Assignee | ||
Comment 12•6 years ago
|
||
first time using lando myself
please work!!!
Comment 13•6 years ago
|
||
Comment 14•6 years ago
|
||
Backed out 2 changesets (bug 1536121) for Reftest failures
Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=241955700&repo=autoland&lineNumber=2420
Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=0fc395a2ac7155ffece75daf20d103144374fbfe
Backout:
https://hg.mozilla.org/integration/autoland/rev/02a9bf38ebf276bcd4950a7ff6286e8688da15b3
Comment 15•6 years ago
|
||
Assignee | ||
Comment 16•6 years ago
|
||
was a simple rebase error in flatten_iframe
Assignee | ||
Updated•6 years ago
|
Comment 17•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/63ab1f265266
https://hg.mozilla.org/mozilla-central/rev/9550416c06ab
Comment 18•5 years ago
|
||
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
Updated•5 years ago
|
Description
•