Boris Zbarsky [:bzbarsky] Reporter
	Description • 13 years ago

Right now a getprop or getelem or callprop on a proxy ends up doing all sorts of stub call stuff and the like (and in the case of getprop/callprop various propcache bits which are guaranteed fail).  It would be good if we could just skip all that and directly call into get().

There are two main options here.  Either we guard on isProxy() and then just do the virtual get() dispatch or we guard on isProxy() and the identity of the proxy handler and jump directly to the right method instead of doing a vtable lookup.

In either case we will unfortunately need to do the AutoPendingProxyOperation thing, I think.

	David Anderson [:dvander] - inactive, e-mail if emergency
	Comment 2 • 13 years ago

Proof of concept. This was a little trickier than I expected because I forgot we have to idify the value?

It speeds up my test case by about 8-10% if I use a native like shapeOf, with a scripted function it gets about 1% slower. No idea what it will do for actual proxies.

	David Anderson [:dvander] - inactive, e-mail if emergency
	Comment 3 • 13 years ago

Some actual tests are needed, like throwing an exception and gc()'ing from inside the handler.

	Andreas Gal :gal
	Comment 4 • 13 years ago

We can call into a wrapper function that does the idify stuff. Can you wire up the set path too? Should be identical mostly.

	David Anderson [:dvander] - inactive, e-mail if emergency
	Comment 5 • 13 years ago

Yeah, that's what I did, though it's kind of hard to specialize further with that around.

I'll do the set path if it turns out this actually does anything :)

	Andreas Gal :gal
	Comment 6 • 13 years ago

Awesome. In case of a scripted proxy we do a lookup inside the ::get code. If I want to make that fast (say for a self-hosted DOM), would you want to do that in your generated code or should I add a specific PIC to the proxy code (caching the shape and functions in the handler)?

	Andreas Gal :gal
	Comment 7 • 13 years ago

And how much slower is the ExternalInvoke I use in the C++ code vs the JIT directly seeing the function call (i.e. in code you emit for a PIC).

	Boris Zbarsky [:bzbarsky] Reporter
	Comment 8 • 13 years ago

So I tried that patch.  As-is, it doesn't speed up nodelists much.  The time that used to be spent in js::proxy_GetProperty is almost exactly replaced by time spent in js::Proxy::get (which is not surprising at all, given how the former is implemented).  The time spent in stubs::GetElem is halfway replaced by time in ProxyGetWrapper, and the time spent in jit-generated code goes up a bit.  Overall, it's a 5ms win or so on my 180ms baseline (100ms baseline if I cache the length at loop start).

Then again, I think we can work on more aggressively specializing (e.g. to the right proxy handler, to int-that-fits-in-id, and so forth).

dvander, the patch not setting hasProxyStub anywhere is just a bug, right?

	David Anderson [:dvander] - inactive, e-mail if emergency
	Comment 9 • 13 years ago

Yeah, I think we'll need to specialize further since just getting rid of the stub call isn't gonna buy a whole lot.

Whoops, yeah I forgot to set hasProxyStub. Another bug, if you tested this on x64, is that it'll push like 6 registers before making the call. That's not really necessary.

	Boris Zbarsky [:bzbarsky] Reporter
	Comment 10 • 13 years ago

I did test on x64, yes.

	David Anderson [:dvander] - inactive, e-mail if emergency
	Comment 11 • 13 years ago

This spills less registers and fixes the hasProxyStub bug. We could specialize further but it would be nice to not have to do the interning... is it possible to have two callbacks, one for ints and one for strings, and not requiring that strings be atomized? If not, we could try getting rid of the wrapper and inserting a fast path instead.

	Andreas Gal :gal
	Comment 12 • 13 years ago

Two wrappers sounds good.

	Tom S [:evilpie]
	Comment 13 • 13 years ago

Why aren't we guarding on the handler shape, and call the "get" trap directly? I guess this would nicely extend to the getter ic, too. We would need to make sure we always pass a string to the hanlder. But i guess there is something in the spec that makes this hard, dunno.

	Boris Zbarsky [:bzbarsky] Reporter
	Comment 14 • 13 years ago

> Why aren't we guarding on the handler shape, and call the "get" trap directly?

The handler in this case is a C++ handler; it has no shape.  We can guard on its pointer identity and call the get directly, though.  Which is the general plan; the attached is just an outline of how to work on getting there.

	David Anderson [:dvander] - inactive, e-mail if emergency
	Comment 15 • 13 years ago

And I think we won't gain much if we have to go through a wrapper, we really want to get rid of the id-ification path.

	Andreas Gal :gal
	Comment 16 • 13 years ago

Why don't you id-ifiy on the hot path if an integer fits into a jsid?

	Andreas Gal :gal
	Comment 17 • 13 years ago

(and also if its GETPROP you already have the atom, no idification there either)

	David Anderson [:dvander] - inactive, e-mail if emergency
	Comment 18 • 13 years ago

What sort of keys are we trying to optimize here, int32 or string? If it's int32 then yeah having a fast path seems fine.

	Andreas Gal :gal
	Comment 19 • 13 years ago

The common cases we want to be fast are

[integer]

and

.atom

Both should be fast.

	Boris Zbarsky [:bzbarsky] Reporter
	Comment 20 • 13 years ago

Right.  We want GetElem fast for int and GetProp fast for strings.

	David Anderson [:dvander] - inactive, e-mail if emergency
	Comment 21 • 13 years ago

In this version, idify is inlined and the wrapper takes a jsid directly.

	Andreas Gal :gal
	Comment 22 • 13 years ago

atoms, not strings, the stuff is already atomized for getprop

	Boris Zbarsky [:bzbarsky] Reporter
	Comment 23 • 13 years ago

Comment on attachment 526119 [details] [diff] [review]
getelem IC for proxies, int32 keys only

This does speed up index access some; gets us faster than Safari, looks like.

But it's broken somehow.  This function that I have that recursively walks the DOM tree:

  function visitDOM(d) {
    var c = d.childNodes;
    for (var i = 0; i < c.length; ++i) {
      visitDOM(c[i]);
    }
  }

ends up throwing due to |d| being undefined.  If I test c[i] before making the recursive call, it's never undefined.  If I assign c[i] to a var and then pass the var to the recursive call everything works.  I'm guessing we leave state in a register somewhere and it gets clobbered...

	Boris Zbarsky [:bzbarsky] Reporter
	Comment 24 • 13 years ago

One thought I had earlier today:  The ideal way to compile proxy access for purposes of the DOM is this:

1)  Do some sort of fast "own property" get on the proxy.  The contract for this is that it does NOT walk the proto chain and has a way to report when the proxy itself had no prop for this.

2)  If #1 reported no prop, do a normal get (complete with normal ICs and so forth) on the proto.

That would let us take out our poor-man's attempt at ICs that we have in the DOM proxy handlers right now, and should work way better.  A wrinkle is that some proxies have "own" props that don't shadow proto props.  If we can have _that_ (so have the jitcode decide whether to ask the proto first or not) handled automatically, that would be even faster, since the common cases where stuff is going to be on the proto would just skip #1 above altogether.

	Boris Zbarsky [:bzbarsky] Reporter
	Comment 25 • 12 years ago

djvj, when you get bored with other DOM stuff, this would be pretty useful!

	Kannan Vijayan [:djvj]
	Comment 26 • 11 years ago

Took wrong bug.

	Tom S [:evilpie]
	Comment 27 • 9 years ago

This probably only works on x64, because of register use. For some reason this is not faster or even slower for me, when testing with a ScriptedDirectProxy. Might be interesting to test DOM proxies.