Bug 216241

Summary:

Comparing styles with large but identical custom property maps is slow

Product:

WebKit

Reporter:

Antti Koivisto <koivisto>

Component:

CSS

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

changseok, darin, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, kondapallykalyan, macpherson, menard, pdr, simon.fraser

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
patch	darin: review+
patch	none

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].