[webkit-reviews] review denied: [Bug 25779] Allow Strings to be created with one malloc node with no copying : [Attachment 30639] Additional changes
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sun May 24 22:41:56 PDT 2009
Maciej Stachowiak <mjs at apple.com> has denied Dave Moore
<davemoore at google.com>'s request for review:
Bug 25779: Allow Strings to be created with one malloc node with no copying
https://bugs.webkit.org/show_bug.cgi?id=25779
Attachment 30639: Additional changes
https://bugs.webkit.org/attachment.cgi?id=30639&action=review
------- Additional Comments from Maciej Stachowiak <mjs at apple.com>
This patch should really go in a separate bugzilla bug, as originally suggested
by Darin. It is awkward to have a new patch for review on a resolved bug. r-
for that but I'd be glad to give substantive review when it is attached to a
new bug report. I hope you don't mind the r- for process reasons, it just gets
messy to have bug reports reused for a different purpose, especially when
already resolved. In general I think this work is great, just trying to keep
the bug system clean.
A few quick comments:
1) Please listen to the script and <set EMAIL_ADDRESS environment variable> :-)
2) For determining the tradeoff point for inline buffers - you could make a
microbenchmark to check or make a rough estimate. The tradeoff is basically
this: (a) FastMalloc allocate a StringImpl + allocate a buffer of N UChars. (b)
One-shot allocate a StringImpl with extra space for N UChars, and copy N
UChars. When N is large, the copy will be more expensive than the extra
allocation of the StringImpl body. It should be possible to estimate the
tradeoff point with microbenchmarks. Also, since space for N UChars will be
reserved on the stack, there is a practical limit to N, to avoid risk of
blowing the stack.
More information about the webkit-reviews
mailing list