[V8] ScriptWrappable should hold the wrapper handle directly (Dromaeo/dom-modify...
authorabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 23 Oct 2012 18:14:22 +0000 (18:14 +0000)
committerabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 23 Oct 2012 18:14:22 +0000 (18:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=97974

Reviewed by Eric Seidel.

Previously, we stored a pointer to a handle to a wrapper in Node. That
is an extra layer of indirection that slows down finding the wrapper
for the node. A handle is just a pointer, so we might as we just store
the handle in the Node directly. That speeds up dom-modify and
dom-traverse by about 2.5%.

This change also lets us get rid of the ChunkedTable we were using to
store all the wrappers because they're now stored in the Nodes
directly.

* bindings/scripts/CodeGeneratorV8.pm:
(GenerateHeader):
* bindings/v8/IntrusiveDOMWrapperMap.h:
(WebCore::IntrusiveDOMWrapperMap::IntrusiveDOMWrapperMap):
(WebCore::IntrusiveDOMWrapperMap::get):
(WebCore::IntrusiveDOMWrapperMap::set):
(WebCore::IntrusiveDOMWrapperMap::contains):
(WebCore::IntrusiveDOMWrapperMap::visit):
(WebCore::IntrusiveDOMWrapperMap::removeIfPresent):
(WebCore::IntrusiveDOMWrapperMap::clear):
* bindings/v8/ScriptWrappable.h:
(WebCore::ScriptWrappable::ScriptWrappable):
(WebCore::ScriptWrappable::wrapper):
(WebCore::ScriptWrappable::setWrapper):
(WebCore::ScriptWrappable::disposeWrapper):
(WebCore::ScriptWrappable::reportMemoryUsage):
(ScriptWrappable):
* bindings/v8/V8DOMWrapper.h:
(WebCore::V8DOMWrapper::getCachedWrapper):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@132245 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/bindings/scripts/CodeGeneratorV8.pm
Source/WebCore/bindings/v8/IntrusiveDOMWrapperMap.h
Source/WebCore/bindings/v8/ScriptWrappable.h
Source/WebCore/bindings/v8/StaticDOMDataStore.h
Source/WebCore/bindings/v8/V8DOMWrapper.h

index c3afb65..ecf92da 100644 (file)
@@ -1,3 +1,40 @@
+2012-10-23  Adam Barth  <abarth@webkit.org>
+
+        [V8] ScriptWrappable should hold the wrapper handle directly (Dromaeo/dom-modify and dom-traverse get ~2.5% faster)
+        https://bugs.webkit.org/show_bug.cgi?id=97974
+
+        Reviewed by Eric Seidel.
+
+        Previously, we stored a pointer to a handle to a wrapper in Node. That
+        is an extra layer of indirection that slows down finding the wrapper
+        for the node. A handle is just a pointer, so we might as we just store
+        the handle in the Node directly. That speeds up dom-modify and
+        dom-traverse by about 2.5%.
+
+        This change also lets us get rid of the ChunkedTable we were using to
+        store all the wrappers because they're now stored in the Nodes
+        directly.
+
+        * bindings/scripts/CodeGeneratorV8.pm:
+        (GenerateHeader):
+        * bindings/v8/IntrusiveDOMWrapperMap.h:
+        (WebCore::IntrusiveDOMWrapperMap::IntrusiveDOMWrapperMap):
+        (WebCore::IntrusiveDOMWrapperMap::get):
+        (WebCore::IntrusiveDOMWrapperMap::set):
+        (WebCore::IntrusiveDOMWrapperMap::contains):
+        (WebCore::IntrusiveDOMWrapperMap::visit):
+        (WebCore::IntrusiveDOMWrapperMap::removeIfPresent):
+        (WebCore::IntrusiveDOMWrapperMap::clear):
+        * bindings/v8/ScriptWrappable.h:
+        (WebCore::ScriptWrappable::ScriptWrappable):
+        (WebCore::ScriptWrappable::wrapper):
+        (WebCore::ScriptWrappable::setWrapper):
+        (WebCore::ScriptWrappable::disposeWrapper):
+        (WebCore::ScriptWrappable::reportMemoryUsage):
+        (ScriptWrappable):
+        * bindings/v8/V8DOMWrapper.h:
+        (WebCore::V8DOMWrapper::getCachedWrapper):
+
 2012-10-23  Kentaro Hara  <haraken@chromium.org>
 
         [V8] Replace SetGlobalGCPrologueCallback() with AddGCPrologueCallback()
index 25c452b..54772ff 100644 (file)
@@ -556,8 +556,11 @@ inline v8::Handle<v8::Value> toV8Fast(Node* node, const v8::AccessorInfo& info,
     // whether the holder's inline wrapper is the same wrapper we see in the
     // v8::AccessorInfo.
     v8::Handle<v8::Object> holderWrapper = info.Holder();
-    if (holder->wrapper() && *holder->wrapper() == holderWrapper && node->wrapper())
-        return *node->wrapper();
+    if (holder->wrapper() == holderWrapper) {
+        v8::Handle<v8::Object> wrapper = node->wrapper();
+        if (!wrapper.IsEmpty())
+            return wrapper;
+    }
     return toV8Slow(node, holderWrapper, info.GetIsolate());
 }
 END
index 676050a..d66c761 100644 (file)
 #define IntrusiveDOMWrapperMap_h
 
 #include "DOMDataStore.h"
+#include "V8DOMMap.h"
 #include "V8Node.h"
 #include "WebCoreMemoryInstrumentation.h"
 
 namespace WebCore {
 
-template <class T, int CHUNK_SIZE, class Traits>
-class ChunkedTable {
-  public:
-    ChunkedTable() : m_chunks(0), m_current(0), m_last(0) { }
-
-    T* add(T element)
+class DOMNodeWrapperMap : public AbstractWeakReferenceMap<Node, v8::Object> {
+public:
+    DOMNodeWrapperMap(v8::WeakReferenceCallback callback)
+        : AbstractWeakReferenceMap<Node, v8::Object>(callback)
     {
-        if (m_current == m_last) {
-            m_chunks = new Chunk(m_chunks);
-            m_current = m_chunks->m_entries;
-            m_last = m_current + CHUNK_SIZE;
-        }
-        ASSERT((m_chunks->m_entries <= m_current) && (m_current < m_last));
-        T* p = m_current++;
-        *p = element;
-        return p;
     }
 
-    void remove(T* element)
+    virtual v8::Persistent<v8::Object> get(Node* node)
     {
-        ASSERT(element);
-        ASSERT(m_current > m_chunks->m_entries);
-        m_current--;
-        if (element != m_current)
-            Traits::move(element, m_current);
-        if (m_current == m_chunks->m_entries) {
-            Chunk* toDelete = m_chunks;
-            m_chunks = toDelete->m_previous;
-            m_current = m_last = m_chunks ? m_chunks->m_entries + CHUNK_SIZE : 0;
-            delete toDelete;
-        }
-        ASSERT(!m_chunks || ((m_chunks->m_entries < m_current) && (m_current <= m_last)));
+        return node->wrapper();
     }
 
-    void clear()
+    virtual void set(Node* node, v8::Persistent<v8::Object> wrapper)
     {
-        if (!m_chunks)
-            return;
-
-        clearEntries(m_chunks->m_entries, m_current);
-        Chunk* last = m_chunks;
-        while (true) {
-            Chunk* previous = last->m_previous;
-            if (!previous)
-                break;
-            delete last;
-            clearEntries(previous->m_entries, previous->m_entries + CHUNK_SIZE);
-            last = previous;
-        }
-
-        m_chunks = last;
-        m_current = m_chunks->m_entries;
-        m_last = m_current + CHUNK_SIZE;
+        ASSERT(node && node->wrapper().IsEmpty());
+        ASSERT(wrapper.WrapperClassId() == v8DOMSubtreeClassId);
+        node->setWrapper(wrapper);
+        wrapper.MakeWeak(node, weakReferenceCallback());
     }
 
-    void visit(DOMDataStore* store, typename Traits::Visitor* visitor)
+    virtual bool contains(Node* node)
     {
-        if (!m_chunks)
-            return;
-
-        visitEntries(store, m_chunks->m_entries, m_current, visitor);
-        for (Chunk* chunk = m_chunks->m_previous; chunk; chunk = chunk->m_previous)
-            visitEntries(store, chunk->m_entries, chunk->m_entries + CHUNK_SIZE, visitor);
+        return !node->wrapper().IsEmpty();
     }
 
-    void reportMemoryUsage(MemoryObjectInfo* memoryObjectInfo) const
+    virtual void visit(DOMDataStore* store, Visitor* visitor)
     {
-        MemoryClassInfo info(memoryObjectInfo, this, WebCoreMemoryTypes::Binding);
-        for (Chunk* chunk = m_chunks; chunk; chunk = chunk->m_previous)
-            info.addMember(chunk);
+        ASSERT_NOT_REACHED();
     }
 
-  private:
-    struct Chunk {
-        explicit Chunk(Chunk* previous) : m_previous(previous) { }
-        Chunk* const m_previous;
-        T m_entries[CHUNK_SIZE];
-    };
-
-    static void clearEntries(T* first, T* last)
+    virtual bool removeIfPresent(Node* node, v8::Persistent<v8::Object> value)
     {
-        for (T* entry = first; entry < last; entry++)
-            Traits::clear(entry);
+        ASSERT(node);
+        v8::Persistent<v8::Object> wrapper = node->wrapper();
+        if (wrapper.IsEmpty())
+            return false;
+        if (wrapper != value)
+            return false;
+        node->disposeWrapper();
+        return true;
     }
 
-    static void visitEntries(DOMDataStore* store, T* first, T* last, typename Traits::Visitor* visitor)
+    virtual void clear()
     {
-        for (T* entry = first; entry < last; entry++)
-            Traits::visit(store, entry, visitor);
+        ASSERT_NOT_REACHED();
     }
 
-    Chunk* m_chunks;
-    T* m_current;
-    T* m_last;
-};
-
-
-class IntrusiveDOMWrapperMap : public AbstractWeakReferenceMap<Node, v8::Object> {
-public:
-    IntrusiveDOMWrapperMap(v8::WeakReferenceCallback callback)
-        : AbstractWeakReferenceMap<Node, v8::Object>(callback) { }
-
-    virtual v8::Persistent<v8::Object> get(Node* obj)
+    virtual void reportMemoryUsage(MemoryObjectInfo* memoryObjectInfo) const OVERRIDE
     {
-        v8::Persistent<v8::Object>* wrapper = obj->wrapper();
-        return wrapper ? *wrapper : v8::Persistent<v8::Object>();
+        // This data structure doesn't use any extra memory.
     }
+};
 
-    virtual void set(Node* obj, v8::Persistent<v8::Object> wrapper)
+class ActiveDOMNodeWrapperMap : public DOMWrapperMap<Node> {
+public:
+    ActiveDOMNodeWrapperMap(v8::WeakReferenceCallback callback)
+        : DOMWrapperMap<Node>(callback)
     {
-        ASSERT(obj);
-        ASSERT(!obj->wrapper());
-        ASSERT(wrapper.WrapperClassId() == v8DOMSubtreeClassId);
-        v8::Persistent<v8::Object>* entry = m_table.add(wrapper);
-        obj->setWrapper(entry);
-        wrapper.MakeWeak(obj, weakReferenceCallback());
     }
 
-    virtual bool contains(Node* obj)
+    virtual v8::Persistent<v8::Object> get(Node* node) OVERRIDE
     {
-        return obj->wrapper();
+        return node->wrapper();
     }
 
-    virtual void visit(DOMDataStore* store, Visitor* visitor)
+    virtual void set(Node* node, v8::Persistent<v8::Object> wrapper) OVERRIDE
     {
-        m_table.visit(store, visitor);
+        ASSERT(node && node->wrapper().IsEmpty());
+        ASSERT(wrapper.WrapperClassId() == v8DOMSubtreeClassId);
+        node->setWrapper(wrapper);
+        DOMWrapperMap<Node>::set(node, wrapper);
     }
 
-    virtual bool removeIfPresent(Node* obj, v8::Persistent<v8::Object> value)
+    virtual bool removeIfPresent(Node* node, v8::Persistent<v8::Object> value) OVERRIDE
     {
-        ASSERT(obj);
-        v8::Persistent<v8::Object>* entry = obj->wrapper();
-        if (!entry)
+        if (!DOMWrapperMap<Node>::removeIfPresent(node, value))
             return false;
-        if (*entry != value)
-            return false;
-        obj->clearWrapper();
-        m_table.remove(entry);
-        value.Dispose();
+        // Notice that we call "clearWrapper" here rather than disposeWrapper because
+        // our base class will call Dispose on the wrapper for us.
+        node->clearWrapper();
         return true;
     }
 
-
-    virtual void clear()
-    {
-        m_table.clear();
-    }
-
-    virtual void reportMemoryUsage(MemoryObjectInfo* memoryObjectInfo) const OVERRIDE
+    virtual void clear() OVERRIDE
     {
-        MemoryClassInfo info(memoryObjectInfo, this, WebCoreMemoryTypes::Binding);
-        info.addMember(m_table);
+        ASSERT_NOT_REACHED();
     }
-
-private:
-    static int const numberOfEntries = (1 << 10) - 1;
-
-    struct ChunkedTableTraits {
-        typedef IntrusiveDOMWrapperMap::Visitor Visitor;
-
-        static void move(v8::Persistent<v8::Object>* target, v8::Persistent<v8::Object>* source)
-        {
-            *target = *source;
-            Node* node = V8Node::toNative(*target);
-            ASSERT(node);
-            node->setWrapper(target);
-        }
-
-        static void clear(v8::Persistent<v8::Object>* entry)
-        {
-            Node* node = V8Node::toNative(*entry);
-            ASSERT(node->wrapper() == entry);
-
-            node->clearWrapper();
-            entry->Dispose();
-        }
-
-        static void visit(DOMDataStore* store, v8::Persistent<v8::Object>* entry, Visitor* visitor)
-        {
-            Node* node = V8Node::toNative(*entry);
-            ASSERT(node->wrapper() == entry);
-
-            visitor->visitDOMWrapper(store, node, *entry);
-        }
-    };
-
-    typedef ChunkedTable<v8::Persistent<v8::Object>, numberOfEntries, ChunkedTableTraits> Table;
-    Table m_table;
 };
 
 } // namespace WebCore
index 2915cc7..4b0df21 100644 (file)
@@ -38,29 +38,42 @@ namespace WebCore {
 
 class ScriptWrappable {
 public:
-    ScriptWrappable() : m_wrapper(0) { }
+    ScriptWrappable()
+    {
+    }
 
-    v8::Persistent<v8::Object>* wrapper() const
+    v8::Persistent<v8::Object> wrapper() const
     {
         return m_wrapper;
     }
 
-    void setWrapper(v8::Persistent<v8::Object>* wrapper)
+    void setWrapper(v8::Persistent<v8::Object> wrapper)
     {
-        ASSERT(wrapper);
+        ASSERT(!wrapper.IsEmpty());
         m_wrapper = wrapper;
     }
 
-    void clearWrapper() { m_wrapper = 0; }
+    void clearWrapper()
+    {
+        ASSERT(!m_wrapper.IsEmpty());
+        m_wrapper.Clear();
+    }
+
+    void disposeWrapper()
+    {
+        ASSERT(!m_wrapper.IsEmpty());
+        m_wrapper.Dispose();
+        m_wrapper.Clear();
+    }
 
     void reportMemoryUsage(MemoryObjectInfo* memoryObjectInfo) const
     {
         MemoryClassInfo info(memoryObjectInfo, this, WebCoreMemoryTypes::DOM);
-        info.addWeakPointer(m_wrapper);
+        info.addWeakPointer(const_cast<v8::Persistent<v8::Object>*>(&m_wrapper));
     }
 
 private:
-    v8::Persistent<v8::Object>* m_wrapper;
+    v8::Persistent<v8::Object> m_wrapper;
 };
 
 } // namespace WebCore
index f9c57fc..8109305 100644 (file)
@@ -52,8 +52,8 @@ public:
     virtual void reportMemoryUsage(MemoryObjectInfo*) const OVERRIDE;
 
 private:
-    IntrusiveDOMWrapperMap m_staticDomNodeMap;
-    IntrusiveDOMWrapperMap m_staticActiveDomNodeMap;
+    DOMNodeWrapperMap m_staticDomNodeMap;
+    ActiveDOMNodeWrapperMap m_staticActiveDomNodeMap;
     DOMWrapperMap<void> m_staticDomObjectMap;
     DOMWrapperMap<void> m_staticActiveDomObjectMap;
 };
index ad1bcaf..fad82c9 100644 (file)
@@ -113,18 +113,15 @@ namespace WebCore {
         {
             ASSERT(isMainThread());
             if (LIKELY(!DOMWrapperWorld::isolatedWorldsExist())) {
-                v8::Persistent<v8::Object>* wrapper = node->wrapper();
-                if (LIKELY(!!wrapper))
-                    return *wrapper;
+                v8::Persistent<v8::Object> wrapper = node->wrapper();
+                if (LIKELY(!wrapper.IsEmpty()))
+                    return wrapper;
             }
 
             V8DOMWindowShell* context = V8DOMWindowShell::getEntered();
-            if (LIKELY(!context)) {
-                v8::Persistent<v8::Object>* wrapper = node->wrapper();
-                if (!wrapper)
-                    return v8::Handle<v8::Object>();
-                return *wrapper;
-            }
+            if (LIKELY(!context))
+                return node->wrapper();
+
             DOMDataStore* store = context->world()->domDataStore();
             DOMNodeMapping& domNodeMap = node->isActiveNode() ? store->activeDomNodeMap() : store->domNodeMap();
             return domNodeMap.get(node);