Bug 216241 – Comparing styles with large but identical custom property maps is slow

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Antti Koivisto]

In full style rebuild we lose sharing of equal StyleCustomPropertyData between the old and the new style. Custom properties are usually defined in document element and this lack of sharing inherits to all elements. Without sharing equality comparisons end up requiring deep comparison, which can be slow if there are thousands of properties.

[Antti Koivisto]

<rdar://problem/67946605>

[Antti Koivisto]

Created attachment 408181 [details]
patch

[Darin Adler]

Comment on attachment 408181 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=408181&action=review

> Source/WebCore/ChangeLog:18
> +        Going to full screen and back on youtube, this patch reduces time in the longest style update from ~460ms to ~90ms.

Amazing.

> Source/WebCore/rendering/style/RenderStyle.cpp:2337
> +    if (m_rareInheritedData->customProperties.ptr() == other.m_rareInheritedData->customProperties.ptr())
> +        return;
> +    if (*m_rareInheritedData->customProperties == *other.m_rareInheritedData->customProperties)
> +        const_cast<StyleRareInheritedData&>(m_rareInheritedData.get()).customProperties = other.m_rareInheritedData->customProperties;

Here’s another way to write it that I like slightly better:

    auto& properties = const_cast<StyleRareInheritedData&>(m_rareInheritedData->customProperties);
    auto& otherProperties = other.m_rareInheritedData->customProperties;
    if (properties.ptr() != otherProperties.ptr() && *properties == *otherProperties)
        priorities = otherProperties;

Not sure you will prefer it.

> Source/WebCore/style/StyleBuilder.cpp:196
> +        Ref<CSSCustomPropertyValue> valueToApply = downcast<CSSCustomPropertyValue>(*property.cssValue[index]);

Could we use auto and makeRef here?

> Source/WebCore/style/StyleBuilder.cpp:233
> +        Ref<CSSCustomPropertyValue> valueToApply = downcast<CSSCustomPropertyValue>(*property.cssValue[index]);

Could we use auto and makeRef here?

[Antti Koivisto]

Created attachment 408216 [details]
patch

[EWS]

Committed r266717: <https://trac.webkit.org/changeset/266717>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 408216 [details].