Bug 216241

Summary: Comparing styles with large but identical custom property maps is slow
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: CSSAssignee: 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: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
darin: review+
patch none

Antti Koivisto
Reported 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.
Attachments
patch (8.44 KB, patch)
2020-09-07 08:14 PDT, Antti Koivisto 		darin: review+
DetailsFormatted DiffDiff
patch (8.37 KB, patch)
2020-09-08 00:40 PDT, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Antti Koivisto
Comment 1 2020-09-07 04:54:22 PDT
<rdar://problem/67946605>
Antti Koivisto
Comment 2 2020-09-07 08:14:07 PDT
Created attachment 408181 [details] patch
Darin Adler
Comment 3 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?
Antti Koivisto
Comment 4 2020-09-08 00:40:25 PDT
Created attachment 408216 [details] patch
EWS
Comment 5 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].
Note You need to log in before you can comment on or make changes to this bug.