Last Comment Bug 514120 - Style resolution shows up as a serious cost when wrapping DOM nodes that have no frames
: Style resolution shows up as a serious cost when wrapping DOM nodes that have...
Status: RESOLVED FIXED
: dev-doc-complete, perf
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: ---
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
Depends on:
Blocks: 467263
  Show dependency treegraph
 
Reported: 2009-09-01 20:48 PDT by Boris Zbarsky [:bz]
Modified: 2011-09-26 22:01 PDT (History)
14 users (show)
bzbarsky: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Benchmarky thing (879 bytes, text/html)
2009-09-30 14:02 PDT, Boris Zbarsky [:bz]
no flags Details
Like so (1.16 KB, patch)
2009-09-30 14:04 PDT, Boris Zbarsky [:bz]
jonas: review+
enndeakin: review+
Details | Diff | Review

Description Boris Zbarsky [:bz] 2009-09-01 20:48:21 PDT
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?
Comment 1 Boris Zbarsky [:bz] 2009-09-17 21:36:30 PDT
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.
Comment 2 Jonas Sicking (:sicking) 2009-09-18 00:03:29 PDT
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.
Comment 3 Boris Zbarsky [:bz] 2009-09-18 05:55:56 PDT
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.
Comment 4 Jonas Sicking (:sicking) 2009-09-30 09:13:11 PDT
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.
Comment 5 Boris Zbarsky [:bz] 2009-09-30 14:02:07 PDT
Created attachment 403859 [details]
Benchmarky thing

Patch coming up that speeds this up by over 2x.
Comment 6 Boris Zbarsky [:bz] 2009-09-30 14:04:02 PDT
Created attachment 403861 [details] [diff] [review]
Like so
Comment 7 Jonas Sicking (:sicking) 2009-09-30 14:41:12 PDT
Would it be faster to do a namespace check and switch the order of the tests instead?
Comment 8 Boris Zbarsky [:bz] 2009-09-30 15:38:29 PDT
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.
Comment 9 Jonas Sicking (:sicking) 2009-09-30 15:45:05 PDT
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 10 Jonas Sicking (:sicking) 2009-09-30 15:47:55 PDT
Comment on attachment 403861 [details] [diff] [review]
Like so

Anyhow, r=me on this I guess.
Comment 11 Boris Zbarsky [:bz] 2009-09-30 15:59:54 PDT
Yeah, I'll switch to a namespace check.
Comment 12 Neil Deakin 2009-10-01 06:28:18 PDT
Comment on attachment 403861 [details] [diff] [review]
Like so

I'm not a superreviewer but the patch looks ok
Comment 13 Boris Zbarsky [:bz] 2009-10-01 13:20:04 PDT
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.
Comment 14 Eric Shepherd [:sheppy] 2011-01-27 12:07:02 PST
Documented here:

https://developer.mozilla.org/en/CSS/display#XBL_bindings_and_display.3anone
Comment 15 Raha Chrost 2011-09-06 10:36:14 PDT
get the result at [url=http://google.com]big G[/url]
Comment 16 Raha Chrost 2011-09-06 10:37:49 PDT
get the result at <a href="http://google.com">big G</a> "big G":http://goog.le

Note You need to log in before you can comment on or make changes to this bug.