Changeset 49403 in webkit

Timestamp:

Oct 9, 2009 2:24:28 PM (15 years ago)

Author:

eric@webkit.org

Message:

2009-10-09 Jens Alfke <​snej@chromium.org>

Reviewed by Darin Adler.

Optimization: Many StringImpl transformations are no-ops and should just return 'this'
​https://bugs.webkit.org/show_bug.cgi?id=30186

Optimized StringImpl methods lower(), stripWhiteSpace() and simplifyWhiteSpace() to
detect no-ops and return this instead of creating a new instance.
Empirical testing shows that the majority of calls to these methods are no-ops, making
this worthwhile even if (in the case of lower()) the non-no-op case is slightly slowed.
Upper() is very rarely a no-op, so it wasn't worthwhile to optimize it.

• platform/text/StringImpl.cpp: (WebCore::StringImpl::lower): (WebCore::StringImpl::upper): Just add a comment explaining why this wasn't optimized (WebCore::StringImpl::stripWhiteSpace): (WebCore::StringImpl::simplifyWhiteSpace):

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/text/StringImpl.cpp (modified) (5 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2009-10-09  Jens Alfke  <snej@chromium.org>
+
+        Reviewed by Darin Adler.
+        
+        Optimization: Many StringImpl transformations are no-ops and should just return 'this'
+        https://bugs.webkit.org/show_bug.cgi?id=30186
+
+        Optimized StringImpl methods lower(), stripWhiteSpace() and simplifyWhiteSpace() to
+        detect no-ops and return this instead of creating a new instance.
+        Empirical testing shows that the majority of calls to these methods are no-ops, making
+        this worthwhile even if (in the case of lower()) the non-no-op case is slightly slowed.
+        Upper() is very rarely a no-op, so it wasn't worthwhile to optimize it.
+
+        * platform/text/StringImpl.cpp:
+        (WebCore::StringImpl::lower):
+        (WebCore::StringImpl::upper): Just add a comment explaining why this wasn't optimized
+        (WebCore::StringImpl::stripWhiteSpace):
+        (WebCore::StringImpl::simplifyWhiteSpace):
+
 2009-10-09  Dirk Schulze  <krit@webkit.org>
 

trunk/WebCore/platform/text/StringImpl.cpp

@@ -176 +176 @@
 PassRefPtr<StringImpl> StringImpl::lower()
 {
-    UChar* data;
-    PassRefPtr<StringImpl> newImpl = createUninitialized(m_length, data);
+    // First scan the string for uppercase and non-ASCII characters:
     int32_t length = m_length;
-
-    // Do a faster loop for the case where all the characters are ASCII.
     UChar ored = 0;
+    bool noUpper = true;
     for (int i = 0; i < length; i++) {
         UChar c = m_data[i];
         ored |= c;
-        data[i] = toASCIILower(c);
-    }
-    if (!(ored & ~0x7F))
+        noUpper = noUpper && !isASCIIUpper(c);
+    }
+    
+    // Nothing to do if the string is all ASCII with no uppercase.
+    if (noUpper && !(ored & ~0x7F))
+        return this;
+
+    UChar* data;
+    RefPtr<StringImpl> newImpl = createUninitialized(m_length, data);
+
+    if (!(ored & ~0x7F)) {
+        // Do a faster loop for the case where all the characters are ASCII.
+        for (int i = 0; i < length; i++) {
+            UChar c = m_data[i];
+            data[i] = toASCIILower(c);
+        }
         return newImpl;
-
+    }
+    
     // Do a slower implementation for cases that include non-ASCII characters.
     bool error;
@@ -204 +216 @@
 PassRefPtr<StringImpl> StringImpl::upper()
 {
+    // This function could be optimized for no-op cases the way lower() is,
+    // but in empirical testing, few actual calls to upper() are no-ops, so
+    // it wouldn't be worth the extra time for pre-scanning.
     UChar* data;
     PassRefPtr<StringImpl> newImpl = createUninitialized(m_length, data);
@@ -288 +303 @@
         end--;
 
+    if (!start && end == m_length - 1)
+        return this;
     return create(m_data + start, end + 1 - start);
 }
@@ -330 +347 @@
     const UChar* fromend = from + m_length;
     int outc = 0;
+    bool changedToSpace = false;
     
     UChar* to = data.characters();
     
     while (true) {
-        while (from != fromend && isSpaceOrNewline(*from))
+        while (from != fromend && isSpaceOrNewline(*from)) {
+            if (*from != ' ')
+                changedToSpace = true;
             from++;
+        }
         while (from != fromend && !isSpaceOrNewline(*from))
             to[outc++] = *from++;
@@ -346 +367 @@
     if (outc > 0 && to[outc - 1] == ' ')
         outc--;
+    
+    if (static_cast<unsigned>(outc) == m_length && !changedToSpace)
+        return this;
     
     data.shrink(outc);