Changeset 107367 in webkit


Ignore:
Timestamp:
Feb 9, 2012 11:09:50 PM (12 years ago)
Author:
abarth@webkit.org
Message:

Dromaeo/dom-traverse.html should go fast
​https://bugs.webkit.org/show_bug.cgi?id=78307

Reviewed by Eric Seidel.

This patch improves Dromaeo/dom-traverse.html by roughly 2.5% by
removing a branch. Previously, we null-checked the result of
V8DOMWrapper::getWrapper in a hot code path, but the only case where we
return a non-empty wrapper comes from a cold code path. By pushing the
null check into the cold codepath, we eliminate the branch from the
hot code path.

This patch also annotates the branches in the hot code path with their
likely outcome. I didn't measure a statistically significant
improvement with that aspect of the change, but it seems worthwhile.

  • bindings/scripts/CodeGeneratorV8.pm:

(GenerateHeader):

  • bindings/v8/V8DOMWrapper.cpp:

(WebCore::getExistingWrapperInline):
(WebCore):
(WebCore::V8DOMWrapper::getExistingWrapperSlow):
(WebCore::V8DOMWrapper::getWrapperSlow):

  • bindings/v8/V8DOMWrapper.h:

(WebCore::V8DOMWrapper::getExistingWrapper):
(V8DOMWrapper):
(WebCore::V8DOMWrapper::getWrapper):

  • bindings/v8/custom/V8NodeCustom.cpp:

(WebCore::toV8Slow):

Location:
trunk/Source/WebCore
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r107366 r107367  
     12012-02-09  Adam Barth  <abarth@webkit.org>
     2
     3        Dromaeo/dom-traverse.html should go fast
     4        https://bugs.webkit.org/show_bug.cgi?id=78307
     5
     6        Reviewed by Eric Seidel.
     7
     8        This patch improves Dromaeo/dom-traverse.html by roughly 2.5% by
     9        removing a branch.  Previously, we null-checked the result of
     10        V8DOMWrapper::getWrapper in a hot code path, but the only case where we
     11        return a non-empty wrapper comes from a cold code path.  By pushing the
     12        null check into the cold codepath, we eliminate the branch from the
     13        hot code path.
     14
     15        This patch also annotates the branches in the hot code path with their
     16        likely outcome.  I didn't measure a statistically significant
     17        improvement with that aspect of the change, but it seems worthwhile.
     18
     19        * bindings/scripts/CodeGeneratorV8.pm:
     20        (GenerateHeader):
     21        * bindings/v8/V8DOMWrapper.cpp:
     22        (WebCore::getExistingWrapperInline):
     23        (WebCore):
     24        (WebCore::V8DOMWrapper::getExistingWrapperSlow):
     25        (WebCore::V8DOMWrapper::getWrapperSlow):
     26        * bindings/v8/V8DOMWrapper.h:
     27        (WebCore::V8DOMWrapper::getExistingWrapper):
     28        (V8DOMWrapper):
     29        (WebCore::V8DOMWrapper::getWrapper):
     30        * bindings/v8/custom/V8NodeCustom.cpp:
     31        (WebCore::toV8Slow):
     32
    1332012-02-09  Emil A Eklund  <eae@chromium.org>
    234

  • trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm

    r107314 r107367  
    428428{
    429429END
    430     my $getWrapper = IsNodeSubType($dataNode) ? "V8DOMWrapper::getWrapper(impl)" : "${domMapFunction}.get(impl)";
     430    my $getWrapper = IsNodeSubType($dataNode) ? "V8DOMWrapper::getExistingWrapper(impl)" : "${domMapFunction}.get(impl)";
    431431    push(@headerContent, <<END);
    432432    return ${getWrapper};
    … …  
    470470inline v8::Handle<v8::Value> toV8(Node* impl, bool forceNewObject = false)
    471471{
    472     if (!impl)
     472    if (UNLIKELY(!impl))
    473473        return v8::Null();
    474     if (!forceNewObject) {
    475         v8::Handle<v8::Value> wrapper = V8DOMWrapper::getWrapper(impl);
    476         if (!wrapper.IsEmpty())
    477             return wrapper;
    478     }
    479     return toV8Slow(impl, forceNewObject);
     474    if (UNLIKELY(forceNewObject))
     475        return toV8Slow(impl, forceNewObject);
     476    return V8DOMWrapper::getWrapper(impl);
    480477}
    481478END

  • trunk/Source/WebCore/bindings/v8/V8DOMWrapper.cpp

    r106698 r107367  
    7373typedef HashMap<void*, v8::Object*> DOMObjectMap;
    7474
     75static ALWAYS_INLINE v8::Handle<v8::Object> getExistingWrapperInline(Node* node)
     76{
     77    V8IsolatedContext* context = V8IsolatedContext::getEntered();
     78    if (LIKELY(!context)) {
     79        v8::Persistent<v8::Object>* wrapper = node->wrapper();
     80        if (!wrapper)
     81            return v8::Handle<v8::Object>();
     82        return *wrapper;
     83    }
     84    DOMDataStore* store = context->world()->domDataStore();
     85    DOMNodeMapping& domNodeMap = node->isActiveNode() ? store->activeDomNodeMap() : store->domNodeMap();
     86    return domNodeMap.get(node);
     87}
     88
    7589// The caller must have increased obj's ref count.
    7690void V8DOMWrapper::setJSWrapperForDOMObject(void* object, v8::Persistent<v8::Object> wrapper)
    … …  
    293307}
    294308
    295 v8::Handle<v8::Object> V8DOMWrapper::getWrapperSlow(Node* node)
    296 {
    297     V8IsolatedContext* context = V8IsolatedContext::getEntered();
    298     if (LIKELY(!context)) {
    299         v8::Persistent<v8::Object>* wrapper = node->wrapper();
    300         if (!wrapper)
    301             return v8::Handle<v8::Object>();
    302         return *wrapper;
    303     }
    304     DOMDataStore* store = context->world()->domDataStore();
    305     DOMNodeMapping& domNodeMap = node->isActiveNode() ? store->activeDomNodeMap() : store->domNodeMap();
    306     return domNodeMap.get(node);
     309v8::Handle<v8::Object> V8DOMWrapper::getExistingWrapperSlow(Node* node)
     310{
     311    return getExistingWrapperInline(node);
     312}
     313
     314v8::Handle<v8::Value> V8DOMWrapper::getWrapperSlow(Node* node)
     315{
     316    v8::Handle<v8::Object> wrapper = getExistingWrapperInline(node);
     317    if (!wrapper.IsEmpty())
     318        return wrapper;
     319    return toV8Slow(node, false);
    307320}
    308321

  • trunk/Source/WebCore/bindings/v8/V8DOMWrapper.h

    r97771 r107367  
    125125        static v8::Local<v8::Object> instantiateV8Object(V8Proxy* proxy, WrapperTypeInfo*, void* impl);
    126126
    127         static v8::Handle<v8::Object> getWrapper(Node* node)
     127        static v8::Handle<v8::Object> getExistingWrapper(Node* node)
    128128        {
    129129            ASSERT(isMainThread());
    130130            if (LIKELY(!IsolatedWorld::count())) {
    131131                v8::Persistent<v8::Object>* wrapper = node->wrapper();
    132                 if (wrapper)
     132                if (LIKELY(!!wrapper))
     133                    return *wrapper;
     134            }
     135            return getExistingWrapperSlow(node);
     136        }
     137
     138        static v8::Handle<v8::Value> getWrapper(Node* node)
     139        {
     140            ASSERT(isMainThread());
     141            if (LIKELY(!IsolatedWorld::count())) {
     142                v8::Persistent<v8::Object>* wrapper = node->wrapper();
     143                if (LIKELY(!!wrapper))
    133144                    return *wrapper;
    134145            }
    … …  
    137148
    138149    private:
    139         static v8::Handle<v8::Object> getWrapperSlow(Node*);
     150        static v8::Handle<v8::Object> getExistingWrapperSlow(Node*);
     151        static v8::Handle<v8::Value> getWrapperSlow(Node*);
    140152    };
    141153

  • trunk/Source/WebCore/bindings/v8/custom/V8NodeCustom.cpp

    r95901 r107367  
    138138
    139139    if (!forceNewObject) {
    140         v8::Handle<v8::Value> wrapper = V8DOMWrapper::getWrapper(impl);
     140        v8::Handle<v8::Value> wrapper = V8DOMWrapper::getExistingWrapper(impl);
    141141        if (!wrapper.IsEmpty())
    142142            return wrapper;
Note: See TracChangeset for help on using the changeset viewer.