RotatedBuffer::DrawBufferWithRotation takes close to 10% of time when opening the Firefox menu

RESOLVED FIXED in mozilla51

Status

()

Core
Graphics: Layers
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jaws, Unassigned)

Tracking

(Blocks: 1 bug)

unspecified
mozilla51
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [gfx-noted] [qx:p-])

Attachments

(3 attachments)

Created attachment 8782111 [details]
profile.json

On average it is about 90ms, see attached profile.

I see bug 951846 is also about DrawBufferWithRotation being slow, so maybe there is some optimization we can make within the function or something we can change to the panel to not his this code path?

Bas or Vincent, do you have any ideas here?
Flags: needinfo?(bas)
Created attachment 8782112 [details]
scratchpad script to start recording, open and close the firefox menu 10 times, then stop recording
Also adding mconley in the hopes that he may know who can help out here on the platform side.
Blocks: 1252224
Flags: needinfo?(mconley)
So this is pretty smooth for me, and I'm not seeing anything stand out in the profiler.

jaws, are you on a machine where you're seeing low frame rates for this transition?
Flags: needinfo?(mconley) → needinfo?(jaws)
I'm on Windows 10. Yeah, with that profiler I'm seeing an average of 26fps but we should have a goal of 60fps. There is a noticeable jank during the opening of the panel, and I'm attributing it to the DrawBufferWithRotation call since it takes on average 90ms, about 6 frames.
Flags: needinfo?(jaws)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #6)
> I'm on Windows 10. Yeah, with that profiler I'm seeing an average of 26fps
> but we should have a goal of 60fps. There is a noticeable jank during the
> opening of the panel, and I'm attributing it to the DrawBufferWithRotation
> call since it takes on average 90ms, about 6 frames.

Note that the Performance Profiler from the Browser Toolbox (which I assume you're using) is also going to contain noise from the Performance Profiler itself rendering things. Best to use the SPS Profiler to avoid that noise.
ni'ing jaws for an SPS Profiler profile instead of one from the Browser Toolbox.
Flags: needinfo?(jaws)
Note that I'm also seeing the DrawBufferWithRotation samples in my profile:
Zooming in on the samples between a "start" and "shown" marker:

https://cleopatra.io/#report=bb1de7cb2d9244369ce19404f3711268e51b8a1f&filter=%5B%7B%22type%22%3A%22RangeSampleFilter%22,%22start%22%3A22600995,%22end%22%3A22609660%7D,%7B%22type%22%3A%22RangeSampleFilter%22,%22start%22%3A22601807,%22end%22%3A22602018%7D%5D&selection=0,23,24,25,26,27,28,1,29,30,31,32,33,34,35,36,37,38,39,40,41,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,57,58,59,57,58,59,57,58,59,60,61,62,63

DrawBufferWithRotation seems to account for about 20% of the samples.

Zooming in further, I can see a Style flush (#66) that's caused be "adjustArrowPositioning" in popup.xml:400, which is in response to a onpopupshowing event that's handled in popup.xml:468. I believe that's causing us to skip a frame here.

There's also a weird Reflow (#13), that seems to be caused by a tooltip callback, where nsXULTooltipListener is attempting to show a tooltip which causes the Reflow. That's weird - maybe during the test I had my mouse over something, and it has polluted the this particular set of samples.

The rest of the Style flushes seem to be caused by keyframe animations.

I also see, towards the start of the panel opening, that we call syncContainerWithMainView in the multiview, which causes a style flush, and causes us to skip some frames.

PanelUI's _adjustLabelsForAutoHyphens also takes some time.

I also see us skipping some frames while we draw the text in the menu... I wonder if we could save some time by hiding that text until the animation is done.

So, to conclude:
1) We need to investigate and try to avoid the style flush caused by adjustArrowPositioning
2) We need to investigate and try to avoid the style flush caused by syncContainerWithMainView 
3) We need to understand what the keyframe animations are, since, as far as I understand it, the menu is supposed to be scaling up (which shouldn't require extra paints).
4) We might want to investigate hiding the text while the menu is opening
Thanks for the really detailed investigation here Mike! I will file separate bugs for each point of comment #9 but leave open this one to investigate why it shows up in about 20% of the samples. Leaving open needinfo for me to run the SPS on my machine to see what different results we might get, though I think you already covered it all.

Updated

2 years ago
Whiteboard: [gfx-noted]
This is interesting, that profile seems to suggest we're doing readback from a D2D surface. That's bad. My guess, is that the firefox menu, because of being small, does not get acceleration, but a temporary surface that we allocate here somehow does. That's not completely surprising and fairly stupid, that needs fixing. Will fix.
Flags: needinfo?(bas)
Depends on: 1296658
Flags: needinfo?(jaws)
Whiteboard: [gfx-noted] → [gfx-noted] [qx:p-]
Marking this as fixed. Tested locally with the profiler. Thanks Bas!

Through nine samples, pre-patch this took on average 25.38ms
Through nine samples, post-patch this took on average 8.49ms
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.