Closed
Bug 1420628
Opened 7 years ago
Closed 6 years ago
gfxContext + TextDrawTarget creation and destruction slows down nsDisplayText::CreateWebRenderCommands
Categories
(Core :: Graphics: WebRender, enhancement, P2)
Core
Graphics: WebRender
Tracking
()
RESOLVED
FIXED
mozilla65
People
(Reporter: mstange, Assigned: mattwoodrow)
References
(Blocks 2 open bugs)
Details
(Whiteboard: [wr-reserve])
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
Here's a profile I got from scrolling https://w3c.github.io/ServiceWorker/ with a profiler interval of 0.2ms on macOS: https://perfht.ml/2k2CpCJ Around 20% of the nsDisplayText::CreateWebRenderCommands is taken up by memory allocation + deallocation of gfxContext and TextDrawTarget. It would be nice to avoid that cost. For example, we could have a gfxContext wrapping a TextDrawTarget stored on the StackingContextHelper, and give the TextDrawTarget a way to reset itself. Then every nsDisplayText::CreateWebRenderCommands invocation could just get that context from the StackingContextHelper, and the StackingContextHelper would reset the TextDrawTarget just before handing out a pointer to that gfxContext.
Updated•7 years ago
|
Whiteboard: [wr-mvp] [triage]
Updated•7 years ago
|
Blocks: stage-wr-nightly
Priority: -- → P3
Whiteboard: [wr-mvp] [triage] → [wr-mvp] [triage][wr-reserve-candidate]
Updated•7 years ago
|
Whiteboard: [wr-mvp] [triage][wr-reserve-candidate] → [wr-reserve]
Updated•7 years ago
|
Priority: P3 → P2
Comment 2•6 years ago
|
||
One way to help this would be to eliminate the need for a gfxContext in our text drawing code. One of the pieces to doing that is to fix the use of an implicit gfxPattern and make it explicit. To make this work we'll need to adjust the gfxPattern matrix so that the context scaling that happens during text drawing doesn't affect the pattern matrix. Today this is all happening in implicitly on the gfxContext.
Assignee | ||
Comment 3•6 years ago
|
||
I have a patch that implements Markus' idea for this (written before I saw this bug), and it makes a fairly significant difference. I do agree that getting rid of the gfxContext dependency would be nice, but I don't think it's something we want on our critical path right now, and it's the simpler of the two objects to retain.
Assignee: nobody → matt.woodrow
Assignee | ||
Comment 4•6 years ago
|
||
MozReview-Commit-ID: HoRHQVEdJeT
Comment 5•6 years ago
|
||
I thought you might tackle this. Do you have an idea of how much difference it makes?
Assignee | ||
Comment 6•6 years ago
|
||
On the testcase we were discussing yesterday (reddit comments page, with an inline youtube video playing at the top), the patch queue in differential saves about 20% (2.5ms -> 2.0ms). This was the most significant chunk of that. I have a displaylist_mutate testcase that includes text, which may be less realistic, but easier to measure. I'll test it out.
Comment 7•6 years ago
|
||
Very nice.
Assignee | ||
Comment 8•6 years ago
|
||
It's a bit hard to measure dispaylist_mutate scores, since we're so scene building bound. Using displaylist_mutate with text [1] (lots of opacity items, containing text and background color), this patch alone removes ~5ms per frame (45ms -> 40ms) from the RenderLayers section. [1] https://mattwoodrow.github.io/dl-test/dl-test.html?layer=flattened
Pushed by mwoodrow@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f5c8fe43b6ee Avoid allocations for each text display item. r=jrmuizel
Comment 10•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f5c8fe43b6ee
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox65:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Updated•2 years ago
|
Blocks: wr-displaylist-perf
You need to log in
before you can comment on or make changes to this bug.
Description
•