Closed Bug 514120 Opened 15 years ago Closed 15 years ago

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

Categories

(Core :: DOM: Core & HTML, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, perf)

Attachments

(2 files)

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
Attached file Benchmarky thing (deleted) —
Patch coming up that speeds this up by over 2x.
Attached patch Like so (deleted) — Splinter Review
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
Closed: 15 years ago
Flags: in-testsuite?
Keywords: dev-doc-needed
Resolution: --- → FIXED
Whiteboard: [doc-waiting-1.9.3]
Assignee: nobody → bzbarsky
get the result at [url=http://google.com]big G[/url]
get the result at <a href="http://google.com">big G</a> "big G":http://goog.le
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: