[V8] Enumerate Nodes via the V8 heap rather than via a list in WebCore
authorabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 23 Oct 2012 17:34:37 +0000 (17:34 +0000)
committerabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 23 Oct 2012 17:34:37 +0000 (17:34 +0000)
https://bugs.webkit.org/show_bug.cgi?id=100033

Reviewed by Eric Seidel.

This patch changes how we enumerate nodes during garbage collection.
After this patch, we use V8's list of open handles to enumerate node
wrappers rather than using a separate list that we maintain in WebCore
for this purpose. A future patch will remove the list in WebCore for a
DOM performance gain.

* bindings/js/ScriptProfiler.h:
(WebCore):
(WebCore::ScriptProfiler::visitNodeWrappers):
* bindings/scripts/CodeGeneratorV8.pm:
(GenerateToV8Converters):
* bindings/scripts/test/V8/V8TestNode.cpp:
(WebCore::V8TestNode::wrapSlow):
* bindings/v8/IntrusiveDOMWrapperMap.h:
(WebCore::IntrusiveDOMWrapperMap::set):
* bindings/v8/ScriptProfiler.cpp:
(WebCore::ScriptProfiler::visitNodeWrappers):
* bindings/v8/ScriptProfiler.h:
(WebCore):
(ScriptProfiler):
* bindings/v8/V8DOMMap.cpp:
(WebCore::NodeWrapperVisitor::~NodeWrapperVisitor):
(WebCore):
(WebCore::visitAllDOMNodes):
* bindings/v8/V8DOMMap.h:
(WebCore):
(NodeWrapperVisitor):
* bindings/v8/V8DOMWrapper.cpp:
(WebCore::V8DOMWrapper::setJSWrapperForDOMNode):
(WebCore::V8DOMWrapper::setJSWrapperForActiveDOMNode):
* bindings/v8/V8GCController.cpp:
(WebCore::NodeVisitor::visitNodeWrapper):
(WebCore::V8GCController::gcPrologue):
(WebCore::V8GCController::gcEpilogue):
* inspector/BindingVisitors.h:
(WebCore::WrappedNodeVisitor::~WrappedNodeVisitor):
* inspector/InspectorMemoryAgent.cpp:
(WebCore):

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

13 files changed:
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/ScriptProfiler.h
Source/WebCore/bindings/scripts/CodeGeneratorV8.pm
Source/WebCore/bindings/scripts/test/V8/V8TestNode.cpp
Source/WebCore/bindings/v8/IntrusiveDOMWrapperMap.h
Source/WebCore/bindings/v8/ScriptProfiler.cpp
Source/WebCore/bindings/v8/ScriptProfiler.h
Source/WebCore/bindings/v8/V8DOMMap.cpp
Source/WebCore/bindings/v8/V8DOMMap.h
Source/WebCore/bindings/v8/V8DOMWrapper.cpp
Source/WebCore/bindings/v8/V8GCController.cpp
Source/WebCore/inspector/BindingVisitors.h
Source/WebCore/inspector/InspectorMemoryAgent.cpp

index 7a3f984..ab080ee 100644 (file)
@@ -1,3 +1,49 @@
+2012-10-23  Adam Barth  <abarth@webkit.org>
+
+        [V8] Enumerate Nodes via the V8 heap rather than via a list in WebCore
+        https://bugs.webkit.org/show_bug.cgi?id=100033
+
+        Reviewed by Eric Seidel.
+
+        This patch changes how we enumerate nodes during garbage collection.
+        After this patch, we use V8's list of open handles to enumerate node
+        wrappers rather than using a separate list that we maintain in WebCore
+        for this purpose. A future patch will remove the list in WebCore for a
+        DOM performance gain.
+
+        * bindings/js/ScriptProfiler.h:
+        (WebCore):
+        (WebCore::ScriptProfiler::visitNodeWrappers):
+        * bindings/scripts/CodeGeneratorV8.pm:
+        (GenerateToV8Converters):
+        * bindings/scripts/test/V8/V8TestNode.cpp:
+        (WebCore::V8TestNode::wrapSlow):
+        * bindings/v8/IntrusiveDOMWrapperMap.h:
+        (WebCore::IntrusiveDOMWrapperMap::set):
+        * bindings/v8/ScriptProfiler.cpp:
+        (WebCore::ScriptProfiler::visitNodeWrappers):
+        * bindings/v8/ScriptProfiler.h:
+        (WebCore):
+        (ScriptProfiler):
+        * bindings/v8/V8DOMMap.cpp:
+        (WebCore::NodeWrapperVisitor::~NodeWrapperVisitor):
+        (WebCore):
+        (WebCore::visitAllDOMNodes):
+        * bindings/v8/V8DOMMap.h:
+        (WebCore):
+        (NodeWrapperVisitor):
+        * bindings/v8/V8DOMWrapper.cpp:
+        (WebCore::V8DOMWrapper::setJSWrapperForDOMNode):
+        (WebCore::V8DOMWrapper::setJSWrapperForActiveDOMNode):
+        * bindings/v8/V8GCController.cpp:
+        (WebCore::NodeVisitor::visitNodeWrapper):
+        (WebCore::V8GCController::gcPrologue):
+        (WebCore::V8GCController::gcEpilogue):
+        * inspector/BindingVisitors.h:
+        (WebCore::WrappedNodeVisitor::~WrappedNodeVisitor):
+        * inspector/InspectorMemoryAgent.cpp:
+        (WebCore):
+
 2012-10-22  Andrey Kosyakov  <caseq@chromium.org>
 
         Web Inspector: paint rectangles are incorrectly shown in case subframes are present
index 19dc192..ce4870e 100644 (file)
@@ -38,7 +38,7 @@ namespace WebCore {
 
 class ExternalArrayVisitor;
 class ExternalStringVisitor;
-class NodeWrapperVisitor;
+class WrappedNodeVisitor;
 class Page;
 class ScriptObject;
 class ScriptValue;
@@ -74,7 +74,7 @@ public:
     static bool isSampling() { return false; }
     static bool hasHeapProfiler() { return false; }
     // FIXME: Implement this counter for JSC. See bug 73936 for more details.
-    static void visitNodeWrappers(NodeWrapperVisitor*) { }
+    static void visitNodeWrappers(WrappedNodeVisitor*) { }
     // FIXME: Support these methods for JSC. See bug 90358.
     static void visitExternalStrings(ExternalStringVisitor*) { }
     static void visitExternalArrays(ExternalArrayVisitor*) { }
index b3f30c6..25c452b 100644 (file)
@@ -3455,19 +3455,9 @@ END
         return wrapper;
 
     installPerContextProperties(wrapper, impl.get());
-END
-
-    push(@implContent, <<END);
     v8::Persistent<v8::Object> wrapperHandle = V8DOMWrapper::setJSWrapperFor${domMapName}(impl, wrapper, isolate);
     if (!hasDependentLifetime)
         wrapperHandle.MarkIndependent();
-END
-    if (IsNodeSubType($dataNode)) {
-        push(@implContent, <<END);
-    wrapperHandle.SetWrapperClassId(v8DOMSubtreeClassId);
-END
-    }
-    push(@implContent, <<END);
     return wrapper;
 }
 END
index 89bfedc..a5de4b6 100644 (file)
@@ -138,7 +138,6 @@ v8::Handle<v8::Object> V8TestNode::wrapSlow(PassRefPtr<TestNode> impl, v8::Handl
     v8::Persistent<v8::Object> wrapperHandle = V8DOMWrapper::setJSWrapperForDOMNode(impl, wrapper, isolate);
     if (!hasDependentLifetime)
         wrapperHandle.MarkIndependent();
-    wrapperHandle.SetWrapperClassId(v8DOMSubtreeClassId);
     return wrapper;
 }
 
index ae20d03..676050a 100644 (file)
@@ -149,6 +149,7 @@ public:
     {
         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());
index 0186839..b53929a 100644 (file)
@@ -177,20 +177,25 @@ void ScriptProfiler::initialize()
     v8::HeapProfiler::DefineWrapperClass(v8DOMSubtreeClassId, &retainedDOMInfo);
 }
 
-void ScriptProfiler::visitNodeWrappers(NodeWrapperVisitor* visitor)
+void ScriptProfiler::visitNodeWrappers(WrappedNodeVisitor* visitor)
 {
-    class VisitorAdapter : public DOMWrapperMap<Node>::Visitor {
+    class VisitorAdapter : public NodeWrapperVisitor {
     public:
-        VisitorAdapter(NodeWrapperVisitor* visitor) : m_visitor(visitor) { }
+        VisitorAdapter(WrappedNodeVisitor* visitor)
+            : m_visitor(visitor)
+        {
+        }
 
-        virtual void visitDOMWrapper(DOMDataStore*, Node* node, v8::Persistent<v8::Object>)
+        virtual void visitNodeWrapper(Node* node, v8::Persistent<v8::Object>)
         {
             m_visitor->visitNode(node);
         }
+
     private:
-        NodeWrapperVisitor* m_visitor;
+        WrappedNodeVisitor* m_visitor;
     } adapter(visitor);
-    visitDOMNodes(&adapter);
+
+    visitAllDOMNodes(&adapter);
 }
 
 void ScriptProfiler::visitExternalStrings(ExternalStringVisitor* visitor)
index 76a4f2a..f845941 100644 (file)
@@ -43,7 +43,7 @@ namespace WebCore {
 
 class ExternalArrayVisitor;
 class ExternalStringVisitor;
-class NodeWrapperVisitor;
+class WrappedNodeVisitor;
 class Page;
 class ScriptObject;
 class ScriptValue;
@@ -79,7 +79,7 @@ public:
     static bool isSampling() { return true; }
     static bool hasHeapProfiler() { return true; }
     static void initialize();
-    static void visitNodeWrappers(NodeWrapperVisitor*);
+    static void visitNodeWrappers(WrappedNodeVisitor*);
     static void visitExternalStrings(ExternalStringVisitor*);
     static void visitExternalArrays(ExternalArrayVisitor*);
     static void collectBindingMemoryInfo(MemoryInstrumentation*);
index 751642f..e6dab90 100644 (file)
@@ -52,6 +52,10 @@ DOMDataStoreHandle::~DOMDataStoreHandle()
         V8PerIsolateData::current()->unregisterDOMDataStore(m_store.get());
 }
 
+NodeWrapperVisitor::~NodeWrapperVisitor()
+{
+}
+
 DOMNodeMapping& getDOMNodeMap(v8::Isolate* isolate)
 {
     return DOMData::getCurrentStore(isolate).domNodeMap();
@@ -84,16 +88,33 @@ void removeAllDOMObjects()
     DOMData::removeObjectsFromWrapperMap<void>(&store, store.activeDomObjectMap());
 }
 
-void visitDOMNodes(DOMWrapperMap<Node>::Visitor* visitor)
+void visitAllDOMNodes(NodeWrapperVisitor* visitor)
 {
     v8::HandleScope scope;
 
-    DOMDataList& list = DOMDataStore::allStores();
-    for (size_t i = 0; i < list.size(); ++i) {
-        DOMDataStore* store = list[i];
-
-        store->domNodeMap().visit(store, visitor);
-    }
+    class VisitorAdapter : public v8::PersistentHandleVisitor {
+    public:
+        explicit VisitorAdapter(NodeWrapperVisitor* visitor)
+            : m_visitor(visitor)
+        {
+        }
+
+        virtual void VisitPersistentHandle(v8::Persistent<v8::Value> value, uint16_t classId)
+        {
+            if (classId != v8DOMSubtreeClassId)
+                return;
+            ASSERT(V8Node::HasInstance(value));
+            ASSERT(value->IsObject());
+            ASSERT(!value.IsIndependent());
+            v8::Persistent<v8::Object> wrapper = v8::Persistent<v8::Object>::Cast(value);
+            m_visitor->visitNodeWrapper(V8Node::toNative(wrapper), wrapper);
+        }
+
+    private:
+        NodeWrapperVisitor* m_visitor;
+    } visitorAdapter(visitor);
+
+    v8::V8::VisitHandlesWithClassIds(&visitorAdapter);
 }
 
 void visitActiveDOMNodes(DOMWrapperMap<Node>::Visitor* visitor)
index ca45dfd..c716f30 100644 (file)
@@ -165,9 +165,16 @@ namespace WebCore {
     // A map from DOM node to its JS wrapper.
     DOMNodeMapping& getDOMNodeMap(v8::Isolate* = 0);
     DOMNodeMapping& getActiveDOMNodeMap(v8::Isolate* = 0);
-    void visitDOMNodes(DOMWrapperMap<Node>::Visitor*);
     void visitActiveDOMNodes(DOMWrapperMap<Node>::Visitor*);
 
+
+    class NodeWrapperVisitor {
+    public:
+        virtual ~NodeWrapperVisitor();
+        virtual void visitNodeWrapper(Node*, v8::Persistent<v8::Object> wrapper) = 0;
+    };
+    void visitAllDOMNodes(NodeWrapperVisitor*);
+
     // A map from a DOM object (non-node) to its JS wrapper. This map does not contain the DOM objects which can have pending activity (active dom objects).
     DOMWrapperMap<void>& getDOMObjectMap(v8::Isolate* = 0);
     void visitDOMObjects(DOMWrapperMap<void>::Visitor*);
index c768da8..1728358 100644 (file)
@@ -73,6 +73,7 @@ v8::Persistent<v8::Object> V8DOMWrapper::setJSWrapperForDOMNode(PassRefPtr<Node>
     v8::Persistent<v8::Object> wrapperHandle = v8::Persistent<v8::Object>::New(wrapper);
     ASSERT(maybeDOMWrapper(wrapperHandle));
     ASSERT(!node->isActiveNode());
+    wrapperHandle.SetWrapperClassId(v8DOMSubtreeClassId);
     getDOMNodeMap(isolate).set(node.leakRef(), wrapperHandle);
     return wrapperHandle;
 }
@@ -82,6 +83,7 @@ v8::Persistent<v8::Object> V8DOMWrapper::setJSWrapperForActiveDOMNode(PassRefPtr
     v8::Persistent<v8::Object> wrapperHandle = v8::Persistent<v8::Object>::New(wrapper);
     ASSERT(maybeDOMWrapper(wrapperHandle));
     ASSERT(node->isActiveNode());
+    wrapperHandle.SetWrapperClassId(v8DOMSubtreeClassId);
     getActiveDOMNodeMap(isolate).set(node.leakRef(), wrapperHandle);
     return wrapperHandle;
 }
index 95d91e2..3a4d486 100644 (file)
@@ -169,9 +169,9 @@ bool operator<(const ImplicitConnection& left, const ImplicitConnection& right)
     return left.root() < right.root();
 }
 
-class NodeVisitor : public DOMWrapperMap<Node>::Visitor {
+class NodeVisitor : public NodeWrapperVisitor {
 public:
-    void visitDOMWrapper(DOMDataStore*, Node* node, v8::Persistent<v8::Object> wrapper)
+    void visitNodeWrapper(Node* node, v8::Persistent<v8::Object> wrapper)
     {
         if (node->hasEventListeners())
             addImplicitReferencesForNodeWithEventListeners(node, wrapper);
@@ -218,8 +218,7 @@ void V8GCController::gcPrologue()
     visitActiveDOMNodes(&activeNodeVisitor);
 
     NodeVisitor nodeVisitor;
-    visitDOMNodes(&nodeVisitor);
-    visitActiveDOMNodes(&nodeVisitor);
+    visitAllDOMNodes(&nodeVisitor);
     nodeVisitor.applyGrouping();
 
     ObjectVisitor objectVisitor;
@@ -307,11 +306,6 @@ void V8GCController::gcEpilogue()
     }
 #endif
 
-#ifndef NDEBUG
-    EnsureWeakDOMNodeVisitor weakDOMNodeVisitor;
-    visitDOMNodes(&weakDOMNodeVisitor);
-#endif
-
     TRACE_EVENT_END0("v8", "GC");
 }
 
index 7530c92..c885703 100644 (file)
@@ -38,11 +38,11 @@ namespace WebCore {
 
 class Node;
 
-class NodeWrapperVisitor {
+class WrappedNodeVisitor {
 public:
     virtual void visitNode(Node*) = 0;
 protected:
-    virtual ~NodeWrapperVisitor() { }
+    virtual ~WrappedNodeVisitor() { }
 };
 
 class ExternalStringVisitor {
index fdc72af..d6ee3a0 100644 (file)
@@ -276,7 +276,7 @@ private:
     CharacterDataStatistics& m_characterDataStatistics;
 };
 
-class CounterVisitor : public NodeWrapperVisitor, public ExternalStringVisitor {
+class CounterVisitor : public WrappedNodeVisitor, public ExternalStringVisitor {
 public:
     CounterVisitor(Page* page)
         : m_page(page)
@@ -450,7 +450,7 @@ static void reportRenderTreeInfo(WTF::MemoryInstrumentationClient& memoryInstrum
 
 namespace {
 
-class DOMTreesIterator : public NodeWrapperVisitor {
+class DOMTreesIterator : public WrappedNodeVisitor {
 public:
     DOMTreesIterator(MemoryInstrumentationImpl& memoryInstrumentation, Page* page)
         : m_page(page)