Changeset 154241 in webkit

Timestamp:

Aug 17, 2013, 3:05:27 PM (12 years ago)

Author:

Antti Koivisto

Message:

<​https://webkit.org/b/119960> Remove some optimizations made obsolete by use of StringBuilder

Reviewed by Andreas Kling.

• dom/ScriptElement.cpp:

(WebCore::ScriptElement::scriptContent):

StringBuilder already optimizes for the single string case. If there is only one the original string is returned.

• dom/Text.cpp:

(WebCore::Text::wholeText):

No need to traverse twice to compute the capacity. StringBuilder handles this efficiently.
Also in the common case there is only one string and the optimization here is actually hurting by disabling the StringBuilder one.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/ScriptElement.cpp (modified) (1 diff)
• dom/Text.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-08-17  Antti Koivisto  <antti@apple.com>
+
+        <https://webkit.org/b/119960> Remove some optimizations made obsolete by use of StringBuilder
+
+        Reviewed by Andreas Kling.
+
+        * dom/ScriptElement.cpp:
+        (WebCore::ScriptElement::scriptContent):
+        
+            StringBuilder already optimizes for the single string case. If there is only one the original string is returned.
+
+        * dom/Text.cpp:
+        (WebCore::Text::wholeText):
+        
+            No need to traverse twice to compute the capacity. StringBuilder handles this efficiently. 
+            Also in the common case there is only one string and the optimization here is actually hurting by disabling the StringBuilder one.
+
 2013-08-17  Antti Koivisto  <antti@apple.com>
 

trunk/Source/WebCore/dom/ScriptElement.cpp

@@ -395 +395 @@
 String ScriptElement::scriptContent() const
 {
-    StringBuilder content;
-    Text* firstTextNode = 0;
-    bool foundMultipleTextNodes = false;
-
-    for (Text* textNode = TextNodeTraversal::firstChild(m_element); textNode; textNode = TextNodeTraversal::nextSibling(textNode)) {
-        if (foundMultipleTextNodes)
-            content.append(textNode->data());
-        else if (firstTextNode) {
-            content.append(firstTextNode->data());
-            content.append(textNode->data());
-            foundMultipleTextNodes = true;
-        } else
-            firstTextNode = textNode;
-    }
-
-    if (firstTextNode && !foundMultipleTextNodes)
-        return firstTextNode->data();
-
-    return content.toString();
+    return TextNodeTraversal::contentsAsString(m_element);
 }
 

trunk/Source/WebCore/dom/Text.cpp

@@ -121 +121 @@
     const Text* startText = earliestLogicallyAdjacentTextNode(this);
     const Text* endText = latestLogicallyAdjacentTextNode(this);
-
-    Node* onePastEndText = TextNodeTraversal::nextSibling(endText);
-    Checked<unsigned> resultLength = 0;
-    for (const Text* text = startText; text != onePastEndText; text = TextNodeTraversal::nextSibling(text))
-        resultLength += text->length();
+    const Node* onePastEndText = TextNodeTraversal::nextSibling(endText);
 
     StringBuilder result;
-    result.reserveCapacity(resultLength.unsafeGet());
     for (const Text* text = startText; text != onePastEndText; text = TextNodeTraversal::nextSibling(text))
         result.append(text->data());
-    ASSERT(result.length() == resultLength.unsafeGet());
-
     return result.toString();
 }