<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" /><style type="text/css"><!--
#msg dl { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fc0 solid; padding: 6px; }
#msg ul, pre { overflow: auto; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<title>[24832] trunk/WebCore</title>
</head>
<body>
<div id="msg">
<dl>
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/24832">24832</a></dd>
<dt>Author</dt> <dd>antti</dd>
<dt>Date</dt> <dd>2007-08-02 16:45:27 -0700 (Thu, 02 Aug 2007)</dd>
</dl>
<h3>Log Message</h3>
<pre> 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:]):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkWebCoreChangeLog">trunk/WebCore/ChangeLog</a></li>
<li><a href="#trunkWebCoreeditingTextIteratorcpp">trunk/WebCore/editing/TextIterator.cpp</a></li>
<li><a href="#trunkWebCoreeditingTextIteratorh">trunk/WebCore/editing/TextIterator.h</a></li>
<li><a href="#trunkWebCorepagemacWebCoreFrameBridgemm">trunk/WebCore/page/mac/WebCoreFrameBridge.mm</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/ChangeLog (24831 => 24832)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/ChangeLog 2007-08-02 22:50:26 UTC (rev 24831)
+++ trunk/WebCore/ChangeLog 2007-08-02 23:45:27 UTC (rev 24832)
</span><span class="lines">@@ -1,3 +1,35 @@
</span><ins>+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:]):
+
</ins><span class="cx"> 2007-08-02 David Hyatt <hyatt@apple.com>
</span><span class="cx">
</span><span class="cx"> Fix for 5374437, allow comment nodes to be the child of a document.
</span></span></pre></div>
<a id="trunkWebCoreeditingTextIteratorcpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/editing/TextIterator.cpp (24831 => 24832)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/editing/TextIterator.cpp 2007-08-02 22:50:26 UTC (rev 24831)
+++ trunk/WebCore/editing/TextIterator.cpp 2007-08-02 23:45:27 UTC (rev 24832)
</span><span class="lines">@@ -1237,12 +1237,56 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> // --------
</span><ins>+
+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;
+}
</ins><span class="cx">
</span><span class="cx"> DeprecatedString plainText(const Range* r)
</span><span class="cx"> {
</span><del>- DeprecatedString result("");
- for (TextIterator it(r); !it.atEnd(); it.advance())
- result.append(reinterpret_cast<const DeprecatedChar*>(it.characters()), it.length());
</del><ins>+ unsigned length;
+ UChar* buf = plainTextToMallocAllocatedBuffer(r, length);
+ if (!buf)
+ return DeprecatedString("");
+ DeprecatedString result(reinterpret_cast<const DeprecatedChar*>(buf), length);
+ free(buf);
</ins><span class="cx"> return result;
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkWebCoreeditingTextIteratorh"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/editing/TextIterator.h (24831 => 24832)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/editing/TextIterator.h 2007-08-02 22:50:26 UTC (rev 24831)
+++ trunk/WebCore/editing/TextIterator.h 2007-08-02 23:45:27 UTC (rev 24832)
</span><span class="lines">@@ -48,6 +48,7 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> DeprecatedString plainText(const Range*);
</span><ins>+UChar* plainTextToMallocAllocatedBuffer(const Range*, unsigned& bufferLength);
</ins><span class="cx"> PassRefPtr<Range> findPlainText(const Range*, const String&, bool forward, bool caseSensitive);
</span><span class="cx">
</span><span class="cx"> // Iterates through the DOM range, returning all the text, and 0-length boundaries
</span></span></pre></div>
<a id="trunkWebCorepagemacWebCoreFrameBridgemm"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/page/mac/WebCoreFrameBridge.mm (24831 => 24832)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/page/mac/WebCoreFrameBridge.mm 2007-08-02 22:50:26 UTC (rev 24831)
+++ trunk/WebCore/page/mac/WebCoreFrameBridge.mm 2007-08-02 23:45:27 UTC (rev 24832)
</span><span class="lines">@@ -362,14 +362,26 @@
</span><span class="cx"> {
</span><span class="cx"> String text = m_frame->selectedText();
</span><span class="cx"> text.replace('\\', m_frame->backslashAsCurrencySymbol());
</span><del>- return [[(NSString*)text copy] autorelease];
</del><ins>+ return text;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> - (NSString *)stringForRange:(DOMRange *)range
</span><span class="cx"> {
</span><del>- String text = plainText([range _range]);
- text.replace('\\', m_frame->backslashAsCurrencySymbol());
- return [[(NSString*)text copy] autorelease];
</del><ins>+ // 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];
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> - (void)reapplyStylesForDeviceType:(WebCoreDeviceType)deviceType
</span><span class="lines">@@ -803,8 +815,11 @@
</span><span class="cx">
</span><span class="cx"> - (void)setBaseBackgroundColor:(NSColor *)backgroundColor
</span><span class="cx"> {
</span><ins>+ float a = [backgroundColor alphaComponent];
</ins><span class="cx"> if (m_frame && m_frame->view()) {
</span><span class="cx"> NSColor *deviceColor = [backgroundColor colorUsingColorSpaceName:NSDeviceRGBColorSpace];
</span><ins>+ float a2 = [deviceColor alphaComponent];
+ if (a && a2);
</ins><span class="cx"> Color color = Color(makeRGBA((int)(255 * [deviceColor redComponent]),
</span><span class="cx"> (int)(255 * [deviceColor blueComponent]),
</span><span class="cx"> (int)(255 * [deviceColor greenComponent]),
</span></span></pre>
</div>
</div>
</body>
</html>