136301 - mousing over menus causes reflow

Status: NEW

(Core :: XUL, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

While profiling bug 5693, I discovered that mousing over menus causes a reflow.
 I first found the style rule that was guilty, and fixed it, and it's *still*
causing a reflow.  So then I discovered that nsBoxFrame::AttributeChanged
unconditionally does a MarkDirty (at the end of the function).  In this case the
attribute is _moz_menuactive.  So this means that any time an attribute changes
on a XUL element, we reflow.  At least that's what it seems like to me, and
looks like in my profile.

I'll attach the modern skin CSS patch, but it seems (based on my profile) like
menu mouseover performance could be about 50% better if we didn't reflow.  (And
this is with the patch for bug 5693.)

FWIW, I was testing by moving the mouse pointer up and down over my bookmarks menu.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: CSS patch (modern) to prevent style system from generating any reflows

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This unconditional MarkDirty was added in:

revision 1.171
date: 2001/12/03 23:41:08;  author: evaughan%netscape.com;  state: Exp;  lines:
+48 -15
Facelift for nsBoxToBlockAdaptor -r hyatt -sr waterson #110328
Grid memory leak -r naving -sr hewitt #109490


jag moved it accidentially while checking in bug 959, and then moved it back
soon thereafter to fix assertions at startup.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

See bug 110328 comment 3 and bug 110328 comment 4.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

See also bug 113528, a similar problem, it seems.

Comment 5

[Brian Ryner (not reading)]

Comment on attachment 78294 [details] [diff] [review]
CSS patch (modern) to prevent style system from generating any reflows

r=bryner

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I think that patch wasn't the main problem anyway -- I think that's just the
buttons at the end of the menus.  Then again, the fact that they resize on
:hover is annoying...

Comment 7

[Jay Knight]

I see this at http://home.olemiss.edu/~jhknight/ (links on left) in 2002071004.
 Is it the same bug?

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

No, this is a bug about *menus*, not some HTML simulation thereof.

Comment 9

[Markus Hübner]

Do we still reflow?

Comment 10

[Ryan VanderMeulen [:RyanVM]]

dbaron, was this FIXED by the Reflow Branch by chance?

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

From glancing at the nsBoxFrame::AttributeChanged, it appears to be much more careful now about triggering FrameNeedsReflow(), so this is probably fixed/irrelevant