Don't malloc RenderGeometryMap steps individually
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 Jun 2012 18:58:21 +0000 (18:58 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 Jun 2012 18:58:21 +0000 (18:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=90074

Reviewed by Simon Fraser.

Mallocs and frees for steps under RenderGeometryMap::pus/popMappingsToAncestor can total ~2% of the profile when animating transforms.

* rendering/RenderGeometryMap.cpp:
(WebCore):
(WebCore::RenderGeometryMap::absolutePoint):
(WebCore::RenderGeometryMap::absoluteRect):
(WebCore::RenderGeometryMap::mapToAbsolute):
(WebCore::RenderGeometryMap::push):
(WebCore::RenderGeometryMap::pushView):
(WebCore::RenderGeometryMap::popMappingsToAncestor):
* rendering/RenderGeometryMap.h:
(WebCore):
(WebCore::RenderGeometryMapStep::RenderGeometryMapStep):

    Move to header.

(RenderGeometryMapStep):
(RenderGeometryMap):

    Make the step vector hold RenderGeometryMapSteps instead of RenderGeometryMapStep*'s.

(WTF):

    Give RenderGeometryMapSteps SimpleClassVectorTraits. This is needed for dealing with OwnPtr in the struct (and makes it faster too).
    The type is simple enought to move by memcpy.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@121446 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderGeometryMap.cpp
Source/WebCore/rendering/RenderGeometryMap.h

index bb534cd..967de18 100644 (file)
@@ -1,3 +1,36 @@
+2012-06-28  Antti Koivisto  <antti@apple.com>
+
+        Don't malloc RenderGeometryMap steps individually
+        https://bugs.webkit.org/show_bug.cgi?id=90074
+
+        Reviewed by Simon Fraser.
+
+        Mallocs and frees for steps under RenderGeometryMap::pus/popMappingsToAncestor can total ~2% of the profile when animating transforms.
+
+        * rendering/RenderGeometryMap.cpp:
+        (WebCore):
+        (WebCore::RenderGeometryMap::absolutePoint):
+        (WebCore::RenderGeometryMap::absoluteRect):
+        (WebCore::RenderGeometryMap::mapToAbsolute):
+        (WebCore::RenderGeometryMap::push):
+        (WebCore::RenderGeometryMap::pushView):
+        (WebCore::RenderGeometryMap::popMappingsToAncestor):
+        * rendering/RenderGeometryMap.h:
+        (WebCore):
+        (WebCore::RenderGeometryMapStep::RenderGeometryMapStep):
+        
+            Move to header.
+
+        (RenderGeometryMapStep):
+        (RenderGeometryMap):
+        
+            Make the step vector hold RenderGeometryMapSteps instead of RenderGeometryMapStep*'s.
+
+        (WTF):
+        
+            Give RenderGeometryMapSteps SimpleClassVectorTraits. This is needed for dealing with OwnPtr in the struct (and makes it faster too).
+            The type is simple enought to move by memcpy.
+
 2012-06-28  Kalev Lember  <kalevlember@gmail.com>
 
         [GTK] Remove Windows support from plugins/gtk/
index a1f50ed..e6993db 100644 (file)
 
 namespace WebCore {
 
-// Stores data about how to map from one renderer to its container.
-class RenderGeometryMapStep {
-    WTF_MAKE_NONCOPYABLE(RenderGeometryMapStep);
-public:
-    RenderGeometryMapStep(const RenderObject* renderer, bool accumulatingTransform, bool isNonUniform, bool isFixedPosition, bool hasTransform)
-        : m_renderer(renderer)
-        , m_accumulatingTransform(accumulatingTransform)
-        , m_isNonUniform(isNonUniform)
-        , m_isFixedPosition(isFixedPosition)
-        , m_hasTransform(hasTransform)
-    {
-    }
-        
-    const RenderObject* m_renderer;
-    LayoutSize m_offset;
-    OwnPtr<TransformationMatrix> m_transform; // Includes offset if non-null.
-    bool m_accumulatingTransform;
-    bool m_isNonUniform; // Mapping depends on the input point, e.g. because of CSS columns.
-    bool m_isFixedPosition;
-    bool m_hasTransform;
-};
-
-
 RenderGeometryMap::RenderGeometryMap()
     : m_insertionPosition(notFound)
     , m_nonUniformStepsCount(0)
@@ -81,7 +58,7 @@ FloatPoint RenderGeometryMap::absolutePoint(const FloatPoint& p) const
     }
 
 #if !ASSERT_DISABLED
-    FloatPoint rendererMappedResult = m_mapping.last()->m_renderer->localToAbsolute(p, false, true);
+    FloatPoint rendererMappedResult = m_mapping.last().m_renderer->localToAbsolute(p, false, true);
     ASSERT(rendererMappedResult == result);
 #endif
 
@@ -102,7 +79,7 @@ FloatRect RenderGeometryMap::absoluteRect(const FloatRect& rect) const
     }
 
 #if !ASSERT_DISABLED
-    FloatRect rendererMappedResult = m_mapping.last()->m_renderer->localToAbsoluteQuad(rect).boundingBox();
+    FloatRect rendererMappedResult = m_mapping.last().m_renderer->localToAbsoluteQuad(rect).boundingBox();
     // Inspector creates renderers with negative width <https://bugs.webkit.org/show_bug.cgi?id=87194>.
     // Taking FloatQuad bounds avoids spurious assertions because of that.
     ASSERT(enclosingIntRect(rendererMappedResult) == enclosingIntRect(FloatQuad(result).boundingBox()));
@@ -116,36 +93,36 @@ void RenderGeometryMap::mapToAbsolute(TransformState& transformState) const
     // If the mapping includes something like columns, we have to go via renderers.
     if (hasNonUniformStep()) {
         bool fixed = false;
-        m_mapping.last()->m_renderer->mapLocalToContainer(0, fixed, true, transformState, RenderObject::ApplyContainerFlip);
+        m_mapping.last().m_renderer->mapLocalToContainer(0, fixed, true, transformState, RenderObject::ApplyContainerFlip);
         return;
     }
     
     bool inFixed = false;
 
     for (int i = m_mapping.size() - 1; i >= 0; --i) {
-        const RenderGeometryMapStep* currStep = m_mapping[i].get();
+        const RenderGeometryMapStep& currentStep = m_mapping[i];
 
         // If this box has a transform, it acts as a fixed position container
         // for fixed descendants, which prevents the propagation of 'fixed'
         // unless the layer itself is also fixed position.
-        if (currStep->m_hasTransform && !currStep->m_isFixedPosition)
+        if (currentStep.m_hasTransform && !currentStep.m_isFixedPosition)
             inFixed = false;
-        else if (currStep->m_isFixedPosition)
+        else if (currentStep.m_isFixedPosition)
             inFixed = true;
 
         if (!i) {
-            if (currStep->m_transform)
-                transformState.applyTransform(*currStep->m_transform.get());
+            if (currentStep.m_transform)
+                transformState.applyTransform(*currentStep.m_transform.get());
 
             // The root gets special treatment for fixed position
             if (inFixed)
-                transformState.move(currStep->m_offset.width(), currStep->m_offset.height());
+                transformState.move(currentStep.m_offset.width(), currentStep.m_offset.height());
         } else {
-            TransformState::TransformAccumulation accumulate = currStep->m_accumulatingTransform ? TransformState::AccumulateTransform : TransformState::FlattenTransform;
-            if (currStep->m_transform)
-                transformState.applyTransform(*currStep->m_transform.get(), accumulate);
+            TransformState::TransformAccumulation accumulate = currentStep.m_accumulatingTransform ? TransformState::AccumulateTransform : TransformState::FlattenTransform;
+            if (currentStep.m_transform)
+                transformState.applyTransform(*currentStep.m_transform.get(), accumulate);
             else
-                transformState.move(currStep->m_offset.width(), currStep->m_offset.height(), accumulate);
+                transformState.move(currentStep.m_offset.width(), currentStep.m_offset.height(), accumulate);
         }
     }
 
@@ -184,48 +161,51 @@ void RenderGeometryMap::pushMappingsToAncestor(const RenderLayer* layer, const R
 void RenderGeometryMap::push(const RenderObject* renderer, const LayoutSize& offsetFromContainer, bool accumulatingTransform, bool isNonUniform, bool isFixedPosition, bool hasTransform)
 {
     ASSERT(m_insertionPosition != notFound);
-    
-    OwnPtr<RenderGeometryMapStep> step = adoptPtr(new RenderGeometryMapStep(renderer, accumulatingTransform, isNonUniform, isFixedPosition, hasTransform));
-    step->m_offset = offsetFromContainer;
-    
-    stepInserted(*step.get());
-    m_mapping.insert(m_insertionPosition, step.release());
+
+    m_mapping.insert(m_insertionPosition, RenderGeometryMapStep(renderer, accumulatingTransform, isNonUniform, isFixedPosition, hasTransform));
+
+    RenderGeometryMapStep& step = m_mapping[m_insertionPosition];
+    step.m_offset = offsetFromContainer;
+
+    stepInserted(step);
 }
 
 void RenderGeometryMap::push(const RenderObject* renderer, const TransformationMatrix& t, bool accumulatingTransform, bool isNonUniform, bool isFixedPosition, bool hasTransform)
 {
     ASSERT(m_insertionPosition != notFound);
 
-    OwnPtr<RenderGeometryMapStep> step = adoptPtr(new RenderGeometryMapStep(renderer, accumulatingTransform, isNonUniform, isFixedPosition, hasTransform));
+    m_mapping.insert(m_insertionPosition, RenderGeometryMapStep(renderer, accumulatingTransform, isNonUniform, isFixedPosition, hasTransform));
+    
+    RenderGeometryMapStep& step = m_mapping[m_insertionPosition];
     if (!t.isIntegerTranslation())
-        step->m_transform = adoptPtr(new TransformationMatrix(t));
+        step.m_transform = adoptPtr(new TransformationMatrix(t));
     else
-        step->m_offset = LayoutSize(t.e(), t.f());
-    
-    stepInserted(*step.get());
-    m_mapping.insert(m_insertionPosition, step.release());
+        step.m_offset = LayoutSize(t.e(), t.f());
+
+    stepInserted(step);
 }
 
 void RenderGeometryMap::pushView(const RenderView* view, const LayoutSize& scrollOffset, const TransformationMatrix* t)
 {
     ASSERT(m_insertionPosition != notFound);
+    ASSERT(!m_mapping.size()); // The view should always be the first thing pushed.
 
-    OwnPtr<RenderGeometryMapStep> step = adoptPtr(new RenderGeometryMapStep(view, false, false, false, t));
-    step->m_offset = scrollOffset;
+    m_mapping.insert(m_insertionPosition, RenderGeometryMapStep(view, false, false, false, t));
+    
+    RenderGeometryMapStep& step = m_mapping[m_insertionPosition];
+    step.m_offset = scrollOffset;
     if (t)
-        step->m_transform = adoptPtr(new TransformationMatrix(*t));
-        
-    ASSERT(!m_mapping.size()); // The view should always be the first thing pushed.
-    stepInserted(*step.get());
-    m_mapping.insert(m_insertionPosition, step.release());
+        step.m_transform = adoptPtr(new TransformationMatrix(*t));
+    
+    stepInserted(step);
 }
 
 void RenderGeometryMap::popMappingsToAncestor(const RenderBoxModelObject* ancestorRenderer)
 {
     ASSERT(m_mapping.size());
 
-    while (m_mapping.size() && m_mapping.last()->m_renderer != ancestorRenderer) {
-        stepRemoved(*m_mapping.last().get());
+    while (m_mapping.size() && m_mapping.last().m_renderer != ancestorRenderer) {
+        stepRemoved(m_mapping.last());
         m_mapping.removeLast();
     }
 }
@@ -239,7 +219,7 @@ void RenderGeometryMap::popMappingsToAncestor(const RenderLayer* ancestorLayer)
 void RenderGeometryMap::stepInserted(const RenderGeometryMapStep& step)
 {
     // Offset on the first step is the RenderView's offset, which is only applied when we have fixed-position.s
-    if (m_mapping.size())
+    if (m_mapping.size() > 1)
         m_accumulatedOffset += step.m_offset;
 
     if (step.m_isNonUniform)
index 68002a8..8e63c50 100644 (file)
 
 namespace WebCore {
 
-class RenderGeometryMapStep;
 class RenderLayer;
 
+// Stores data about how to map from one renderer to its container.
+struct RenderGeometryMapStep {
+    RenderGeometryMapStep(const RenderGeometryMapStep& o)
+        : m_renderer(o.m_renderer)
+        , m_accumulatingTransform(o.m_accumulatingTransform)
+        , m_isNonUniform(o.m_isNonUniform)
+        , m_isFixedPosition(o.m_isFixedPosition)
+        , m_hasTransform(o.m_hasTransform)
+    {
+        ASSERT(!o.m_transform);
+    }
+    RenderGeometryMapStep(const RenderObject* renderer, bool accumulatingTransform, bool isNonUniform, bool isFixedPosition, bool hasTransform)
+        : m_renderer(renderer)
+        , m_accumulatingTransform(accumulatingTransform)
+        , m_isNonUniform(isNonUniform)
+        , m_isFixedPosition(isFixedPosition)
+        , m_hasTransform(hasTransform)
+    {
+    }
+    const RenderObject* m_renderer;
+    LayoutSize m_offset;
+    OwnPtr<TransformationMatrix> m_transform; // Includes offset if non-null.
+    bool m_accumulatingTransform;
+    bool m_isNonUniform; // Mapping depends on the input point, e.g. because of CSS columns.
+    bool m_isFixedPosition;
+    bool m_hasTransform;
+};
+
 // Can be used while walking the Renderer tree to cache data about offsets and transforms.
 class RenderGeometryMap {
 public:
@@ -72,8 +99,8 @@ private:
     bool hasNonUniformStep() const { return m_nonUniformStepsCount; }
     bool hasTransformStep() const { return m_transformedStepsCount; }
     bool hasFixedPositionStep() const { return m_fixedStepsCount; }
-    
-    typedef Vector<OwnPtr<RenderGeometryMapStep>, 32> RenderGeometryMapSteps;
+
+    typedef Vector<RenderGeometryMapStep, 32> RenderGeometryMapSteps;
 
     size_t m_insertionPosition;
     int m_nonUniformStepsCount;
@@ -85,4 +112,10 @@ private:
 
 } // namespace WebCore
 
+namespace WTF {
+// This is required for a struct with OwnPtr. We know RenderGeometryMapStep is simple enough that
+// initializing to 0 and moving with memcpy (and then not destructing the original) will work.
+template<> struct VectorTraits<WebCore::RenderGeometryMapStep> : SimpleClassVectorTraits { };
+}
+
 #endif // RenderGeometryMap_h