so that seemingly innocent line of code eats up ~100ms on startup due to being called often(~50times, >2ms each) and invoking a ton of XPConnect junk via http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/browser.xml#314. slow native calls triggered by this line: DOMWindow, interfaces*2, QueryInterface, getInterface

http://mxr.mozilla.org/mobile-browser/source/chrome/content/browser.js#1082 is the line, but other users of contentWindow are affected too

also webNavigation property is a 70ms hit, there are probably similar issues with caching it

looks like webNavigation calling is mostly happening through currentURI

Comment on attachment 354970 [details] [diff] [review] cache contentWindow Can't cache this, since it will change after each load. A more direct way to get the content window from the docshell would likely help, even if it just moved most of this code to C++ and perhaps cached the return value.

(In reply to comment #5) > (From update of attachment 354970 [details] [diff] [review]) > Can't cache this, since it will change after each load. A more direct way to > get the content window from the docshell would likely help, even if it just > moved most of this code to C++ and perhaps cached the return value. Boris, Gavin says you are the person to ask about getting access to these in a more direct manner.

Perhaps these methods could use the new quick stubs?

So... the window doesn't change after each load. It has the same lifetime as the docshell. As far as a more direct getter, I'm not quite sure how much more direct you want. There are two different interfaces that docshell implements, both of which hand out an nsIDOMWindow with a single method call (nsIInterfaceRequestor and nsIWebNavigation). So the only cost you could eliminate here is the QI to the relevant interface, right? But no one's forcing you to cache just the nsIDocShell interface pointer in browser.xml.... Could cache others, if they'd be useful. Certainly the webNavigation property could be cached, just like the window. Just make sure to clear all the relevant caches on frameloader swap. Also, if desired you could certainly cache all the relevant interface constants so you don't keep looking up Components.interfaces and then the relevant interface constant. But I suspect that just caching the objects is simpler and faster. As far as quickstubs go, those could indeed happen here, with some work. Jason's the man to talk to about that. I'm a little surprised that it takes 2ms to execute this line, though, even with all the XPConnect goop. Or is that the fennec timing, not desktop?

(In reply to comment #8) > So... the window doesn't change after each load. It has the same lifetime as > the docshell. > > As far as a more direct getter, I'm not quite sure how much more direct you > want. There are two different interfaces that docshell implements, both of > which hand out an nsIDOMWindow with a single method call (nsIInterfaceRequestor > and nsIWebNavigation). So the only cost you could eliminate here is the QI to > the relevant interface, right? But no one's forcing you to cache just the > nsIDocShell interface pointer in browser.xml.... Could cache others, if they'd > be useful. Certainly the webNavigation property could be cached, just like the > window. Just make sure to clear all the relevant caches on frameloader swap. > > Also, if desired you could certainly cache all the relevant interface constants > so you don't keep looking up Components.interfaces and then the relevant > interface constant. But I suspect that just caching the objects is simpler and > faster. > > As far as quickstubs go, those could indeed happen here, with some work. > Jason's the man to talk to about that. From what you said, caching the object sounds like the way to go > > I'm a little surprised that it takes 2ms to execute this line, though, even > with all the XPConnect goop. Or is that the fennec timing, not desktop? That's fennec timing on the device, it's approx 100x slower than desktop in general.

Taras - since Boris says caching the contentWindow and webNavigation are OK (I would have guessed contentWindow was a no-no too), then how about a patch that adds a cache for webNavigation too? Since currentURI hits it a lot. Just remember to NULL out the caches in swapDocShells, probably at the end of the function.

Or you could just add them to fieldsToSwap.

Comment on attachment 355163 [details] [diff] [review] browser.xml opt 1/2 We can't rely on the browser-defined Cc/Ci/Cr constants from a toolkit file. We could use fields for these, but I'm not sure that's worth it.

Comment on attachment 355164 [details] [diff] [review] cache contentWindow and webNavigation 2/2 Hmm, I thought you'd need to clear these out in destroy() to avoid potentially troublesome circular references (at least until bug 372107 is fixed?), but it appears that we don't already do that for _docShell. Perhaps we should?

(In reply to comment #14) > (From update of attachment 355163 [details] [diff] [review]) > We can't rely on the browser-defined Cc/Ci/Cr constants from a toolkit file. We > could use fields for these, but I'm not sure that's worth it. It is, most of the interfaces calls come from browser.xml. That costs >50ms of startup for no good reason. Fields will have to do.

(In reply to comment #15) > (From update of attachment 355164 [details] [diff] [review]) > Hmm, I thought you'd need to clear these out in destroy() to avoid potentially > troublesome circular references (at least until bug 372107 is fixed?), but it > appears that we don't already do that for _docShell. Perhaps we should? Ok, talked to bz about this, and he pointed out that removal of the <xul:browser> should kill the docshell anyways, and his changes to XBL teardown and field handling should should have us covered otherwise (and we can probably investigate removing at least part of destroy()).

resubmitting without Ci to avoid any confusion.

Pushed 599fc0933fd8