Bug 216241 - Comparing styles with large but identical custom property maps is slow
Summary: Comparing styles with large but identical custom property maps is slow
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-09-07 04:27 PDT by Antti Koivisto
Modified: 2020-09-08 01:48 PDT (History)
11 users (show)

See Also:


Attachments
patch (8.44 KB, patch)
2020-09-07 08:14 PDT, Antti Koivisto
darin: review+
Details | Formatted Diff | Diff
patch (8.37 KB, patch)
2020-09-08 00:40 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2020-09-07 04:27:54 PDT
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.
Comment 1 Antti Koivisto 2020-09-07 04:54:22 PDT
<rdar://problem/67946605>
Comment 2 Antti Koivisto 2020-09-07 08:14:07 PDT
Created attachment 408181 [details]
patch
Comment 3 Darin Adler 2020-09-07 10:20:03 PDT
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?
Comment 4 Antti Koivisto 2020-09-08 00:40:25 PDT
Created attachment 408216 [details]
patch
Comment 5 EWS 2020-09-08 01:48:42 PDT
Committed r266717: <https://trac.webkit.org/changeset/266717>

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