getComputedDOMStyle is too expensive

RESOLVED FIXED in mozilla0.9

Status

()

defect
RESOLVED FIXED
18 years ago
18 years ago

People

(Reporter: hyatt, Assigned: hyatt)

Tracking

Trunk
mozilla0.9
x86
Windows 2000
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

18 years ago
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.
(Assignee)

Updated

18 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9
(Assignee)

Comment 1

18 years ago
I will be adding a dump of the offending elements shortly.  
(Assignee)

Comment 2

18 years ago
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.
(Assignee)

Comment 4

18 years ago
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.
(Assignee)

Updated

18 years ago
Summary: Need a style context cache to avoid double-resolves prior to frame construction → getComputedDOMStyle is too expensive
I'm normally worried about special cases like this, but my need for speed overrules.

sr=shaver.

Comment 6

18 years ago
r=jag
(Assignee)

Comment 7

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

Comment 8

18 years ago
sr=scc
a=brendan@mozilla.org for checkin to the 0.8.1 branch -- hyatt, please get this
patch in.  Thanks,

/be
(Assignee)

Comment 10

18 years ago
Fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
Did you check this into the 0.8.1 branch too? bonsai suggests only trunk...
(Assignee)

Comment 12

18 years ago
I don't want to put this in the branch.
hyatt: you have final say, but we drivers@mozilla.org 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
(Assignee)

Comment 14

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

Updated

18 years ago
Blocks: 49141
You need to log in before you can comment on or make changes to this bug.