104206 2012-12-05 21:17:27 -0800 [V8] Optimize v8StringOrNull() and v8StringOrUndefined() 2012-12-05 21:48:49 -0800 1 1 1 Unclassified WebKit WebCore JavaScript 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 haraken haraken abarth japhet webkit.review.bot oldest_to_newest 784550 0 haraken 2012-12-05 21:17:27 -0800 Currently v8StringOrNull() and v8StringOrUndefined() are checking a null string twice. It is redundant. 784554 1 177921 haraken 2012-12-05 21:18:54 -0800 Created attachment 177921 Patch 784555 2 177921 abarth 2012-12-05 21:26:26 -0800 Comment on attachment 177921 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177921&action=review > Source/WebCore/bindings/v8/V8Binding.h:161 > + if (UNLIKELY(!isolate)) > + isolate = v8::Isolate::GetCurrent(); Some day we'll make isolate mandatory and remove all these branches. 784557 3 haraken 2012-12-05 21:28:28 -0800 (In reply to comment #2) > (From update of attachment 177921 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=177921&action=review > > > Source/WebCore/bindings/v8/V8Binding.h:161 > > + if (UNLIKELY(!isolate)) > > + isolate = v8::Isolate::GetCurrent(); > > Some day we'll make isolate mandatory and remove all these branches. I'm doing it today:) Actually all call sites of v8StringOrNull() and v8StringOrUndefined() have an isolate. Almost all call sites of v8String() have an isolate. (I'll add v8StringWithCheck(), which has an isolate check, and will use it for call sites that don't have an isolate.) 784567 4 haraken 2012-12-05 21:32:17 -0800 Also I've implemented an isolate-version V8 APIs for Local<T>::New() and IsWeak(), which will land this week or next week. It will speed up v8String() and others. 784585 5 177921 webkit.review.bot 2012-12-05 21:48:46 -0800 Comment on attachment 177921 Patch Clearing flags on attachment: 177921 Committed r136797: <http://trac.webkit.org/changeset/136797> 784586 6 webkit.review.bot 2012-12-05 21:48:49 -0800 All reviewed patches have been landed. Closing bug. 177921 2012-12-05 21:18:54 -0800 2012-12-05 21:48:45 -0800 Patch bug-104206-20121206141618.patch text/plain 2728 haraken U3VidmVyc2lvbiBSZXZpc2lvbjogMTM2Nzc3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNTEzYmFmMzY5MjY0ZTVi YTUxYWJiMjIxNGVlN2YzMDBkMzAzOTRhMS4uZDE4MzVkMzZlNTRlNjY0OGMwNWExYmYyMzRhNTFj ZmI4ODIzOTVlZSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDIzIEBACiAyMDEyLTEyLTA1ICBLZW50 YXJvIEhhcmEgIDxoYXJha2VuQGNocm9taXVtLm9yZz4KIAorICAgICAgICBbVjhdIE9wdGltaXpl IHY4U3RyaW5nT3JOdWxsKCkgYW5kIHY4U3RyaW5nT3JVbmRlZmluZWQoKQorICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTA0MjA2CisKKyAgICAgICAgUmV2 aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQ3VycmVudGx5IHY4U3RyaW5nT3JO dWxsKCkgYW5kIHY4U3RyaW5nT3JVbmRlZmluZWQoKSBhcmUgY2hlY2tpbmcKKyAgICAgICAgYSBu dWxsIHN0cmluZyB0d2ljZS4gSXQgaXMgcmVkdW5kYW50LgorCisgICAgICAgIFtkaXYubG9jYWxO YW1lXQorICAgICAgICBCZWZvcmUgdGhpcyBwYXRjaDogMjAuMDMgbnMKKyAgICAgICAgQWZ0ZXIg dGhpcyBwYXRjaDogIDE5LjM0IG5zCisKKyAgICAgICAgKiBiaW5kaW5ncy92OC9WOEJpbmRpbmcu aDoKKyAgICAgICAgKFdlYkNvcmU6OnY4U3RyaW5nT3JOdWxsKToKKyAgICAgICAgKFdlYkNvcmU6 OnY4U3RyaW5nT3JVbmRlZmluZWQpOgorCisyMDEyLTEyLTA1ICBLZW50YXJvIEhhcmEgIDxoYXJh a2VuQGNocm9taXVtLm9yZz4KKwogICAgICAgICBVbnJldmlld2VkLCByb2xsaW5nIG91dCByMTM2 NDgxLgogICAgICAgICBodHRwOi8vdHJhYy53ZWJraXQub3JnL2NoYW5nZXNldC8xMzY0ODEKICAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEwMzg2OApkaWZm IC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvYmluZGluZ3MvdjgvVjhCaW5kaW5nLmggYi9Tb3VyY2Uv V2ViQ29yZS9iaW5kaW5ncy92OC9WOEJpbmRpbmcuaAppbmRleCA3NWUyYWQ3ZGJjNTQzODgyZWFk NmQxY2U3OTM0YmU4MmQ3MWZiNzFiLi5mMWU4OTQyOTg0MDM2NTQ3NzE2ODAxMjA5ZDI1MTczNDBh OWNlZmJlIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9iaW5kaW5ncy92OC9WOEJpbmRpbmcu aAorKysgYi9Tb3VyY2UvV2ViQ29yZS9iaW5kaW5ncy92OC9WOEJpbmRpbmcuaApAQCAtMTU1LDE0 ICsxNTUsMjIgQEAgbmFtZXNwYWNlIFdlYkNvcmUgewogICAgICAgICByZXR1cm4gVjhQZXJJc29s YXRlRGF0YTo6ZnJvbShpc29sYXRlKS0+c3RyaW5nQ2FjaGUoKS0+djhFeHRlcm5hbFN0cmluZyhz dHJpbmdJbXBsLCBpc29sYXRlKTsKICAgICB9CiAKLSAgICBpbmxpbmUgdjg6OkhhbmRsZTx2ODo6 VmFsdWU+IHY4U3RyaW5nT3JOdWxsKGNvbnN0IFN0cmluZyYgc3RyLCB2ODo6SXNvbGF0ZSogaXNv bGF0ZSA9IDApCisgICAgaW5saW5lIHY4OjpIYW5kbGU8djg6OlZhbHVlPiB2OFN0cmluZ09yTnVs bChjb25zdCBTdHJpbmcmIHN0cmluZywgdjg6Oklzb2xhdGUqIGlzb2xhdGUgPSAwKQogICAgIHsK LSAgICAgICAgcmV0dXJuIHN0ci5pc051bGwoKSA/IHY4OjpIYW5kbGU8djg6OlZhbHVlPih2OE51 bGxXaXRoQ2hlY2soaXNvbGF0ZSkpIDogdjg6OkhhbmRsZTx2ODo6VmFsdWU+KHY4U3RyaW5nKHN0 ciwgaXNvbGF0ZSkpOworICAgICAgICBpZiAoVU5MSUtFTFkoIWlzb2xhdGUpKQorICAgICAgICAg ICAgaXNvbGF0ZSA9IHY4OjpJc29sYXRlOjpHZXRDdXJyZW50KCk7CisgICAgICAgIGlmIChzdHJp bmcuaXNOdWxsKCkpCisgICAgICAgICAgICByZXR1cm4gdjhOdWxsKGlzb2xhdGUpOworICAgICAg ICByZXR1cm4gVjhQZXJJc29sYXRlRGF0YTo6ZnJvbShpc29sYXRlKS0+c3RyaW5nQ2FjaGUoKS0+ djhFeHRlcm5hbFN0cmluZyhzdHJpbmcuaW1wbCgpLCBpc29sYXRlKTsKICAgICB9CiAKLSAgICBp bmxpbmUgdjg6OkhhbmRsZTx2ODo6VmFsdWU+IHY4U3RyaW5nT3JVbmRlZmluZWQoY29uc3QgU3Ry aW5nJiBzdHIsIHY4OjpJc29sYXRlKiBpc29sYXRlID0gMCkKKyAgICBpbmxpbmUgdjg6OkhhbmRs ZTx2ODo6VmFsdWU+IHY4U3RyaW5nT3JVbmRlZmluZWQoY29uc3QgU3RyaW5nJiBzdHJpbmcsIHY4 OjpJc29sYXRlKiBpc29sYXRlID0gMCkKICAgICB7Ci0gICAgICAgIHJldHVybiBzdHIuaXNOdWxs KCkgPyB2ODo6SGFuZGxlPHY4OjpWYWx1ZT4odjg6OlVuZGVmaW5lZCgpKSA6IHY4OjpIYW5kbGU8 djg6OlZhbHVlPih2OFN0cmluZyhzdHIsIGlzb2xhdGUpKTsKKyAgICAgICAgaWYgKFVOTElLRUxZ KCFpc29sYXRlKSkKKyAgICAgICAgICAgIGlzb2xhdGUgPSB2ODo6SXNvbGF0ZTo6R2V0Q3VycmVu dCgpOworICAgICAgICBpZiAoc3RyaW5nLmlzTnVsbCgpKQorICAgICAgICAgICAgcmV0dXJuIHY4 OjpVbmRlZmluZWQoaXNvbGF0ZSk7CisgICAgICAgIHJldHVybiBWOFBlcklzb2xhdGVEYXRhOjpm cm9tKGlzb2xhdGUpLT5zdHJpbmdDYWNoZSgpLT52OEV4dGVybmFsU3RyaW5nKHN0cmluZy5pbXBs KCksIGlzb2xhdGUpOwogICAgIH0KIAogICAgIGlubGluZSB2ODo6SGFuZGxlPHY4OjpJbnRlZ2Vy PiB2OEludGVnZXIoaW50IHZhbHVlLCB2ODo6SXNvbGF0ZSogaXNvbGF0ZSA9IDApCg==