Changeset 142419 in webkit


Ignore:
Timestamp:
Feb 10, 2013 5:16:19 PM (11 years ago)
Author:
haraken@chromium.org
Message:

[V8] Remove V8GCController::m_edenNodes and make minor DOM GC more precise
https://bugs.webkit.org/show_bug.cgi?id=108579

Reviewed by Adam Barth.

Currently V8GCController::m_edenNodes stores a list of nodes whose
wrappers have been created since the latest GC. The reason why we
needed m_edenNodes is that there was no way to know a list of wrappers
in the new space of V8. By using m_edenNodes, we had been approximating
'wrappers in the new space' by 'wrappers that have been created since
the latest GC'.

Now V8 provides VisitHandlesForPartialDependence(), with which WebKit
can know a list of wrappers in the new space. By using the API, we can
remove V8GCController::m_edenNodes. The benefit is that (1) we no longer
need to keep m_edenNodes and that (2) it enables more precise minor
DOM GC (Remember that m_edenNodes was just an approximation).

Performance benchmark: https://bugs.webkit.org/attachment.cgi?id=185940
The benchmark runs 300 iterations, each of which creates 100000 elements.
The benchmark measures average, min, median, max and stdev of execution times
of the 300 iterations. This will tell us the worst-case overhead of this change.

Before:

mean=59.91ms, min=39ms, median=42ms, max=258ms, stdev=47.48ms

After:

mean=58.75ms, min=35ms, median=41ms, max=250ms, stdev=47.32ms

As shown above, I couldn't observe any performance regression.

No tests. No change in behavior.

  • bindings/v8/DOMDataStore.h:

(WebCore::DOMDataStore::setWrapperInObject):

  • bindings/v8/DOMWrapperWorld.h:

(DOMWrapperWorld):
(WebCore::DOMWrapperWorld::getWorldWithoutContextCheck):

  • bindings/v8/V8Binding.h:

(WebCore):
(WebCore::worldForEnteredContextIfIsolated):
(WebCore::worldForEnteredContextWithoutContextCheck):

  • bindings/v8/V8DOMWindowShell.cpp:

(WebCore::V8DOMWindowShell::initializeIfNeeded):

  • bindings/v8/V8GCController.cpp:

(WebCore::gcTree):
(WebCore):
(MinorGCWrapperVisitor):
(WebCore::MinorGCWrapperVisitor::MinorGCWrapperVisitor):
(WebCore::MinorGCWrapperVisitor::notifyFinished):
(WebCore::MajorGCWrapperVisitor::MajorGCWrapperVisitor):
(WebCore::V8GCController::gcPrologue):
(WebCore::V8GCController::minorGCPrologue):
(WebCore::V8GCController::majorGCPrologue):

  • bindings/v8/V8GCController.h:

(V8GCController):

Location:
trunk/Source/WebCore
Files:
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r142416 r142419  
     12013-02-04  Kentaro Hara  <haraken@chromium.org>
     2
     3        [V8] Remove V8GCController::m_edenNodes and make minor DOM GC more precise
     4        https://bugs.webkit.org/show_bug.cgi?id=108579
     5
     6        Reviewed by Adam Barth.
     7
     8        Currently V8GCController::m_edenNodes stores a list of nodes whose
     9        wrappers have been created since the latest GC. The reason why we
     10        needed m_edenNodes is that there was no way to know a list of wrappers
     11        in the new space of V8. By using m_edenNodes, we had been approximating
     12        'wrappers in the new space' by 'wrappers that have been created since
     13        the latest GC'.
     14
     15        Now V8 provides VisitHandlesForPartialDependence(), with which WebKit
     16        can know a list of wrappers in the new space. By using the API, we can
     17        remove V8GCController::m_edenNodes. The benefit is that (1) we no longer
     18        need to keep m_edenNodes and that (2) it enables more precise minor
     19        DOM GC (Remember that m_edenNodes was just an approximation).
     20
     21        Performance benchmark: https://bugs.webkit.org/attachment.cgi?id=185940
     22        The benchmark runs 300 iterations, each of which creates 100000 elements.
     23        The benchmark measures average, min, median, max and stdev of execution times
     24        of the 300 iterations. This will tell us the worst-case overhead of this change.
     25
     26        Before:
     27          mean=59.91ms, min=39ms, median=42ms, max=258ms, stdev=47.48ms
     28
     29        After:
     30          mean=58.75ms, min=35ms, median=41ms, max=250ms, stdev=47.32ms
     31
     32        As shown above, I couldn't observe any performance regression.
     33
     34        No tests. No change in behavior.
     35
     36        * bindings/v8/DOMDataStore.h:
     37        (WebCore::DOMDataStore::setWrapperInObject):
     38        * bindings/v8/DOMWrapperWorld.h:
     39        (DOMWrapperWorld):
     40        (WebCore::DOMWrapperWorld::getWorldWithoutContextCheck):
     41        * bindings/v8/V8Binding.h:
     42        (WebCore):
     43        (WebCore::worldForEnteredContextIfIsolated):
     44        (WebCore::worldForEnteredContextWithoutContextCheck):
     45        * bindings/v8/V8DOMWindowShell.cpp:
     46        (WebCore::V8DOMWindowShell::initializeIfNeeded):
     47        * bindings/v8/V8GCController.cpp:
     48        (WebCore::gcTree):
     49        (WebCore):
     50        (MinorGCWrapperVisitor):
     51        (WebCore::MinorGCWrapperVisitor::MinorGCWrapperVisitor):
     52        (WebCore::MinorGCWrapperVisitor::notifyFinished):
     53        (WebCore::MajorGCWrapperVisitor::MajorGCWrapperVisitor):
     54        (WebCore::V8GCController::gcPrologue):
     55        (WebCore::V8GCController::minorGCPrologue):
     56        (WebCore::V8GCController::majorGCPrologue):
     57        * bindings/v8/V8GCController.h:
     58        (V8GCController):
     59
    1602013-02-10  Tim Horton  <timothy_horton@apple.com>
    261
  • trunk/Source/WebCore/bindings/v8/DOMDataStore.h

    r142159 r142419  
    163163        object->setWrapper(wrapper, isolate, configuration);
    164164    }
    165     static void setWrapperInObject(Node* object, v8::Persistent<v8::Object> wrapper, v8::Isolate* isolate, const WrapperConfiguration& configuration)
    166     {
    167         object->setWrapper(wrapper, isolate, configuration);
    168         V8GCController::didCreateWrapperForNode(object);
    169     }
    170165
    171166    Type m_type;
  • trunk/Source/WebCore/bindings/v8/DOMWrapperWorld.h

    r142209 r142419  
    6363
    6464    static void assertContextHasCorrectPrototype(v8::Handle<v8::Context>);
     65    // FIXME: Rename this method to getWorld().
    6566    static DOMWrapperWorld* isolated(v8::Handle<v8::Context> context)
    6667    {
    6768        assertContextHasCorrectPrototype(context);
     69        return static_cast<DOMWrapperWorld*>(context->GetAlignedPointerFromEmbedderData(v8ContextIsolatedWorld));
     70    }
     71    static DOMWrapperWorld* getWorldWithoutContextCheck(v8::Handle<v8::Context> context)
     72    {
    6873        return static_cast<DOMWrapperWorld*>(context->GetAlignedPointerFromEmbedderData(v8ContextIsolatedWorld));
    6974    }
  • trunk/Source/WebCore/bindings/v8/V8Binding.h

    r142250 r142419  
    438438    Frame* toFrameIfNotDetached(v8::Handle<v8::Context>);
    439439
     440    // FIXME: Rename this method to worldForEnteredContext().
    440441    inline DOMWrapperWorld* worldForEnteredContextIfIsolated()
    441442    {
    442         if (!v8::Context::InContext())
     443        v8::Handle<v8::Context> context = v8::Context::GetEntered();
     444        if (context.IsEmpty())
    443445            return 0;
    444         return DOMWrapperWorld::isolated(v8::Context::GetEntered());
     446        return DOMWrapperWorld::isolated(context);
     447    }
     448
     449    // This is a slightly different version of worldForEnteredContext().
     450    // The difference is just that worldForEnteredContextWithoutContextCheck()
     451    // does not call assertContextHasCorrectPrototype() (which is enabled on
     452    // Debug builds only). Because assertContextHasCorrectPrototype() crashes
     453    // if it is called when a current context is not completely initialized,
     454    // you have to use worldForEnteredContextWithoutContextCheck() if you need
     455    // to get a DOMWrapperWorld while a current context is being initialized.
     456    // See https://bugs.webkit.org/show_bug.cgi?id=108579#c15 for more details.
     457    inline DOMWrapperWorld* worldForEnteredContextWithoutContextCheck()
     458    {
     459        v8::Handle<v8::Context> context = v8::Context::GetEntered();
     460        if (context.IsEmpty())
     461            return 0;
     462        return DOMWrapperWorld::getWorldWithoutContextCheck(context);
    445463    }
    446464
  • trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.cpp

    r142159 r142419  
    209209        return false;
    210210
     211    m_world->setIsolatedWorldField(m_context.get());
     212
    211213    bool isMainWorld = m_world->isMainWorld();
    212214
    213215    v8::Local<v8::Context> context = v8::Local<v8::Context>::New(m_context.get());
    214216    v8::Context::Scope contextScope(context);
    215 
    216     m_world->setIsolatedWorldField(m_context.get());
    217217
    218218    if (m_global.isEmpty()) {
  • trunk/Source/WebCore/bindings/v8/V8GCController.cpp

    r142250 r142419  
    179179}
    180180
    181 class WrapperVisitor : public v8::PersistentHandleVisitor {
     181static void gcTree(v8::Isolate* isolate, Node* startNode)
     182{
     183    Vector<v8::Persistent<v8::Value>, initialNodeVectorSize> newSpaceWrappers;
     184
     185    // We traverse a DOM tree in the DFS order starting from startNode.
     186    // The traversal order does not matter for correctness but does matter for performance.
     187    Node* node = startNode;
     188    // To make each minor GC time bounded, we might need to give up
     189    // traversing at some point for a large DOM tree. That being said,
     190    // I could not observe the need even in pathological test cases.
     191    do {
     192        ASSERT(node);
     193        if (!node->wrapper().IsEmpty()) {
     194            // FIXME: Remove the special handling for image elements.
     195            // The same special handling is in V8GCController::opaqueRootForGC().
     196            // Maybe should image elements be active DOM nodes?
     197            // See https://code.google.com/p/chromium/issues/detail?id=164882
     198            if (!node->isV8CollectableDuringMinorGC() || (node->hasTagName(HTMLNames::imgTag) && static_cast<HTMLImageElement*>(node)->hasPendingActivity())) {
     199                // This node is not in the new space of V8. This indicates that
     200                // the minor GC cannot anyway judge reachability of this DOM tree.
     201                // Thus we give up traversing the DOM tree.
     202                return;
     203            }
     204            node->setV8CollectableDuringMinorGC(false);
     205            newSpaceWrappers.append(node->wrapper());
     206        }
     207        if (node->firstChild()) {
     208            node = node->firstChild();
     209            continue;
     210        }
     211        while (!node->nextSibling()) {
     212            if (!node->parentNode())
     213                break;
     214            node = node->parentNode();
     215        }
     216        if (node->parentNode())
     217            node = node->nextSibling();
     218    } while (node != startNode);
     219
     220    // We completed the DOM tree traversal. All wrappers in the DOM tree are
     221    // stored in newSpaceWrappers and are expected to exist in the new space of V8.
     222    // We report those wrappers to V8 as an object group.
     223    for (size_t i = 0; i < newSpaceWrappers.size(); i++)
     224        newSpaceWrappers[i].MarkPartiallyDependent(isolate);
     225    if (newSpaceWrappers.size() > 0)
     226        v8::V8::AddObjectGroup(&newSpaceWrappers[0], newSpaceWrappers.size());
     227}
     228
     229// Regarding a minor GC algorithm for DOM nodes, see this document:
     230// https://docs.google.com/a/google.com/presentation/d/1uifwVYGNYTZDoGLyCb7sXa7g49mWNMW2gaWvMN5NLk8/edit#slide=id.p
     231class MinorGCWrapperVisitor : public v8::PersistentHandleVisitor {
    182232public:
    183     explicit WrapperVisitor(v8::Isolate* isolate)
     233    explicit MinorGCWrapperVisitor(v8::Isolate* isolate)
     234        : m_isolate(isolate)
     235    {
     236        UNUSED_PARAM(m_isolate);
     237    }
     238
     239    virtual void VisitPersistentHandle(v8::Persistent<v8::Value> value, uint16_t classId) OVERRIDE
     240    {
     241        // A minor DOM GC can collect only Nodes.
     242        if (classId != v8DOMNodeClassId)
     243            return;
     244
     245        // To make minor GC cycle time bounded, we limit the number of wrappers handled
     246        // by each minor GC cycle to 10000. This value was selected so that the minor
     247        // GC cycle time is bounded to 20 ms in a case where the new space size
     248        // is 16 MB and it is full of wrappers (which is almost the worst case).
     249        // Practically speaking, as far as I crawled real web applications,
     250        // the number of wrappers handled by each minor GC cycle is at most 3000.
     251        // So this limit is mainly for pathological micro benchmarks.
     252        const unsigned wrappersHandledByEachMinorGC = 10000;
     253        if (m_nodesInNewSpace.size() >= wrappersHandledByEachMinorGC)
     254            return;
     255
     256        ASSERT(value->IsObject());
     257        v8::Persistent<v8::Object> wrapper = v8::Persistent<v8::Object>::Cast(value);
     258        ASSERT(V8DOMWrapper::maybeDOMWrapper(value));
     259        ASSERT(V8Node::HasInstance(wrapper, m_isolate));
     260        Node* node = V8Node::toNative(wrapper);
     261        m_nodesInNewSpace.append(node);
     262        node->setV8CollectableDuringMinorGC(true);
     263    }
     264
     265    void notifyFinished()
     266    {
     267        for (size_t i = 0; i < m_nodesInNewSpace.size(); i++) {
     268            ASSERT(!m_nodesInNewSpace[i]->wrapper().IsEmpty());
     269            if (m_nodesInNewSpace[i]->isV8CollectableDuringMinorGC()) // This branch is just for performance.
     270                gcTree(m_isolate, m_nodesInNewSpace[i]);
     271        }
     272    }
     273
     274private:
     275    Vector<Node*> m_nodesInNewSpace;
     276    v8::Isolate* m_isolate;
     277};
     278
     279class MajorGCWrapperVisitor : public v8::PersistentHandleVisitor {
     280public:
     281    explicit MajorGCWrapperVisitor(v8::Isolate* isolate)
    184282        : m_isolate(isolate)
    185283    {
     
    249347};
    250348
    251 // Regarding a minor GC algorithm for DOM nodes, see this document:
    252 // https://docs.google.com/a/google.com/presentation/d/1uifwVYGNYTZDoGLyCb7sXa7g49mWNMW2gaWvMN5NLk8/edit#slide=id.p
    253 //
    254 // m_edenNodes stores nodes that have wrappers that have been created since the last minor/major GC.
    255 Vector<Node*>* V8GCController::m_edenNodes = 0;
    256 
    257 static void gcTree(v8::Isolate* isolate, Node* startNode)
    258 {
    259     Vector<v8::Persistent<v8::Value>, initialNodeVectorSize> newSpaceWrappers;
    260 
    261     // We traverse a DOM tree in the DFS order starting from startNode.
    262     // The traversal order does not matter for correctness but does matter for performance.
    263     Node* node = startNode;
    264     // To make each minor GC time bounded, we might need to give up
    265     // traversing at some point for a large DOM tree. That being said,
    266     // I could not observe the need even in pathological test cases.
    267     do {
    268         ASSERT(node);
    269         if (!node->wrapper().IsEmpty()) {
    270             // FIXME: Remove the special handling for image elements.
    271             // The same special handling is in V8GCController::opaqueRootForGC().
    272             // Maybe should image elements be active DOM nodes?
    273             // See https://code.google.com/p/chromium/issues/detail?id=164882
    274             if (!node->isV8CollectableDuringMinorGC() || (node->hasTagName(HTMLNames::imgTag) && static_cast<HTMLImageElement*>(node)->hasPendingActivity())) {
    275                 // The fact that we encounter a node that is not in the Eden space
    276                 // implies that its wrapper might be in the old space of V8.
    277                 // This indicates that the minor GC cannot anyway judge reachability
    278                 // of this DOM tree. Thus we give up traversing the DOM tree.
    279                 return;
    280             }
    281             // A once traversed node is removed from the Eden space.
    282             node->setV8CollectableDuringMinorGC(false);
    283             newSpaceWrappers.append(node->wrapper());
    284         }
    285         if (node->firstChild()) {
    286             node = node->firstChild();
    287             continue;
    288         }
    289         while (!node->nextSibling()) {
    290             if (!node->parentNode())
    291                 break;
    292             node = node->parentNode();
    293         }
    294         if (node->parentNode())
    295             node = node->nextSibling();
    296     } while (node != startNode);
    297 
    298     // We completed the DOM tree traversal. All wrappers in the DOM tree are
    299     // stored in newSpaceWrappers and are expected to exist in the new space of V8.
    300     // We report those wrappers to V8 as an object group.
    301     for (size_t i = 0; i < newSpaceWrappers.size(); i++)
    302         newSpaceWrappers[i].MarkPartiallyDependent(isolate);
    303     if (newSpaceWrappers.size() > 0)
    304         v8::V8::AddObjectGroup(&newSpaceWrappers[0], newSpaceWrappers.size());
    305 }
    306 
    307 void V8GCController::didCreateWrapperForNode(Node* node)
    308 {
    309     // To make minor GC cycle time bounded, we limit the number of wrappers handled
    310     // by each minor GC cycle to 10000. This value was selected so that the minor
    311     // GC cycle time is bounded to 20 ms in a case where the new space size
    312     // is 16 MB and it is full of wrappers (which is almost the worst case).
    313     // Practically speaking, as far as I crawled real web applications,
    314     // the number of wrappers handled by each minor GC cycle is at most 3000.
    315     // So this limit is mainly for pathological micro benchmarks.
    316     const unsigned wrappersHandledByEachMinorGC = 10000;
    317     ASSERT(!node->wrapper().IsEmpty());
    318     if (!m_edenNodes)
    319         m_edenNodes = adoptPtr(new Vector<Node*>).leakPtr();
    320     if (m_edenNodes->size() <= wrappersHandledByEachMinorGC) {
    321         // A node of a newly created wrapper is put into the Eden space.
    322         m_edenNodes->append(node);
    323         node->setV8CollectableDuringMinorGC(true);
    324     }
    325 }
    326 
    327349void V8GCController::gcPrologue(v8::GCType type, v8::GCCallbackFlags flags)
    328350{
     
    333355    else if (type == v8::kGCTypeMarkSweepCompact)
    334356        majorGCPrologue();
    335 
    336     if (isMainThreadOrGCThread() && m_edenNodes) {
    337         // The Eden space is cleared at every minor/major GC.
    338         m_edenNodes->clear();
    339     }
    340357}
    341358
     
    344361    TRACE_EVENT_BEGIN0("v8", "GC");
    345362
    346     if (isMainThreadOrGCThread() && m_edenNodes) {
    347         for (size_t i = 0; i < m_edenNodes->size(); i++) {
    348             ASSERT(!m_edenNodes->at(i)->wrapper().IsEmpty());
    349             if (m_edenNodes->at(i)->isV8CollectableDuringMinorGC()) // This branch is just for performance.
    350                 gcTree(isolate, m_edenNodes->at(i));
     363    if (isMainThread()) {
     364        v8::Isolate* isolate = v8::Isolate::GetCurrent();
     365        v8::HandleScope scope;
     366
     367        // A minor GC can handle the main world only.
     368        DOMWrapperWorld* world = worldForEnteredContextWithoutContextCheck();
     369        if (world && world->isMainWorld()) {
     370            MinorGCWrapperVisitor visitor(isolate);
     371            v8::V8::VisitHandlesForPartialDependence(isolate, &visitor);
     372            visitor.notifyFinished();
    351373        }
    352374    }
     
    361383    v8::HandleScope scope;
    362384
    363     WrapperVisitor visitor(isolate);
     385    MajorGCWrapperVisitor visitor(isolate);
    364386    v8::V8::VisitHandlesWithClassIds(&visitor);
    365387    visitor.notifyFinished();
  • trunk/Source/WebCore/bindings/v8/V8GCController.h

    r142250 r142419  
    5353
    5454    static Node* opaqueRootForGC(Node*, v8::Isolate*);
    55     static void didCreateWrapperForNode(Node*);
    5655
    5756private:
Note: See TracChangeset for help on using the changeset viewer.