- Revision
- 122524
- Author
- [email protected]
- Date
- 2012-07-12 16:14:40 -0700 (Thu, 12 Jul 2012)
Log 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 <[email protected]> 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):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (122523 => 122524)
--- trunk/Source/WebCore/ChangeLog 2012-07-12 23:13:02 UTC (rev 122523)
+++ trunk/Source/WebCore/ChangeLog 2012-07-12 23:14:40 UTC (rev 122524)
@@ -1,3 +1,48 @@
+2012-07-12 Elliott Sprehn <[email protected]>
+
+ Unneeded tree walking when adding or removing children due to RenderCounter / RenderQuote logic
+ https://bugs.webkit.org/show_bug.cgi?id=89900
+
+ 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):
+
2012-07-12 Ryosuke Niwa <[email protected]>
Merge HTMLCollectionWithArrayStorage into HTMLCollection
Modified: trunk/Source/WebCore/rendering/RenderCounter.cpp (122523 => 122524)
--- trunk/Source/WebCore/rendering/RenderCounter.cpp 2012-07-12 23:13:02 UTC (rev 122523)
+++ trunk/Source/WebCore/rendering/RenderCounter.cpp 2012-07-12 23:14:40 UTC (rev 122524)
@@ -474,6 +474,7 @@
, m_counterNode(0)
, m_nextForSameCounter(0)
{
+ view()->addRenderCounter();
}
RenderCounter::~RenderCounter()
@@ -484,6 +485,13 @@
}
}
+void RenderCounter::willBeDestroyed()
+{
+ if (view())
+ view()->removeRenderCounter();
+ RenderText::willBeDestroyed();
+}
+
const char* RenderCounter::renderName() const
{
return "RenderCounter";
@@ -596,14 +604,17 @@
// map associated with a renderer, so there is no risk in leaking the map.
}
-void RenderCounter::rendererRemovedFromTree(RenderObject* removedRenderer)
+void RenderCounter::rendererRemovedFromTree(RenderObject* renderer)
{
- RenderObject* currentRenderer = removedRenderer->lastLeafChild();
+ ASSERT(renderer->view());
+ if (!renderer->view()->hasRenderCounters())
+ return;
+ RenderObject* currentRenderer = renderer->lastLeafChild();
if (!currentRenderer)
- currentRenderer = removedRenderer;
+ currentRenderer = renderer;
while (true) {
destroyCounterNodes(currentRenderer);
- if (currentRenderer == removedRenderer)
+ if (currentRenderer == renderer)
break;
currentRenderer = currentRenderer->previousInPreOrder();
}
@@ -647,6 +658,9 @@
void RenderCounter::rendererSubtreeAttached(RenderObject* renderer)
{
+ ASSERT(renderer->view());
+ if (!renderer->view()->hasRenderCounters())
+ return;
Node* node = renderer->node();
if (node)
node = node->parentNode();
Modified: trunk/Source/WebCore/rendering/RenderCounter.h (122523 => 122524)
--- trunk/Source/WebCore/rendering/RenderCounter.h 2012-07-12 23:13:02 UTC (rev 122523)
+++ trunk/Source/WebCore/rendering/RenderCounter.h 2012-07-12 23:14:40 UTC (rev 122524)
@@ -40,6 +40,9 @@
static void rendererRemovedFromTree(RenderObject*);
static void rendererStyleChanged(RenderObject*, const RenderStyle* oldStyle, const RenderStyle* newStyle);
+protected:
+ virtual void willBeDestroyed();
+
private:
virtual const char* renderName() const;
virtual bool isCounter() const;
Modified: trunk/Source/WebCore/rendering/RenderObjectChildList.cpp (122523 => 122524)
--- trunk/Source/WebCore/rendering/RenderObjectChildList.cpp 2012-07-12 23:13:02 UTC (rev 122523)
+++ trunk/Source/WebCore/rendering/RenderObjectChildList.cpp 2012-07-12 23:14:40 UTC (rev 122524)
@@ -154,8 +154,12 @@
oldChild->setNextSibling(0);
oldChild->setParent(0);
- RenderCounter::rendererRemovedFromTree(oldChild);
- RenderQuote::rendererRemovedFromTree(oldChild);
+ // rendererRemovedFromTree walks the whole subtree. We can improve performance
+ // by skipping this step when destroying the entire tree.
+ if (!owner->documentBeingDestroyed()) {
+ RenderCounter::rendererRemovedFromTree(oldChild);
+ RenderQuote::rendererRemovedFromTree(oldChild);
+ }
if (AXObjectCache::accessibilityEnabled())
owner->document()->axObjectCache()->childrenChanged(owner);
Modified: trunk/Source/WebCore/rendering/RenderQuote.cpp (122523 => 122524)
--- trunk/Source/WebCore/rendering/RenderQuote.cpp 2012-07-12 23:13:02 UTC (rev 122523)
+++ trunk/Source/WebCore/rendering/RenderQuote.cpp 2012-07-12 23:14:40 UTC (rev 122524)
@@ -57,12 +57,20 @@
, m_next(0)
, m_previous(0)
{
+ view()->addRenderQuote();
}
RenderQuote::~RenderQuote()
{
}
+void RenderQuote::willBeDestroyed()
+{
+ if (view())
+ view()->removeRenderQuote();
+ RenderText::willBeDestroyed();
+}
+
const char* RenderQuote::renderName() const
{
return "RenderQuote";
@@ -278,7 +286,8 @@
void RenderQuote::rendererSubtreeAttached(RenderObject* renderer)
{
- if (renderer->documentBeingDestroyed())
+ ASSERT(renderer->view());
+ if (!renderer->view()->hasRenderQuotes())
return;
for (RenderObject* descendant = renderer; descendant; descendant = descendant->nextInPreOrder(renderer))
if (descendant->isQuote()) {
@@ -287,17 +296,18 @@
}
}
-void RenderQuote::rendererRemovedFromTree(RenderObject* subtreeRoot)
+void RenderQuote::rendererRemovedFromTree(RenderObject* renderer)
{
- if (subtreeRoot->documentBeingDestroyed())
+ ASSERT(renderer->view());
+ if (!renderer->view()->hasRenderQuotes())
return;
- for (RenderObject* descendant = subtreeRoot; descendant; descendant = descendant->nextInPreOrder(subtreeRoot))
+ for (RenderObject* descendant = renderer; descendant; descendant = descendant->nextInPreOrder(renderer))
if (descendant->isQuote()) {
RenderQuote* removedQuote = toRenderQuote(descendant);
RenderQuote* lastQuoteBefore = removedQuote->m_previous;
removedQuote->m_previous = 0;
int depth = removedQuote->m_depth;
- for (descendant = descendant->nextInPreOrder(subtreeRoot); descendant; descendant = descendant->nextInPreOrder(subtreeRoot))
+ for (descendant = descendant->nextInPreOrder(renderer); descendant; descendant = descendant->nextInPreOrder(renderer))
if (descendant->isQuote())
removedQuote = toRenderQuote(descendant);
RenderQuote* quoteAfter = removedQuote->m_next;
Modified: trunk/Source/WebCore/rendering/RenderQuote.h (122523 => 122524)
--- trunk/Source/WebCore/rendering/RenderQuote.h 2012-07-12 23:13:02 UTC (rev 122523)
+++ trunk/Source/WebCore/rendering/RenderQuote.h 2012-07-12 23:14:40 UTC (rev 122524)
@@ -35,6 +35,7 @@
static void rendererRemovedFromTree(RenderObject*);
protected:
virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle);
+ virtual void willBeDestroyed();
private:
virtual const char* renderName() const;
virtual bool isQuote() const { return true; };
Modified: trunk/Source/WebCore/rendering/RenderView.cpp (122523 => 122524)
--- trunk/Source/WebCore/rendering/RenderView.cpp 2012-07-12 23:13:02 UTC (rev 122523)
+++ trunk/Source/WebCore/rendering/RenderView.cpp 2012-07-12 23:14:40 UTC (rev 122524)
@@ -62,6 +62,8 @@
, m_pageLogicalHeightChanged(false)
, m_layoutState(0)
, m_layoutStateDisableCount(0)
+ , m_renderQuoteCount(0)
+ , m_renderCounterCount(0)
{
// Clear our anonymous bit, set because RenderObject assumes
// any renderer with document as the node is anonymous.
Modified: trunk/Source/WebCore/rendering/RenderView.h (122523 => 122524)
--- trunk/Source/WebCore/rendering/RenderView.h 2012-07-12 23:13:02 UTC (rev 122523)
+++ trunk/Source/WebCore/rendering/RenderView.h 2012-07-12 23:14:40 UTC (rev 122524)
@@ -192,6 +192,17 @@
void setFixedPositionedObjectsNeedLayout();
+ // FIXME: This is a work around because the current implementation of counters and quotes
+ // requires walking the entire tree repeatedly and most pages don't actually use either
+ // feature so we shouldn't take the performance hit when not needed. Long term we should
+ // rewrite the counter and quotes code.
+ void addRenderQuote() { m_renderQuoteCount++; }
+ void removeRenderQuote() { ASSERT(m_renderQuoteCount > 0); m_renderQuoteCount--; }
+ bool hasRenderQuotes() { return m_renderQuoteCount; }
+ void addRenderCounter() { m_renderCounterCount++; }
+ void removeRenderCounter() { ASSERT(m_renderCounterCount > 0); m_renderCounterCount--; }
+ bool hasRenderCounters() { return m_renderCounterCount; }
+
protected:
virtual void mapLocalToContainer(RenderBoxModelObject* repaintContainer, bool useTransforms, bool fixed, TransformState&, ApplyContainerFlipOrNot = ApplyContainerFlip, bool* wasFixed = 0) const;
virtual const RenderObject* pushMappingToContainer(const RenderBoxModelObject* ancestorToStopAt, RenderGeometryMap&) const;
@@ -287,6 +298,9 @@
#endif
OwnPtr<FlowThreadController> m_flowThreadController;
RefPtr<IntervalArena> m_intervalArena;
+
+ unsigned m_renderQuoteCount;
+ unsigned m_renderCounterCount;
};
inline RenderView* toRenderView(RenderObject* object)
