Closed Bug 747285 Opened 12 years ago Closed 2 years ago

Generate faster DOM getters/setters

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bzbarsky, Unassigned)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

This is a tracking bug for three task, to be done in this order:

A)  A way to hang DOM-specific metadata off property getters/setters
B)  DOM codegen to create said metadata
C)  JIT changes to make use of the metadata

Background: we need to make our new-binding getters (already much faster than our old bindings) about 2x faster still to compete with Safari.  That means reducing both the jitcode and native code involved.

What a DOM getter has to do right now is the following:

1)  Verify that the "this" object it has is the right type.  This is done by
    checking for the DOM_OBJECT JSClass flag, then checking that:

      ((DOMJSClass*)obj->getClass())->mInterfaceChain[protoIndex] ==
        protoId 

    where protoIndex and protoId are fixed for a given getter.  There are some
    complications involving checking for a proxy wrapper as |this| and
    unwrapping it and whatnot that make this slower than I would like.
2)  Extract the pointer to the native object from a reserved slot whose index is
    stored in the DOMJSClass.  This is always the same slot, actually, except
    for global objects because they have existing global object reserved slots
    and the DOM slot has to come after those.
3)  For setters only, convert the argument to the C++ type.
4)  Call the underlying C++ method.
5)  For getters only, wrap the result up to be returned to JS.

Doing #1, #2, and part of #5 (see discussion in bug 747066) in the jitcode would let us make them much faster than now, I believe.  In particular, part (A) of this bug would involve hanging the metadata needed to do #1 and #2 off getters/setters; the JIT would then invoke functions with these signatures:

  JSBool (Getter*)(JSContext* cx, void* thisObj, Value* rv);
  JSBool (Setter*)(JSContext* cx, void* thisObj, Value val);

(though we can do a Value* for the setter too if that would be easier, e.g. for rooting purposes).  The thisObj would be a pointer to the C++ object.

For #1 and #2, there has been discussion of three possible approaches.

One is to have an IC keyed on object shape, and shape of the proto that the prop was found on, as usual.  Since shape implies both the getter function identity and the JSClass pointer, the check for #1 could be done entirely at jit-compile time, the determination of the right reserved slot index could be done at jit-compile time, so all the jitcode would have to do after verifying that the IC is valid is read the slot at the baked-in index and call the baked-in function pointer.  This would likely be very fast, but would be subject to the IC blowing out due to polymorphism in the DOM.

A second is to have a more complicated IC, where we actually guard on the expression in #1 above.  That would be somewhat slower, and it's not clear that we wouldn't still need a shape guard to quickly get the actual getter function.

Finally, we could teach TI about the DOM's subtyping relationships, do something to invalidate TI info when people shadow DOM methods, and use TI to generate code that's about as fast as approach one, but without the worries about too many shapes.

In practice, a combination of approach 3 and approach 1 (for cases when people do shadow DOM stuff) seems like it would work best.

In any case, what we need is to actually make some decisions about how to handle (A) above.  That probably involves changes to some core JS data structures (Shape or JSFunction or whatever is needed).

Once that's done, I can make (B) happen, and then we can work on (C).

Ideally we could do this in the next several months.  I realize this is competing with IonMonkey stand-up, but I'm also assuming that (C) won't happen until Ion is up and running.  I would really like us to not be still blocked on (B) or worse yet (A) come July, though.  So anything we can do to make progress on (A) would be much appreciated.

I'll file separate bugs blocking this one for (A), (B), and (C).  In the end, (C) might need to be broken down still further.
Depends on: 747286
Depends on: 747287
Depends on: 747288
One other note: One of the pieces of metadata I would like to attach to DOM getters is some notion of idempotency or whatever we need to make ImageData.data, for example, CSE-able: for a given ImageData object it always returns the same result, with no side-effects.  We also have DOM getters that guarantee no side-effects (but not necessarily constant return values), and DOM getters that guarantee return values that are constant as long as no DOM method calls or setters happen, but not globally constant.
Depends on: 747289
Blocks: 747290
And one more note: if there are particular calling conventions that would most help here, we can do that on the DOM side too.  Obviously less of a problem on x86-64, for methods with only 3 arguments as here: they'd just all go in registers....
Boris, on the IonMonkey side: What benchmarks should we run to test performance improvements? In particular it'd be nice to measure ahead of time whether we can get away with fast-paths at the IR-level, or need actual ICs.
That's a good question.

This all applies to new DOM bindings, and right now not many objects are using them yet.  In particular, there's really no way to get non-monomorphic behavior right now.  There's also no decent non-microbenchmark where this will make a difference so far (though we hope to change that in the next few months).  Will that give you good enough data to answer the fast-path vs IC question?

With that caveat, right now you can test performance of the .readyState getter or the .multipart getter on the XMLHttpRequest object to measure getter overhead.  You can test the .multipart setter on the same object to measure setter overhead.  Testing method calls is a bit harder, but we don't really have as concrete a plan for them yet.
(In reply to Boris Zbarsky (:bz) from comment #4)
> This all applies to new DOM bindings, and right now not many objects are
> using them yet.  In particular, there's really no way to get non-monomorphic
> behavior right now.  There's also no decent non-microbenchmark where this
> will make a difference so far (though we hope to change that in the next few
> months).  Will that give you good enough data to answer the fast-path vs IC
> question?

Sort of - if there are old benchmarks that won't yet exercise the new DOM paths, would those be useful in determining whether we can extract monomorphic sites out of polymorphic DOM access?

IonMonkey will need to do something for generic getters/setters anyway, and maybe that's a good place to start.
Assignee: general → nicolas.b.pierron
Assignee: nicolas.b.pierron → efaust
Status: NEW → ASSIGNED
Depends on: 818912
For what it's worth, most of this seems to be done now, except the subtyping relations bit.

The bug assignee didn't login in Bugzilla in the last 7 months.
:sdetar, could you have a look please?
For more information, please visit auto_nag documentation.

Assignee: efaustbmo → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(sdetar)

This bug had no activity for 10 years and a lot of this work has been done. Let's close this.

Status: NEW → RESOLVED
Closed: 2 years ago
Flags: needinfo?(sdetar)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.