Title: [121446] trunk/Source/WebCore
Revision
121446
Author
an...@apple.com
Date
2012-06-28 11:58:21 -0700 (Thu, 28 Jun 2012)

Log Message

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.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (121445 => 121446)


--- trunk/Source/WebCore/ChangeLog	2012-06-28 18:15:17 UTC (rev 121445)
+++ trunk/Source/WebCore/ChangeLog	2012-06-28 18:58:21 UTC (rev 121446)
@@ -1,3 +1,36 @@
+2012-06-28  Antti Koivisto  <an...@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  <kalevlem...@gmail.com>
 
         [GTK] Remove Windows support from plugins/gtk/

Modified: trunk/Source/WebCore/rendering/RenderGeometryMap.cpp (121445 => 121446)


--- trunk/Source/WebCore/rendering/RenderGeometryMap.cpp	2012-06-28 18:15:17 UTC (rev 121445)
+++ trunk/Source/WebCore/rendering/RenderGeometryMap.cpp	2012-06-28 18:58:21 UTC (rev 121446)
@@ -33,29 +33,6 @@
 
 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 @@
     }
 
 #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 @@
     }
 
 #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 @@
     // 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::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::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)

Modified: trunk/Source/WebCore/rendering/RenderGeometryMap.h (121445 => 121446)


--- trunk/Source/WebCore/rendering/RenderGeometryMap.h	2012-06-28 18:15:17 UTC (rev 121445)
+++ trunk/Source/WebCore/rendering/RenderGeometryMap.h	2012-06-28 18:58:21 UTC (rev 121446)
@@ -35,9 +35,36 @@
 
 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,9 +99,9 @@
     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;
     int m_transformedStepsCount;
@@ -85,4 +112,10 @@
 
 } // 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
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to