907369 - Speed up transparent cross-compartment wrappers

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Not sure what form this should take: better ICs, or inlining in some cases, or what...  

In particular, this is most noticeable when working with a cross-compartment wrapper for an array or typed array, because [n] access turns into a call into the proxy handler.   Right now we don't even IC that, afaik, so doing that might be a good start, with a followup for something smarter?

Please feel free to spin off separate bugs for named and indexed access here as needed, but I suspect the indexed situation is more dire in the two bugs this blocks.

Comment 1

[Eric Faust [:efaust]]

For what it's worth, after the work done in bug 875452, any ccw specific code should be really straightforward to add to the GetPropertyICs, given the ability to check easily, and with similar work now going on in bug 785467, we should soon be at that level in the SetPropertyICs. I was expecting this to be one of the constraints of the design chosen in bug 875452, so the current implentation should be very amenable to adding additional special proxy types, as long as we can tell them all apart.

To what extent do accesses to these things happen at {GET,SET}ELEM sites that isn't direct forwarding? It wouldn't be hard at all to allow the ICs to step through them if we could tell for certain that A) they were just cross-compartment wrappers and that we could easily mimic those wrapping actions in IC, and B) the class of the object being wrapped, so that we could do appropriate forwarding.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Bobby, sounds like questions for you?

No matter what, we need to do some JS_WrapValue (on the thing being set on sets and the thing being gotten on gets)... or a faster equivalent that knows to skip doing things to, e.g., integers.

Comment 3

[Bobby Holley (:bholley)]

(In reply to Eric Faust [:efaust] from comment #1)
> To what extent do accesses to these things happen at {GET,SET}ELEM sites
> that isn't direct forwarding?
> It wouldn't be hard at all to allow the ICs to
> step through them if we could tell for certain that A) they were just
> cross-compartment wrappers and that we could easily mimic those wrapping
> actions in IC, and B) the class of the object being wrapped, so that we
> could do appropriate forwarding.

For things with CrossCompartmentWrapper::singleton for a proxy handler, we don't have any interest security policies, so the only work that needs to happen is (1) wrapping all the arguments (and |this|) into the target compartment, and (2) wrapping the return value back. See the PIERCE stuff in jswrapper.cpp.

> (from IRC)
> Can we know with enough confidence to bake it into jitcode,
> the...say JSClass? of the object the CCW forwards to

Modulo wrapper recomputation, yes. And for wrapper computation, I think it's totally fine to throw away all the JIT code for a compartment.

Comment 4

[Eric Faust [:efaust]]

More information needs to be collected about what paths actually need the help.

Bobby and I spoke for about a half hour on Vidyo, as well as off and on with Boris on IRC, and people seemed more or less in agreement that we should try to get the JITs involved as little as possible. i.e. if we really care about CCW typed array element accesses, then we should do that, and maybe not worry so much about the general case.

Boris, can you take a look at writing some tests and profiling and so on, so that we can start to make a more informed decision about what our performance priorities are here?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: A bare-bones performance testcase

On this testcase, for me, a cross-compartment get on a typed array takes about 80ns.  A cross-compartment get of an object takes about 210ns.

For the typed array case, a profile shows that we spend about 5% of the time under TypedArrayObjectTemplate<int>::obj_getGeneric (which is the OOL "call the generic method" thing for typed arrays).  All the rest is wrapper overhead.  30% of the time is wrapping of the "receiver" into the target compartment of the CCW.  Another 6% or so is the "wrapping" of the jsid and the return value (which quickly bail out).  And then theres the various js::GetElement, js::Proxy::get, etc bits.

For the object case, we end up doing a bunch of ion::GetPropertyId::update, which spends 13% of its time in AutoFlushCache ctors.  That sounds like us failing to add an IC stub over and over, right?  Only 52% of the time here is under proxy_GetGeneric.  That breaks down as 22% wrapping of the return value, 8% wrapping of the receiver, 5% wrapping of the id, 9% calling the actual DirectProxyHandler::get.  Of that last, only 6% ends up under baseops::GetProperty.

So we would be able to cut the time here in half just by having a really generic proxy IC, looks like.  If we want to win more than that, we'd need ... something.  Not sure what.

xpc::WrapperFactory::PrepareForWrapping is being pretty expensive (about half of the JSObject wrap time for the return value).  I wonder whether we can speed that up somehow.

Comment 6

[Bobby Holley (:bholley)]

(In reply to Boris Zbarsky [:bz] from comment #5)
> xpc::WrapperFactory::PrepareForWrapping is being pretty expensive (about
> half of the JSObject wrap time for the return value).  I wonder whether we
> can speed that up somehow.

I have a strategy for this in bug 907937.

Comment 7

[Bobby Holley (:bholley)]

Bug 907937 gets us an 11% typedarray win an a 26% object win. See bug 907937 comment 11.

Comment 8

[Guilherme Lima]

This is fixed on Nightly, am I right?

Comment 9

[Jan de Mooij [:jandem]]

(In reply to Guilherme Lima from comment #8)

This is fixed on Nightly, am I right?

For the attached test I get similar results for all four sub tests on Nightly (~3, Ion is likely loop hoisting things and so are other engines). As expected the cross-global cases are much slower on Firefox 65.

We still have transparent cross-compartment wrappers but I don't think we're going to spend significant time optimizing them with the recent changes.