Bug 76592 – [v8]Low efficiency of writing long string in npruntime infrastructure

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Li Yin]

[v8]Low efficiency of writing long string in npruntime infrastructure

[Li Yin]

Created attachment 123069 [details]
Patch

[Li Yin]

In chromium-gtk mode, when the big string was transported from web page to plug-in through invoke function, the elapsed time is too long. I found the root cause was that the time of WriteUtf8 operation was too expensive, when the data type of String was ConsString.

Pseudo-code in JavaScript side:
var str="";
var num=1024*1024;
for(var i=0; i<num; i++)
    str+="A";
plugin_object.writeString(str);

Suppose that plugin_object is a plugin object, and has the function "writeString" in plug-in, although the this function could do nothing.

[Li Yin]

Specifying the WriteOptions to HINT_MANY_WRITES_EXPECTED in the calling of WriteUtf8,
the efficiency of WriteUtf8 can be greatly improved. 
On the other hand, it almost has no negative effect for short sting.

[Adam Barth]

Comment on attachment 123069 [details]
Patch

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.

[Li Yin]

1. The efficiency will be improved by 300 times in the best case, when the size of string reaches 1MB.

2. HINT_MANY_WRITES_EXPECTED is internal data type in the class, it can't be accessed in the V8NPUtils.cpp.

[Adam Barth]

> 2. HINT_MANY_WRITES_EXPECTED is internal data type in the class, it can't be accessed in the V8NPUtils.cpp.

You can't write v8::String::HINT_MANY_WRITES_EXPECTED ?  It looks like it's declared public in v8.h.

[Li Yin]

Created attachment 123076 [details]
Patch

[Li Yin]

(In reply to comment #6)
> > 2. HINT_MANY_WRITES_EXPECTED is internal data type in the class, it can't be accessed in the V8NPUtils.cpp.
> 
> You can't write v8::String::HINT_MANY_WRITES_EXPECTED ?  It looks like it's declared public in v8.h.

Yeah, thanks for your reminding and review, and I committed the new patch based on your suggestion, please check it. 
Thanks.

[Adam Barth]

Comment on attachment 123076 [details]
Patch

Looks great.  Thanks!

[WebKit Review Bot]

Comment on attachment 123076 [details]
Patch

Clearing flags on attachment: 123076

Committed r105389: <http://trac.webkit.org/changeset/105389>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.