Update V8GCController to use new V8 GC-related APIs.

Source/bindings/v8/V8GCController.cpp

 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
  * 
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  * 
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 29 matching lines @@
 #include "bindings/v8/V8RecursionScope.h"
 #include "bindings/v8/WrapperTypeInfo.h"
 #include "core/dom/Attr.h"
 #include "core/html/HTMLImageElement.h"
 #include "core/platform/MemoryUsageSupport.h"
 #include "core/platform/chromium/TraceEvent.h"
 #include <algorithm>
 
 namespace WebCore {
 
-class ImplicitConnection {
-public:
-    ImplicitConnection(void* root, v8::Persistent<v8::Object> wrapper)
-        : m_root(root)
-        , m_wrapper(wrapper)
-        , m_rootNode(0)
-    {
-    }
-    ImplicitConnection(Node* root, v8::Persistent<v8::Object> wrapper)
-        : m_root(root)
-        , m_wrapper(wrapper)
-        , m_rootNode(root)
-    {
-    }
-
-    void* root() const { return m_root; }
-    v8::Persistent<v8::Object> wrapper() const { return m_wrapper; }
-
-    PassOwnPtr<RetainedObjectInfo> retainedObjectInfo()
-    {
-        if (!m_rootNode)
-            return nullptr;
-        return adoptPtr(new RetainedDOMInfo(m_rootNode));
-    }
-
-private:
-    void* m_root;
-    v8::Persistent<v8::Object> m_wrapper;
-    Node* m_rootNode;
-};
-
-bool operator<(const ImplicitConnection& left, const ImplicitConnection& right)
-{
-    return left.root() < right.root();
-}
-
-struct ImplicitReference {
-    ImplicitReference(void* parent, v8::Persistent<v8::Object> child)
-        : parent(parent)
-        , child(child)
-    {
-    }
-
-    void* parent;
-    v8::Persistent<v8::Object> child;
-};
-
-bool operator<(const ImplicitReference& left, const ImplicitReference& right)
-{
-    return left.parent < right.parent;
-}
-
-class WrapperGrouper {
-public:
-    WrapperGrouper()
-    {
-        m_liveObjects.append(V8PerIsolateData::current()->ensureLiveRoot());
-    }
-
-    void addObjectWrapperToGroup(void* root, v8::Persistent<v8::Object> wrapper)
-    {
-        m_connections.append(ImplicitConnection(root, wrapper));
-    }
-
-    void addNodeWrapperToGroup(Node* root, v8::Persistent<v8::Object> wrapper)
-    {
-        m_connections.append(ImplicitConnection(root, wrapper));
-    }
-
-    void addImplicitReference(void* parent, v8::Persistent<v8::Object> child)
-    {
-        m_references.append(ImplicitReference(parent, child));
-        m_rootGroupMap.add(parent, v8::Persistent<v8::Object>());
-    }
-
-    void keepAlive(v8::Persistent<v8::Value> wrapper)
-    {
-        m_liveObjects.append(wrapper);
-    }
-
-    void apply()
-    {
-        if (m_liveObjects.size() > 1)
-            v8::V8::AddObjectGroup(m_liveObjects.data(), m_liveObjects.size());
-
-        std::sort(m_connections.begin(), m_connections.end());
-        Vector<v8::Persistent<v8::Value> > group;
-        ImplicitConnection* connectionIterator = m_connections.begin();
-        const ImplicitConnection* connectionIteratorEnd = m_connections.end();
-        while (connectionIterator < connectionIteratorEnd) {
-            void* root = connectionIterator->root();
-            v8::Persistent<v8::Object> groupRepresentativeWrapper = connectionIterator->wrapper();
-            OwnPtr<RetainedObjectInfo> retainedObjectInfo = connectionIterator->retainedObjectInfo();
-
-            do {
-                group.append(connectionIterator->wrapper());
-                ++connectionIterator;
-            } while (connectionIterator < connectionIteratorEnd && root == connectionIterator->root());
-
-            if (group.size() > 1)
-                v8::V8::AddObjectGroup(group.data(), group.size(), retainedObjectInfo.leakPtr());
-
-            HashMap<void*, v8::Persistent<v8::Object> >::iterator iter = m_rootGroupMap.find(root);
-            if (iter != m_rootGroupMap.end())
-                iter->value = groupRepresentativeWrapper;
-
-            group.shrink(0);
-        }
-
-        std::sort(m_references.begin(), m_references.end());
-        const ImplicitReference* referenceIterator = m_references.begin();
-        const ImplicitReference* referenceIteratorEnd = m_references.end();
-        while (referenceIterator < referenceIteratorEnd) {
-            void* parent = referenceIterator->parent;
-            v8::Persistent<v8::Object> parentWrapper = m_rootGroupMap.get(parent);
-            if (parentWrapper.IsEmpty()) {
-                ++referenceIterator;
-                continue;
-            }
-
-            Vector<v8::Persistent<v8::Value> > children;
-            do {
-                children.append(referenceIterator->child);
-                ++referenceIterator;
-            } while (referenceIterator < referenceIteratorEnd && parent == referenceIterator->parent);
-
-            v8::V8::AddImplicitReferences(parentWrapper, children.data(), children.size());
-        }
-    }
-
-private:
-    Vector<v8::Persistent<v8::Value> > m_liveObjects;
-    Vector<ImplicitConnection> m_connections;
-    Vector<ImplicitReference> m_references;
-    HashMap<void*, v8::Persistent<v8::Object> > m_rootGroupMap;
-};
-
 // FIXME: This should use opaque GC roots.
-static void addImplicitReferencesForNodeWithEventListeners(Node* node, v8::Persistent<v8::Object> wrapper)
+static void addImplicitReferencesForNodeWithEventListeners(v8::Isolate* isolate, Node* node, const v8::Persistent<v8::Object>& wrapper)

[haraken, 2013/04/25 14:55:54]

addImplicitReferencesForNodeWithEventListeners =>
addReferenceFromNodeToEventListeners

[marja, 2013/04/26 08:14:47]

As agreed offline: addReferencesFromNodeToEventListeners.

 {
     ASSERT(node->hasEventListeners());
 
-    Vector<v8::Persistent<v8::Value> > listeners;
-
     EventListenerIterator iterator(node);
     while (EventListener* listener = iterator.nextListener()) {
         if (listener->type() != EventListener::JSEventListenerType)
             continue;
         V8AbstractEventListener* v8listener = static_cast<V8AbstractEventListener*>(listener);
         if (!v8listener->hasExistingListenerObject())
             continue;
-        listeners.append(v8listener->existingListenerObjectPersistentHandle());
+
+        isolate->SetReference(wrapper, v8listener->existingListenerObjectPersistentHandle());
     }
-
-    if (listeners.isEmpty())
-        return;
-
-    v8::V8::AddImplicitReferences(wrapper, listeners.data(), listeners.size());
 }
 
 Node* V8GCController::opaqueRootForGC(Node* node, v8::Isolate*)
 {
     // FIXME: Remove the special handling for image elements.
     // The same special handling is in V8GCController::gcTree().
     // Maybe should image elements be active DOM nodes?
     // See https://code.google.com/p/chromium/issues/detail?id=164882
     if (node->inDocument() || (node->hasTagName(HTMLNames::imgTag) && static_cast<HTMLImageElement*>(node)->hasPendingActivity()))
         return node->document();
 
     if (node->isAttributeNode()) {
         Node* ownerElement = static_cast<Attr*>(node)->ownerElement();
         if (!ownerElement)
             return node;
         node = ownerElement;
     }
 
     while (Node* parent = node->parentOrShadowHostNode())
         node = parent;
 
     return node;
 }
 
 static void gcTree(v8::Isolate* isolate, Node* startNode)
 {
-    Vector<v8::Persistent<v8::Value>, initialNodeVectorSize> newSpaceWrappers;
+    Vector<Node*, initialNodeVectorSize> newSpaceWrappers;

[haraken, 2013/04/25 14:55:54]

newSpaceWrappers => newSpaceNodes

[marja, 2013/04/26 08:14:47]

Done.

 
     // We traverse a DOM tree in the DFS order starting from startNode.
     // The traversal order does not matter for correctness but does matter for performance.
     Node* node = startNode;
     // To make each minor GC time bounded, we might need to give up
     // traversing at some point for a large DOM tree. That being said,
     // I could not observe the need even in pathological test cases.
     do {
         ASSERT(node);
         if (!node->wrapper().IsEmpty()) {
             // FIXME: Remove the special handling for image elements.
             // The same special handling is in V8GCController::opaqueRootForGC().
             // Maybe should image elements be active DOM nodes?
             // See https://code.google.com/p/chromium/issues/detail?id=164882
             if (!node->isV8CollectableDuringMinorGC() || (node->hasTagName(HTMLNames::imgTag) && static_cast<HTMLImageElement*>(node)->hasPendingActivity())) {
                 // This node is not in the new space of V8. This indicates that
                 // the minor GC cannot anyway judge reachability of this DOM tree.
                 // Thus we give up traversing the DOM tree.
                 return;
             }
             node->setV8CollectableDuringMinorGC(false);
-            newSpaceWrappers.append(node->wrapper());
+            newSpaceWrappers.append(node);
         }
         if (node->firstChild()) {
             node = node->firstChild();
             continue;
         }
         while (!node->nextSibling()) {
             if (!node->parentNode())
                 break;
             node = node->parentNode();
         }
         if (node->parentNode())
             node = node->nextSibling();
     } while (node != startNode);
 
     // We completed the DOM tree traversal. All wrappers in the DOM tree are
     // stored in newSpaceWrappers and are expected to exist in the new space of V8.
     // We report those wrappers to V8 as an object group.
-    v8::Persistent<v8::Value>* wrapperIterator = newSpaceWrappers.begin();
-    const v8::Persistent<v8::Value>* wrapperIteratorEnd = newSpaceWrappers.end();
-    for (; wrapperIterator != wrapperIteratorEnd; ++wrapperIterator)
-        wrapperIterator->MarkPartiallyDependent(isolate);
-    if (newSpaceWrappers.size() > 0)
-        v8::V8::AddObjectGroup(&newSpaceWrappers[0], newSpaceWrappers.size());
+    Node** nodeIterator = newSpaceWrappers.begin();
+    Node** const nodeIteratorEnd = newSpaceWrappers.end();
+    if (nodeIterator == nodeIteratorEnd)
+        return;
+    v8::UniqueId id(reinterpret_cast<intptr_t>(*(*nodeIterator)->wrapper()));
+    for (; nodeIterator != nodeIteratorEnd; ++nodeIterator) {
+        v8::Persistent<v8::Object> wrapper = v8::Persistent<v8::Object>((*nodeIterator)->wrapper());

[haraken, 2013/04/25 14:55:54]

Nit: It looks redundant to access (*nodeIterator)->wrapper() twice.

[marja, 2013/04/26 08:14:47]

Discussed offline -> not changing this. Streamliend the "wrapper = " line
though.

+        wrapper.MarkPartiallyDependent(isolate);
+        isolate->SetObjectGroupId(wrapper, id);
+    }
 }
 
 // Regarding a minor GC algorithm for DOM nodes, see this document:
 // https://docs.google.com/a/google.com/presentation/d/1uifwVYGNYTZDoGLyCb7sXa7g49mWNMW2gaWvMN5NLk8/edit#slide=id.p
 class MinorGCWrapperVisitor : public v8::PersistentHandleVisitor {
 public:
     explicit MinorGCWrapperVisitor(v8::Isolate* isolate)
         : m_isolate(isolate)
     {
         UNUSED_PARAM(m_isolate);
@@ skipping 46 matching lines @@
         }
     }
 
 private:
     Vector<Node*> m_nodesInNewSpace;
     v8::Isolate* m_isolate;
 };
 
 class MajorGCWrapperVisitor : public v8::PersistentHandleVisitor {
 public:
-    explicit MajorGCWrapperVisitor(v8::Isolate* isolate)
-        : m_isolate(isolate)
+    explicit MajorGCWrapperVisitor(v8::Isolate* isolate, bool constructRetainedObjectInfos)
+        : m_isolate(isolate), m_liveRootGroupIdSet(false), m_constructRetainedObjectInfos(constructRetainedObjectInfos)

[haraken, 2013/04/25 14:55:54]

Nit: We normally write one initialization per line.

[marja, 2013/04/26 08:14:47]

Done.

     {
     }
 
     virtual void VisitPersistentHandle(v8::Persistent<v8::Value> value, uint16_t classId) OVERRIDE
     {
         ASSERT(value->IsObject());
         v8::Persistent<v8::Object> wrapper = v8::Persistent<v8::Object>::Cast(value);
 
         if (classId != v8DOMNodeClassId && classId != v8DOMObjectClassId)
             return;
 
         ASSERT(V8DOMWrapper::maybeDOMWrapper(value));
 
         if (value.IsIndependent(m_isolate))
             return;
 
         WrapperTypeInfo* type = toWrapperTypeInfo(wrapper);
         void* object = toNative(wrapper);
 
         if (V8MessagePort::info.equals(type)) {
             // Mark each port as in-use if it's entangled. For simplicity's sake,
             // we assume all ports are remotely entangled, since the Chromium port
             // implementation can't tell the difference.
             MessagePort* port = static_cast<MessagePort*>(object);
             if (port->isEntangled() || port->hasPendingActivity())
-                m_grouper.keepAlive(wrapper);
+                setObjectGroupId(wrapper, ensureLiveRoot());
         } else if (V8MutationObserver::info.equals(type)) {
             // FIXME: Allow opaqueRootForGC to operate on multiple roots and move this logic into V8MutationObserverCustom.
             MutationObserver* observer = static_cast<MutationObserver*>(object);
             HashSet<Node*> observedNodes = observer->getObservedNodes();
             for (HashSet<Node*>::iterator it = observedNodes.begin(); it != observedNodes.end(); ++it)
-                m_grouper.addImplicitReference(V8GCController::opaqueRootForGC(*it, m_isolate), wrapper);
+                m_isolate->SetReferenceFromGroup(v8::UniqueId(reinterpret_cast<intptr_t>(V8GCController::opaqueRootForGC(*it, m_isolate))), wrapper);

[haraken, 2013/04/25 14:55:54]

Nit: Shall we write this in two lines?

v8::UniqueId id(reinterpret_cast<intptr_t>(V8GCController::opaqueRootForGC(*it,
m_isolate));
m_isolate->SetReferenceFromGroup(id, wrapper);

BTW, why can't you simply use:

m_isolate->SetReference(V8GCController::opaqueRootForGC(*it, m_isolate),
wrapper);

?

[marja, 2013/04/26 08:14:47]

Offline discussion -> not going to change the logic from object groups to
implicit references.

Did the "UniqueId on its own line" change.

         } else {
             ActiveDOMObject* activeDOMObject = type->toActiveDOMObject(wrapper);
             if (activeDOMObject && activeDOMObject->hasPendingActivity())
-                m_grouper.keepAlive(wrapper);
+                setObjectGroupId(wrapper, ensureLiveRoot());
         }
 
         if (classId == v8DOMNodeClassId) {
             UNUSED_PARAM(m_isolate);
             ASSERT(V8Node::HasInstanceInAnyWorld(wrapper, m_isolate));
             ASSERT(!wrapper.IsIndependent(m_isolate));
 
             Node* node = static_cast<Node*>(object);
 
             if (node->hasEventListeners())
-                addImplicitReferencesForNodeWithEventListeners(node, wrapper);
-
-            m_grouper.addNodeWrapperToGroup(V8GCController::opaqueRootForGC(node, m_isolate), wrapper);
+                addImplicitReferencesForNodeWithEventListeners(m_isolate, node, wrapper);
+            Node* root = V8GCController::opaqueRootForGC(node, m_isolate);
+            setObjectGroupId(wrapper, root);
+            if (m_constructRetainedObjectInfos)
+                m_groupsWhichNeedRetainerInfo.append(root);
         } else if (classId == v8DOMObjectClassId) {
-            m_grouper.addObjectWrapperToGroup(type->opaqueRootForGC(object, wrapper, m_isolate), wrapper);
+            void* root = type->opaqueRootForGC(object, wrapper, m_isolate);
+            setObjectGroupId(wrapper, root);
         } else {
             ASSERT_NOT_REACHED();
         }
     }
 
-    void notifyFinished()
-    {
-        m_grouper.apply();
+    void notifyFinished() {
+        std::sort(m_groupsWhichNeedRetainerInfo.begin(), m_groupsWhichNeedRetainerInfo.end());
+        Node* already_added = 0;

[haraken, 2013/04/25 14:55:54]

Nit: already_added => alreadyAdded

[marja, 2013/04/26 08:14:47]

Done.

+        v8::HeapProfiler* profiler = m_isolate->GetHeapProfiler();
+        for (size_t i = 0; i < m_groupsWhichNeedRetainerInfo.size(); ++i) {
+            Node* root = m_groupsWhichNeedRetainerInfo[i];
+            if (root != already_added) {
+                profiler->SetRetainedObjectInfo(v8::UniqueId(reinterpret_cast<intptr_t>(root)), new RetainedDOMInfo(root));
+                already_added = root;
+            }
+        }
     }
 
 private:
-    WrapperGrouper m_grouper;
+    void setObjectGroupId(const v8::Persistent<v8::Object>& object, void* root)
+    {
+        v8::UniqueId id(reinterpret_cast<intptr_t>(root));
+        m_isolate->SetObjectGroupId(object, id);
+    }
+
+    void* ensureLiveRoot()
+    {
+        v8::Persistent<v8::Value> liveRoot = V8PerIsolateData::current()->ensureLiveRoot();

[haraken, 2013/04/25 14:55:54]

V8PerIsolateData::current() is heavy. You should use
V8PerIsolateData::from(Isolate*).

[marja, 2013/04/26 08:14:47]

Ahh, thanks for pointing that out. The previous version used
V8PerIsolateData::current() even though it could've used
V8PerIsolateData::from(Isolate*). *But* it did it only once, whereas this CL
does it repeatedly. Fixed.

+        if (!m_liveRootGroupIdSet) {
+            v8::UniqueId id(reinterpret_cast<intptr_t>(*liveRoot));
+            m_isolate->SetObjectGroupId(*liveRoot, id);
+            m_liveRootGroupIdSet = true;
+        }
+        return *liveRoot;

[haraken, 2013/04/25 14:55:54]

How about returning id from this method?

Currently you are doing something redundant: return a root Node from this method
=> pass the root Node to setObjectGroupId() => retrieve an ID from the root
Node.

[marja, 2013/04/26 08:14:47]

Done.

+    }
+
     v8::Isolate* m_isolate;
+    Vector<Node*> m_groupsWhichNeedRetainerInfo;
+    bool m_liveRootGroupIdSet;
+    bool m_constructRetainedObjectInfos;
 };
 
 void V8GCController::gcPrologue(v8::GCType type, v8::GCCallbackFlags flags)
 {
     // It would be nice if the GC callbacks passed the Isolate directly....
     v8::Isolate* isolate = v8::Isolate::GetCurrent();
     if (type == v8::kGCTypeScavenge)
         minorGCPrologue(isolate);
     else if (type == v8::kGCTypeMarkSweepCompact)
-        majorGCPrologue();
+        majorGCPrologue(flags & v8::kGCCallbackFlagConstructRetainedObjectInfos);
 }
 
 void V8GCController::minorGCPrologue(v8::Isolate* isolate)
 {
     TRACE_EVENT_BEGIN0("v8", "GC");
 
     if (isMainThread()) {
         v8::Isolate* isolate = v8::Isolate::GetCurrent();
         v8::HandleScope scope;
 
         MinorGCWrapperVisitor visitor(isolate);
         v8::V8::VisitHandlesForPartialDependence(isolate, &visitor);
         visitor.notifyFinished();
     }
 }
 
 // Create object groups for DOM tree nodes.
-void V8GCController::majorGCPrologue()
+void V8GCController::majorGCPrologue(bool constructRetainedObjectInfos)
 {
     TRACE_EVENT_BEGIN0("v8", "GC");
 
     v8::Isolate* isolate = v8::Isolate::GetCurrent();
     v8::HandleScope scope;
 
-    MajorGCWrapperVisitor visitor(isolate);
+    MajorGCWrapperVisitor visitor(isolate, constructRetainedObjectInfos);
     v8::V8::VisitHandlesWithClassIds(&visitor);
     visitor.notifyFinished();
 
     V8PerIsolateData::from(isolate)->stringCache()->clearOnGC();
 }
 
 static int workingSetEstimateMB = 0;
 
 static Mutex& workingSetEstimateMBMutex()
 {
@@ skipping 83 matching lines @@
         if (!script.IsEmpty()) {
             V8RecursionScope::MicrotaskSuppression scope;
             script->Run();
         }
     }
 
     context.clear();
 }
 
 }  // namespace WebCore