81070 2012-03-13 19:19:30 -0700 Avoid StringImpl::getData16SlowCase() when sorting array 2012-03-14 22:11:56 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 benjamin benjamin ggaren oldest_to_newest 578108 0 benjamin 2012-03-13 19:19:30 -0700 When sorting a JSArray, one of the bottleneck is the conversion from StringImpl::getData16SlowCase() 578113 1 131773 benjamin 2012-03-13 19:27:27 -0700 Created attachment 131773 Patch 578657 2 131773 ggaren 2012-03-14 12:37:05 -0700 Comment on attachment 131773 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=131773&action=review > Source/JavaScriptCore/wtf/text/StringImpl.h:782 > + if (string1 && string2) { It would be better to NULL check string1 and string2 at the head of the function, and return early if NULL, rather than NULL checking more than once in the body of the function, and indenting so much of the code. I think this would work: if (!string1) return -1; if (!string2) return string1->length(); .... 578659 3 ggaren 2012-03-14 12:38:11 -0700 > if (!string2) > return string1->length(); Oops! if (!string2) return string1->length() ? 1 : -1; 578665 4 benjamin 2012-03-14 12:44:24 -0700 > It would be better to NULL check string1 and string2 at the head of the function, and return early if NULL, rather than NULL checking more than once in the body of the function, and indenting so much of the code. I think this would work: Good point! I'll update that Thanks for the review. 579128 5 benjamin 2012-03-14 22:11:56 -0700 Committed r110822: <http://trac.webkit.org/changeset/110822> 131773 2012-03-13 19:27:27 -0700 2012-03-14 12:37:05 -0700 Patch bug-81070-20120313192725.patch text/plain 4696 benjamin U3VidmVyc2lvbiBSZXZpc2lvbjogMTEwNjEyCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCBj MTc0OTAwOTM3ZjcxYjA2NGQ0ZjY1ZDI1MDBmNzcyYzZkNjZkNDM1Li43ZmYwMTU4MWQ0MDA1ZDZh ZTYzNWYwMzYzZWYyNzg2MWQwNWQ2Mzg5IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwyNSBAQAorMjAxMi0wMy0xMyAgQmVuamFtaW4gUG91bGFpbiAgPGJwb3VsYWluQGFwcGxl LmNvbT4KKworICAgICAgICBBdm9pZCBTdHJpbmdJbXBsOjpnZXREYXRhMTZTbG93Q2FzZSgpIHdo ZW4gc29ydGluZyBhcnJheQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9ODEwNzAKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKwor ICAgICAgICBUaGUgZnVuY3Rpb24gY29kZVBvaW50Q29tcGFyZSgpIGlzIHVzZWQgaW50ZW5zaXZl bHkgd2hlbiBzb3J0aW5nIHN0cmluZ3MuCisgICAgICAgIFRoaXMgcGF0Y2ggaW1wcm92ZXMgaXRz IHBlcmZvcm1hbmNlIGJ5OgorICAgICAgICAtQXZvaWRpbmcgY2hhcmFjdGVyIGNvbnZlcnNpb24u CisgICAgICAgIC1JbmxpbmluZyB0aGUgZnVuY3Rpb24uCisKKyAgICAgICAgVGhpcyBtYWtlcyBQ ZWFjZWtlZXBlcidzIGFycmF5Q29tYmluZWQgdGVzdCAzMCUgZmFzdGVyLgorCisgICAgICAgICog d3RmL3RleHQvU3RyaW5nSW1wbC5jcHA6CisgICAgICAgICogd3RmL3RleHQvU3RyaW5nSW1wbC5o OgorICAgICAgICAoV1RGKToKKyAgICAgICAgKFdURjo6Y29kZVBvaW50Q29tcGFyZSk6CisgICAg ICAgIChXVEY6OmNvZGVQb2ludENvbXBhcmU4KToKKyAgICAgICAgKFdURjo6Y29kZVBvaW50Q29t cGFyZTE2KToKKyAgICAgICAgKFdURjo6Y29kZVBvaW50Q29tcGFyZThUbzE2KToKKwogMjAxMi0w My0xMyAgSG9qb25nIEhhbiAgPGhvam9uZy5oYW5Ac2Ftc3VuZy5jb20+CiAKICAgICAgICAgRHVt cCB0aGUgZ2VuZXJhdGVkIGNvZGUgZm9yIEFSTV9UUkFESVRJT05BTApkaWZmIC0tZ2l0IGEvU291 cmNlL0phdmFTY3JpcHRDb3JlL3d0Zi90ZXh0L1N0cmluZ0ltcGwuY3BwIGIvU291cmNlL0phdmFT Y3JpcHRDb3JlL3d0Zi90ZXh0L1N0cmluZ0ltcGwuY3BwCmluZGV4IDNmNzM1NTZiNTE3YWIwMTk4 MGI3MDcwOWQ4MDY0ODc0OWY0OGUwOTIuLjZlMDRjNDFjZTYzNzc4ZWQ2YzE1YmNiYWVmNTNlMzU2 MDZkNzY1YjEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS93dGYvdGV4dC9TdHJp bmdJbXBsLmNwcAorKysgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvd3RmL3RleHQvU3RyaW5nSW1w bC5jcHAKQEAgLTc3NywyOSArNzc3LDYgQEAgc3RhdGljIGlubGluZSBib29sIGVxdWFsSWdub3Jp bmdDYXNlKGNvbnN0IFVDaGFyKiBhLCBjb25zdCBVQ2hhciogYiwgaW50IGxlbmd0aCkKICAgICBy ZXR1cm4gdW1lbWNhc2VjbXAoYSwgYiwgbGVuZ3RoKSA9PSAwOwogfQogCi1pbnQgY29kZVBvaW50 Q29tcGFyZShjb25zdCBTdHJpbmdJbXBsKiBzMSwgY29uc3QgU3RyaW5nSW1wbCogczIpCi17Ci0g ICAgY29uc3QgdW5zaWduZWQgbDEgPSBzMSA/IHMxLT5sZW5ndGgoKSA6IDA7Ci0gICAgY29uc3Qg dW5zaWduZWQgbDIgPSBzMiA/IHMyLT5sZW5ndGgoKSA6IDA7Ci0gICAgY29uc3QgdW5zaWduZWQg bG1pbiA9IGwxIDwgbDIgPyBsMSA6IGwyOwotICAgIGNvbnN0IFVDaGFyKiBjMSA9IHMxID8gczEt PmNoYXJhY3RlcnMoKSA6IDA7Ci0gICAgY29uc3QgVUNoYXIqIGMyID0gczIgPyBzMi0+Y2hhcmFj dGVycygpIDogMDsKLSAgICB1bnNpZ25lZCBwb3MgPSAwOwotICAgIHdoaWxlIChwb3MgPCBsbWlu ICYmICpjMSA9PSAqYzIpIHsKLSAgICAgICAgYzErKzsKLSAgICAgICAgYzIrKzsKLSAgICAgICAg cG9zKys7Ci0gICAgfQotCi0gICAgaWYgKHBvcyA8IGxtaW4pCi0gICAgICAgIHJldHVybiAoYzFb MF0gPiBjMlswXSkgPyAxIDogLTE7Ci0KLSAgICBpZiAobDEgPT0gbDIpCi0gICAgICAgIHJldHVy biAwOwotCi0gICAgcmV0dXJuIChsMSA+IGwyKSA/IDEgOiAtMTsKLX0KLQogc2l6ZV90IFN0cmlu Z0ltcGw6OmZpbmQoVUNoYXIgYywgdW5zaWduZWQgc3RhcnQpCiB7CiAgICAgaWYgKGlzOEJpdCgp KQpkaWZmIC0tZ2l0IGEvU291cmNlL0phdmFTY3JpcHRDb3JlL3d0Zi90ZXh0L1N0cmluZ0ltcGwu aCBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS93dGYvdGV4dC9TdHJpbmdJbXBsLmgKaW5kZXggNjY3 MzM1Yjg2MGI3MWYyNzhmY2U5OTIzZWJmNGFkNDVhODhlMjY5OC4uYzAyYTE3OTc4NmMzZGM2NDVh MjEwOWFjZDMxMWVjZTM4YjQ4OTIyNCAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3Jl L3d0Zi90ZXh0L1N0cmluZ0ltcGwuaAorKysgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvd3RmL3Rl eHQvU3RyaW5nSW1wbC5oCkBAIC03NDIsNyArNzQyLDYwIEBAIGJvb2wgZXF1YWxJZ25vcmluZ051 bGxpdHkoY29uc3QgVmVjdG9yPFVDaGFyLCBpbmxpbmVDYXBhY2l0eT4mIGEsIFN0cmluZ0ltcGwq IGIpCiAgICAgcmV0dXJuICFtZW1jbXAoYS5kYXRhKCksIGItPmNoYXJhY3RlcnMoKSwgYi0+bGVu Z3RoKCkgKiBzaXplb2YoVUNoYXIpKTsKIH0KIAotV1RGX0VYUE9SVF9QUklWQVRFIGludCBjb2Rl UG9pbnRDb21wYXJlKGNvbnN0IFN0cmluZ0ltcGwqLCBjb25zdCBTdHJpbmdJbXBsKik7Cit0ZW1w bGF0ZTx0eXBlbmFtZSBDaGFyYWN0ZXJUeXBlMSwgdHlwZW5hbWUgQ2hhcmFjdGVyVHlwZTI+Citz dGF0aWMgaW5saW5lIGludCBjb2RlUG9pbnRDb21wYXJlKHVuc2lnbmVkIGwxLCB1bnNpZ25lZCBs MiwgY29uc3QgQ2hhcmFjdGVyVHlwZTEqIGMxLCBjb25zdCBDaGFyYWN0ZXJUeXBlMiogYzIpCit7 CisgICAgY29uc3QgdW5zaWduZWQgbG1pbiA9IGwxIDwgbDIgPyBsMSA6IGwyOworICAgIHVuc2ln bmVkIHBvcyA9IDA7CisgICAgd2hpbGUgKHBvcyA8IGxtaW4gJiYgKmMxID09ICpjMikgeworICAg ICAgICBjMSsrOworICAgICAgICBjMisrOworICAgICAgICBwb3MrKzsKKyAgICB9CisKKyAgICBp ZiAocG9zIDwgbG1pbikKKyAgICAgICAgcmV0dXJuIChjMVswXSA+IGMyWzBdKSA/IDEgOiAtMTsK KworICAgIGlmIChsMSA9PSBsMikKKyAgICAgICAgcmV0dXJuIDA7CisKKyAgICByZXR1cm4gKGwx ID4gbDIpID8gMSA6IC0xOworfQorCitzdGF0aWMgaW5saW5lIGludCBjb2RlUG9pbnRDb21wYXJl OChjb25zdCBTdHJpbmdJbXBsKiBzdHJpbmcxLCBjb25zdCBTdHJpbmdJbXBsKiBzdHJpbmcyKQor eworICAgIHJldHVybiBjb2RlUG9pbnRDb21wYXJlKHN0cmluZzEtPmxlbmd0aCgpLCBzdHJpbmcy LT5sZW5ndGgoKSwgc3RyaW5nMS0+Y2hhcmFjdGVyczgoKSwgc3RyaW5nMi0+Y2hhcmFjdGVyczgo KSk7Cit9CisKK3N0YXRpYyBpbmxpbmUgaW50IGNvZGVQb2ludENvbXBhcmUxNihjb25zdCBTdHJp bmdJbXBsKiBzdHJpbmcxLCBjb25zdCBTdHJpbmdJbXBsKiBzdHJpbmcyKQoreworICAgIHJldHVy biBjb2RlUG9pbnRDb21wYXJlKHN0cmluZzEtPmxlbmd0aCgpLCBzdHJpbmcyLT5sZW5ndGgoKSwg c3RyaW5nMS0+Y2hhcmFjdGVyczE2KCksIHN0cmluZzItPmNoYXJhY3RlcnMxNigpKTsKK30KKwor c3RhdGljIGlubGluZSBpbnQgY29kZVBvaW50Q29tcGFyZThUbzE2KGNvbnN0IFN0cmluZ0ltcGwq IHN0cmluZzEsIGNvbnN0IFN0cmluZ0ltcGwqIHN0cmluZzIpCit7CisgICAgcmV0dXJuIGNvZGVQ b2ludENvbXBhcmUoc3RyaW5nMS0+bGVuZ3RoKCksIHN0cmluZzItPmxlbmd0aCgpLCBzdHJpbmcx LT5jaGFyYWN0ZXJzOCgpLCBzdHJpbmcyLT5jaGFyYWN0ZXJzMTYoKSk7Cit9CisKK3N0YXRpYyBp bmxpbmUgaW50IGNvZGVQb2ludENvbXBhcmUoY29uc3QgU3RyaW5nSW1wbCogc3RyaW5nMSwgY29u c3QgU3RyaW5nSW1wbCogc3RyaW5nMikKK3sKKyAgICBpZiAoc3RyaW5nMSAmJiBzdHJpbmcyKSB7 CisgICAgICAgIGJvb2wgc3RyaW5nMUlzOEJpdCA9IHN0cmluZzEtPmlzOEJpdCgpOworICAgICAg ICBib29sIHN0cmluZzJJczhCaXQgPSBzdHJpbmcyLT5pczhCaXQoKTsKKyAgICAgICAgaWYgKHN0 cmluZzFJczhCaXQpIHsKKyAgICAgICAgICAgIGlmIChzdHJpbmcySXM4Qml0KQorICAgICAgICAg ICAgICAgIHJldHVybiBjb2RlUG9pbnRDb21wYXJlOChzdHJpbmcxLCBzdHJpbmcyKTsKKyAgICAg ICAgICAgIHJldHVybiBjb2RlUG9pbnRDb21wYXJlOFRvMTYoc3RyaW5nMSwgc3RyaW5nMik7Cisg ICAgICAgIH0KKyAgICAgICAgaWYgKHN0cmluZzJJczhCaXQpCisgICAgICAgICAgICByZXR1cm4g LWNvZGVQb2ludENvbXBhcmU4VG8xNihzdHJpbmcyLCBzdHJpbmcxKTsKKyAgICAgICAgcmV0dXJu IGNvZGVQb2ludENvbXBhcmUxNihzdHJpbmcxLCBzdHJpbmcyKTsKKyAgICB9CisKKyAgICBjb25z dCB1bnNpZ25lZCBsMSA9IHN0cmluZzEgPyBzdHJpbmcxLT5sZW5ndGgoKSA6IDA7CisgICAgY29u c3QgdW5zaWduZWQgbDIgPSBzdHJpbmcyID8gc3RyaW5nMi0+bGVuZ3RoKCkgOiAwOworICAgIHJl dHVybiAobDEgPiBsMikgPyAxIDogLTE7Cit9CiAKIHN0YXRpYyBpbmxpbmUgYm9vbCBpc1NwYWNl T3JOZXdsaW5lKFVDaGFyIGMpCiB7Cg==