512584 - super fast paths for Components.classes and Components.interfaces

Status: RESOLVED WONTFIX

(Core :: XPConnect, defect, P1)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

We should have support from xpconnect and JS for doing some kind of super fast path transformation of some constructs, for example:

On code that is marked as chrome (that is, has access to xpconnect):

Components.classes["foo"] -- turn into a single GetComponentByClasse(const char *foo) call
Components.interfaces.nsIFoo -- turn into basically a static value (or whatever we use to represent interfaces)
Components.classes["foo"].getService(Components.interfaces.nsIFoo) ->  single GetComponentByClass() call with the right QI on the back end

etc.

These are super common constructs, and we pay lots of xpconnect overhead (accessors off Components, custom indexers, etc.).

For historical reasons, whatever solution should support

var Cc = Components.classes;
var Ci = Components.interfaces;

with the same fast paths supported for Cc and Ci (or whatever variables are used).

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

So basically, we want to quickstub Components?

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

More than quickstubs; I think this would need something more specific than just our current quickstub infrastructure.

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

That is -- quickstubs could fast path Components.classes["foo"] (can it? it could classes.getClass("foo") but can it do the property access?) but it currently can't transform Components.classes["foo"].getService(Components.interfaces.nsIFoo) into a single quickstub'd call, I don't think.  (or .createInstance, etc.)

Comment 4

[Brendan Eich [:brendan]]

Transforming expressions is not something a code-calling bridge can do. Nor can the JS compiler in general (we don't analyze names that carefully, and the global object is hard to impossible to analyze).

Why have we tolerated such long-winded boilerplate for so long?

Better to do the source-to-source change once and hg commit it. We can get static analysis help at scale, I bet.

/be

Comment 5

[Brendan Eich [:brendan]]

The source-to-source idea would be to make some built-ins, with sweet names and convenient APIs, and optimize those. We could even use the #builtins#... jazz that self-hosted JS for TraceMonkey is starting to use, if we could convince ourselves it was safe to expose to chrome.

/be

Comment 6

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Source-to-source could work, though any convenient name may well end up with a namespace collision.  It is horrible boilerplate though.  Should talk about it tomorrow/thursday... one option might be Components.getService("foo", Components.interfaces.nsIFoo), but I don't know how to get rid of Components.interfaces (other than putting nsI* as globals).

For the compiler idea -- can we just set some flag when we create the JS context, and have the compiler make certain assumptions?  (e.g. when it sees a "Components" name, just assume it will always be xpconnect, and act accordingly?)  That would make it hard to catch Cc and Ci, though, hmm.

Comment 7

[(dormant account)]

Can we not make Ci, Cc, etc readonly native properties on the Chrome globalobject?

Existing sources could be modified to remove Ci, Cc, etc via jshydra

Comment 8

[Benjamin Smedberg]

Is the problem here the xpconnect overhead of accessing these objects? A while back I looked at implementing these objects natively, instead of using xpconnect, and it doesn't look especially hard to do... would that solve the problem (would it trace more effectively than using xpconnect)?

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

That's basically what I was suggesting in comment 1... We do have to be a little careful because we expose Components.classes to content but actually allow access (we throw a security exception from XPCWrappedNative::CallMethod).

Comment 10

[Dietrich Ayala (:dietrich)]

is the work in bug 516085 going to be usable for this bug as well?

blake, are there bugs for the issues that block resolving this issue? is there someone on the js team who can work on it?

taras says general win from this -> p1.

Comment 11

[(dormant account)]

Seeing that nobody other than Blake knows what is going on I am going to experiment for now.

It looks like it should be possible to define Components.*, Components.classes and Components.interfaces as a pure js. I'm pretty sure nobody uses Components.QueryInterface.

Comment 12

[(dormant account)]

Attachment: pure-js Components

Here is my first stab. This replaces the Components object with a js one + C++ lookup. This is still missing isSuccessCode() a couple of other pointless utility functions.

next step will be to make .interfaces and .classes native and then i can look at making .nsIFoo/etc xpcom component accessors cheap.

I wonder if we can share a single Components* object across chrome. It may be useful for caching services/etc on js side, but I am not there yet.

Comment 13

[(dormant account)]

Attachment: Components.interfaces & Components.classes are fast now

It turned out to be easy to finish up the rest of major stuff. Posting it here while I do some measurements on n810.

Comment 14

[(dormant account)]

This seems to save about 50ms on fennec startup. Doing something clever to avoid overhead on Components.interfaces/classes.* should bring that up to at least 100ms.

Comment 15

[(dormant account)]

So more data.

With this patch, Components.utils/classes. now amount for 45ms of startup. vs somewhere > 100ms before.

Components.interfaces.* cost 231ms, and components.classes.* cost another 54ms.

Total is 382ms for normal fennec startup. This is for all members of Components. and Components.interfaces/utils.

On top of this there would be cost of Components.Constructor, etc.


This is all a pretty heavy penalty to pay for something that should be free.

Comment 16

[(dormant account)]

OK, I give up. Optimizing the existing approach is a dead-end.

We could redefine what Components.(interfaces/classes).* mean. Returning an xpconnected object only to have it passed in as an argument into getService or something is insanity in terms of performance.

I think getting rid of pointless xpconnect should save us around 10% in warm startup speed on desktop(50ms) or 500ms on mobile.

In order to continue this I'll need someone who is experienced with the Components* API so we can draft up a new/equivalent api.

Comment 17

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

mrbkap, can you help here?

Comment 18

[(dormant account)]

Talked with mrbkap about this. The plan of attack is
a) Convert Components* to pure JSNative objects with some stuff reimplented as native callbacks(no xpconnect).
b) CID and IID objects should become pure js objects of some sort.
c) xpconnect can be taught to convert those into what CID/IID-expecting functions expect.

I think once that is done, the next thing to do will be to move on to switching heavily used xpconnect calls to jsctype ones.

Comment 19

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to comment #18)
> Talked with mrbkap about this. The plan of attack is
> a) Convert Components* to pure JSNative objects with some stuff reimplented as
> native callbacks(no xpconnect).
> b) CID and IID objects should become pure js objects of some sort.
> c) xpconnect can be taught to convert those into what CID/IID-expecting
> functions expect.

Sounds good.  Quickstubs should be taught how to convert those as well.

> I think once that is done, the next thing to do will be to move on to switching
> heavily used xpconnect calls to jsctype ones.

Hm, heavily used xpconnect calls meaning calls that go across xpconnect, or calls into xpconnect itself?  If the former, isn't this just quickstubs?  If the latter, I'm not sure how jsctypes helps (as opposed to just setting up a js native function).

Comment 20

[(dormant account)]

Attachment: mostly native Components.interfaces

this patch 90% works, brings up a browser. Main thing that still files is that js code that does
 if (fo instanceof Ci.someinterface)
always returns false.

I couldnt get rid of nsJSIID sillyness because xpconnect parameter conversion stuff is too voodoo for me. But i did make it so nsJSIIDs are created lazily so declaring stuff like

myconstant = Ci.nsIFoo is basically free until someone passes myconstant into xpconnect.

Comment 21

[(dormant account)]

I combined the two patches and sent them off to the try server. Results are discouraging. Ts is 1-5% faster, but that doesn't seem to be worth the hassle of deCOMtaminating Components.*

The only remotely sensible thing to do would be to do what the name of the bug suggests and add fast Cc.* and Ci.* globals to chrome scripts. I'm not sure if that's worth complicating our code for.

Comment 22

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

I think it's worth it -- every percentage point helps, and this is code that's used all across the browser, not just startup.  A measurable 1-5% faster Ts win is pretty awesome, especially since it's on top of a lot of the "easy win" Ts work that has already been done!

Comment 23

[Damon Sicore (:damons)]

(In reply to comment #21)
> I combined the two patches and sent them off to the try server. Results are
> discouraging. Ts is 1-5% faster, but that doesn't seem to be worth the hassle
> of deCOMtaminating Components.*
> 
> The only remotely sensible thing to do would be to do what the name of the bug
> suggests and add fast Cc.* and Ci.* globals to chrome scripts. I'm not sure if
> that's worth complicating our code for.

Taras, really?  1-5% faster seems worth it to me.  Can we drive this home?

Comment 24

[(dormant account)]

Attachment: combined patch

Comment 25

[(dormant account)]

Attachment: combined patch

This passes xpcshell tests here. Going to wait on try-server and places branch to confirm wins before embarking on finalizing these changes.

Comment 26

[Dietrich Ayala (:dietrich)]

result summary from landing on the Places branch. win is xp. didn't get multiple test runs, so this is comparing a single datapoint for each test. (though, for things like Ts, it's median of 10 runs, for example).

http://spreadsheets.google.com/ccc?key=0AjIcGNW1bFV5dGNLaVhtYlpKeVR5NXMyUGVoS2p2SFE&hl=en

Linux Ts log: http://tinderbox.mozilla.org/showlog.cgi?log=Places/1269987934.1269991379.19265.gz&fulltext=1
Mac Ts log: http://tinderbox.mozilla.org/showlog.cgi?log=Places/1269995479.1269999868.9655.gz&fulltext=1
WinXP Ts log: http://tinderbox.mozilla.org/showlog.cgi?log=Places/1269992772.1269995464.30821.gz&fulltext=1

Comment 27

[Johnathan Nightingale [:johnath]]

It looks like this sort of went to sleep - taras - can we make it go?

Comment 28

[(dormant account)]

(In reply to comment #27)
> It looks like this sort of went to sleep - taras - can we make it go?

I don't think so. This is a pretty delicate retrofit with hard-to-measure gains in overall startup. I think a more effective use of time would be to move on switching chome over to xpconnect-less api variety ala bug 563742.

Comment 29

[(dormant account)]

See comment 28