Style resolution shows up as a serious cost when wrapping DOM nodes that have no frames

RESOLVED FIXED

Status

()

Core
DOM
RESOLVED FIXED
8 years ago
6 years ago

People

(Reporter: bz, Assigned: bz)

Tracking

(Blocks: 1 bug, {dev-doc-complete, perf})

Trunk
x86
Mac OS X
dev-doc-complete, perf
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

If someone touches a node that has no primary frame from JS so that we have to js-wrap it, we end up resolving a style context in GetBindingURL (in nsDOMClassInfo.cpp).  This is not exactly cheap...

The obvious solution is that XBL2 will eventually mean we only need to check a small (empty?) set of selectors here, right?  Is there something we can do in the meantime to speed this up?
Summary: Style resolution shows up as a serious cost when wrapping DOM nodes → Style resolution shows up as a serious cost when wrapping DOM nodes that have no frames
Possible thoughts I've had so far:

1)  Restrict the check to XUL elements, on the assumption that people don't bind
    others much if they have no frame yet and we don't care if they do.
2)  Restrict the check to chrome prescontexts on similar assumptions but for
    content vs chrome.
3)  Have some sort of reduced version of style resolution that would tell us
    whether we need to "really" resolve style (or just hand back the binding URI).

All this is moot if we think we can drop the xbl1 binding mechanism in the near future, of course.
XBL2 should seriously change things here. It might create other issues, but ideally this one should go away.

In the meantime I think 1 or 2 are totally fine solutions.
Well, the real question is whether we can just drop the xbl1 style binding mechanism in Gecko 1.9.3.  If so, we shouldn't worry about those workarounds.
I don't think we can do that no. I think firefox/toolkit depends on thus behavior, and I doubt XBL2 will be able to replace it in the 1.9.3 timeframe.
Blocks: 467263
Created attachment 403859 [details]
Benchmarky thing

Patch coming up that speeds this up by over 2x.
Created attachment 403861 [details] [diff] [review]
Like so
Attachment #403861 - Flags: superreview?(enndeakin)
Attachment #403861 - Flags: review?(jonas)
Would it be faster to do a namespace check and switch the order of the tests instead?
I was considering that...  Once we nuke the primary frame map, the primary frame check will be very fast (inlined member lookup).  It probably makes sense to do a namespace check here, though.  In practice, I suspect it just doesn't matter; this function didn't show up in the profile of that testcase at all with the patch.
Sure, but why not use namespace check? If for no other reason than to ease the relanding of bug 488249.

I guess it makes sense to keep the frame thing as is though.
Comment on attachment 403861 [details] [diff] [review]
Like so

Anyhow, r=me on this I guess.
Attachment #403861 - Flags: review?(jonas) → review+
Yeah, I'll switch to a namespace check.
Comment on attachment 403861 [details] [diff] [review]
Like so

I'm not a superreviewer but the patch looks ok
Attachment #403861 - Flags: superreview?(enndeakin) → review?(enndeakin)
Attachment #403861 - Flags: review?(enndeakin) → review+
Pushed http://hg.mozilla.org/mozilla-central/rev/88f126bdf443

dev-doc-needed: we now no longer attach XBL bindings to elements in display:none subtrees when they're first accessed from JS, unless those elements are XUL.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Flags: in-testsuite?
Keywords: dev-doc-needed
Resolution: --- → FIXED
Whiteboard: [doc-waiting-1.9.3]
Documented here:

https://developer.mozilla.org/en/CSS/display#XBL_bindings_and_display.3anone
Keywords: dev-doc-needed → dev-doc-complete
Whiteboard: [doc-waiting-1.9.3]

Updated

6 years ago
Assignee: nobody → bzbarsky

Comment 15

6 years ago
get the result at [url=http://google.com]big G[/url]

Comment 16

6 years ago
get the result at <a href="http://google.com">big G</a> "big G":http://goog.le
You need to log in before you can comment on or make changes to this bug.