Closed Bug 69142 Opened 19 years ago Closed 19 years ago

Setting attributes on menus thrashes in RecreateFrameForContent on MacOS

Categories

(Core :: CSS Parsing and Computation, defect, major)

PowerPC
Mac System 9.x
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla0.9

People

(Reporter: mikepinkerton, Assigned: hyatt)

References

Details

(Keywords: perf)

Attachments

(1 file)

As part of profiling window activation (when a window comes to the fg), i noticed 
that 50% of activation processing was spent in RecreateFrameForContent() after 
setting attributes on content nodes representing menu items.

Recall that on the mac, menus on the menubar are native and have a rule that 
makes them display:none. This won't ever change, yet the style system, upon 
seeing the attributeChanged message, tries its hardest to squeeze out a frame, 
wastes lots of time, and eventually ends up back at the start w/out a frame.

50% of window activate translates into 25% of window refresh time (the time from 
when you click in a bg window until the paint completes updating all the areas 
damaged from bringing the window to the front). This is very noticable on mac 
(flashing) and contributes to it feeling sluggish. 

ideas?
Hmm. I wonder if we can optimize the computation of a style change's impact such
that display:none causes nothing to happen. Sounds like it is worth looking into.

So, what attribute is changing? Is there some style rule that has an attribute
selector with that attribute? If not, I wonder why shaver's optimization isn't
kicking in (the optimization to skip attribute changed notification reresolves
if there is not rule based on the attrib).
there are rules based on that attribute for _some_ menus, just not those on the 
menubar. i think shaver's optimizations fall by the wayside when anyone, 
anywhere, uses a rule with that attribute. what say you, shaver?

I'm guessing the attribute is the "disabled" attribute, since we're updating 
menu/command state when bringing the window to the fg. 
Blocks: 48274
I have a patch that takes this off the map.  Basically, I looked at the code and
you can't stop RecreateFramesForContent from being called.  It's correct that
the code calls it.

The only reason the method is slow in the case where you have no parent frame is
that ContentInserted is stupid.  I have a simple patch to ContentInserted that
makes it much smarter about not doing any work when the parent content node has
no frame.  This takes RecreateFramesForContent off the map on my Quantify runs.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9
Looking for r and sr.
Patch looks good to me r/sr=attinasi
Make a note about why the container might be null (e.g., ``this happens if we're
inserting into an element that is currently mapped `display: none', for
example''). Do that, sr=waterson
Fix was checked in ages ago.  Not sure how this bug stayed open.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.