Bug 106667 – [V8] Slightly optimize getWrapperFast()

• Component: WebCore JavaScript
• Status: RESOLVED FIXED

[Kentaro Hara]

Slightly optimize getWrapperFast().

[Kentaro Hara]

Created attachment 182348 [details]
Patch

[Adam Barth]

Comment on attachment 182348 [details]
Patch

ok

[Kentaro Hara]

BTW, what do you think about the following idea?

- Use a class id to indicate whether we're in the main world or not. Specifically, we can use v8DOMNodeInMainWorldClassId, v8DOMObjectInMainWorldClassId, v8DOMNodeInIsolateWorldClassId, v8DOMObjectInIsolateWorldClassId. Then we can know whether we're in the main world or not by looking at a class id of info.Holder(). This will simplify a bunch of methods in DOMDataStore.h.

I don't think this will improve performance because the number of if branches doesn't change, but it will simplify the confusing if conditions in DOMDataStore.h.

[WebKit Review Bot]

Comment on attachment 182348 [details]
Patch

Clearing flags on attachment: 182348

Committed r139458: <http://trac.webkit.org/changeset/139458>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Adam Barth]

> BTW, what do you think about the following idea?

I'd rather not pack more bits into the class ID unless there's a performance gain.  The class ID is a very limited space, and we want to save it for things we really need.

Another idea is to use different ObjectTemplates for the main world and for isolated worlds.  That way we could have two implementations of nextSiblingCallback, one for the main world and one for isolated worlds.  The one for the main world could then skip the "am I in the main world" branch entirely and grab at the inline wrapper directly.

[Kentaro Hara]

(In reply to comment #6)
> > BTW, what do you think about the following idea?
> 
> I'd rather not pack more bits into the class ID unless there's a performance gain.  The class ID is a very limited space, and we want to save it for things we really need.

This isn't quite right. A class ID has 16 bit (although I am trying to reduce the size to 8 bit just right now).


> Another idea is to use different ObjectTemplates for the main world and for isolated worlds.  That way we could have two implementations of nextSiblingCallback, one for the main world and one for isolated worlds.  The one for the main world could then skip the "am I in the main world" branch entirely and grab at the inline wrapper directly.

Sounds like a better approach. (This is the approach Dan is also proposing.) My only concern is that it will complicate the generated code. Specifically, we will need to generate something like the following code for all getters/setters/methods, affecting custom bindings:


template<WorldType world>
struct firstChildAttrGetterStruct {

static v8::Handle<v8::Value> firstChildAttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
    Node* imp = V8Node::toNative(info.Holder());
    return toV8Fast<world>(imp->firstChild(), info, imp);
}

};

template<>
struct firstChildAttrGetterStruct<MainWorld>;

template<>
struct firstChildAttrGetterStruct<IsolateWorld>;

[Adam Barth]

We can also just generate two copies of the function and skip the templates.

[Kentaro Hara]

(In reply to comment #8)
> We can also just generate two copies of the function and skip the templates.

For experiment, I removed the following lines to observe the performance impact of generating getters/setters for the main world:

    static v8::Handle<v8::Object> getWrapperFast(T* object, const HolderContainer& container, Wrappable* holder)
    {
        /* if ((!DOMWrapperWorld::isolatedWorldsExist() && isMainWorldObject(object)) || holderContainsWrapper(container, holder)) { */
            if (mainWorldWrapperIsStoredInObject(object))
                return getWrapperFromObject(object);
            return mainWorldStore()->m_wrapperMap.get(object);
        /* }
        return current(container.GetIsolate())->get(object); */
    }

As a result, a micro benchmark for .firstChild improved from 14.1 ns to 13.7 ns. On the other hand, I couldn't observe any performance difference in Dromaeo/dom-traverse.

So it might not be worth doing the optimization.

[Adam Barth]

Interesting.  Thanks!