Changeset 263282 in webkit

Timestamp:

Jun 19, 2020, 1:13:14 PM (6 years ago)

Author:

Chris Dumez

Message:

[iOS] RenderThemeIOS::cssValueToSystemColorMap() does an unnecessary linear search under systemColorFromCSSValueID()
​https://bugs.webkit.org/show_bug.cgi?id=213396

Reviewed by Timothy Hatcher.

RenderThemeIOS::cssValueToSystemColorMap() does an unnecessary linear search under systemColorFromCSSValueID().
cssValueToSystemColorMap() already has the selector, yet it passes a CSSValueID to systemColorFromCSSValueID() which
then does a linear search to match the CSSValueID to a selector. This was very inefficient / unfortunate.

This patch introduces a systemColorFromCSSValueIDSelector() which takes in a selector instead of a CSSValueID. I have
also moved the constructor of the LocalCurrentTraitCollection variable to the call site so that we don't keep
constructing / destroying it for each loop iteration. The traces show us spending a lot of time in its constructor /
destructor.

• rendering/RenderThemeIOS.mm:

(WebCore::systemColorFromCSSValueIDSelector):
(WebCore::RenderThemeIOS::cssValueToSystemColorMap):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/RenderThemeIOS.mm (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-06-19  Chris Dumez  <cdumez@apple.com>
+
+        [iOS] RenderThemeIOS::cssValueToSystemColorMap() does an unnecessary linear search under systemColorFromCSSValueID()
+        https://bugs.webkit.org/show_bug.cgi?id=213396
+
+        Reviewed by Timothy Hatcher.
+
+        RenderThemeIOS::cssValueToSystemColorMap() does an unnecessary linear search under systemColorFromCSSValueID().
+        cssValueToSystemColorMap() already has the selector, yet it passes a CSSValueID to systemColorFromCSSValueID() which
+        then does a linear search to match the CSSValueID to a selector. This was very inefficient / unfortunate.
+
+        This patch introduces a systemColorFromCSSValueIDSelector() which takes in a selector instead of a CSSValueID. I have
+        also moved the constructor of the LocalCurrentTraitCollection variable to the call site so that we don't keep
+        constructing / destroying it for each loop iteration. The traces show us spending a lot of time in its constructor /
+        destructor.
+
+        * rendering/RenderThemeIOS.mm:
+        (WebCore::systemColorFromCSSValueIDSelector):
+        (WebCore::RenderThemeIOS::cssValueToSystemColorMap):
+
 2020-06-19  James Darpinian  <jdarpinian@chromium.org>
 

trunk/Source/WebCore/rendering/RenderThemeIOS.mm

@@ -1284 +1284 @@
 }
 
+static inline Optional<Color> systemColorFromCSSValueIDSelector(CSSValueIDAndSelector idAndSelector)
+{
+    if (auto color = wtfObjCMsgSend<UIColor *>(PAL::getUIColorClass(), idAndSelector.selector))
+        return Color { color.CGColor, Color::Semantic };
+    return WTF::nullopt;
+}
+
 static Optional<Color> systemColorFromCSSValueID(CSSValueID cssValueID, bool useDarkAppearance, bool useElevatedUserInterfaceLevel)
 {
@@ -1315 +1322 @@
     static const NeverDestroyed<CSSValueToSystemColorMap> colorMap = [] {
         CSSValueToSystemColorMap map;
-        for (auto& cssValueIDSelector : cssValueIDSelectorList()) {
-            for (bool useDarkAppearance : { false, true }) {
-                for (bool useElevatedUserInterfaceLevel : { false, true }) {
-                    if (auto color = systemColorFromCSSValueID(cssValueIDSelector.cssValueID, useDarkAppearance, useElevatedUserInterfaceLevel))
-                        map.add(CSSValueKey { cssValueIDSelector.cssValueID, useDarkAppearance, useElevatedUserInterfaceLevel }, *color);
+        for (bool useDarkAppearance : { false, true }) {
+            for (bool useElevatedUserInterfaceLevel : { false, true }) {
+                LocalCurrentTraitCollection localTraitCollection(useDarkAppearance, useElevatedUserInterfaceLevel);
+                for (auto& cssValueIDSelector : cssValueIDSelectorList()) {
+                    if (auto color = systemColorFromCSSValueIDSelector(cssValueIDSelector))
+                        map.add(CSSValueKey { cssValueIDSelector.cssValueID, useDarkAppearance, useElevatedUserInterfaceLevel }, WTFMove(*color));
                 }
             }