Reviewed by Darin.
authorantti <antti@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 2 Aug 2007 23:45:27 +0000 (23:45 +0000)
committerantti <antti@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 2 Aug 2007 23:45:27 +0000 (23:45 +0000)
        <rdar://problem/5355951>
        plainText() fragments TCMalloc heap badly on large pages

        also likely fixes some cases of
        <rdar://problem/5335382>
        CrashTracer: [REGRESSION] 73 crashes in Safari at com.apple.WebCore: WebCore::DeprecatedStringData::increaseUnicodeSize + 52

        If you load http://dscoder.com/test.txt with WebKit build with TCMalloc and system malloc you see that
        Safari RPRVT with TCMalloc is 118.8MB
        Safari RPRVT with system malloc is 69.7MB

        Difference is almost entirely caused by heap fragmentation from a full document plainText() call (for indexing purposes).

        The patch helps in two ways:
        - construct plainText string in pieces to avoid O(n^2) reallocs
        - allocate buffers using system malloc so they can be returned back to OS and don't fragment and grow TCMalloc heap

        This shrinks http://dscoder.com/test.txt RPRVT to 79.0MB and makes full document plainText() take 50ms instead of 500ms.
        The benefits are not limited to extreme cases, web pages above ~200kB can show substantial improvement in RPRVT.

        * editing/TextIterator.cpp:
        (WebCore::plainTextToMallocAllocatedBuffer):
        (WebCore::plainText):
        * editing/TextIterator.h:
        * page/mac/WebCoreFrameBridge.mm:
        (-[WebCoreFrameBridge selectedString]):
        (-[WebCoreFrameBridge stringForRange:]):

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

WebCore/ChangeLog
WebCore/editing/TextIterator.cpp
WebCore/editing/TextIterator.h
WebCore/page/mac/WebCoreFrameBridge.mm

index 9b40416e6390294a8c049fef850a85f6d0e6f48f..9f3ee167e79da5d275c29a475bca6f4c3e07010c 100644 (file)
@@ -1,3 +1,35 @@
+2007-08-02  Antti Koivisto  <antti@apple.com>
+
+        Reviewed by Darin.
+
+        <rdar://problem/5355951>
+        plainText() fragments TCMalloc heap badly on large pages
+        
+        also likely fixes some cases of
+        <rdar://problem/5335382>
+        CrashTracer: [REGRESSION] 73 crashes in Safari at com.apple.WebCore: WebCore::DeprecatedStringData::increaseUnicodeSize + 52
+        
+        If you load http://dscoder.com/test.txt with WebKit build with TCMalloc and system malloc you see that
+        Safari RPRVT with TCMalloc is 118.8MB
+        Safari RPRVT with system malloc is 69.7MB
+        
+        Difference is almost entirely caused by heap fragmentation from a full document plainText() call (for indexing purposes).
+        
+        The patch helps in two ways:
+        - construct plainText string in pieces to avoid O(n^2) reallocs
+        - allocate buffers using system malloc so they can be returned back to OS and don't fragment and grow TCMalloc heap
+    
+        This shrinks http://dscoder.com/test.txt RPRVT to 79.0MB and makes full document plainText() take 50ms instead of 500ms.
+        The benefits are not limited to extreme cases, web pages above ~200kB can show substantial improvement in RPRVT.
+
+        * editing/TextIterator.cpp:
+        (WebCore::plainTextToMallocAllocatedBuffer):
+        (WebCore::plainText):
+        * editing/TextIterator.h:
+        * page/mac/WebCoreFrameBridge.mm:
+        (-[WebCoreFrameBridge selectedString]):
+        (-[WebCoreFrameBridge stringForRange:]):
+
 2007-08-02  David Hyatt  <hyatt@apple.com>
 
         Fix for 5374437, allow comment nodes to be the child of a document.
index c9c8209bce25074b2ce36ab8a5c2d266aa254c09..9c444629a25ce8d1252904ca7a0112933e22393a 100644 (file)
@@ -1237,12 +1237,56 @@ PassRefPtr<Range> TextIterator::rangeFromLocationAndLength(Element *scope, int r
 }
 
 // --------
+    
+UChar* plainTextToMallocAllocatedBuffer(const Range* r, unsigned& bufferLength) 
+{
+    // Do this in pieces to avoid massive reallocations if there is a large amount of text.
+    // Use system malloc for buffers since they can consume lots of memory and current TCMalloc is unable return it back to OS
+    static const unsigned cMaxSegmentSize = 1 << 16;
+    bufferLength = 0;
+    Vector<pair<UChar*, unsigned> >* textSegments = 0;
+    Vector<UChar> textBuffer;
+    textBuffer.reserveCapacity(cMaxSegmentSize);
+    for (TextIterator it(r); !it.atEnd(); it.advance()) {
+        if (textBuffer.size() && textBuffer.size() + it.length() > cMaxSegmentSize) {
+            if (!textSegments)
+                textSegments = new Vector<pair<UChar*, unsigned> >;
+            pair<UChar*, unsigned> newSegment(static_cast<UChar*>(malloc(textBuffer.size() * sizeof(UChar))), textBuffer.size());
+            memcpy(newSegment.first, textBuffer.data(), textBuffer.size() * sizeof(UChar));
+            textSegments->append(newSegment);
+            textBuffer.clear();
+        }
+        textBuffer.append(it.characters(), it.length());
+        bufferLength += it.length();
+    }
+    
+    if (!bufferLength)
+        return 0;
+    
+    // Since we know the size now, we can make a single buffer out of the pieces with one big alloc
+    UChar* resultBuffer = static_cast<UChar*>(malloc(bufferLength * sizeof(UChar)));
+    UChar* resultPos = resultBuffer;
+    if (textSegments) {
+        for (unsigned n = 0; n < textSegments->size(); n++) {
+            const pair<UChar*, unsigned>& s = textSegments->at(n);
+            memcpy(resultPos, s.first, s.second * sizeof(UChar));
+            free(s.first);
+            resultPos += s.second;
+        }
+        delete textSegments;
+    }
+    memcpy(resultPos, textBuffer.data(), textBuffer.size() * sizeof(UChar));
+    return resultBuffer;
+}
 
 DeprecatedString plainText(const Range* r)
 {
-    DeprecatedString result("");
-    for (TextIterator it(r); !it.atEnd(); it.advance())
-        result.append(reinterpret_cast<const DeprecatedChar*>(it.characters()), it.length());
+    unsigned length;
+    UChar* buf = plainTextToMallocAllocatedBuffer(r, length);
+    if (!buf)
+        return DeprecatedString("");
+    DeprecatedString result(reinterpret_cast<const DeprecatedChar*>(buf), length);
+    free(buf);
     return result;
 }
 
index 6ae2d30d27b9514ac3ca51a072788804c1b3526b..9b22c19f76dd607a71e2c4cf5a56c3dd98d35bf7 100644 (file)
@@ -48,6 +48,7 @@ inline bool isCollapsibleWhitespace(UChar c)
 }
 
 DeprecatedString plainText(const Range*);
+UChar* plainTextToMallocAllocatedBuffer(const Range*, unsigned& bufferLength);
 PassRefPtr<Range> findPlainText(const Range*, const String&, bool forward, bool caseSensitive);
 
 // Iterates through the DOM range, returning all the text, and 0-length boundaries
index 4b4d877e9c9e75df0a67390dc3db93e2428da95a..3a7e310ae70df800ebff80759185fc918c979251 100644 (file)
@@ -362,14 +362,26 @@ static inline WebCoreFrameBridge *bridge(Frame *frame)
 {
     String text = m_frame->selectedText();
     text.replace('\\', m_frame->backslashAsCurrencySymbol());
-    return [[(NSString*)text copy] autorelease];
+    return text;
 }
 
 - (NSString *)stringForRange:(DOMRange *)range
 {
-    String text = plainText([range _range]);
-    text.replace('\\', m_frame->backslashAsCurrencySymbol());
-    return [[(NSString*)text copy] autorelease];
+    // This will give a system malloc'd buffer that can be turned directly into an NSString
+    unsigned length;
+    UChar* buf = plainTextToMallocAllocatedBuffer([range _range], length);
+    
+    if (!buf)
+        return [NSString string];
+    
+    UChar backslashAsCurrencySymbol = m_frame->backslashAsCurrencySymbol();
+    if (backslashAsCurrencySymbol != '\\')
+        for (unsigned n = 0; n < length; n++) 
+            if (buf[n] == '\\')
+                buf[n] = backslashAsCurrencySymbol;
+
+    // Transfer buffer ownership to NSString
+    return [[[NSString alloc] initWithCharactersNoCopy:buf length:length freeWhenDone:YES] autorelease];
 }
 
 - (void)reapplyStylesForDeviceType:(WebCoreDeviceType)deviceType
@@ -803,8 +815,11 @@ static HTMLFormElement *formElementFromDOMElement(DOMElement *element)
 
 - (void)setBaseBackgroundColor:(NSColor *)backgroundColor
 {
+    float a = [backgroundColor alphaComponent];
     if (m_frame && m_frame->view()) {
         NSColor *deviceColor = [backgroundColor colorUsingColorSpaceName:NSDeviceRGBColorSpace];
+        float a2 = [deviceColor alphaComponent];
+        if (a && a2);
         Color color = Color(makeRGBA((int)(255 * [deviceColor redComponent]),
                                      (int)(255 * [deviceColor blueComponent]),
                                      (int)(255 * [deviceColor greenComponent]),