From 9b9dc6ac03817c972a8eaa0c04eef03666b96cee Mon Sep 17 00:00:00 2001 From: "antti@apple.com" Date: Thu, 28 Jun 2012 18:58:21 +0000 Subject: [PATCH] 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. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@121446 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- Source/WebCore/ChangeLog | 33 +++++++++ Source/WebCore/rendering/RenderGeometryMap.cpp | 96 ++++++++++---------------- Source/WebCore/rendering/RenderGeometryMap.h | 39 ++++++++++- 3 files changed, 107 insertions(+), 61 deletions(-) diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index bb534cd..967de18 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,36 @@ +2012-06-28 Antti Koivisto + + 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 [GTK] Remove Windows support from plugins/gtk/ diff --git a/Source/WebCore/rendering/RenderGeometryMap.cpp b/Source/WebCore/rendering/RenderGeometryMap.cpp index a1f50ed..e6993db 100644 --- a/Source/WebCore/rendering/RenderGeometryMap.cpp +++ b/Source/WebCore/rendering/RenderGeometryMap.cpp @@ -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 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 . // 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 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 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 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) diff --git a/Source/WebCore/rendering/RenderGeometryMap.h b/Source/WebCore/rendering/RenderGeometryMap.h index 68002a8..8e63c50 100644 --- a/Source/WebCore/rendering/RenderGeometryMap.h +++ b/Source/WebCore/rendering/RenderGeometryMap.h @@ -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 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, 32> RenderGeometryMapSteps; + + typedef Vector 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 : SimpleClassVectorTraits { }; +} + #endif // RenderGeometryMap_h -- 2.7.4