Title: [266689] trunk/Source/WebCore
Revision
266689
Author
wenson_hs...@apple.com
Date
2020-09-06 17:59:52 -0700 (Sun, 06 Sep 2020)

Log Message

[MotionMark - Multiply] Web process spends ~1% of total samples in PropertyCascade::resolveDirectionAndWritingMode
https://bugs.webkit.org/show_bug.cgi?id=216223

Reviewed by Darin Adler.

A few subtests in MotionMark (Leaves, Focus, Design, and especially Multiply) spend large amounts of time in
style resolution (`Document::resolveStyle`) due to constant style changes across many elements during every
frame. In Multiply, ~3-4% of the time underneath `Document::resolveStyle` is spent resolving direction and
writing modes inside `PropertyCascade::resolveDirectionAndWritingMode` (i.e., ~2.3 million invocations). This
helper function is responsible for computing the text direction and CSS writing mode that is used to resolve
direction-aware CSS properties (which are enumerated in `CSSProperty::isDirectionAwareProperty`). Resolving the
direction and writing mode involves iterating over all of the matched CSS properties (`m_matchResult`) in the
property cascade in search of CSS properties for writing and direction, which can be relatively expensive when
there are lots of properties in the cascade.

However, if there are no direction-aware CSS properties in the cascade, this work can actually be elided; to
achieve this, we can store the inherited `Direction` in `m_direction`, and then lazily resolve it if needed.

I measured this locally to yield a little under ~1% in the Multiply subtest in MotionMark. Otherwise, there is
no change in behavior; see below for more details.

* style/PropertyCascade.cpp:
(WebCore::Style::PropertyCascade::PropertyCascade):
(WebCore::Style::PropertyCascade::set):

If we encounter a direction-aware CSS property, then use `direction()` to ensure that the direction and writing
mode are resolved.

(WebCore::Style::PropertyCascade::direction const):

Make this getter call `resolveDirectionAndWritingMode` if needed.

* style/PropertyCascade.h:

Add a new `bool` member to keep track of whether or not the CSS direction has not yet been resolved. Note that
since this member variable fits within the padding after `Direction m_direction;`, this class is still the same
size.

(WebCore::Style::PropertyCascade::direction const): Deleted.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (266688 => 266689)


--- trunk/Source/WebCore/ChangeLog	2020-09-07 00:52:52 UTC (rev 266688)
+++ trunk/Source/WebCore/ChangeLog	2020-09-07 00:59:52 UTC (rev 266689)
@@ -1,3 +1,45 @@
+2020-09-06  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        [MotionMark - Multiply] Web process spends ~1% of total samples in PropertyCascade::resolveDirectionAndWritingMode
+        https://bugs.webkit.org/show_bug.cgi?id=216223
+
+        Reviewed by Darin Adler.
+
+        A few subtests in MotionMark (Leaves, Focus, Design, and especially Multiply) spend large amounts of time in
+        style resolution (`Document::resolveStyle`) due to constant style changes across many elements during every
+        frame. In Multiply, ~3-4% of the time underneath `Document::resolveStyle` is spent resolving direction and
+        writing modes inside `PropertyCascade::resolveDirectionAndWritingMode` (i.e., ~2.3 million invocations). This
+        helper function is responsible for computing the text direction and CSS writing mode that is used to resolve
+        direction-aware CSS properties (which are enumerated in `CSSProperty::isDirectionAwareProperty`). Resolving the
+        direction and writing mode involves iterating over all of the matched CSS properties (`m_matchResult`) in the
+        property cascade in search of CSS properties for writing and direction, which can be relatively expensive when
+        there are lots of properties in the cascade.
+
+        However, if there are no direction-aware CSS properties in the cascade, this work can actually be elided; to
+        achieve this, we can store the inherited `Direction` in `m_direction`, and then lazily resolve it if needed.
+
+        I measured this locally to yield a little under ~1% in the Multiply subtest in MotionMark. Otherwise, there is
+        no change in behavior; see below for more details.
+
+        * style/PropertyCascade.cpp:
+        (WebCore::Style::PropertyCascade::PropertyCascade):
+        (WebCore::Style::PropertyCascade::set):
+
+        If we encounter a direction-aware CSS property, then use `direction()` to ensure that the direction and writing
+        mode are resolved.
+
+        (WebCore::Style::PropertyCascade::direction const):
+
+        Make this getter call `resolveDirectionAndWritingMode` if needed.
+
+        * style/PropertyCascade.h:
+
+        Add a new `bool` member to keep track of whether or not the CSS direction has not yet been resolved. Note that
+        since this member variable fits within the padding after `Direction m_direction;`, this class is still the same
+        size.
+
+        (WebCore::Style::PropertyCascade::direction const): Deleted.
+
 2020-09-06  Myles C. Maxfield  <mmaxfi...@apple.com>
 
         Make GlyphBufferAdvance and GlyphBufferOrigin more robust

Modified: trunk/Source/WebCore/style/PropertyCascade.cpp (266688 => 266689)


--- trunk/Source/WebCore/style/PropertyCascade.cpp	2020-09-07 00:52:52 UTC (rev 266688)
+++ trunk/Source/WebCore/style/PropertyCascade.cpp	2020-09-07 00:59:52 UTC (rev 266689)
@@ -148,7 +148,7 @@
 PropertyCascade::PropertyCascade(const MatchResult& matchResult, OptionSet<CascadeLevel> cascadeLevels, IncludedProperties includedProperties, Direction direction)
     : m_matchResult(matchResult)
     , m_includedProperties(includedProperties)
-    , m_direction(resolveDirectionAndWritingMode(direction))
+    , m_direction(direction)
 {
     buildCascade(cascadeLevels);
 }
@@ -156,7 +156,8 @@
 PropertyCascade::PropertyCascade(const PropertyCascade& parent, OptionSet<CascadeLevel> cascadeLevels)
     : m_matchResult(parent.m_matchResult)
     , m_includedProperties(parent.m_includedProperties)
-    , m_direction(parent.m_direction)
+    , m_direction(parent.direction())
+    , m_directionIsUnresolved(false)
 {
     buildCascade(cascadeLevels);
 }
@@ -196,8 +197,10 @@
 
 void PropertyCascade::set(CSSPropertyID id, CSSValue& cssValue, unsigned linkMatchType, CascadeLevel cascadeLevel, ScopeOrdinal styleScopeOrdinal)
 {
-    if (CSSProperty::isDirectionAwareProperty(id))
-        id = CSSProperty::resolveDirectionAwareProperty(id, m_direction.textDirection, m_direction.writingMode);
+    if (CSSProperty::isDirectionAwareProperty(id)) {
+        auto direction = this->direction();
+        id = CSSProperty::resolveDirectionAwareProperty(id, direction.textDirection, direction.writingMode);
+    }
 
     ASSERT(!shouldApplyPropertyInParseOrder(id));
 
@@ -404,5 +407,14 @@
     return result;
 }
 
+PropertyCascade::Direction PropertyCascade::direction() const
+{
+    if (m_directionIsUnresolved) {
+        m_direction = resolveDirectionAndWritingMode(m_direction);
+        m_directionIsUnresolved = false;
+    }
+    return m_direction;
 }
+
 }
+}

Modified: trunk/Source/WebCore/style/PropertyCascade.h (266688 => 266689)


--- trunk/Source/WebCore/style/PropertyCascade.h	2020-09-07 00:52:52 UTC (rev 266688)
+++ trunk/Source/WebCore/style/PropertyCascade.h	2020-09-07 00:59:52 UTC (rev 266689)
@@ -67,7 +67,7 @@
     const Vector<Property, 8>& deferredProperties() const { return m_deferredProperties; }
     const HashMap<AtomString, Property>& customProperties() const { return m_customProperties; }
 
-    Direction direction() const { return m_direction; }
+    Direction direction() const;
 
     const PropertyCascade* propertyCascadeForRollback(CascadeLevel) const;
 
@@ -85,7 +85,8 @@
 
     const MatchResult& m_matchResult;
     const IncludedProperties m_includedProperties;
-    const Direction m_direction;
+    mutable Direction m_direction;
+    mutable bool m_directionIsUnresolved { true };
 
     Property m_properties[numCSSProperties + 2];
     std::bitset<numCSSProperties + 2> m_propertyIsPresent;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to