76592 2012-01-18 17:48:17 -0800 [v8]Low efficiency of writing long string in npruntime infrastructure 2012-01-18 23:52:49 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 0 li.yin webkit-unassigned abarth japhet webkit.review.bot oldest_to_newest 538358 0 li.yin 2012-01-18 17:48:17 -0800 [v8]Low efficiency of writing long string in npruntime infrastructure 538461 1 123069 li.yin 2012-01-18 21:40:11 -0800 Created attachment 123069 Patch 538474 2 li.yin 2012-01-18 21:53:05 -0800 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. 538476 3 li.yin 2012-01-18 21:55:33 -0800 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. 538477 4 123069 abarth 2012-01-18 21:55:47 -0800 Comment on attachment 123069 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. 538487 5 li.yin 2012-01-18 22:19:12 -0800 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. 538488 6 abarth 2012-01-18 22:23:04 -0800 > 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. 538495 7 123076 li.yin 2012-01-18 22:53:36 -0800 Created attachment 123076 Patch 538498 8 li.yin 2012-01-18 22:57:15 -0800 (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. 538499 9 123076 abarth 2012-01-18 22:57:32 -0800 Comment on attachment 123076 Patch Looks great. Thanks! 538523 10 123076 webkit.review.bot 2012-01-18 23:52:44 -0800 Comment on attachment 123076 Patch Clearing flags on attachment: 123076 Committed r105389: <http://trac.webkit.org/changeset/105389> 538524 11 webkit.review.bot 2012-01-18 23:52:49 -0800 All reviewed patches have been landed. Closing bug. 123069 2012-01-18 21:40:11 -0800 2012-01-18 22:53:16 -0800 Patch bug-76592-20120120134034.patch text/plain 1811 li.yin SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEwNTM3NykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEzIEBACisyMDEyLTAxLTE5ICBMaSBZaW4g IDxsaS55aW5AaW50ZWwuY29tPgorCisgICAgICAgIFt2OF0gTG93IGVmZmljaWVuY3kgb2Ygd3Jp dGluZyBsb25nIHN0cmluZyBmcm9tIHdlYiBhcHBsaWNhdGlvbiB0byBwbHVnaW4uCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD03NjU5MgorCisgICAgICAg IFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogYmluZGluZ3MvdjgvVjhO UFV0aWxzLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OmNvbnZlcnRWOE9iamVjdFRvTlBWYXJpYW50 KToKKwogMjAxMi0wMS0xOCAgSWFuIFZvbGxpY2sgIDx2b2xsaWNrQGNocm9taXVtLm9yZz4KIAog ICAgICAgICBbY2hyb21pdW1dIENyZWF0ZSBhIGJhc2UtY2xhc3MgQ0NBbmltYXRpb24gdG8gcmVw cmVzZW50IGNvbXBvc2l0b3IgYW5pbWF0aW9ucwpJbmRleDogU291cmNlL1dlYkNvcmUvYmluZGlu Z3MvdjgvVjhOUFV0aWxzLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9iaW5kaW5n cy92OC9WOE5QVXRpbHMuY3BwCShyZXZpc2lvbiAxMDUzNzcpCisrKyBTb3VyY2UvV2ViQ29yZS9i aW5kaW5ncy92OC9WOE5QVXRpbHMuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC02MiwxMSArNjIsMTMg QEAgdm9pZCBjb252ZXJ0VjhPYmplY3RUb05QVmFyaWFudCh2ODo6TG9jYQogICAgIGVsc2UgaWYg KG9iamVjdC0+SXNVbmRlZmluZWQoKSkKICAgICAgICAgVk9JRF9UT19OUFZBUklBTlQoKnJlc3Vs dCk7CiAgICAgZWxzZSBpZiAob2JqZWN0LT5Jc1N0cmluZygpKSB7Ci0gICAgICAgIHY4OjpTdHJp bmc6OlV0ZjhWYWx1ZSB1dGY4KG9iamVjdCk7Ci0gICAgICAgIGludCBsZW5ndGggPSB1dGY4Lmxl bmd0aCgpICsgMTsKKyAgICAgICAgdjg6OkhhbmRsZTx2ODo6U3RyaW5nPiBzdHIgPSBvYmplY3Qt PlRvU3RyaW5nKCk7CisgICAgICAgIGludCBsZW5ndGggPSBzdHItPlV0ZjhMZW5ndGgoKSArIDE7 CiAgICAgICAgIGNoYXIqIHV0ZjhDaGFycyA9IHJlaW50ZXJwcmV0X2Nhc3Q8Y2hhcio+KG1hbGxv YyhsZW5ndGgpKTsKLSAgICAgICAgbWVtY3B5KHV0ZjhDaGFycywgKnV0ZjgsIGxlbmd0aCk7Ci0g ICAgICAgIFNUUklOR05fVE9fTlBWQVJJQU5UKHV0ZjhDaGFycywgdXRmOC5sZW5ndGgoKSwgKnJl c3VsdCk7CisgICAgICAgIGlmICghdXRmOENoYXJzKQorICAgICAgICAgICAgcmV0dXJuOworICAg ICAgICBzdHItPldyaXRlVXRmOCh1dGY4Q2hhcnMsIGxlbmd0aCwgMCwgMSk7CisgICAgICAgIFNU UklOR05fVE9fTlBWQVJJQU5UKHV0ZjhDaGFycywgbGVuZ3RoLTEsICpyZXN1bHQpOwogICAgIH0g ZWxzZSBpZiAob2JqZWN0LT5Jc09iamVjdCgpKSB7CiAgICAgICAgIERPTVdpbmRvdyogd2luZG93 ID0gVjhQcm94eTo6cmV0cmlldmVXaW5kb3coVjhQcm94eTo6Y3VycmVudENvbnRleHQoKSk7CiAg ICAgICAgIE5QT2JqZWN0KiBucG9iamVjdCA9IG5wQ3JlYXRlVjhTY3JpcHRPYmplY3QoMCwgdjg6 OkhhbmRsZTx2ODo6T2JqZWN0Pjo6Q2FzdChvYmplY3QpLCB3aW5kb3cpOwo= 123076 2012-01-18 22:53:36 -0800 2012-01-18 23:52:44 -0800 Patch bug-76592-20120120145359.patch text/plain 1921 li.yin SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEwNTM3NykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE2IEBACisyMDEyLTAxLTE5ICBMaSBZaW4g IDxsaS55aW5AaW50ZWwuY29tPgorCisgICAgICAgIFt2OF0gTG93IGVmZmljaWVuY3kgb2Ygd3Jp dGluZyBsb25nIHN0cmluZyBmcm9tIHdlYiBhcHBsaWNhdGlvbiB0byBwbHVnaW4uCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD03NjU5MgorCisgICAgICAg IFRoZSBlZmZpY2llbmN5IHdpbGwgYmUgaW1wcm92ZWQgYnkgMzAwIHRpbWVzIGluIHRoZSBiZXN0 IGNhc2UsIHdoZW4gdGhlCisgICAgICAgIHNpemUgb2Ygc3RyaW5nIHJlYWNoZXMgMU1CLgorCisg ICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogYmluZGluZ3Mv djgvVjhOUFV0aWxzLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OmNvbnZlcnRWOE9iamVjdFRvTlBW YXJpYW50KToKKwogMjAxMi0wMS0xOCAgSWFuIFZvbGxpY2sgIDx2b2xsaWNrQGNocm9taXVtLm9y Zz4KIAogICAgICAgICBbY2hyb21pdW1dIENyZWF0ZSBhIGJhc2UtY2xhc3MgQ0NBbmltYXRpb24g dG8gcmVwcmVzZW50IGNvbXBvc2l0b3IgYW5pbWF0aW9ucwpJbmRleDogU291cmNlL1dlYkNvcmUv YmluZGluZ3MvdjgvVjhOUFV0aWxzLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9i aW5kaW5ncy92OC9WOE5QVXRpbHMuY3BwCShyZXZpc2lvbiAxMDUzNzcpCisrKyBTb3VyY2UvV2Vi Q29yZS9iaW5kaW5ncy92OC9WOE5QVXRpbHMuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC02MiwxMSAr NjIsMTEgQEAgdm9pZCBjb252ZXJ0VjhPYmplY3RUb05QVmFyaWFudCh2ODo6TG9jYQogICAgIGVs c2UgaWYgKG9iamVjdC0+SXNVbmRlZmluZWQoKSkKICAgICAgICAgVk9JRF9UT19OUFZBUklBTlQo KnJlc3VsdCk7CiAgICAgZWxzZSBpZiAob2JqZWN0LT5Jc1N0cmluZygpKSB7Ci0gICAgICAgIHY4 OjpTdHJpbmc6OlV0ZjhWYWx1ZSB1dGY4KG9iamVjdCk7Ci0gICAgICAgIGludCBsZW5ndGggPSB1 dGY4Lmxlbmd0aCgpICsgMTsKKyAgICAgICAgdjg6OkhhbmRsZTx2ODo6U3RyaW5nPiBzdHIgPSBv YmplY3QtPlRvU3RyaW5nKCk7CisgICAgICAgIGludCBsZW5ndGggPSBzdHItPlV0ZjhMZW5ndGgo KSArIDE7CiAgICAgICAgIGNoYXIqIHV0ZjhDaGFycyA9IHJlaW50ZXJwcmV0X2Nhc3Q8Y2hhcio+ KG1hbGxvYyhsZW5ndGgpKTsKLSAgICAgICAgbWVtY3B5KHV0ZjhDaGFycywgKnV0ZjgsIGxlbmd0 aCk7Ci0gICAgICAgIFNUUklOR05fVE9fTlBWQVJJQU5UKHV0ZjhDaGFycywgdXRmOC5sZW5ndGgo KSwgKnJlc3VsdCk7CisgICAgICAgIHN0ci0+V3JpdGVVdGY4KHV0ZjhDaGFycywgbGVuZ3RoLCAw LCB2ODo6U3RyaW5nOjpISU5UX01BTllfV1JJVEVTX0VYUEVDVEVEKTsKKyAgICAgICAgU1RSSU5H Tl9UT19OUFZBUklBTlQodXRmOENoYXJzLCBsZW5ndGgtMSwgKnJlc3VsdCk7CiAgICAgfSBlbHNl IGlmIChvYmplY3QtPklzT2JqZWN0KCkpIHsKICAgICAgICAgRE9NV2luZG93KiB3aW5kb3cgPSBW OFByb3h5OjpyZXRyaWV2ZVdpbmRvdyhWOFByb3h5OjpjdXJyZW50Q29udGV4dCgpKTsKICAgICAg ICAgTlBPYmplY3QqIG5wb2JqZWN0ID0gbnBDcmVhdGVWOFNjcmlwdE9iamVjdCgwLCB2ODo6SGFu ZGxlPHY4OjpPYmplY3Q+OjpDYXN0KG9iamVjdCksIHdpbmRvdyk7Cg==