From a36470c68ccaef89cc4700283b8bdab485bb830e Mon Sep 17 00:00:00 2001 From: "benjamin@webkit.org" Date: Fri, 27 Jan 2012 22:47:39 +0000 Subject: [PATCH] Speed up the prefix matching of cssPropertyName() https://bugs.webkit.org/show_bug.cgi?id=77158 Patch by Benjamin Poulain 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 --- Source/WebCore/ChangeLog | 28 +++++ .../bindings/js/JSCSSStyleDeclarationCustom.cpp | 113 ++++++++++++++++----- 2 files changed, 114 insertions(+), 27 deletions(-) diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index b432dfb..b64b457 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,31 @@ +2012-01-27 Benjamin Poulain + + 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 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 --- 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 +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++])); -- 2.7.4