Changeset 122524 in webkit


Ignore:
Timestamp:
Jul 12, 2012 4:14:40 PM (12 years ago)
Author:
commit-queue@webkit.org
Message:

Unneeded tree walking when adding or removing children due to RenderCounter / RenderQuote logic
​https://bugs.webkit.org/show_bug.cgi?id=89900

Patch by Elliott Sprehn <Elliott Sprehn> on 2012-07-12
Reviewed by Eric Seidel and Abhishek Arya.

Previously we would walk the all children a renderer whenever adding
or removing a child renderer from its RenderObjectChildList to look for
RenderQuote and RenderCounter instances to update. This patch introduces
a counter in RenderView for the number of RenderQuote and RenderCounter
instances in that document so we can avoid these traversals.

No tests needed since this is just a short circuiting of logic and the existing
tests should cover it.

  • rendering/RenderCounter.cpp:

(WebCore::RenderCounter::RenderCounter): Increment instance counter.
(WebCore::RenderCounter::willBeDestroyed): Decrement instance counter.
(WebCore):
(WebCore::RenderCounter::rendererRemovedFromTree): Short circuit when counter is zero.
(WebCore::RenderCounter::rendererSubtreeAttached): Short circuit when counter is zero.

  • rendering/RenderCounter.h:

(RenderCounter):

  • rendering/RenderObjectChildList.cpp:

(WebCore::RenderObjectChildList::removeChildNode): Short circuit calling into Counter and Quote code when the document is being destroyed.

  • rendering/RenderQuote.cpp:

(WebCore::RenderQuote::RenderQuote):
(WebCore::RenderQuote::willBeDestroyed):
(WebCore):
(WebCore::RenderQuote::rendererSubtreeAttached): Increment instance counter.
(WebCore::RenderQuote::rendererRemovedFromTree): Decrement instance counter.

  • rendering/RenderQuote.h:

(RenderQuote):

  • rendering/RenderView.cpp:

(WebCore::RenderView::RenderView):

  • rendering/RenderView.h: Methods for managing the RenderQuote and RenderCounter counts.

(RenderView):
(WebCore::RenderView::addRenderQuote):
(WebCore::RenderView::removeRenderQuote):
(WebCore::RenderView::hasRenderQuotes):
(WebCore::RenderView::addRenderCounter):
(WebCore::RenderView::removeRenderCounter):
(WebCore::RenderView::hasRenderCounters):

Location:
trunk/Source/WebCore
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r122518 r122524  
     12012-07-12  Elliott Sprehn  <esprehn@gmail.com>
     2
     3        Unneeded tree walking when adding or removing children due to RenderCounter / RenderQuote logic
     4        https://bugs.webkit.org/show_bug.cgi?id=89900
     5
     6        Reviewed by Eric Seidel and Abhishek Arya.
     7
     8        Previously we would walk the all children a renderer whenever adding
     9        or removing a child renderer from its RenderObjectChildList to look for
     10        RenderQuote and RenderCounter instances to update. This patch introduces
     11        a counter in RenderView for the number of RenderQuote and RenderCounter
     12        instances in that document so we can avoid these traversals.
     13
     14        No tests needed since this is just a short circuiting of logic and the existing
     15        tests should cover it.
     16
     17        * rendering/RenderCounter.cpp:
     18        (WebCore::RenderCounter::RenderCounter): Increment instance counter.
     19        (WebCore::RenderCounter::willBeDestroyed): Decrement instance counter.
     20        (WebCore):
     21        (WebCore::RenderCounter::rendererRemovedFromTree): Short circuit when counter is zero.
     22        (WebCore::RenderCounter::rendererSubtreeAttached): Short circuit when counter is zero.
     23        * rendering/RenderCounter.h:
     24        (RenderCounter):
     25        * rendering/RenderObjectChildList.cpp:
     26        (WebCore::RenderObjectChildList::removeChildNode): Short circuit calling into Counter and Quote code when the document is being destroyed.
     27        * rendering/RenderQuote.cpp:
     28        (WebCore::RenderQuote::RenderQuote):
     29        (WebCore::RenderQuote::willBeDestroyed):
     30        (WebCore):
     31        (WebCore::RenderQuote::rendererSubtreeAttached): Increment instance counter.
     32        (WebCore::RenderQuote::rendererRemovedFromTree): Decrement instance counter.
     33        * rendering/RenderQuote.h:
     34        (RenderQuote):
     35        * rendering/RenderView.cpp:
     36        (WebCore::RenderView::RenderView):
     37        * rendering/RenderView.h: Methods for managing the RenderQuote and RenderCounter counts.
     38        (RenderView):
     39        (WebCore::RenderView::addRenderQuote):
     40        (WebCore::RenderView::removeRenderQuote):
     41        (WebCore::RenderView::hasRenderQuotes):
     42        (WebCore::RenderView::addRenderCounter):
     43        (WebCore::RenderView::removeRenderCounter):
     44        (WebCore::RenderView::hasRenderCounters):
     45
    1462012-07-12  Ryosuke Niwa  <rniwa@webkit.org>
    247

  • trunk/Source/WebCore/rendering/RenderCounter.cpp

    r115736 r122524  
    475475    , m_nextForSameCounter(0)
    476476{
     477    view()->addRenderCounter();
    477478}
    478479
    … …  
    483484        ASSERT(!m_counterNode);
    484485    }
     486}
     487
     488void RenderCounter::willBeDestroyed()
     489{
     490    if (view())
     491        view()->removeRenderCounter();
     492    RenderText::willBeDestroyed();
    485493}
    486494
    … …  
    597605}
    598606
    599 void RenderCounter::rendererRemovedFromTree(RenderObject* removedRenderer)
    600 {
    601     RenderObject* currentRenderer = removedRenderer->lastLeafChild();
     607void RenderCounter::rendererRemovedFromTree(RenderObject* renderer)
     608{
     609    ASSERT(renderer->view());
     610    if (!renderer->view()->hasRenderCounters())
     611        return;
     612    RenderObject* currentRenderer = renderer->lastLeafChild();
    602613    if (!currentRenderer)
    603         currentRenderer = removedRenderer;
     614        currentRenderer = renderer;
    604615    while (true) {
    605616        destroyCounterNodes(currentRenderer);
    606         if (currentRenderer == removedRenderer)
     617        if (currentRenderer == renderer)
    607618            break;
    608619        currentRenderer = currentRenderer->previousInPreOrder();
    … …  
    648659void RenderCounter::rendererSubtreeAttached(RenderObject* renderer)
    649660{
     661    ASSERT(renderer->view());
     662    if (!renderer->view()->hasRenderCounters())
     663        return;
    650664    Node* node = renderer->node();
    651665    if (node)

  • trunk/Source/WebCore/rendering/RenderCounter.h

    r86358 r122524  
    4141    static void rendererStyleChanged(RenderObject*, const RenderStyle* oldStyle, const RenderStyle* newStyle);
    4242
     43protected:
     44    virtual void willBeDestroyed();
     45
    4346private:
    4447    virtual const char* renderName() const;

  • trunk/Source/WebCore/rendering/RenderObjectChildList.cpp

    r122502 r122524  
    155155    oldChild->setParent(0);
    156156
    157     RenderCounter::rendererRemovedFromTree(oldChild);
    158     RenderQuote::rendererRemovedFromTree(oldChild);
     157    // rendererRemovedFromTree walks the whole subtree. We can improve performance
     158    // by skipping this step when destroying the entire tree.
     159    if (!owner->documentBeingDestroyed()) {
     160        RenderCounter::rendererRemovedFromTree(oldChild);
     161        RenderQuote::rendererRemovedFromTree(oldChild);
     162    }
    159163
    160164    if (AXObjectCache::accessibilityEnabled())

  • trunk/Source/WebCore/rendering/RenderQuote.cpp

    r116908 r122524  
    5858    , m_previous(0)
    5959{
     60    view()->addRenderQuote();
    6061}
    6162
    6263RenderQuote::~RenderQuote()
    6364{
     65}
     66
     67void RenderQuote::willBeDestroyed()
     68{
     69    if (view())
     70        view()->removeRenderQuote();
     71    RenderText::willBeDestroyed();
    6472}
    6573
    … …  
    279287void RenderQuote::rendererSubtreeAttached(RenderObject* renderer)
    280288{
    281     if (renderer->documentBeingDestroyed())
     289    ASSERT(renderer->view());
     290    if (!renderer->view()->hasRenderQuotes())
    282291        return;
    283292    for (RenderObject* descendant = renderer; descendant; descendant = descendant->nextInPreOrder(renderer))
    … …  
    288297}
    289298
    290 void RenderQuote::rendererRemovedFromTree(RenderObject* subtreeRoot)
    291 {
    292     if (subtreeRoot->documentBeingDestroyed())
     299void RenderQuote::rendererRemovedFromTree(RenderObject* renderer)
     300{
     301    ASSERT(renderer->view());
     302    if (!renderer->view()->hasRenderQuotes())
    293303        return;
    294     for (RenderObject* descendant = subtreeRoot; descendant; descendant = descendant->nextInPreOrder(subtreeRoot))
     304    for (RenderObject* descendant = renderer; descendant; descendant = descendant->nextInPreOrder(renderer))
    295305        if (descendant->isQuote()) {
    296306            RenderQuote* removedQuote = toRenderQuote(descendant);
    … …  
    298308            removedQuote->m_previous = 0;
    299309            int depth = removedQuote->m_depth;
    300             for (descendant = descendant->nextInPreOrder(subtreeRoot); descendant; descendant = descendant->nextInPreOrder(subtreeRoot))
     310            for (descendant = descendant->nextInPreOrder(renderer); descendant; descendant = descendant->nextInPreOrder(renderer))
    301311                if (descendant->isQuote())
    302312                    removedQuote = toRenderQuote(descendant);

  • trunk/Source/WebCore/rendering/RenderQuote.h

    r95901 r122524  
    3636protected:
    3737    virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle);
     38    virtual void willBeDestroyed();
    3839private:
    3940    virtual const char* renderName() const;

  • trunk/Source/WebCore/rendering/RenderView.cpp

    r120757 r122524  
    6363    , m_layoutState(0)
    6464    , m_layoutStateDisableCount(0)
     65    , m_renderQuoteCount(0)
     66    , m_renderCounterCount(0)
    6567{
    6668    // Clear our anonymous bit, set because RenderObject assumes

  • trunk/Source/WebCore/rendering/RenderView.h

    r120757 r122524  
    193193    void setFixedPositionedObjectsNeedLayout();
    194194
     195    // FIXME: This is a work around because the current implementation of counters and quotes
     196    // requires walking the entire tree repeatedly and most pages don't actually use either
     197    // feature so we shouldn't take the performance hit when not needed. Long term we should
     198    // rewrite the counter and quotes code.
     199    void addRenderQuote() { m_renderQuoteCount++; }
     200    void removeRenderQuote() { ASSERT(m_renderQuoteCount > 0); m_renderQuoteCount--; }
     201    bool hasRenderQuotes() { return m_renderQuoteCount; }
     202    void addRenderCounter() { m_renderCounterCount++; }
     203    void removeRenderCounter() { ASSERT(m_renderCounterCount > 0); m_renderCounterCount--; }
     204    bool hasRenderCounters() { return m_renderCounterCount; }
     205
    195206protected:
    196207    virtual void mapLocalToContainer(RenderBoxModelObject* repaintContainer, bool useTransforms, bool fixed, TransformState&, ApplyContainerFlipOrNot = ApplyContainerFlip, bool* wasFixed = 0) const;
    … …  
    288299    OwnPtr<FlowThreadController> m_flowThreadController;
    289300    RefPtr<IntervalArena> m_intervalArena;
     301
     302    unsigned m_renderQuoteCount;
     303    unsigned m_renderCounterCount;
    290304};
    291305
Note: See TracChangeset for help on using the changeset viewer.