50517 2010-12-04 06:08:50 -0800 Add a fast case for ASCII strings in HashAndUTF8CharactersTranslator::equal 2010-12-31 04:43:55 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Other OS X 10.5 RESOLVED FIXED P2 Normal --- 51612 43085 1 paroga paroga commit-queue darin eric oldest_to_newest 317263 0 paroga 2010-12-04 06:08:50 -0800 Add a fast case for ASCII strings in HashAndUTF8CharactersTranslator::equal 317264 1 75607 paroga 2010-12-04 06:10:36 -0800 Created attachment 75607 Patch 320781 2 75607 eric 2010-12-12 02:33:51 -0800 Comment on attachment 75607 Patch Again, on what benchmark? So much code for unclear gains. Please document your measurement techniques in the ChangeLog. 321187 3 75607 darin 2010-12-13 10:06:47 -0800 Comment on attachment 75607 Patch Again, I don’t quite see eye to eye with Eric on this. This optimization looks like a good idea and is not a lot more code. But I’d like to understand more what this makes faster. What platform? What test? 321290 4 eric 2010-12-13 11:38:33 -0800 I'm also not sure Darin and I disagree here either. :) 323090 5 paroga 2010-12-16 03:05:29 -0800 Same as in https://bugs.webkit.org/show_bug.cgi?id=50516#c9 (and sorry for the delay): avg median stdev OS X 10.6 original >4000 >4000 ;-) r74063 2185.72 2182 12.126895728091345 r74063 with patch 2127.83 2118.5 75.47424130125455 -2.65% -2,91% Maybe someone else can confirm the speed increase? I'm not sure if this (and the change in bug 50516 - both) cause such a "huge" performance win. 323295 6 75607 darin 2010-12-16 11:06:31 -0800 Comment on attachment 75607 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=75607&action=review I think it’s OK to land this patch. I’m going to change the review to review+. Eric, please do let me know if you object. I am not trying to override you. > JavaScriptCore/wtf/text/AtomicString.cpp:235 > + return equalUTF16WithUTF8(string->characters(), string->characters() + string->length(), buffer.characters, buffer.characters + buffer.length); We have stringCharacters in a local variable, and should use it instead of string->characters(). 323401 7 eric 2010-12-16 13:57:15 -0800 I would like Patrick to update the ChangeLog to explain his testing, and ideally add the benchmark to our repository under WebCore/benchmarks. Without clear (repeatable) testing, optimizing like this makes little sense to me. :) Thank you for reviewing Darin. 324846 8 75607 eric 2010-12-20 23:04:24 -0800 Comment on attachment 75607 Patch Eh. Looking at this patch again, it looks clearly right. Still would be good to know what perf testing is being done here. 326932 9 77465 paroga 2010-12-26 15:31:30 -0800 Created attachment 77465 Patch I'll cq+ this, when the xml parser benchmark is landed. 328154 10 77465 commit-queue 2010-12-31 04:43:48 -0800 Comment on attachment 77465 Patch Clearing flags on attachment: 77465 Committed r74829: <http://trac.webkit.org/changeset/74829> 328156 11 commit-queue 2010-12-31 04:43:55 -0800 All reviewed patches have been landed. Closing bug. 75607 2010-12-04 06:10:36 -0800 2010-12-26 15:31:30 -0800 Patch bug-50517-20101204151034.patch text/plain 2060 paroga ZGlmZiAtLWdpdCBhL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZyBiL0phdmFTY3JpcHRDb3JlL0No YW5nZUxvZwppbmRleCBlZTJhYTk5MDYyYWU2NTFhNTFjMzU1NDhlZGJmYWFjNDE5ZWI4ZGM0Li5i MDUyZTA5NjUwMTYzMjFmOTBiNGUzY2Q2MmZkYTg3MTFhNmVlMjllIDEwMDY0NAotLS0gYS9KYXZh U2NyaXB0Q29yZS9DaGFuZ2VMb2cKKysrIGIvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCkBAIC0x LDMgKzEsMTMgQEAKKzIwMTAtMTItMDQgIFBhdHJpY2sgR2Fuc3RlcmVyICA8cGFyb2dhQHdlYmtp dC5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg QWRkIGEgZmFzdCBjYXNlIGZvciBBU0NJSSBzdHJpbmdzIGluIEhhc2hBbmRVVEY4Q2hhcmFjdGVy c1RyYW5zbGF0b3I6OmVxdWFsCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3df YnVnLmNnaT9pZD01MDUxNworCisgICAgICAgICogd3RmL3RleHQvQXRvbWljU3RyaW5nLmNwcDoK KyAgICAgICAgKFdURjo6SGFzaEFuZFVURjhDaGFyYWN0ZXJzVHJhbnNsYXRvcjo6ZXF1YWwpOgor CiAyMDEwLTEyLTAzICBHYXZpbiBCYXJyYWNsb3VnaCAgPGJhcnJhY2xvdWdoQGFwcGxlLmNvbT4K IAogICAgICAgICBSdWJiZXIgc3RhbXBlZCBieSBPbGl2ZXIgSHVudC4KZGlmZiAtLWdpdCBhL0ph dmFTY3JpcHRDb3JlL3d0Zi90ZXh0L0F0b21pY1N0cmluZy5jcHAgYi9KYXZhU2NyaXB0Q29yZS93 dGYvdGV4dC9BdG9taWNTdHJpbmcuY3BwCmluZGV4IGFjYmNkMzRkMDBjMjI3MmY0ZmVhY2MyODdk OTQ1MGE1N2VjY2VlZWQuLjYxNDg3Y2Y2N2UwNGUzN2RiZGZjZGI0OTU3Njc5OGZlZDY0ZGE3MWIg MTAwNjQ0Ci0tLSBhL0phdmFTY3JpcHRDb3JlL3d0Zi90ZXh0L0F0b21pY1N0cmluZy5jcHAKKysr IGIvSmF2YVNjcmlwdENvcmUvd3RmL3RleHQvQXRvbWljU3RyaW5nLmNwcApAQCAtMjI1LDcgKzIy NSwyMiBAQCBzdHJ1Y3QgSGFzaEFuZFVURjhDaGFyYWN0ZXJzVHJhbnNsYXRvciB7CiAKICAgICBz dGF0aWMgYm9vbCBlcXVhbChTdHJpbmdJbXBsKiBjb25zdCYgc3RyaW5nLCBjb25zdCBIYXNoQW5k VVRGOENoYXJhY3RlcnMmIGJ1ZmZlcikKICAgICB7Ci0gICAgICAgIHJldHVybiBlcXVhbFVURjE2 V2l0aFVURjgoc3RyaW5nLT5jaGFyYWN0ZXJzKCksIHN0cmluZy0+Y2hhcmFjdGVycygpICsgc3Ry aW5nLT5sZW5ndGgoKSwgYnVmZmVyLmNoYXJhY3RlcnMsIGJ1ZmZlci5jaGFyYWN0ZXJzICsgYnVm ZmVyLmxlbmd0aCk7CisgICAgICAgIGlmIChidWZmZXIudXRmMTZMZW5ndGggIT0gc3RyaW5nLT5s ZW5ndGgoKSkKKyAgICAgICAgICAgIHJldHVybiBmYWxzZTsKKworICAgICAgICBjb25zdCBVQ2hh ciogc3RyaW5nQ2hhcmFjdGVycyA9IHN0cmluZy0+Y2hhcmFjdGVycygpOworCisgICAgICAgIC8v IElmIGJ1ZmZlciBjb250YWlucyBvbmx5IEFTQ0lJIGNoYXJhY3RlcnMgVVRGLTggYW5kIFVURjE2 IGxlbmd0aCBhcmUgdGhlIHNhbWUuCisgICAgICAgIGlmIChidWZmZXIudXRmMTZMZW5ndGggIT0g YnVmZmVyLmxlbmd0aCkKKyAgICAgICAgICAgIHJldHVybiBlcXVhbFVURjE2V2l0aFVURjgoc3Ry aW5nLT5jaGFyYWN0ZXJzKCksIHN0cmluZy0+Y2hhcmFjdGVycygpICsgc3RyaW5nLT5sZW5ndGgo KSwgYnVmZmVyLmNoYXJhY3RlcnMsIGJ1ZmZlci5jaGFyYWN0ZXJzICsgYnVmZmVyLmxlbmd0aCk7 CisKKyAgICAgICAgZm9yICh1bnNpZ25lZCBpID0gMDsgaSA8IGJ1ZmZlci5sZW5ndGg7ICsraSkg eworICAgICAgICAgICAgQVNTRVJUKGlzQVNDSUkoYnVmZmVyLmNoYXJhY3RlcnNbaV0pKTsKKyAg ICAgICAgICAgIGlmIChzdHJpbmdDaGFyYWN0ZXJzW2ldICE9IGJ1ZmZlci5jaGFyYWN0ZXJzW2ld KQorICAgICAgICAgICAgICAgIHJldHVybiBmYWxzZTsKKyAgICAgICAgfQorCisgICAgICAgIHJl dHVybiB0cnVlOwogICAgIH0KIAogICAgIHN0YXRpYyB2b2lkIHRyYW5zbGF0ZShTdHJpbmdJbXBs KiYgbG9jYXRpb24sIGNvbnN0IEhhc2hBbmRVVEY4Q2hhcmFjdGVycyYgYnVmZmVyLCB1bnNpZ25l ZCBoYXNoKQo= 77465 2010-12-26 15:31:30 -0800 2010-12-31 04:43:48 -0800 Patch bug-50517.patch text/plain 1982 paroga ZGlmZiAtLWdpdCBhL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZyBiL0phdmFTY3JpcHRDb3JlL0No YW5nZUxvZwppbmRleCBlM2RkZWIxLi45MGYwOGIxIDEwMDY0NAotLS0gYS9KYXZhU2NyaXB0Q29y ZS9DaGFuZ2VMb2cKKysrIGIvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUg QEAKKzIwMTAtMTItMjYgIFBhdHJpY2sgR2Fuc3RlcmVyICA8cGFyb2dhQHdlYmtpdC5vcmc+CisK KyAgICAgICAgUmV2aWV3ZWQgYnkgRGFyaW4gQWRsZXIuCisKKyAgICAgICAgQWRkIGEgZmFzdCBj YXNlIGZvciBBU0NJSSBzdHJpbmdzIGluIEhhc2hBbmRVVEY4Q2hhcmFjdGVyc1RyYW5zbGF0b3I6 OmVxdWFsCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD01 MDUxNworCisgICAgICAgIFRoaXMgY2hhbmdlIHNob3dzIGFib3V0IDIlIHBlcmZvcm1hbmNlIHdp biBvbiB0aGUgeG1sLXBhcnNlciBiZW5jaG1hcmsuCisKKyAgICAgICAgKiB3dGYvdGV4dC9BdG9t aWNTdHJpbmcuY3BwOgorICAgICAgICAoV1RGOjpIYXNoQW5kVVRGOENoYXJhY3RlcnNUcmFuc2xh dG9yOjplcXVhbCk6CisKIDIwMTAtMTItMjYgIFhhbiBMb3BleiAgPHhsb3BlekBpZ2FsaWEuY29t PgogCiAgICAgICAgIFJldmlld2VkIGJ5IEVyaWMgU2VpZGVsLgpkaWZmIC0tZ2l0IGEvSmF2YVNj cmlwdENvcmUvd3RmL3RleHQvQXRvbWljU3RyaW5nLmNwcCBiL0phdmFTY3JpcHRDb3JlL3d0Zi90 ZXh0L0F0b21pY1N0cmluZy5jcHAKaW5kZXggYWNiY2QzNC4uOTNhZDIxZCAxMDA2NDQKLS0tIGEv SmF2YVNjcmlwdENvcmUvd3RmL3RleHQvQXRvbWljU3RyaW5nLmNwcAorKysgYi9KYXZhU2NyaXB0 Q29yZS93dGYvdGV4dC9BdG9taWNTdHJpbmcuY3BwCkBAIC0yMjUsNyArMjI1LDIyIEBAIHN0cnVj dCBIYXNoQW5kVVRGOENoYXJhY3RlcnNUcmFuc2xhdG9yIHsKIAogICAgIHN0YXRpYyBib29sIGVx dWFsKFN0cmluZ0ltcGwqIGNvbnN0JiBzdHJpbmcsIGNvbnN0IEhhc2hBbmRVVEY4Q2hhcmFjdGVy cyYgYnVmZmVyKQogICAgIHsKLSAgICAgICAgcmV0dXJuIGVxdWFsVVRGMTZXaXRoVVRGOChzdHJp bmctPmNoYXJhY3RlcnMoKSwgc3RyaW5nLT5jaGFyYWN0ZXJzKCkgKyBzdHJpbmctPmxlbmd0aCgp LCBidWZmZXIuY2hhcmFjdGVycywgYnVmZmVyLmNoYXJhY3RlcnMgKyBidWZmZXIubGVuZ3RoKTsK KyAgICAgICAgaWYgKGJ1ZmZlci51dGYxNkxlbmd0aCAhPSBzdHJpbmctPmxlbmd0aCgpKQorICAg ICAgICAgICAgcmV0dXJuIGZhbHNlOworCisgICAgICAgIGNvbnN0IFVDaGFyKiBzdHJpbmdDaGFy YWN0ZXJzID0gc3RyaW5nLT5jaGFyYWN0ZXJzKCk7CisKKyAgICAgICAgLy8gSWYgYnVmZmVyIGNv bnRhaW5zIG9ubHkgQVNDSUkgY2hhcmFjdGVycyBVVEYtOCBhbmQgVVRGMTYgbGVuZ3RoIGFyZSB0 aGUgc2FtZS4KKyAgICAgICAgaWYgKGJ1ZmZlci51dGYxNkxlbmd0aCAhPSBidWZmZXIubGVuZ3Ro KQorICAgICAgICAgICAgcmV0dXJuIGVxdWFsVVRGMTZXaXRoVVRGOChzdHJpbmdDaGFyYWN0ZXJz LCBzdHJpbmdDaGFyYWN0ZXJzICsgc3RyaW5nLT5sZW5ndGgoKSwgYnVmZmVyLmNoYXJhY3RlcnMs IGJ1ZmZlci5jaGFyYWN0ZXJzICsgYnVmZmVyLmxlbmd0aCk7CisKKyAgICAgICAgZm9yICh1bnNp Z25lZCBpID0gMDsgaSA8IGJ1ZmZlci5sZW5ndGg7ICsraSkgeworICAgICAgICAgICAgQVNTRVJU KGlzQVNDSUkoYnVmZmVyLmNoYXJhY3RlcnNbaV0pKTsKKyAgICAgICAgICAgIGlmIChzdHJpbmdD aGFyYWN0ZXJzW2ldICE9IGJ1ZmZlci5jaGFyYWN0ZXJzW2ldKQorICAgICAgICAgICAgICAgIHJl dHVybiBmYWxzZTsKKyAgICAgICAgfQorCisgICAgICAgIHJldHVybiB0cnVlOwogICAgIH0KIAog ICAgIHN0YXRpYyB2b2lkIHRyYW5zbGF0ZShTdHJpbmdJbXBsKiYgbG9jYXRpb24sIGNvbnN0IEhh c2hBbmRVVEY4Q2hhcmFjdGVycyYgYnVmZmVyLCB1bnNpZ25lZCBoYXNoKQo=