515349 - Over 1000 PresShell::FlushPendingNotifications during blank page startup

Status: NEW

(Firefox :: General, defect)

Description

[Joel Reymont (:joelr)]

There are over 1000 PresShell::FlushPendingNotifications during blank page startup. I don't know whether all of them do work, only 9 trigger XUL document reflows in ProcessReflowCommands. 

The method itself is quite branchy, with lots of ifs. I also wonder if some of the flushes can be optimized away. It could be that the same things are flushed over and over again and I'm curious if flushed could be queued up and duplicate or unnecessary flushes eliminated.

Comment 1

[Joel Reymont (:joelr)]

To be more precise, there are 1109 flushes of which 9 trigger XUL document reflows with root count greater than 0. 

Boris suggested to bin this by flush type, fully expecting most of these to be style flushes, assuming there will only be three bins (style/layout/paint) and that the paint bin will be empty. I'm working on this.

Comment 2

[Joel Reymont (:joelr)]

Attachment: All flushes (1109)

Call sites of all the blank page startup flushes.

Comment 3

[Joel Reymont (:joelr)]

Attachment: Flushes that trigger XUL document reflows (with flush type)

Comment 4

[Joel Reymont (:joelr)]

Boris, this is what you asked for, pending flushes by type.

./d firefox-bin flush-pending-by-type.d
dtrace -Zqw -x dynvarsize=64m -x evaltime=preinit -p 13540  -s flush-pending-by-type.d -s sigcont.d

----------------------
type 4, elapsed: 0.1458990ms
type 3, elapsed: 3.7413888ms
type 5, elapsed: 18.2747276ms
type 4, cpu    : 0.1288225ms
type 3, cpu    : 3.5871132ms
type 5, cpu    : 6.2415977ms
type 5, count  : 27 times
type 4, count  : 35 times
type 3, count  : 1046 times

Comment 5

[Joel Reymont (:joelr)]

Flush_Style            = 3, /* As above, plus flush style reresolution */
Flush_Frames           = Flush_Style,
Flush_Layout           = 4, /* As above, plus flush reflow */
Flush_Display          = 5  /* As above, plus flush painting */

Comment 6

[Joel Reymont (:joelr)]

The above is wrong, my apologies. This is from the trunk:

Flush_Style            = 3, /* As above, plus flush style reresolution */
Flush_Frames           = Flush_Style,
Flush_InterruptibleLayout = 4, /* As above, plus flush reflow, but allow it to be interrupted (so an incomplete layout may result) */
Flush_Layout           = 5, /* As above, but layout must run to completion */
Flush_Display          = 6  /* As above, plus flush painting */

Comment 7

[Joel Reymont (:joelr)]

bz: joelr: ok, so almost all Flush_Frames/Flush_Style flushes
bz: joelr: so we're seeing one style flush per menuitem
bz: joelr: (all of them no-ops, of course)
bz: er...
bz: correction
joelr: bz: how do you tell these are menu items?
bz: it's not one flush per menuitem
bz: It's N per menuitem
bz: where N is the number of parents
bz:               XUL`PresShell::FlushPendingNotifications(mozFlushType)+0x15d
bz:               XUL`nsComputedDOMStyle::GetPropertyCSSValue(nsAString_internal const&, nsIDOMCSSValue**)+0x20b
bz:               XUL`nsMenuItemIconX::GetIconURI(nsIURI**)+0x29c
bz:               XUL`nsMenuItemIconX::SetupIcon()+0x52
bz:               XUL`nsMenuX::LoadMenuItem(nsIContent*)+0x131
bz:               XUL`nsMenuX::MenuConstruct()+0xcd
bz: is the stack
bz: so I can tell they're menuitems by looking at those class names. 
bz: I think almost all of these are the menu stuff
bz: Might be worth seeing whether we can make that not flush just to cut down on the noise
bz: because there are no pending style changes
bz: joelr: in any case, there are about 60 calls here that could trigger reflow, right?
bz: joelr: the rest is just restyling stuff
joelr: bz: i don't know how to tell if the calls "could" trigger reflow
bz: joelr: might be worth seeing whether we can fix that menu thing, though
bz: joelr: by looking at the flush type
bz: joelr: and comparing to the mozFlushType enum values

Comment 8

[Boris Zbarsky [:bzbarsky]]

It's probably worth filing a separate bug on the MenuConstruct() setup, at least if MenuConstruct() takes an appreciable amount of time.

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.