[Webkit-unassigned] [Bug 31415] [v8] do not copy data twice when converting short v8 string into WebCore::String

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=31415


Dmitry Titov <dimich at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #43080|review?                     |review-
               Flag|                            |




--- Comment #5 from Dmitry Titov <dimich at chromium.org>  2009-11-12 14:48:40 PST ---
(From update of attachment 43080)
> Index: WebCore/ChangeLog

> +        Do not use WebCore::String::String(const UChar*, int length) to convert
> +        short v8 strings.
> +
> +        Plus added string traits.

It is good to add the reason for the change to the ChangeLog as well as what
was done.
Ideally, perf optimizations would come with some measurement to show the actual
win.

> +    static String convertFromV8(v8::Handle<v8::String> v8String, int length) {
> +        ASSERT(v8String->Length() == length);
> +        // NOTE: as of now, String(const UChar*, int) performs String::createUninitialized
> +        // anyway, so no need to optimize like we do for AtomicString below.
> +        UChar* buffer;
> +        String result = String::createUninitialized(length, buffer);
> +        v8String->Write(reinterpret_cast<uint16_t*>(buffer), 0, length);
> +        return result;

It is unfortunate that this introduces almost a duplicate of the code... Could
you consider having the StringTraits to have some simple bool flag, like
"useInlineBuffer" and have a single copy of the code, something like this:
if (StringTraits<StringType>::useInlineBuffer() && length <= inlineBufferSize)
... use inline buffer ...
} else {
... use createUninitialized ...
}
or some other way to keep the code in one place?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.