[webkit-reviews] review denied: [Bug 76592] [v8]Low efficiency of writing long string in npruntime infrastructure : [Attachment 123069] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jan 18 21:55:46 PST 2012


Adam Barth <abarth at webkit.org> has denied joey <li.yin at intel.com>'s request for
review:
Bug 76592: [v8]Low efficiency of writing long string in npruntime
infrastructure
https://bugs.webkit.org/show_bug.cgi?id=76592

Attachment 123069: Patch
https://bugs.webkit.org/attachment.cgi?id=123069&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=123069&action=review


Looks reasonable.  A couple nits below.  Thanks for the patch!

> Source/WebCore/ChangeLog:9
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   * bindings/v8/V8NPUtils.cpp:
> +	   (WebCore::convertV8ObjectToNPVariant):

Can you say something about what gets faster and by how much?  That's useful
information to have in the ChangeLog.

> Source/WebCore/bindings/v8/V8NPUtils.cpp:69
> +	   if (!utf8Chars)
> +	       return;

in WebKit, malloc crashes if it can't allocated memory, so this null check
isn't needed.

> Source/WebCore/bindings/v8/V8NPUtils.cpp:70
> +	   str->WriteUtf8(utf8Chars, length, 0, 1);

Can we use HINT_MANY_WRITES_EXPECTED rather than 1 as the fourth parameter? 
As-is it's hard to know what the 1 means.


More information about the webkit-reviews mailing list