72345 - getComputedDOMStyle is too expensive

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[David Hyatt]

See my upcoming post to n.p.m.performance.  9% of new navigator window time is 
spent resolving style on objects that *will* have frames (but that don't at the 
time the style resolution happens to determine the binding).

We need the capability to cache resolved style contexts from 
GetComputedDOMStyle in a cache on the presshell.  Frame construction can then 
check this cache to avoid doing a double-resolve.

Comment 1

[David Hyatt]

I will be adding a dump of the offending elements shortly.  

Comment 2

[David Hyatt]

Ok, with some clever limiting on my end, I cut it from 9% to 5%, but the
remaining 5% all have bindings that must be loaded (and thus are a legitimate
problem).

Attaching the patch to cut the calls to GetComputedDOMStyle from 55 down to 17.

I notice that 3 of the 17 calls are from nsGenericElement (in other words,
non-XUL elements).  I'll investigate that next.

Comment 3

[David Hyatt]

Attachment: Patch to limit elements that can have bindings checked in the "no-frame" case.

Comment 4

[David Hyatt]

Ok, the three HTML elements are: HTML, HTML, and INPUT.  The two <html> tags are
from about:blank.  There are two because about:blank is loaded twice (the source
of another bug).  The reason <html> has the computed DOM style check done is the
scrollbars.  The scrollbar is parented to the <html> element, and the creation
of the scrollbar script object forces the creation of the <html> script object,
but no frame is in the map for it yet.

Stopping about:blank from having scrollbars (another bug I've filed) will fix this.

The input element is in the XUL document, and it needs bindings, so it's ok.

I'm going to work now on eliminating scrollbars from about:blank and on stopping
the double load.  The time from this should go down even further.

Comment 5

[Mike Shaver (:shaver emeritus)]

I'm normally worried about special cases like this, but my need for speed overrules.

sr=shaver.

Comment 6

[Peter "jag" Annema]

r=jag

Comment 7

[David Hyatt]

With the about:blank patches, I'm down to 13, and 4%, for a 5% speedup.  All right!

Comment 8

[Scott Collins]

sr=scc

Comment 9

[Brendan Eich [:brendan]]

a=[e-mail] for checkin to the 0.8.1 branch -- hyatt, please get this
patch in.  Thanks,

/be

Comment 10

[David Hyatt]

Fixed.

Comment 11

[Peter "jag" Annema]

Did you check this into the 0.8.1 branch too? bonsai suggests only trunk...

Comment 12

[David Hyatt]

I don't want to put this in the branch.

Comment 13

[Brendan Eich [:brendan]]

hyatt: you have final say, but we [e-mail] thought this was a 0.8.1
candidate.  No worries, and please do say more about why you think this should
not go on the branch.

/be

Comment 14

[David Hyatt]

There is a risk of regressions... bindings not being installed.  I want to see
it shake out first.