REGRESSION (r166422): All RenderBox objects grew 104 bytes from addin…

…g repaint timers. <https://webkit.org/b/133027> <rdar://problem/16867410> Instead of storing a rarely-used repaint timer on every RenderBox, store one on the RenderView, and keep a hash set of renderers needing repaint. Renderers get a flag tracking whether they have a pending lazy repaint. This way we can avoid hash lookups in the common case. Also added a static assertion to catch RenderBox growing in the future. Reviewed by Antti Koivisto. * rendering/RenderBox.cpp: (WebCore::SameSizeAsRenderBox::~SameSizeAsRenderBox): (WebCore::RenderBox::RenderBox): (WebCore::RenderBox::~RenderBox): (WebCore::RenderBox::paintBoxDecorations): (WebCore::RenderBox::layoutOverflowRectForPropagation): * rendering/RenderBox.h: * rendering/RenderElement.cpp: (WebCore::RenderElement::RenderElement): * rendering/RenderElement.h: (WebCore::RenderElement::setRenderBoxNeedsLazyRepaint): (WebCore::RenderElement::renderBoxNeedsLazyRepaint): * rendering/RenderView.cpp: (WebCore::RenderView::RenderView): (WebCore::RenderView::scheduleLazyRepaint): (WebCore::RenderView::unscheduleLazyRepaint): (WebCore::RenderView::lazyRepaintTimerFired): * rendering/RenderView.h: Canonical link: https://commits.webkit.org/151057@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@168993 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · May 17, 2014 · 4801aeb · 4801aeb
1 parent 32e1517
commit 4801aeb
Show file tree

Hide file tree

Showing 7 changed files with 94 additions and 14 deletions.
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,38 @@
+2014-05-17  Andreas Kling  <akling@apple.com>
+
+        REGRESSION (r166422): All RenderBox objects grew 104 bytes from adding repaint timers.
+        <https://webkit.org/b/133027>
+        <rdar://problem/16867410>
+
+        Instead of storing a rarely-used repaint timer on every RenderBox, store one
+        on the RenderView, and keep a hash set of renderers needing repaint.
+
+        Renderers get a flag tracking whether they have a pending lazy repaint.
+        This way we can avoid hash lookups in the common case.
+
+        Also added a static assertion to catch RenderBox growing in the future.
+
+        Reviewed by Antti Koivisto.
+
+        * rendering/RenderBox.cpp:
+        (WebCore::SameSizeAsRenderBox::~SameSizeAsRenderBox):
+        (WebCore::RenderBox::RenderBox):
+        (WebCore::RenderBox::~RenderBox):
+        (WebCore::RenderBox::paintBoxDecorations):
+        (WebCore::RenderBox::layoutOverflowRectForPropagation):
+        * rendering/RenderBox.h:
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::RenderElement):
+        * rendering/RenderElement.h:
+        (WebCore::RenderElement::setRenderBoxNeedsLazyRepaint):
+        (WebCore::RenderElement::renderBoxNeedsLazyRepaint):
+        * rendering/RenderView.cpp:
+        (WebCore::RenderView::RenderView):
+        (WebCore::RenderView::scheduleLazyRepaint):
+        (WebCore::RenderView::unscheduleLazyRepaint):
+        (WebCore::RenderView::lazyRepaintTimerFired):
+        * rendering/RenderView.h:
+
 2014-05-16  Jer Noble  <jer.noble@apple.com>
 
         [Mac][MSE] setCurrentTime() goes down fastSeek path in MediaPlayerPrivateMediaSourceAVFObjC.

diff --git a/Source/WebCore/rendering/RenderBox.cpp b/Source/WebCore/rendering/RenderBox.cpp
@@ -67,6 +67,16 @@
 
 namespace WebCore {
 
+struct SameSizeAsRenderBox : public RenderBoxModelObject {
+    virtual ~SameSizeAsRenderBox() { }
+    LayoutRect frameRect;
+    LayoutBoxExtent marginBox;
+    LayoutUnit preferredLogicalWidths[2];
+    void* pointers[2];
+};
+
+COMPILE_ASSERT(sizeof(RenderBox) == sizeof(SameSizeAsRenderBox), RenderBox_should_stay_small);
+
 using namespace HTMLNames;
 
 // Used by flexible boxes when flexing this element and by table cells.
@@ -103,7 +113,6 @@ RenderBox::RenderBox(Element& element, PassRef<RenderStyle> style, unsigned base
     , m_minPreferredLogicalWidth(-1)
     , m_maxPreferredLogicalWidth(-1)
     , m_inlineBoxWrapper(0)
-    , m_repaintTimer(this, &RenderBox::repaintTimerFired)
 {
     setIsBox();
 }
@@ -113,14 +122,13 @@ RenderBox::RenderBox(Document& document, PassRef<RenderStyle> style, unsigned ba
     , m_minPreferredLogicalWidth(-1)
     , m_maxPreferredLogicalWidth(-1)
     , m_inlineBoxWrapper(0)
-    , m_repaintTimer(this, &RenderBox::repaintTimerFired)
 {
     setIsBox();
 }
 
 RenderBox::~RenderBox()
 {
-    m_repaintTimer.stop();
+    view().unscheduleLazyRepaint(*this);
     if (hasControlStatesForRenderer(this))
         removeControlStatesForRenderer(this);
 }
@@ -1255,7 +1263,7 @@ void RenderBox::paintBoxDecorations(PaintInfo& paintInfo, const LayoutPoint& pai
     bool themePainted = style().hasAppearance() && !theme().paint(*this, controlStates, paintInfo, paintRect);
 
     if (controlStates && controlStates->needsRepaint())
-        m_repaintTimer.startOneShot(0);
+        view().scheduleLazyRepaint(*this);
 
     if (!themePainted) {
         if (bleedAvoidance == BackgroundBleedBackgroundOverBorder)
@@ -4781,10 +4789,4 @@ LayoutUnit RenderBox::offsetFromLogicalTopOfFirstPage() const
     return containerBlock->offsetFromLogicalTopOfFirstPage() + logicalTop();
 }
 
-void RenderBox::repaintTimerFired(Timer<RenderBox>&)
-{
-    if (!document().inPageCache())
-        this->repaint();
-}
-
 } // namespace WebCore
diff --git a/Source/WebCore/rendering/RenderBox.h b/Source/WebCore/rendering/RenderBox.h
@@ -723,12 +723,8 @@ class RenderBox : public RenderBoxModelObject {
     RefPtr<RenderOverflow> m_overflow;
 
 private:
-    void repaintTimerFired(Timer<RenderBox>&);
-
     // Used to store state between styleWillChange and styleDidChange
     static bool s_hadOverflowClip;
-
-    Timer<RenderBox> m_repaintTimer;
 };
 
 RENDER_OBJECT_TYPE_CASTS(RenderBox, isBox())

diff --git a/Source/WebCore/rendering/RenderElement.cpp b/Source/WebCore/rendering/RenderElement.cpp
@@ -83,6 +83,7 @@ RenderElement::RenderElement(Element& element, PassRef<RenderStyle> style, unsig
     , m_ancestorLineBoxDirty(false)
     , m_hasInitializedStyle(false)
     , m_renderInlineAlwaysCreatesLineBoxes(false)
+    , m_renderBoxNeedsLazyRepaint(false)
     , m_hasPausedImageAnimations(false)
     , m_firstChild(nullptr)
     , m_lastChild(nullptr)
@@ -96,6 +97,7 @@ RenderElement::RenderElement(Document& document, PassRef<RenderStyle> style, uns
     , m_ancestorLineBoxDirty(false)
     , m_hasInitializedStyle(false)
     , m_renderInlineAlwaysCreatesLineBoxes(false)
+    , m_renderBoxNeedsLazyRepaint(false)
     , m_hasPausedImageAnimations(false)
     , m_firstChild(nullptr)
     , m_lastChild(nullptr)

diff --git a/Source/WebCore/rendering/RenderElement.h b/Source/WebCore/rendering/RenderElement.h
@@ -155,6 +155,9 @@ class RenderElement : public RenderObject {
 
     RenderNamedFlowThread* renderNamedFlowThreadWrapper();
 
+    void setRenderBoxNeedsLazyRepaint(bool b) { m_renderBoxNeedsLazyRepaint = b; }
+    bool renderBoxNeedsLazyRepaint() const { return m_renderBoxNeedsLazyRepaint; }
+
 protected:
     enum BaseTypeFlags {
         RenderLayerModelObjectFlag = 1 << 0,
@@ -224,6 +227,7 @@ class RenderElement : public RenderObject {
     bool m_hasInitializedStyle : 1;
 
     bool m_renderInlineAlwaysCreatesLineBoxes : 1;
+    bool m_renderBoxNeedsLazyRepaint : 1;
     bool m_hasPausedImageAnimations : 1;
 
     RenderObject* m_firstChild;

diff --git a/Source/WebCore/rendering/RenderView.cpp b/Source/WebCore/rendering/RenderView.cpp
@@ -102,6 +102,7 @@ RenderView::RenderView(Document& document, PassRef<RenderStyle> style)
     , m_selectionEndPos(-1)
     , m_rendererCount(0)
     , m_maximalOutlineSize(0)
+    , m_lazyRepaintTimer(this, &RenderView::lazyRepaintTimerFired)
     , m_pageLogicalHeight(0)
     , m_pageLogicalHeightChanged(false)
     , m_layoutState(nullptr)
@@ -136,6 +137,38 @@ RenderView::~RenderView()
 {
 }
 
+void RenderView::scheduleLazyRepaint(RenderBox& renderer)
+{
+    if (renderer.renderBoxNeedsLazyRepaint())
+        return;
+    renderer.setRenderBoxNeedsLazyRepaint(true);
+    m_renderersNeedingLazyRepaint.add(&renderer);
+    if (!m_lazyRepaintTimer.isActive())
+        m_lazyRepaintTimer.startOneShot(0);
+}
+
+void RenderView::unscheduleLazyRepaint(RenderBox& renderer)
+{
+    if (!renderer.renderBoxNeedsLazyRepaint())
+        return;
+    renderer.setRenderBoxNeedsLazyRepaint(false);
+    m_renderersNeedingLazyRepaint.remove(&renderer);
+    if (m_renderersNeedingLazyRepaint.isEmpty())
+        m_lazyRepaintTimer.stop();
+}
+
+void RenderView::lazyRepaintTimerFired(Timer<RenderView>&)
+{
+    bool shouldRepaint = !document().inPageCache();
+
+    for (auto& renderer : m_renderersNeedingLazyRepaint) {
+        if (shouldRepaint)
+            renderer->repaint();
+        renderer->setRenderBoxNeedsLazyRepaint(false);
+    }
+    m_renderersNeedingLazyRepaint.clear();
+}
+
 bool RenderView::hitTest(const HitTestRequest& request, HitTestResult& result)
 {
     return hitTest(request, result.hitTestLocation(), result);

diff --git a/Source/WebCore/rendering/RenderView.h b/Source/WebCore/rendering/RenderView.h
@@ -246,6 +246,9 @@ class RenderView final : public RenderBlockFlow, public SelectionSubtreeRoot {
         bool m_wasAccumulatingRepaintRegion;
     };
 
+    void scheduleLazyRepaint(RenderBox&);
+    void unscheduleLazyRepaint(RenderBox&);
+
 protected:
     virtual void mapLocalToContainer(const RenderLayerModelObject* repaintContainer, TransformState&, MapCoordinatesFlags = ApplyContainerFlip, bool* wasFixed = 0) const override;
     virtual const RenderObject* pushMappingToContainer(const RenderLayerModelObject* ancestorToStopAt, RenderGeometryMap&) const override;
@@ -339,6 +342,11 @@ class RenderView final : public RenderBlockFlow, public SelectionSubtreeRoot {
 
     bool shouldUsePrintingLayout() const;
 
+    void lazyRepaintTimerFired(Timer<RenderView>&);
+
+    Timer<RenderView> m_lazyRepaintTimer;
+    HashSet<RenderBox*> m_renderersNeedingLazyRepaint;
+
     std::unique_ptr<ImageQualityController> m_imageQualityController;
     LayoutUnit m_pageLogicalHeight;
     bool m_pageLogicalHeightChanged;