Jared Wein [:jaws] (please needinfo? me) Reporter
	Description • 8 years ago

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?

	Jared Wein [:jaws] (please needinfo? me) Reporter
	Comment 2 • 8 years ago

Also adding mconley in the hopes that he may know who can help out here on the platform side.

	Mike Conley (:mconley) (:⚙️)
	Comment 3 • 8 years ago

I've taken jaws' Scratchpad and modified it for use with the Gecko Profiler Add-on.

I gathered the following profile: https://cleopatra.io/#report=551e1b62404afa81d2f63ddec1270a02584e19ab&filter=%5B%7B%22type%22%3A%22RangeSampleFilter%22,%22start%22%3A22600995,%22end%22%3A22609660%7D%5D&selection=0,23,24,25,26,27,28,1,29,30,31,32,33,34,35,36,37,38,39,131,40,132,133,134,135,136,137

	Mike Conley (:mconley) (:⚙️)
	Comment 5 • 8 years ago

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?

	Jared Wein [:jaws] (please needinfo? me) Reporter
	Comment 6 • 8 years ago

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.

	Mike Conley (:mconley) (:⚙️)
	Comment 7 • 8 years ago

(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.

	Mike Conley (:mconley) (:⚙️)
	Comment 8 • 8 years ago

ni'ing jaws for an SPS Profiler profile instead of one from the Browser Toolbox.

	Mike Conley (:mconley) (:⚙️)
	Comment 9 • 8 years ago

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

	Jared Wein [:jaws] (please needinfo? me) Reporter
	Comment 10 • 8 years ago

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.

	Bas Schouten (:bas.schouten)
	Comment 11 • 8 years ago

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.

	Jared Wein [:jaws] (please needinfo? me) Reporter
	Comment 12 • 8 years ago

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