Closed Bug 1420628 Opened 2 years ago Closed Last year
Context + Text Draw Target creation and destruction slows down ns Display Text::Create Web Render Commands
47 bytes, text/x-phabricator-request
|Details | Review|
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.
2 years ago
Priority: -- → P3
Whiteboard: [wr-mvp] [triage] → [wr-mvp] [triage][wr-reserve-candidate]
Whiteboard: [wr-mvp] [triage][wr-reserve-candidate] → [wr-reserve]
2 years ago
Priority: P3 → P2
I don't think this should be a blocker on the first release.
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.
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
I thought you might tackle this. Do you have an idea of how much difference it makes?
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.
It's a bit hard to measure dispaylist_mutate scores, since we're so scene building bound. Using displaylist_mutate with text  (lots of opacity items, containing text and background color), this patch alone removes ~5ms per frame (45ms -> 40ms) from the RenderLayers section.  https://mattwoodrow.github.io/dl-test/dl-test.html?layer=flattened
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/f5c8fe43b6ee Avoid allocations for each text display item. r=jrmuizel
You need to log in before you can comment on or make changes to this bug.