Optimize parseHTMLInteger()
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 27 Feb 2016 18:30:58 +0000 (18:30 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 27 Feb 2016 18:30:58 +0000 (18:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=154772

Reviewed by Ryosuke Niwa.

Optimize parseHTMLInteger() a bit now that it is used a lot more and
that it has decent API test coverage. In particular, we now:
- Avoid using a StringBuilder for the digits.
- Get rid of a is8Bit() branch.
- Only traverse the input string once.

* html/parser/HTMLParserIdioms.cpp:
(WebCore::parseHTMLIntegerInternal):
(WebCore::parseHTMLInteger):
(WebCore::parseHTMLNonNegativeInteger): Deleted.

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

Source/WebCore/ChangeLog
Source/WebCore/html/parser/HTMLParserIdioms.cpp

index 346bd42..3b78da8 100644 (file)
@@ -1,3 +1,21 @@
+2016-02-27  Chris Dumez  <cdumez@apple.com>
+
+        Optimize parseHTMLInteger()
+        https://bugs.webkit.org/show_bug.cgi?id=154772
+
+        Reviewed by Ryosuke Niwa.
+
+        Optimize parseHTMLInteger() a bit now that it is used a lot more and
+        that it has decent API test coverage. In particular, we now:
+        - Avoid using a StringBuilder for the digits.
+        - Get rid of a is8Bit() branch.
+        - Only traverse the input string once.
+
+        * html/parser/HTMLParserIdioms.cpp:
+        (WebCore::parseHTMLIntegerInternal):
+        (WebCore::parseHTMLInteger):
+        (WebCore::parseHTMLNonNegativeInteger): Deleted.
+
 2016-02-27  Andreas Kling  <akling@apple.com>
 
         [iOS] Discard decoded image data on top-level navigation.
index 03ca192..b599617 100644 (file)
@@ -155,66 +155,54 @@ double parseToDoubleForNumberType(const String& string)
 template <typename CharacterType>
 static bool parseHTMLIntegerInternal(const CharacterType* position, const CharacterType* end, int& value)
 {
-    // Step 3
-    bool isPositive = true;
-
-    // Step 4
-    while (position < end) {
-        if (!isHTMLSpace(*position))
-            break;
+    while (position < end && isHTMLSpace(*position))
         ++position;
-    }
 
-    // Step 5
     if (position == end)
         return false;
-    ASSERT_WITH_SECURITY_IMPLICATION(position < end);
 
-    // Step 6
+    bool isNegative = false;
     if (*position == '-') {
-        isPositive = false;
+        isNegative = true;
         ++position;
     } else if (*position == '+')
         ++position;
-    if (position == end)
-        return false;
-    ASSERT_WITH_SECURITY_IMPLICATION(position < end);
 
-    // Step 7
-    if (!isASCIIDigit(*position))
+    if (position == end || !isASCIIDigit(*position))
         return false;
 
-    // Step 8
-    StringBuilder cleanCharacters;
-    if (!isPositive)
-        cleanCharacters.append('-');
-    while (position < end) {
-        if (!isASCIIDigit(*position))
-            break;
-        cleanCharacters.append(*position++);
-    }
+    constexpr int intMax = std::numeric_limits<int>::max();
+    constexpr int base = 10;
+    constexpr int maxMultiplier = intMax / base;
+
+    unsigned result = 0;
+    do {
+        int digitValue = *position - '0';
 
-    // Step 9
-    bool ok;
-    if (cleanCharacters.is8Bit())
-        value = charactersToIntStrict(cleanCharacters.characters8(), cleanCharacters.length(), &ok);
-    else
-        value = charactersToIntStrict(cleanCharacters.characters16(), cleanCharacters.length(), &ok);
-    return ok;
+        if (result > maxMultiplier || (result == maxMultiplier && digitValue > (intMax % base) + isNegative))
+            return false;
+
+        result = base * result + digitValue;
+        ++position;
+    } while (position < end && isASCIIDigit(*position));
+
+    value = isNegative ? -result : result;
+    return true;
 }
 
 // https://html.spec.whatwg.org/multipage/infrastructure.html#rules-for-parsing-integers
 bool parseHTMLInteger(const String& input, int& value)
 {
-    // Step 1
-    // Step 2
     unsigned length = input.length();
-    if (!length || input.is8Bit()) {
-        const LChar* start = input.characters8();
+    if (!length)
+        return false;
+
+    if (LIKELY(input.is8Bit())) {
+        auto* start = input.characters8();
         return parseHTMLIntegerInternal(start, start + length, value);
     }
 
-    const UChar* start = input.characters16();
+    auto* start = input.characters16();
     return parseHTMLIntegerInternal(start, start + length, value);
 }