Closed Bug 533639 Opened 15 years ago Closed 13 years ago

Consider better jit codegen for DOM bindings (dromaeo)

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED WONTFIX
Tracking Status
blocking2.0 --- -
status2.0 --- wanted

People

(Reporter: bzbarsky, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(6 files, 1 obsolete file)

We (me, Peter, Blake) were looking at some dromaeo profiles today, and we seem to be spending enough time on wrapping/unwrapping even now that there's no way we can get to chrome's performance on those tests without some radical change. So a halfway-radical change proposal: bake the QI result into the trace. More precisely, since we guard on shape before making the quickstub call (we do, right?) and since shape includes JSClass (right?) we are in fact guaranteed that if we had a certain DOM class before we still have the same one. At least for nodes. Not quite as sure about nodelists and such, given our classinfo-name stuff, but I think it's true there too. Given that, the idea is to bake into trace the offset from nsISupports* to the interface the quickstub actually wants. Then we need "only" check whether we have a security wrapper, and if not get our nsISupports* and add the offset. This works as long as the interface can be reached via the element's offset table. Andreas' suggestion was that the trace allocate a 4-word scratch area when it traces a quickstub call, and pass the pointer to the call. We can then stash the offset there the first time we call the quickstub, if the interface is reached via the offset table, and get it from there after that. Thoughts? Is there some way we can make this even thinner somehow?
Assignee: nobody → general
Component: DOM → JavaScript Engine
QA Contact: general → general
Can you post the shark profile as ascii?? just so we know what callstack we talk about.
Attached file Microbenchmark that's 57% unwrapping (deleted) —
This speeds up the second microbenchmark some; in particular unwrapping drops to 28%. That's a 40% speedup on the testcase overall; a 70% speedup of the unwrapping itself. I also tried putting the pic use before the security wrapper thing, and that moved unwrapping down to 24%. So a 44% overall speedup; a 77% speedup of the unwrapping. If we're ok doing the pic check before the unwrap, I bet we can win even bigger by simply hoisting it out of the unwrap function altogether (and therefore saving the function call overhead). Remaining issues: 1) Decide what to do with that security stuff. 2) Figure out how to make getters also work; we can't use the traceable native stuff there. 3) Figure out whether we can enter this code sometimes with slimwrappers and sometimes with XPCNativeWrapper on the same trace, and if so how we can handle the situation. I suspect it cam, in fact.
For issue #3, which we are in fact hitting, and crashing as a result, can we just cheat and if the nsISupports has the vtable pointer to the XPCNativeWrapper vtable grab it's mIdentity? Or something? Issue 4: 4) In a debug build, we die trying to call getElementsByTagName: 0x003b5f42 in JS_Assert (s=0x463e94 "prefixc <= 3", file=0x4605a8 "/Users/bzbarsky/mozilla/tracemonkey/mozilla/js/src/jstracer.cpp", ln=10566) at /Users/bzbarsky/mozilla/tracemonkey/mozilla/js/src/jsutil.cpp:71 The relevant part is: p *trcinfo->specializations $6 = { builtin = 0x118ed6f8, prefix = 0x118e3d55 "IfTC", argtypes = 0x118da664 "s", flags = 9 } Is there something that actually fails for the case of prefixc == 4, or do we just need to bump the assert?
mrbkap pointed out that for method calls we don't guard on the shape of |this| or anything (in particular not if coming through JSOP_CALL). So I'm going to back out the method part of this for now, and make the property part, where we do have shape guards, work.
Just put the shape into the PIC.
The assert seems bogus.
(In reply to comment #9) > mrbkap pointed out that for method calls we don't guard on the shape of |this| > or anything (in particular not if coming through JSOP_CALL). It's there, under TR::record_JSOP_CALLPROP -> test_property_cache etc. The shape of the receiver guards the method (function-valued property) value. This is a big and important optimization, you can count on it. /be
> Just put the shape into the PIC. Good idea. I tested putting the "is this a security wrapper?" check in the pic, and it didn't seem to help that much. Still doing some profiling to see what else is up. Peterv is worried that we might have objects with different vtable layouts sharing classinfo... Not quite sure what to do about that yet. :(
What does help a lot is inlining the pic codepath and saving the call overhead. On the second microbenchmark in this bug, that drops the runtime to about 30ms (from 68 initially). That's compared to 42ms without inlining. Now the caveat is that it's not exactly a tiny bit of code to inline...
Still need a solution for getters/setters. Andreas thinks we should just use a member of the JSContext as a secret handshake. Brendan didn't sound happy with it. And of course still need to sort out the offset table situation.
Attachment #416734 - Attachment is obsolete: true
blocking2.0: --- → ?
Depends on: 397177
blocking2.0: ? → beta1
This probably needs to be remeasured with peterv's new fast paths.
Blocks: domperf
blocking2.0: beta1+ → final+
blocking2.0: final+ → -
status2.0: --- → wanted
bz, is this bug still relevant or has DOM perf improvement moved on to other bugs?
That's a good question. I _think_ we have other plans for doing this now, and the approach from this bug is not going to get used.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: