62527 2011-06-12 21:12:22 -0700 [EFL][WK2] Change string operation to makeString() 2011-06-15 09:20:09 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED INVALID P2 Normal --- 62732 61838 0 enmi.lee webkit-unassigned antognolli+webkit gyuyoung.kim kenneth leandro lucas.de.marchi rakuco ryuan.choi sangseok.lim tonikitoo webkit.review.bot youngtaeck.song zimmermann oldest_to_newest 419414 0 enmi.lee 2011-06-12 21:12:22 -0700 I've changed string operation codes to use makeString(). 419419 1 96918 enmi.lee 2011-06-12 21:25:52 -0700 Created attachment 96918 Patch 419422 2 gyuyoung.kim 2011-06-12 21:39:14 -0700 LGTM. 419522 3 rakuco 2011-06-13 06:10:42 -0700 Informal r+ from my side. 419545 4 96918 kenneth 2011-06-13 07:22:11 -0700 Comment on attachment 96918 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=96918&action=review > Source/WebKit2/ChangeLog:6 > + [EFL][WK2] Change string operation to makeString() > + https://bugs.webkit.org/show_bug.cgi?id=62527 Please add one line explaining why, like why that makes sense, why that is better etc? like 'use makeStrings() instead of creating individual strings and concatenating them' 420373 5 97109 enmi.lee 2011-06-14 06:40:37 -0700 Created attachment 97109 Patch 420374 6 enmi.lee 2011-06-14 06:41:37 -0700 (In reply to comment #4) > (From update of attachment 96918 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=96918&action=review > > > Source/WebKit2/ChangeLog:6 > > + [EFL][WK2] Change string operation to makeString() > > + https://bugs.webkit.org/show_bug.cgi?id=62527 > > Please add one line explaining why, like why that makes sense, why that is better etc? > > like 'use makeStrings() instead of creating individual strings and concatenating them' Done. I've added one line for explaining. 420447 7 97109 leandro 2011-06-14 08:49:04 -0700 Comment on attachment 97109 Patch Patch looks OK; changelog could use more work. 420448 8 97109 eric 2011-06-14 08:50:13 -0700 Comment on attachment 97109 Patch Looks OK to me. 420862 9 97109 gyuyoung.kim 2011-06-14 17:09:05 -0700 Comment on attachment 97109 Patch Informal r+ 420909 10 97207 enmi.lee 2011-06-14 18:01:49 -0700 Created attachment 97207 Patch 420910 11 enmi.lee 2011-06-14 18:02:46 -0700 (In reply to comment #10) > Created an attachment (id=97207) [details] > Patch modify "use" to "Use" 420917 12 97207 gyuyoung.kim 2011-06-14 18:09:03 -0700 Comment on attachment 97207 Patch Internal r+ 421035 13 97207 webkit.review.bot 2011-06-14 23:13:17 -0700 Comment on attachment 97207 Patch Clearing flags on attachment: 97207 Committed r88907: <http://trac.webkit.org/changeset/88907> 421036 14 webkit.review.bot 2011-06-14 23:13:22 -0700 All reviewed patches have been landed. Closing bug. 421256 15 zimmermann 2011-06-15 07:25:39 -0700 This patch is wrong. makeString is deprecated, using String operator+ gives exactly the same performance. This has been changed recently. use "osVersion = name.sysname + " " + name.machine;" You are not required to cast to Strings first. The StringAppend template solution will reduce this String concatting operation to just a single malloc of the final size, no appending is done at all. 421365 16 leandro 2011-06-15 09:20:09 -0700 (In reply to comment #15) > This patch is wrong. makeString is deprecated, using String operator+ gives > exactly the same performance. This has been changed recently. > > use "osVersion = name.sysname + " " + name.machine;" > You are not required to cast to Strings first. > > The StringAppend template solution will reduce this String concatting operation > to just a single malloc of the final size, no appending is done at all. You're right. I've rolled this out as r88944. Changing this bug status to 'invalid'. 96918 2011-06-12 21:25:52 -0700 2011-06-14 06:40:29 -0700 Patch bug-62527-20110613132549.patch text/plain 1879 enmi.lee U3VidmVyc2lvbiBSZXZpc2lvbjogODg2MjQKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwppbmRleCBlNzU5NTQ4YzU3YzM2NjEw N2QxOWY1OTgwN2E4NWRlNzYxOTE3OThlLi5mYTQ4OTA2OTUxNjZmNGNhYWFmZWQzMmNhMTJkM2Fj NDMwNTBiZDNlIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKKysrIGIvU291 cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTMgQEAKKzIwMTEtMDYtMTIgIEV1bm1p IExlZSAgPGV1bm1pMTUubGVlQHNhbXN1bmcuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIFtFRkxdW1dLMl0gQ2hhbmdlIHN0cmluZyBvcGVyYXRp b24gdG8gbWFrZVN0cmluZygpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3df YnVnLmNnaT9pZD02MjUyNworCisgICAgICAgICogVUlQcm9jZXNzL2VmbC9XZWJQYWdlUHJveHlF ZmwuY3BwOgorICAgICAgICAoV2ViS2l0OjpXZWJQYWdlUHJveHk6OnN0YW5kYXJkVXNlckFnZW50 KToKKwogMjAxMS0wNi0xMiAgQWRhbSBCYXJ0aCAgPGFiYXJ0aEB3ZWJraXQub3JnPgogCiAgICAg ICAgIFJldmlld2VkIGJ5IEVyaWMgU2VpZGVsLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIv VUlQcm9jZXNzL2VmbC9XZWJQYWdlUHJveHlFZmwuY3BwIGIvU291cmNlL1dlYktpdDIvVUlQcm9j ZXNzL2VmbC9XZWJQYWdlUHJveHlFZmwuY3BwCmluZGV4IGFlMzE0Yjg1Nzg1MjEzNjA2NDBmMjY4 NTM5YWI0N2YzZTVjOGE2YTkuLjhhOGFkMTBiYzcyZDkxZmRjMDYzYjNmNDUxNzIxNzJiNTNmMzgy MjYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9lZmwvV2ViUGFnZVByb3h5 RWZsLmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvZWZsL1dlYlBhZ2VQcm94eUVm bC5jcHAKQEAgLTMwLDYgKzMwLDcgQEAKICNpbmNsdWRlICJQYWdlQ2xpZW50SW1wbC5oIgogCiAj aW5jbHVkZSA8c3lzL3V0c25hbWUuaD4KKyNpbmNsdWRlIDx3dGYvdGV4dC9TdHJpbmdDb25jYXRl bmF0ZS5oPgogCiBuYW1lc3BhY2UgV2ViS2l0IHsKIApAQCAtNDAsOSArNDEsOSBAQCBFdmFzX09i amVjdCogV2ViUGFnZVByb3h5Ojp2aWV3T2JqZWN0KCkKIAogU3RyaW5nIFdlYlBhZ2VQcm94eTo6 c3RhbmRhcmRVc2VyQWdlbnQoY29uc3QgU3RyaW5nJiBhcHBsaWNhdGlvbk5hbWVGb3JVc2VyQWdl bnQpCiB7Ci0gICAgV1RGOjpTdHJpbmcgcGxhdGZvcm07Ci0gICAgV1RGOjpTdHJpbmcgdmVyc2lv bjsKLSAgICBXVEY6OlN0cmluZyBvc1ZlcnNpb247CisgICAgU3RyaW5nIHBsYXRmb3JtOworICAg IFN0cmluZyB2ZXJzaW9uOworICAgIFN0cmluZyBvc1ZlcnNpb247CiAKICNpZiBQTEFURk9STShY MTEpCiAgICAgcGxhdGZvcm0gPSAiWDExIjsKQEAgLTUzLDcgKzU0LDcgQEAgU3RyaW5nIFdlYlBh Z2VQcm94eTo6c3RhbmRhcmRVc2VyQWdlbnQoY29uc3QgU3RyaW5nJiBhcHBsaWNhdGlvbk5hbWVG b3JVc2VyQWdlbnQKICAgICAgICAgICAgICAgICAgICAgICAgICBTdHJpbmc6Om51bWJlcihXRUJL SVRfVVNFUl9BR0VOVF9NSU5PUl9WRVJTSU9OKSwgJysnKTsKICAgICBzdHJ1Y3QgdXRzbmFtZSBu YW1lOwogICAgIGlmICh1bmFtZSgmbmFtZSkgIT0gLTEpCi0gICAgICAgIG9zVmVyc2lvbiA9IFdU Rjo6U3RyaW5nKG5hbWUuc3lzbmFtZSkgKyAiICIgKyBXVEY6OlN0cmluZyhuYW1lLm1hY2hpbmUp OworICAgICAgICBvc1ZlcnNpb24gPSBtYWtlU3RyaW5nKG5hbWUuc3lzbmFtZSwgJyAnLCBuYW1l Lm1hY2hpbmUpOwogICAgIGVsc2UKICAgICAgICAgb3NWZXJzaW9uID0gIlVua25vd24iOwogCg== 97109 2011-06-14 06:40:37 -0700 2011-06-14 18:01:40 -0700 Patch bug-62527-20110614224038.patch text/plain 1990 enmi.lee U3VidmVyc2lvbiBSZXZpc2lvbjogODg3OTIKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwppbmRleCBlZjczZDBlZjVjZTk4ZmMx YWU4ODQ2MTA1YjY2ZWZjNTk5MWVhMjUyLi5jZTc3Y2ZmMGUzMTRmMDdmZWZhMmUzYWJjMmI5ZGEw MjMyNjY4YTU4IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKKysrIGIvU291 cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTEtMDYtMTQgIEV1bm1p IExlZSAgPGV1bm1pMTUubGVlQHNhbXN1bmcuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIFtFRkxdW1dLMl0gQ2hhbmdlIHN0cmluZyBvcGVyYXRp b24gdG8gbWFrZVN0cmluZygpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3df YnVnLmNnaT9pZD02MjUyNworCisgICAgICAgIHVzZSBtYWtlU3RyaW5nKCkgYmVjYXVzZSBpdCBp cyBiZXR0ZXIgdGhhbiBjcmVhdGluZyBpbmRpdmlkdWFsIHN0cmluZ3MgYW5kIGNvbmNhdGVuYXRp bmcgdGhlbS4KKworICAgICAgICAqIFVJUHJvY2Vzcy9lZmwvV2ViUGFnZVByb3h5RWZsLmNwcDoK KyAgICAgICAgKFdlYktpdDo6V2ViUGFnZVByb3h5OjpzdGFuZGFyZFVzZXJBZ2VudCk6CisKIDIw MTEtMDYtMTMgIEFuZGVycyBDYXJsc3NvbiAgPGFuZGVyc2NhQGFwcGxlLmNvbT4KIAogICAgICAg ICBSZXZpZXdlZCBieSBTYW0gV2VpbmlnLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvVUlQ cm9jZXNzL2VmbC9XZWJQYWdlUHJveHlFZmwuY3BwIGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNz L2VmbC9XZWJQYWdlUHJveHlFZmwuY3BwCmluZGV4IGFlMzE0Yjg1Nzg1MjEzNjA2NDBmMjY4NTM5 YWI0N2YzZTVjOGE2YTkuLjhhOGFkMTBiYzcyZDkxZmRjMDYzYjNmNDUxNzIxNzJiNTNmMzgyMjYg MTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9lZmwvV2ViUGFnZVByb3h5RWZs LmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvZWZsL1dlYlBhZ2VQcm94eUVmbC5j cHAKQEAgLTMwLDYgKzMwLDcgQEAKICNpbmNsdWRlICJQYWdlQ2xpZW50SW1wbC5oIgogCiAjaW5j bHVkZSA8c3lzL3V0c25hbWUuaD4KKyNpbmNsdWRlIDx3dGYvdGV4dC9TdHJpbmdDb25jYXRlbmF0 ZS5oPgogCiBuYW1lc3BhY2UgV2ViS2l0IHsKIApAQCAtNDAsOSArNDEsOSBAQCBFdmFzX09iamVj dCogV2ViUGFnZVByb3h5Ojp2aWV3T2JqZWN0KCkKIAogU3RyaW5nIFdlYlBhZ2VQcm94eTo6c3Rh bmRhcmRVc2VyQWdlbnQoY29uc3QgU3RyaW5nJiBhcHBsaWNhdGlvbk5hbWVGb3JVc2VyQWdlbnQp CiB7Ci0gICAgV1RGOjpTdHJpbmcgcGxhdGZvcm07Ci0gICAgV1RGOjpTdHJpbmcgdmVyc2lvbjsK LSAgICBXVEY6OlN0cmluZyBvc1ZlcnNpb247CisgICAgU3RyaW5nIHBsYXRmb3JtOworICAgIFN0 cmluZyB2ZXJzaW9uOworICAgIFN0cmluZyBvc1ZlcnNpb247CiAKICNpZiBQTEFURk9STShYMTEp CiAgICAgcGxhdGZvcm0gPSAiWDExIjsKQEAgLTUzLDcgKzU0LDcgQEAgU3RyaW5nIFdlYlBhZ2VQ cm94eTo6c3RhbmRhcmRVc2VyQWdlbnQoY29uc3QgU3RyaW5nJiBhcHBsaWNhdGlvbk5hbWVGb3JV c2VyQWdlbnQKICAgICAgICAgICAgICAgICAgICAgICAgICBTdHJpbmc6Om51bWJlcihXRUJLSVRf VVNFUl9BR0VOVF9NSU5PUl9WRVJTSU9OKSwgJysnKTsKICAgICBzdHJ1Y3QgdXRzbmFtZSBuYW1l OwogICAgIGlmICh1bmFtZSgmbmFtZSkgIT0gLTEpCi0gICAgICAgIG9zVmVyc2lvbiA9IFdURjo6 U3RyaW5nKG5hbWUuc3lzbmFtZSkgKyAiICIgKyBXVEY6OlN0cmluZyhuYW1lLm1hY2hpbmUpOwor ICAgICAgICBvc1ZlcnNpb24gPSBtYWtlU3RyaW5nKG5hbWUuc3lzbmFtZSwgJyAnLCBuYW1lLm1h Y2hpbmUpOwogICAgIGVsc2UKICAgICAgICAgb3NWZXJzaW9uID0gIlVua25vd24iOwogCg== 97207 2011-06-14 18:01:49 -0700 2011-06-14 23:13:17 -0700 Patch bug-62527-20110615100142.patch text/plain 1990 enmi.lee U3VidmVyc2lvbiBSZXZpc2lvbjogODg3OTIKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwppbmRleCBlZjczZDBlZjVjZTk4ZmMx YWU4ODQ2MTA1YjY2ZWZjNTk5MWVhMjUyLi4wZjJiM2YyYmUzZWY3ZGZmN2RiMGQzMDdjNWEzNDgw N2JlZjliMzQyIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKKysrIGIvU291 cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTEtMDYtMTQgIEV1bm1p IExlZSAgPGV1bm1pMTUubGVlQHNhbXN1bmcuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIFtFRkxdW1dLMl0gQ2hhbmdlIHN0cmluZyBvcGVyYXRp b24gdG8gbWFrZVN0cmluZygpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3df YnVnLmNnaT9pZD02MjUyNworCisgICAgICAgIFVzZSBtYWtlU3RyaW5nKCkgYmVjYXVzZSBpdCBp cyBiZXR0ZXIgdGhhbiBjcmVhdGluZyBpbmRpdmlkdWFsIHN0cmluZ3MgYW5kIGNvbmNhdGVuYXRp bmcgdGhlbS4KKworICAgICAgICAqIFVJUHJvY2Vzcy9lZmwvV2ViUGFnZVByb3h5RWZsLmNwcDoK KyAgICAgICAgKFdlYktpdDo6V2ViUGFnZVByb3h5OjpzdGFuZGFyZFVzZXJBZ2VudCk6CisKIDIw MTEtMDYtMTMgIEFuZGVycyBDYXJsc3NvbiAgPGFuZGVyc2NhQGFwcGxlLmNvbT4KIAogICAgICAg ICBSZXZpZXdlZCBieSBTYW0gV2VpbmlnLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvVUlQ cm9jZXNzL2VmbC9XZWJQYWdlUHJveHlFZmwuY3BwIGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNz L2VmbC9XZWJQYWdlUHJveHlFZmwuY3BwCmluZGV4IGFlMzE0Yjg1Nzg1MjEzNjA2NDBmMjY4NTM5 YWI0N2YzZTVjOGE2YTkuLjhhOGFkMTBiYzcyZDkxZmRjMDYzYjNmNDUxNzIxNzJiNTNmMzgyMjYg MTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9lZmwvV2ViUGFnZVByb3h5RWZs LmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvZWZsL1dlYlBhZ2VQcm94eUVmbC5j cHAKQEAgLTMwLDYgKzMwLDcgQEAKICNpbmNsdWRlICJQYWdlQ2xpZW50SW1wbC5oIgogCiAjaW5j bHVkZSA8c3lzL3V0c25hbWUuaD4KKyNpbmNsdWRlIDx3dGYvdGV4dC9TdHJpbmdDb25jYXRlbmF0 ZS5oPgogCiBuYW1lc3BhY2UgV2ViS2l0IHsKIApAQCAtNDAsOSArNDEsOSBAQCBFdmFzX09iamVj dCogV2ViUGFnZVByb3h5Ojp2aWV3T2JqZWN0KCkKIAogU3RyaW5nIFdlYlBhZ2VQcm94eTo6c3Rh bmRhcmRVc2VyQWdlbnQoY29uc3QgU3RyaW5nJiBhcHBsaWNhdGlvbk5hbWVGb3JVc2VyQWdlbnQp CiB7Ci0gICAgV1RGOjpTdHJpbmcgcGxhdGZvcm07Ci0gICAgV1RGOjpTdHJpbmcgdmVyc2lvbjsK LSAgICBXVEY6OlN0cmluZyBvc1ZlcnNpb247CisgICAgU3RyaW5nIHBsYXRmb3JtOworICAgIFN0 cmluZyB2ZXJzaW9uOworICAgIFN0cmluZyBvc1ZlcnNpb247CiAKICNpZiBQTEFURk9STShYMTEp CiAgICAgcGxhdGZvcm0gPSAiWDExIjsKQEAgLTUzLDcgKzU0LDcgQEAgU3RyaW5nIFdlYlBhZ2VQ cm94eTo6c3RhbmRhcmRVc2VyQWdlbnQoY29uc3QgU3RyaW5nJiBhcHBsaWNhdGlvbk5hbWVGb3JV c2VyQWdlbnQKICAgICAgICAgICAgICAgICAgICAgICAgICBTdHJpbmc6Om51bWJlcihXRUJLSVRf VVNFUl9BR0VOVF9NSU5PUl9WRVJTSU9OKSwgJysnKTsKICAgICBzdHJ1Y3QgdXRzbmFtZSBuYW1l OwogICAgIGlmICh1bmFtZSgmbmFtZSkgIT0gLTEpCi0gICAgICAgIG9zVmVyc2lvbiA9IFdURjo6 U3RyaW5nKG5hbWUuc3lzbmFtZSkgKyAiICIgKyBXVEY6OlN0cmluZyhuYW1lLm1hY2hpbmUpOwor ICAgICAgICBvc1ZlcnNpb24gPSBtYWtlU3RyaW5nKG5hbWUuc3lzbmFtZSwgJyAnLCBuYW1lLm1h Y2hpbmUpOwogICAgIGVsc2UKICAgICAgICAgb3NWZXJzaW9uID0gIlVua25vd24iOwogCg==