Speed up the prefix matching of cssPropertyName()

https://bugs.webkit.org/show_bug.cgi?id=77158

Patch by Benjamin Poulain <bpoulain@apple.com> on 2012-01-27
Reviewed by Geoffrey Garen.

This patch improves the performance by:
-not checking the PropertyName with all 7 prefix
 (now, in the worse case, 2 prefixes are checked)
-avoiding the conversion 8bits->16bits by using String::operator[]
 instead of ::characters()

To avoid checking every prefix, the code branch based on the first
characters of the propertyName.
The remaining of the prefix is checked character by characters like before.

When accessing CSS property, this gives a 13% improvement when there is no prefix.
There is no performance regression for the matching of prefix, including for the first one
of the previous matching code ("css").

* bindings/js/JSCSSStyleDeclarationCustom.cpp:
():
(WebCore::matchesCSSPropertyNamePrefix):
(WebCore):
(WebCore::getCSSPropertyNamePrefix):
(WebCore::cssPropertyName):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@106154 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index b432dfb..b64b457 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,31 @@
+2012-01-27  Benjamin Poulain  <bpoulain@apple.com>
+
+        Speed up the prefix matching of cssPropertyName()
+        https://bugs.webkit.org/show_bug.cgi?id=77158
+
+        Reviewed by Geoffrey Garen.
+
+        This patch improves the performance by:
+        -not checking the PropertyName with all 7 prefix
+         (now, in the worse case, 2 prefixes are checked)
+        -avoiding the conversion 8bits->16bits by using String::operator[]
+         instead of ::characters()
+
+        To avoid checking every prefix, the code branch based on the first
+        characters of the propertyName.
+        The remaining of the prefix is checked character by characters like before.
+
+        When accessing CSS property, this gives a 13% improvement when there is no prefix.
+        There is no performance regression for the matching of prefix, including for the first one
+        of the previous matching code ("css").
+
+        * bindings/js/JSCSSStyleDeclarationCustom.cpp:
+        ():
+        (WebCore::matchesCSSPropertyNamePrefix):
+        (WebCore):
+        (WebCore::getCSSPropertyNamePrefix):
+        (WebCore::cssPropertyName):
+
 2012-01-27  Ken Buchanan  <kenrb@chromium.org>
 
         Crash in updateFirstLetter() from unnecessary anonymous block

diff --git a/Source/WebCore/bindings/js/JSCSSStyleDeclarationCustom.cpp b/Source/WebCore/bindings/js/JSCSSStyleDeclarationCustom.cpp
index 1fab242..23fda48 100644 (file)
--- a/Source/WebCore/bindings/js/JSCSSStyleDeclarationCustom.cpp
+++ b/Source/WebCore/bindings/js/JSCSSStyleDeclarationCustom.cpp
@@ -56,31 +56,85 @@ void JSCSSStyleDeclaration::visitChildren(JSCell* cell, SlotVisitor& visitor)
     visitor.addOpaqueRoot(root(thisObject->impl()));
 }
 
-// Check for a CSS prefix.
-// Passed prefix is all lowercase.
-// First character of the prefix within the property name may be upper or lowercase.
-// Other characters in the prefix within the property name must be lowercase.
-// The prefix within the property name must be followed by a capital letter.
-static bool hasCSSPropertyNamePrefix(const Identifier& propertyName, const char* prefix)
+enum PropertyNamePrefix
 {
+    PropertyNamePrefixNone,
+    PropertyNamePrefixCSS,
+    PropertyNamePrefixPixel,
+    PropertyNamePrefixPos,
+    PropertyNamePrefixApple,
+    PropertyNamePrefixEpub,
+    PropertyNamePrefixKHTML,
+    PropertyNamePrefixWebKit
+};
+
+template<size_t prefixCStringLength>
+static inline bool matchesCSSPropertyNamePrefix(const StringImpl& propertyName, const char (&prefix)[prefixCStringLength])
+{
+    size_t prefixLength = prefixCStringLength - 1;
+
+    ASSERT(toASCIILower(propertyName[0]) == prefix[0]);
+    const size_t offset = 1;
+
 #ifndef NDEBUG
-    ASSERT(*prefix);
-    for (const char* p = prefix; *p; ++p)
-        ASSERT(isASCIILower(*p));
+    for (size_t i = 0; i < prefixLength; ++i)
+        ASSERT(isASCIILower(prefix[i]));
+    ASSERT(!prefix[prefixLength]);
     ASSERT(propertyName.length());
 #endif
 
-    if (toASCIILower(propertyName.characters()[0]) != prefix[0])
+    // The prefix within the property name must be followed by a capital letter.
+    // Other characters in the prefix within the property name must be lowercase.
+    if (propertyName.length() < (prefixLength + 1))
         return false;
 
-    unsigned length = propertyName.length();
-    for (unsigned i = 1; i < length; ++i) {
-        if (!prefix[i])
-            return isASCIIUpper(propertyName.characters()[i]);
-        if (propertyName.characters()[i] != prefix[i])
+    for (size_t i = offset; i < prefixLength; ++i) {
+        if (propertyName[i] != prefix[i])
             return false;
     }
-    return false;
+
+    if (!isASCIIUpper(propertyName[prefixLength]))
+        return false;
+    return true;
+}
+
+static PropertyNamePrefix getCSSPropertyNamePrefix(const StringImpl& propertyName)
+{
+    ASSERT(propertyName.length());
+
+    // First character of the prefix within the property name may be upper or lowercase.
+    UChar firstChar = toASCIILower(propertyName[0]);
+    switch (firstChar) {
+    case 'a':
+        if (matchesCSSPropertyNamePrefix(propertyName, "apple"))
+            return PropertyNamePrefixApple;
+        break;
+    case 'c':
+        if (matchesCSSPropertyNamePrefix(propertyName, "css"))
+            return PropertyNamePrefixCSS;
+        break;
+    case 'k':
+        if (matchesCSSPropertyNamePrefix(propertyName, "khtml"))
+            return PropertyNamePrefixKHTML;
+        break;
+    case 'e':
+        if (matchesCSSPropertyNamePrefix(propertyName, "epub"))
+            return PropertyNamePrefixEpub;
+        break;
+    case 'p':
+        if (matchesCSSPropertyNamePrefix(propertyName, "pos"))
+            return PropertyNamePrefixPos;
+        if (matchesCSSPropertyNamePrefix(propertyName, "pixel"))
+            return PropertyNamePrefixPixel;
+        break;
+    case 'w':
+        if (matchesCSSPropertyNamePrefix(propertyName, "webkit"))
+            return PropertyNamePrefixWebKit;
+        break;
+    default:
+        break;
+    }
+    return PropertyNamePrefixNone;
 }
 
 static String cssPropertyName(const Identifier& propertyName, bool* hadPixelOrPosPrefix = 0)
@@ -95,26 +149,31 @@ static String cssPropertyName(const Identifier& propertyName, bool* hadPixelOrPo
     StringBuilder builder;
     builder.reserveCapacity(length);
 
+    const StringImpl* propertyNameString = propertyName.impl();
     unsigned i = 0;
-
-    if (hasCSSPropertyNamePrefix(propertyName, "css"))
+    switch (getCSSPropertyNamePrefix(*propertyNameString)) {
+    case PropertyNamePrefixNone:
+        if (isASCIIUpper((*propertyNameString)[0]))
+            return String();
+        break;
+    case PropertyNamePrefixCSS:
         i += 3;
-    else if (hasCSSPropertyNamePrefix(propertyName, "pixel")) {
+        break;
+    case PropertyNamePrefixPixel:
         i += 5;
         if (hadPixelOrPosPrefix)
             *hadPixelOrPosPrefix = true;
-    } else if (hasCSSPropertyNamePrefix(propertyName, "pos")) {
+        break;
+    case PropertyNamePrefixPos:
         i += 3;
         if (hadPixelOrPosPrefix)
             *hadPixelOrPosPrefix = true;
-    } else if (hasCSSPropertyNamePrefix(propertyName, "webkit")
-            || hasCSSPropertyNamePrefix(propertyName, "khtml")
-            || hasCSSPropertyNamePrefix(propertyName, "apple")
-            || hasCSSPropertyNamePrefix(propertyName, "epub"))
+        break;
+    case PropertyNamePrefixApple:
+    case PropertyNamePrefixEpub:
+    case PropertyNamePrefixKHTML:
+    case PropertyNamePrefixWebKit:
         builder.append('-');
-    else {
-        if (isASCIIUpper(propertyName.characters()[0]))
-            return String();
     }
 
     builder.append(toASCIILower(propertyName.characters()[i++]));