REGRESSION(r106756): 10% performance hit on DOM/Template.
authorkling@webkit.org <kling@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 6 Feb 2012 23:14:11 +0000 (23:14 +0000)
committerkling@webkit.org <kling@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 6 Feb 2012 23:14:11 +0000 (23:14 +0000)
<http://webkit.org/b/77831>

Reviewed by Ryosuke Niwa.

Let the StylePropertySet used for element attribute style have the element as its parent.
This is accomplished by adding an m_parentIsElement bit to StylePropertySet and sharing
some of the internal logic with inline styles.

In the end, this means that CSSParser will now pick up the document's CSSValuePool when
parsing properties for attribute styles, which fixes the perf regression from r106756.

* css/StylePropertySet.cpp:
(WebCore::StylePropertySet::StylePropertySet):
(WebCore::StylePropertySet::contextStyleSheet):

    Find contextStyleSheet via the parentElement() when there is one.

(WebCore::StylePropertySet::setNeedsStyleRecalc):

    Always set FullStyleChange for attribute style mutations. We can probably use the
    same lighter invalidation as inline styles, but that's a topic for another patch.

* css/StylePropertySet.h:
(WebCore::StylePropertySet::createInline):
(WebCore::StylePropertySet::createAttributeStyle):
(WebCore::StylePropertySet::parentRuleInternal):
(WebCore::StylePropertySet::clearParentRule):
(StylePropertySet):
(WebCore::StylePropertySet::parentElement):
(WebCore::StylePropertySet::clearParentElement):

    Added m_parentIsElement bit and update assertions as appropriate to not just
    cover the inline style case. Added a createAttributeStyle() helper to create
    a StylePropertySet for use as Element::attributeStyle().

* dom/StyledElement.h:
* dom/ElementAttributeData.h:
* dom/ElementAttributeData.cpp:
(WebCore::ElementAttributeData::ensureAttributeStyle):

    Use StylePropertySet::createAttributeStyle().

* dom/StyledElement.cpp:
(WebCore::StyledElement::removeCSSProperties):
(WebCore::StyledElement::addCSSProperty):
(WebCore::StyledElement::addCSSImageProperty):

    Remove setNeedsStyleRecalc() calls since that is now handled automatically by
    StylePropertySet's mutation methods.

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

Source/WebCore/ChangeLog
Source/WebCore/css/StylePropertySet.cpp
Source/WebCore/css/StylePropertySet.h
Source/WebCore/dom/ElementAttributeData.cpp
Source/WebCore/dom/ElementAttributeData.h
Source/WebCore/dom/StyledElement.cpp
Source/WebCore/dom/StyledElement.h

index 59df398..0131f7b 100644 (file)
@@ -1,3 +1,56 @@
+2012-02-06  Andreas Kling  <awesomekling@apple.com>
+
+        REGRESSION(r106756): 10% performance hit on DOM/Template.
+        <http://webkit.org/b/77831>
+
+        Reviewed by Ryosuke Niwa.
+
+        Let the StylePropertySet used for element attribute style have the element as its parent.
+        This is accomplished by adding an m_parentIsElement bit to StylePropertySet and sharing
+        some of the internal logic with inline styles.
+
+        In the end, this means that CSSParser will now pick up the document's CSSValuePool when
+        parsing properties for attribute styles, which fixes the perf regression from r106756.
+
+        * css/StylePropertySet.cpp:
+        (WebCore::StylePropertySet::StylePropertySet):
+        (WebCore::StylePropertySet::contextStyleSheet):
+
+            Find contextStyleSheet via the parentElement() when there is one.
+
+        (WebCore::StylePropertySet::setNeedsStyleRecalc):
+
+            Always set FullStyleChange for attribute style mutations. We can probably use the
+            same lighter invalidation as inline styles, but that's a topic for another patch.
+
+        * css/StylePropertySet.h:
+        (WebCore::StylePropertySet::createInline):
+        (WebCore::StylePropertySet::createAttributeStyle):
+        (WebCore::StylePropertySet::parentRuleInternal):
+        (WebCore::StylePropertySet::clearParentRule):
+        (StylePropertySet):
+        (WebCore::StylePropertySet::parentElement):
+        (WebCore::StylePropertySet::clearParentElement):
+
+            Added m_parentIsElement bit and update assertions as appropriate to not just
+            cover the inline style case. Added a createAttributeStyle() helper to create
+            a StylePropertySet for use as Element::attributeStyle().
+
+        * dom/StyledElement.h:
+        * dom/ElementAttributeData.h:
+        * dom/ElementAttributeData.cpp:
+        (WebCore::ElementAttributeData::ensureAttributeStyle):
+
+            Use StylePropertySet::createAttributeStyle().
+
+        * dom/StyledElement.cpp:
+        (WebCore::StyledElement::removeCSSProperties):
+        (WebCore::StyledElement::addCSSProperty):
+        (WebCore::StyledElement::addCSSImageProperty):
+
+            Remove setNeedsStyleRecalc() calls since that is now handled automatically by
+            StylePropertySet's mutation methods.
+
 2012-02-06  Kentaro Hara  <haraken@chromium.org>
 
         In AppleWebKit, stop rebuilding IDLs that need not to be rebuilt
index e6a48ea..11ec771 100644 (file)
@@ -169,6 +169,7 @@ bool StyleAttributeMutationScope::s_shouldDeliver = false;
 
 StylePropertySet::StylePropertySet()
     : m_strictParsing(false)
+    , m_parentIsElement(false)
     , m_isInlineStyleDeclaration(false)
     , m_parent(static_cast<CSSRule*>(0))
 {
@@ -176,6 +177,7 @@ StylePropertySet::StylePropertySet()
 
 StylePropertySet::StylePropertySet(CSSRule* parentRule)
     : m_strictParsing(!parentRule || parentRule->useStrictParsing())
+    , m_parentIsElement(false)
     , m_isInlineStyleDeclaration(false)
     , m_parent(parentRule)
 {
@@ -184,6 +186,7 @@ StylePropertySet::StylePropertySet(CSSRule* parentRule)
 StylePropertySet::StylePropertySet(CSSRule* parentRule, const Vector<CSSProperty>& properties)
     : m_properties(properties)
     , m_strictParsing(!parentRule || parentRule->useStrictParsing())
+    , m_parentIsElement(false)
     , m_isInlineStyleDeclaration(false)
     , m_parent(parentRule)
 {
@@ -192,6 +195,7 @@ StylePropertySet::StylePropertySet(CSSRule* parentRule, const Vector<CSSProperty
 
 StylePropertySet::StylePropertySet(CSSRule* parentRule, const CSSProperty* const * properties, int numProperties)
     : m_strictParsing(!parentRule || parentRule->useStrictParsing())
+    , m_parentIsElement(false)
     , m_isInlineStyleDeclaration(false)
     , m_parent(parentRule)
 {
@@ -214,11 +218,12 @@ StylePropertySet::StylePropertySet(CSSRule* parentRule, const CSSProperty* const
     }
 }
 
-StylePropertySet::StylePropertySet(StyledElement* parentElement
+StylePropertySet::StylePropertySet(StyledElement* parentElement, bool isInlineStyle)
     : m_strictParsing(false)
-    , m_isInlineStyleDeclaration(true)
+    , m_parentIsElement(true)
+    , m_isInlineStyleDeclaration(isInlineStyle)
     , m_parent(parentElement)
-{ 
+{
 }
 
 StylePropertySet::~StylePropertySet()
@@ -239,7 +244,7 @@ void StylePropertySet::deref()
 
 CSSStyleSheet* StylePropertySet::contextStyleSheet() const
 {
-    if (m_isInlineStyleDeclaration) {
+    if (m_parentIsElement) {
         Document* document = m_parent.element ? m_parent.element->document() : 0;
         return document ? document->elementSheet() : 0;
     }
@@ -705,10 +710,16 @@ String StylePropertySet::removeProperty(int propertyID, bool notifyChanged, bool
 
 void StylePropertySet::setNeedsStyleRecalc()
 {
-    if (isInlineStyleDeclaration()) {
+    if (m_parentIsElement) {
         StyledElement* element = parentElement();
         if (!element)
             return;
+
+        if (!m_isInlineStyleDeclaration) {
+            element->setNeedsStyleRecalc();
+            return;
+        }
+
         element->setNeedsStyleRecalc(InlineStyleChange);
         element->invalidateStyleAttribute();
         StyleAttributeMutationScope(this).didInvalidateStyleAttr();
index 65b5954..a61b376 100644 (file)
@@ -55,8 +55,12 @@ public:
         return adoptRef(new StylePropertySet(0, properties));
     }
     static PassRefPtr<StylePropertySet> createInline(StyledElement* element)
-    { 
-        return adoptRef(new StylePropertySet(element));
+    {
+        return adoptRef(new StylePropertySet(element, /*isInlineStyle*/ true));
+    }
+    static PassRefPtr<StylePropertySet> createAttributeStyle(StyledElement* element)
+    {
+        return adoptRef(new StylePropertySet(element, /*isInlineStyle*/ false));
     }
 
     void deref();
@@ -109,12 +113,12 @@ public:
 
     PassRefPtr<StylePropertySet> copyPropertiesInSet(const int* set, unsigned length) const;
 
-    CSSRule* parentRuleInternal() const { return m_isInlineStyleDeclaration ? 0 : m_parent.rule; }
-    void clearParentRule() { ASSERT(!m_isInlineStyleDeclaration); m_parent.rule = 0; }
-    
-    StyledElement* parentElement() const { ASSERT(m_isInlineStyleDeclaration); return m_parent.element; }
-    void clearParentElement() { ASSERT(m_isInlineStyleDeclaration); m_parent.element = 0; }
-    
+    CSSRule* parentRuleInternal() const { return m_parentIsElement ? 0 : m_parent.rule; }
+    void clearParentRule() { ASSERT(!m_parentIsElement); m_parent.rule = 0; }
+
+    StyledElement* parentElement() const { ASSERT(m_parentIsElement); return m_parent.element; }
+    void clearParentElement() { ASSERT(m_parentIsElement); m_parent.element = 0; }
+
     CSSStyleSheet* contextStyleSheet() const;
     
     String asText() const;
@@ -126,7 +130,7 @@ private:
     StylePropertySet(CSSRule* parentRule);
     StylePropertySet(CSSRule* parentRule, const Vector<CSSProperty>&);
     StylePropertySet(CSSRule* parentRule, const CSSProperty* const *, int numProperties);
-    StylePropertySet(StyledElement*);
+    StylePropertySet(StyledElement*, bool isInlineStyle);
 
     void setNeedsStyleRecalc();
 
@@ -155,6 +159,7 @@ private:
     Vector<CSSProperty, 4> m_properties;
 
     bool m_strictParsing : 1;
+    bool m_parentIsElement : 1;
     bool m_isInlineStyleDeclaration : 1;
 
     union Parent {
index c27f124..cea62f3 100644 (file)
@@ -53,12 +53,10 @@ void ElementAttributeData::destroyInlineStyleDecl()
     m_inlineStyleDecl = 0;
 }
 
-StylePropertySet* ElementAttributeData::ensureAttributeStyle()
+StylePropertySet* ElementAttributeData::ensureAttributeStyle(StyledElement* element)
 {
-    if (!m_attributeStyle) {
-        m_attributeStyle = StylePropertySet::create();
-        m_attributeStyle->setStrictParsing(false);
-    }
+    if (!m_attributeStyle)
+        m_attributeStyle = StylePropertySet::createAttributeStyle(element);
     return m_attributeStyle.get();
 }
 
index 0283728..4080659 100644 (file)
@@ -47,7 +47,7 @@ public:
     void destroyInlineStyleDecl();
 
     StylePropertySet* attributeStyle() const { return m_attributeStyle.get(); }
-    StylePropertySet* ensureAttributeStyle();
+    StylePropertySet* ensureAttributeStyle(StyledElement*);
 
 private:
     friend class NamedNodeMap;
index 69bf059..95c2f18 100644 (file)
@@ -110,8 +110,6 @@ void StyledElement::removeCSSProperties(int id1, int id2, int id3, int id4, int
     if (!style)
         return;
 
-    setNeedsStyleRecalc(FullStyleChange);
-
     ASSERT(id1 != CSSPropertyInvalid);
     style->removeProperty(id1);
 
@@ -142,20 +140,16 @@ void StyledElement::addCSSProperty(int id, const String &value)
 {
     if (!ensureAttributeStyle()->setProperty(id, value))
         removeCSSProperty(id);
-    else
-        setNeedsStyleRecalc(FullStyleChange);
 }
 
 void StyledElement::addCSSProperty(int id, int value)
 {
     ensureAttributeStyle()->setProperty(id, value);
-    setNeedsStyleRecalc(FullStyleChange);
 }
 
 void StyledElement::addCSSImageProperty(int id, const String& url)
 {
     ensureAttributeStyle()->setProperty(CSSProperty(id, CSSImageValue::create(url)));
-    setNeedsStyleRecalc(FullStyleChange);
 }
 
 void StyledElement::addCSSLength(int id, const String &value)
index f941b8b..52f9cbc 100644 (file)
@@ -52,7 +52,7 @@ public:
     virtual CSSStyleDeclaration* style() OVERRIDE { return ensureInlineStyleDecl()->ensureCSSStyleDeclaration(); }
 
     StylePropertySet* attributeStyle() const { return attributeData() ? attributeData()->attributeStyle() : 0; }
-    StylePropertySet* ensureAttributeStyle() { return ensureAttributeData()->ensureAttributeStyle(); }
+    StylePropertySet* ensureAttributeStyle() { return ensureAttributeData()->ensureAttributeStyle(this); }
 
     const SpaceSplitString& classNames() const;