[webkit-reviews] review granted: [Bug 221766] Reduce string copies when converting from NSString/CFStringRef to WTF::String : [Attachment 420020] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Alex Christensen
<achristensen at apple.com>'s request for review:
Bug 221766: Reduce string copies when converting from NSString/CFStringRef to
WTF::String
https://bugs.webkit.org/show_bug.cgi?id=221766

Attachment 420020: Patch

https://bugs.webkit.org/attachment.cgi?id=420020&action=review




--- Comment #2 from Geoffrey Garen <ggaren at apple.com> ---
Comment on attachment 420020
  --> https://bugs.webkit.org/attachment.cgi?id=420020
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=420020&action=review

r=me -- but please revert the buffer casting behavior before landing (or post a
new patch)

> Source/WTF/wtf/text/cf/StringCF.cpp:52
> +    buffer.resize(size * sizeof(UChar) / sizeof(LChar));
> +
> +    StringBuffer<UChar>& ucharBuffer =
*reinterpret_cast<StringBuffer<UChar>*>(&buffer);

I don't think we should do this part. It's not type-safe or
strict-aliasing-safe. Also, in order for the resize() to be an optimization,
malloc would need to over-allocate the original buffer by 2X. In bmalloc, that
will never happen.

So, I recommend just using a new StringBuffer. Or, if I've missed something and
there's a reason to believe that resize() will be faster, then I recommend
adding a releaseRaw() that gives you the malloc buffer, along with an adopting
constructor. That's type-safe and strict-aliasing-safe.

(Side note: I think the argument to resize() should have been just "size".)