Attachment 126227 Details for Bug 78199 – Perhaps a patch

[patch] Perhaps a patch

bug-78199.diff (text/plain), 3.72 KB, created by Andreas Kling on 2012-02-08 19:56:57 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Andreas Kling

Created: 2012-02-08 19:56:57 PST

Size: 3.72 KB

patch

obsolete

>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index ae5726a..a4ef6c5 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,26 @@
>+2012-02-08  Andreas Kling  <awesomekling@apple.com>
>+
>+        Increased style sharing for elements with presentation attributes.
>+        <http://webkit.org/b/78199>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When determining whether two elements can share style, we can do a lot better.
>+        Instead of comparing the attribute maps for exact equality, do a property-by-property
>+        comparison of the attributeStyle() and the additionalAttributeStyle() (if any.)
>+
>+        This increases our style sharing hit rate and shaves 100ms off of each cycle on
>+        Chromium's "Moz" page cycler test on my machine.
>+
>+        The function that compares attribute styles has O(n^2) runtime in the worst case,
>+        where n is the number of properties in the styles. However, given the low number of
>+        properties found in attribute styles, this should be fine, and it doesn't seem to
>+        heat up in profiles.
>+
>+        * css/CSSStyleSelector.cpp:
>+        (WebCore::attributeStylesEqual):
>+        (WebCore::CSSStyleSelector::canShareStyleWithElement):
>+
> 2012-02-08  Adam Klein  <adamk@chromium.org>
> 
>         Simplify and correct mutation delivery timing for JSC
>diff --git a/Source/WebCore/css/CSSStyleSelector.cpp b/Source/WebCore/css/CSSStyleSelector.cpp
>index ffd7c4f..b92f9be 100644
>--- a/Source/WebCore/css/CSSStyleSelector.cpp
>+++ b/Source/WebCore/css/CSSStyleSelector.cpp
>@@ -1194,6 +1194,29 @@ bool CSSStyleSelector::canShareStyleWithControl(StyledElement* element) const
>     return true;
> }
> 
>+static inline bool attributeStylesEqual(StylePropertySet* a, StylePropertySet* b)
>+{
>+    if (a == b)
>+        return true;
>+    if (a->propertyCount() != b->propertyCount())
>+        return false;
>+    for (unsigned i = 0; i < a->propertyCount(); ++i) {
>+        const CSSProperty& aProperty = a->propertyAt(i);
>+        unsigned j;
>+        for (j = 0; j < b->propertyCount(); ++j) {
>+            const CSSProperty& bProperty = b->propertyAt(j);
>+            if (aProperty.id() != bProperty.id())
>+                continue;
>+            if (aProperty.value() != bProperty.value())
>+                return false;
>+            break;
>+        }
>+        if (j == b->propertyCount())
>+            return false;
>+    }
>+    return true;
>+}
>+
> bool CSSStyleSelector::canShareStyleWithElement(StyledElement* element) const
> {
>     RenderStyle* style = element->renderStyle();
>@@ -1210,6 +1233,8 @@ bool CSSStyleSelector::canShareStyleWithElement(StyledElement* element) const
>         return false;
>     if (!!element->attributeStyle() != !!m_styledElement->attributeStyle())
>         return false;
>+    if (!!element->additionalAttributeStyle() != !!m_styledElement->additionalAttributeStyle())
>+        return false;
>     if (element->isLink() != m_element->isLink())
>         return false;
>     if (style->affectedByUncommonAttributeSelectors())
>@@ -1274,7 +1299,10 @@ bool CSSStyleSelector::canShareStyleWithElement(StyledElement* element) const
>     if (element->hasClass() && m_element->getAttribute(classAttr) != element->getAttribute(classAttr))
>         return false;
> 
>-    if (element->attributeStyle() && !element->attributeMap()->mapsEquivalent(m_styledElement->attributeMap()))
>+    if (element->attributeStyle() && !attributeStylesEqual(element->attributeStyle(), m_styledElement->attributeStyle()))
>+        return false;
>+
>+    if (element->additionalAttributeStyle() && !attributeStylesEqual(element->additionalAttributeStyle().get(), m_styledElement->additionalAttributeStyle().get()))
>         return false;
> 
>     if (element->isLink() && m_elementLinkState != style->insideLink())

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 78199: 126227