RESOLVED FIXED Bug 66399
Reduce usages of String::createUninitialized
https://bugs.webkit.org/show_bug.cgi?id=66399
Summary Reduce usages of String::createUninitialized
Annie Sullivan
Reported 2011-08-17 13:04:04 PDT
WTF::String is supposed to be immutable, but createUninitialized() allows callers to hang on to a non-const reference to the underlying data buffer. Some of these usages could be replaced by StringBuilder.
Attachments
Patch (7.30 KB, patch)
2011-08-17 13:08 PDT, Annie Sullivan
no flags
Annie Sullivan
Comment 1 2011-08-17 13:08:52 PDT
Annie Sullivan
Comment 2 2011-08-17 13:09:25 PDT
If anyone has suggestions for performance benchmark tests to run before submitting, please comment.
James Robinson
Comment 3 2011-08-17 13:12:04 PDT
PerformanceTests/Parser has a good test for the html parser. I'm not sure if that test hits these codepaths or not.
Annie Sullivan
Comment 4 2011-08-17 13:40:49 PDT
(In reply to comment #3) > PerformanceTests/Parser has a good test for the html parser. I'm not sure if that test hits these codepaths or not. Numbers from html-parser.html look similar before/after applying my patch. Before patch: avg 1605.3 median 1604 stdev 9.935290634903437 min 1594 max 1629 After patch: avg 1595.1 median 1592 stdev 6.579513659838393 min 1589 max 1609
Adam Barth
Comment 5 2011-08-17 14:04:44 PDT
Comment on attachment 104219 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=104219&action=review > Source/WebCore/html/parser/HTMLSourceTracker.cpp:59 > + int length = token.endIndex() - token.startIndex(); int => size_t probably
Adam Barth
Comment 6 2011-08-17 14:06:55 PDT
There shouldn't be any perf impact. ReserveCapacity effectively does the same thing by pre-allocating the buffer.
WebKit Review Bot
Comment 7 2011-08-17 21:23:13 PDT
Comment on attachment 104219 [details] Patch Clearing flags on attachment: 104219 Committed r93281: <http://trac.webkit.org/changeset/93281>
WebKit Review Bot
Comment 8 2011-08-17 21:23:18 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.