45589 2010-09-11 08:52:12 -0700 Inline UTF8SequenceLength 2010-09-22 10:57:09 -0700 1 1 1 Unclassified WebKit Web Template Framework 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED P2 Normal --- 0 paroga webkit-unassigned abarth commit-queue darin eric webkit.review.bot oldest_to_newest 277712 0 paroga 2010-09-11 08:52:12 -0700 see patch 277716 1 67299 paroga 2010-09-11 08:57:42 -0700 Created attachment 67299 Patch 279418 2 67299 eric 2010-09-14 23:53:48 -0700 Comment on attachment 67299 Patch Please update your ChangeLog to explain "why". 279492 3 paroga 2010-09-15 03:30:30 -0700 (In reply to comment #2) > (From update of attachment 67299 [details]) > Please update your ChangeLog to explain "why". Doesn't "inline" already explain it all? 280912 4 67299 paroga 2010-09-17 15:25:58 -0700 Comment on attachment 67299 Patch (In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 67299 [details] [details]) > > Please update your ChangeLog to explain "why". > Doesn't "inline" already explain it all? There is no real "why". It's only a cleanup. 280998 5 darin 2010-09-17 17:04:48 -0700 Does it make the code faster? Smaller? Easier to read? 281099 6 68003 paroga 2010-09-18 01:28:32 -0700 Created attachment 68003 Patch Added a "to improve performance" in the ChangeLog. 281204 7 68003 darin 2010-09-18 16:17:29 -0700 Comment on attachment 68003 Patch OK. So does this improve performance? 282930 8 68371 paroga 2010-09-22 07:49:34 -0700 Created attachment 68371 Changes used for testing > OK. So does this improve performance? I've done a quick performance test with this patch. I've got ~8% speed improvement on WinCE (time dropped from 2385 to 2203). 282943 9 darin 2010-09-22 08:18:00 -0700 (In reply to comment #8) > I've done a quick performance test with this patch. > I've got ~8% speed improvement on WinCE (time dropped from 2385 to 2203). Sorry to keep pushing on this, but did you try just calling the inline version as-is rather than merging them? I’d expect an even-better speed improvement if you just changed the call site in convertUTF8ToUTF16 and didn’t make the other changes. 282967 10 68378 paroga 2010-09-22 09:14:11 -0700 Created attachment 68378 Patch (In reply to comment #9) > Sorry to keep pushing on this, but did you try just calling the inline version as-is rather than merging them? I’d expect an even-better speed improvement if you just changed the call site in convertUTF8ToUTF16 and didn’t make the other changes. I don't know why, but you're right! It's faster with the newest patch. 282984 11 darin 2010-09-22 09:45:33 -0700 (In reply to comment #10) > (In reply to comment #9) > > Sorry to keep pushing on this, but did you try just calling the inline version as-is rather than merging them? I’d expect an even-better speed improvement if you just changed the call site in convertUTF8ToUTF16 and didn’t make the other changes. > I don't know why, but you're right! It's faster with the newest patch. I believe I wrote the original function, and I followed the principle of inlining the hot part of the code and keeping the less-hot part out of line. This often makes things faster than just inlining everything. Seeing that your original patch reversed that decision, I asked you to measure. Thanks for the extra effort. 283032 12 68378 commit-queue 2010-09-22 10:29:05 -0700 Comment on attachment 68378 Patch Clearing flags on attachment: 68378 Committed r68055: <http://trac.webkit.org/changeset/68055> 283033 13 commit-queue 2010-09-22 10:29:10 -0700 All reviewed patches have been landed. Closing bug. 283053 14 webkit.review.bot 2010-09-22 10:57:09 -0700 http://trac.webkit.org/changeset/68055 might have broken Qt Windows 32-bit Release The following changes are on the blame list: http://trac.webkit.org/changeset/68056 http://trac.webkit.org/changeset/68055 67299 2010-09-11 08:57:42 -0700 2010-09-18 01:28:32 -0700 Patch 0001-UTF8SequenceLength.patch text/plain 2083 paroga ZGlmZiAtLWdpdCBhL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZyBiL0phdmFTY3JpcHRDb3JlL0No YW5nZUxvZwppbmRleCA1ODg5YmJkLi5iMWQ3MWE4IDEwMDY0NAotLS0gYS9KYXZhU2NyaXB0Q29y ZS9DaGFuZ2VMb2cKKysrIGIvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUg QEAKKzIwMTAtMDktMTEgIFBhdHJpY2sgR2Fuc3RlcmVyICA8cGFyb2dhQHBhcm9nYS5jb20+CisK KyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgSW5saW5lIFVU RjhTZXF1ZW5jZUxlbmd0aAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9NDU1ODkKKworICAgICAgICAqIHd0Zi91bmljb2RlL1VURjguY3BwOgorICAgICAg ICAoV1RGOjpVbmljb2RlOjppbmxpbmVVVEY4U2VxdWVuY2VMZW5ndGgpOiBJbnRlZ3JhdGUgaW5s aW5lVVRGOFNlcXVlbmNlTGVuZ3RoTm9uQVNDSUkuCisgICAgICAgIChXVEY6OlVuaWNvZGU6OlVU RjhTZXF1ZW5jZUxlbmd0aCk6IFVzZSBpbmxpbmVVVEY4U2VxdWVuY2VMZW5ndGggaW5zdGVhZCBv ZiBhIGNvcHkgb2YgaXQuCisgICAgICAgIChXVEY6OlVuaWNvZGU6OmNvbnZlcnRVVEY4VG9VVEYx Nik6IFVzZSBpbmxpbmUgdmVyc2lvbiBvZiBVVEY4U2VxdWVuY2VMZW5ndGguCisKIDIwMTAtMDkt MTAgIEZyaWRyaWNoIFN0cmJhICA8ZnJpZHJpY2guc3RyYmFAYmx1ZXdpbi5jaD4KIAogICAgICAg ICBSZXZpZXdlZCBieSBBbmRyZWFzIEtsaW5nLgpkaWZmIC0tZ2l0IGEvSmF2YVNjcmlwdENvcmUv d3RmL3VuaWNvZGUvVVRGOC5jcHAgYi9KYXZhU2NyaXB0Q29yZS93dGYvdW5pY29kZS9VVEY4LmNw cAppbmRleCAyMWQ1ODU2Li41MDJjMGUyIDEwMDY0NAotLS0gYS9KYXZhU2NyaXB0Q29yZS93dGYv dW5pY29kZS9VVEY4LmNwcAorKysgYi9KYXZhU2NyaXB0Q29yZS93dGYvdW5pY29kZS9VVEY4LmNw cApAQCAtMjksOCArMjksMTAgQEAKIG5hbWVzcGFjZSBXVEYgewogbmFtZXNwYWNlIFVuaWNvZGUg ewogCi1pbmxpbmUgaW50IGlubGluZVVURjhTZXF1ZW5jZUxlbmd0aE5vbkFTQ0lJKGNoYXIgYjAp CitpbmxpbmUgaW50IGlubGluZVVURjhTZXF1ZW5jZUxlbmd0aChjaGFyIGIwKQogeworICAgIGlm ICgoYjAgJiAweDgwKSAhPSAweDgwKQorICAgICAgICByZXR1cm4gMTsKICAgICBpZiAoKGIwICYg MHhDMCkgIT0gMHhDMCkKICAgICAgICAgcmV0dXJuIDA7CiAgICAgaWYgKChiMCAmIDB4RTApID09 IDB4QzApCkBAIC00MiwxNCArNDQsOSBAQCBpbmxpbmUgaW50IGlubGluZVVURjhTZXF1ZW5jZUxl bmd0aE5vbkFTQ0lJKGNoYXIgYjApCiAgICAgcmV0dXJuIDA7CiB9CiAKLWlubGluZSBpbnQgaW5s aW5lVVRGOFNlcXVlbmNlTGVuZ3RoKGNoYXIgYjApCi17Ci0gICAgcmV0dXJuIChiMCAmIDB4ODAp ID09IDAgPyAxIDogaW5saW5lVVRGOFNlcXVlbmNlTGVuZ3RoTm9uQVNDSUkoYjApOwotfQotCiBp bnQgVVRGOFNlcXVlbmNlTGVuZ3RoKGNoYXIgYjApCiB7Ci0gICAgcmV0dXJuIChiMCAmIDB4ODAp ID09IDAgPyAxIDogaW5saW5lVVRGOFNlcXVlbmNlTGVuZ3RoTm9uQVNDSUkoYjApOworICAgIHJl dHVybiBpbmxpbmVVVEY4U2VxdWVuY2VMZW5ndGgoYjApOwogfQogCiBpbnQgZGVjb2RlVVRGOFNl cXVlbmNlKGNvbnN0IGNoYXIqIHNlcXVlbmNlKQpAQCAtMjQwLDcgKzIzNyw3IEBAIENvbnZlcnNp b25SZXN1bHQgY29udmVydFVURjhUb1VURjE2KAogICAgIFVDaGFyKiB0YXJnZXQgPSAqdGFyZ2V0 U3RhcnQ7CiAgICAgd2hpbGUgKHNvdXJjZSA8IHNvdXJjZUVuZCkgewogICAgICAgICBVQ2hhcjMy IGNoID0gMDsKLSAgICAgICAgaW50IGV4dHJhQnl0ZXNUb1JlYWQgPSBVVEY4U2VxdWVuY2VMZW5n dGgoKnNvdXJjZSkgLSAxOworICAgICAgICBpbnQgZXh0cmFCeXRlc1RvUmVhZCA9IGlubGluZVVU RjhTZXF1ZW5jZUxlbmd0aCgqc291cmNlKSAtIDE7CiAgICAgICAgIGlmIChzb3VyY2UgKyBleHRy YUJ5dGVzVG9SZWFkID49IHNvdXJjZUVuZCkgewogICAgICAgICAgICAgcmVzdWx0ID0gc291cmNl RXhoYXVzdGVkOwogICAgICAgICAgICAgYnJlYWs7Cg== 68003 2010-09-18 01:28:32 -0700 2010-09-22 09:14:11 -0700 Patch bug-45589.patch text/plain 2106 paroga ZGlmZiAtLWdpdCBhL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZyBiL0phdmFTY3JpcHRDb3JlL0No YW5nZUxvZwppbmRleCA1ODg5YmJkLi5iMWQ3MWE4IDEwMDY0NAotLS0gYS9KYXZhU2NyaXB0Q29y ZS9DaGFuZ2VMb2cKKysrIGIvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUg QEAKKzIwMTAtMDktMTEgIFBhdHJpY2sgR2Fuc3RlcmVyICA8cGFyb2dhQHBhcm9nYS5jb20+CisK KyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgSW5saW5lIFVU RjhTZXF1ZW5jZUxlbmd0aAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9NDU1ODkKKworICAgICAgICAqIHd0Zi91bmljb2RlL1VURjguY3BwOgorICAgICAg ICAoV1RGOjpVbmljb2RlOjppbmxpbmVVVEY4U2VxdWVuY2VMZW5ndGgpOiBJbnRlZ3JhdGUgaW5s aW5lVVRGOFNlcXVlbmNlTGVuZ3RoTm9uQVNDSUkuCisgICAgICAgIChXVEY6OlVuaWNvZGU6OlVU RjhTZXF1ZW5jZUxlbmd0aCk6IFVzZSBpbmxpbmVVVEY4U2VxdWVuY2VMZW5ndGggaW5zdGVhZCBv ZiBhIGNvcHkgb2YgaXQuCisgICAgICAgIChXVEY6OlVuaWNvZGU6OmNvbnZlcnRVVEY4VG9VVEYx Nik6IFVzZSBpbmxpbmUgdmVyc2lvbiBvZiBVVEY4U2VxdWVuY2VMZW5ndGggdG8gaW1wcm92ZSBw ZXJmb3JtYW5jZS4KKwogMjAxMC0wOS0xMCAgRnJpZHJpY2ggU3RyYmEgIDxmcmlkcmljaC5zdHJi YUBibHVld2luLmNoPgogCiAgICAgICAgIFJldmlld2VkIGJ5IEFuZHJlYXMgS2xpbmcuCmRpZmYg LS1naXQgYS9KYXZhU2NyaXB0Q29yZS93dGYvdW5pY29kZS9VVEY4LmNwcCBiL0phdmFTY3JpcHRD b3JlL3d0Zi91bmljb2RlL1VURjguY3BwCmluZGV4IDIxZDU4NTYuLjUwMmMwZTIgMTAwNjQ0Ci0t LSBhL0phdmFTY3JpcHRDb3JlL3d0Zi91bmljb2RlL1VURjguY3BwCisrKyBiL0phdmFTY3JpcHRD b3JlL3d0Zi91bmljb2RlL1VURjguY3BwCkBAIC0yOSw4ICsyOSwxMCBAQAogbmFtZXNwYWNlIFdU RiB7CiBuYW1lc3BhY2UgVW5pY29kZSB7CiAKLWlubGluZSBpbnQgaW5saW5lVVRGOFNlcXVlbmNl TGVuZ3RoTm9uQVNDSUkoY2hhciBiMCkKK2lubGluZSBpbnQgaW5saW5lVVRGOFNlcXVlbmNlTGVu Z3RoKGNoYXIgYjApCiB7CisgICAgaWYgKChiMCAmIDB4ODApICE9IDB4ODApCisgICAgICAgIHJl dHVybiAxOwogICAgIGlmICgoYjAgJiAweEMwKSAhPSAweEMwKQogICAgICAgICByZXR1cm4gMDsK ICAgICBpZiAoKGIwICYgMHhFMCkgPT0gMHhDMCkKQEAgLTQyLDE0ICs0NCw5IEBAIGlubGluZSBp bnQgaW5saW5lVVRGOFNlcXVlbmNlTGVuZ3RoTm9uQVNDSUkoY2hhciBiMCkKICAgICByZXR1cm4g MDsKIH0KIAotaW5saW5lIGludCBpbmxpbmVVVEY4U2VxdWVuY2VMZW5ndGgoY2hhciBiMCkKLXsK LSAgICByZXR1cm4gKGIwICYgMHg4MCkgPT0gMCA/IDEgOiBpbmxpbmVVVEY4U2VxdWVuY2VMZW5n dGhOb25BU0NJSShiMCk7Ci19Ci0KIGludCBVVEY4U2VxdWVuY2VMZW5ndGgoY2hhciBiMCkKIHsK LSAgICByZXR1cm4gKGIwICYgMHg4MCkgPT0gMCA/IDEgOiBpbmxpbmVVVEY4U2VxdWVuY2VMZW5n dGhOb25BU0NJSShiMCk7CisgICAgcmV0dXJuIGlubGluZVVURjhTZXF1ZW5jZUxlbmd0aChiMCk7 CiB9CiAKIGludCBkZWNvZGVVVEY4U2VxdWVuY2UoY29uc3QgY2hhciogc2VxdWVuY2UpCkBAIC0y NDAsNyArMjM3LDcgQEAgQ29udmVyc2lvblJlc3VsdCBjb252ZXJ0VVRGOFRvVVRGMTYoCiAgICAg VUNoYXIqIHRhcmdldCA9ICp0YXJnZXRTdGFydDsKICAgICB3aGlsZSAoc291cmNlIDwgc291cmNl RW5kKSB7CiAgICAgICAgIFVDaGFyMzIgY2ggPSAwOwotICAgICAgICBpbnQgZXh0cmFCeXRlc1Rv UmVhZCA9IFVURjhTZXF1ZW5jZUxlbmd0aCgqc291cmNlKSAtIDE7CisgICAgICAgIGludCBleHRy YUJ5dGVzVG9SZWFkID0gaW5saW5lVVRGOFNlcXVlbmNlTGVuZ3RoKCpzb3VyY2UpIC0gMTsKICAg ICAgICAgaWYgKHNvdXJjZSArIGV4dHJhQnl0ZXNUb1JlYWQgPj0gc291cmNlRW5kKSB7CiAgICAg ICAgICAgICByZXN1bHQgPSBzb3VyY2VFeGhhdXN0ZWQ7CiAgICAgICAgICAgICBicmVhazsK 68371 2010-09-22 07:49:34 -0700 2010-09-22 07:49:34 -0700 Changes used for testing test.patch text/plain 957 paroga ZGlmZiAtLWdpdCBhL0phdmFTY3JpcHRDb3JlL3d0Zi91bmljb2RlL1VURjguY3BwIGIvSmF2YVNj cmlwdENvcmUvd3RmL3VuaWNvZGUvVVRGOC5jcHAKaW5kZXggMjFkNTg1Ni4uYTE2ZDUwMyAxMDA2 NDQKLS0tIGEvSmF2YVNjcmlwdENvcmUvd3RmL3VuaWNvZGUvVVRGOC5jcHAKKysrIGIvSmF2YVNj cmlwdENvcmUvd3RmL3VuaWNvZGUvVVRGOC5jcHAKQEAgLTIzNSw2ICsyMzUsOSBAQCBDb252ZXJz aW9uUmVzdWx0IGNvbnZlcnRVVEY4VG9VVEYxNigKICAgICBjb25zdCBjaGFyKiogc291cmNlU3Rh cnQsIGNvbnN0IGNoYXIqIHNvdXJjZUVuZCwgCiAgICAgVUNoYXIqKiB0YXJnZXRTdGFydCwgVUNo YXIqIHRhcmdldEVuZCwgYm9vbCBzdHJpY3QpCiB7CisgICAgc3RhdGljIGRvdWJsZSB0aW1lID0g MDsKKyAgICBkb3VibGUgdGltZVMgPSBXVEY6OmN1cnJlbnRUaW1lKCk7CisKICAgICBDb252ZXJz aW9uUmVzdWx0IHJlc3VsdCA9IGNvbnZlcnNpb25PSzsKICAgICBjb25zdCBjaGFyKiBzb3VyY2Ug PSAqc291cmNlU3RhcnQ7CiAgICAgVUNoYXIqIHRhcmdldCA9ICp0YXJnZXRTdGFydDsKQEAgLTI5 NSw2ICsyOTgsMTQgQEAgQ29udmVyc2lvblJlc3VsdCBjb252ZXJ0VVRGOFRvVVRGMTYoCiAgICAg ICAgICAgICAqdGFyZ2V0KysgPSAoVUNoYXIpKChjaCAmIDB4MDNGRikgKyAweERDMDApOwogICAg ICAgICB9CiAgICAgfQorCisgICAgdGltZSArPSBXVEY6OmN1cnJlbnRUaW1lKCkgLSB0aW1lUzsK KworICAgIHN0YXRpYyB1bnNpZ25lZCBjbnQgPSAwOworICAgIGNudCsrOworICAgIGlmICgoY250 ICYgMHhGRikgPT0gMCkKKyAgICAgICAgc3RkOjpjb3V0IDw8IGNudCA8PCAiID0gIiA8PCAodGlt ZSAqIDEwMDApIDw8IHN0ZDo6ZW5kbDsKKwogICAgICpzb3VyY2VTdGFydCA9IHNvdXJjZTsKICAg ICAqdGFyZ2V0U3RhcnQgPSB0YXJnZXQ7CiAgICAgcmV0dXJuIHJlc3VsdDsK 68378 2010-09-22 09:14:11 -0700 2010-09-22 10:29:05 -0700 Patch bug-45589.patch text/plain 1190 paroga ZGlmZiAtLWdpdCBhL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZyBiL0phdmFTY3JpcHRDb3JlL0No YW5nZUxvZwppbmRleCA2OWUyN2FjLi42NzBkMGJiIDEwMDY0NAotLS0gYS9KYXZhU2NyaXB0Q29y ZS9DaGFuZ2VMb2cKKysrIGIvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTMg QEAKKzIwMTAtMDktMjIgIFBhdHJpY2sgR2Fuc3RlcmVyICA8cGFyb2dhQHdlYmtpdC5vcmc+CisK KyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgSW5saW5lIFVU RjhTZXF1ZW5jZUxlbmd0aAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9NDU1ODkKKworICAgICAgICAqIHd0Zi91bmljb2RlL1VURjguY3BwOgorICAgICAg ICAoV1RGOjpVbmljb2RlOjpjb252ZXJ0VVRGOFRvVVRGMTYpOiBVc2UgaW5saW5lIHZlcnNpb24g b2YgVVRGOFNlcXVlbmNlTGVuZ3RoIHRvIGltcHJvdmUgcGVyZm9ybWFuY2UuCisKIDIwMTAtMDkt MjEgIEdlb2ZmcmV5IEdhcmVuICA8Z2dhcmVuQGFwcGxlLmNvbT4KIAogICAgICAgICBSZXZpZXdl ZCBieSBPbGl2ZXIgSHVudC4KZGlmZiAtLWdpdCBhL0phdmFTY3JpcHRDb3JlL3d0Zi91bmljb2Rl L1VURjguY3BwIGIvSmF2YVNjcmlwdENvcmUvd3RmL3VuaWNvZGUvVVRGOC5jcHAKaW5kZXggMjFk NTg1Ni4uNDBjNTYwOSAxMDA2NDQKLS0tIGEvSmF2YVNjcmlwdENvcmUvd3RmL3VuaWNvZGUvVVRG OC5jcHAKKysrIGIvSmF2YVNjcmlwdENvcmUvd3RmL3VuaWNvZGUvVVRGOC5jcHAKQEAgLTI0MCw3 ICsyNDAsNyBAQCBDb252ZXJzaW9uUmVzdWx0IGNvbnZlcnRVVEY4VG9VVEYxNigKICAgICBVQ2hh ciogdGFyZ2V0ID0gKnRhcmdldFN0YXJ0OwogICAgIHdoaWxlIChzb3VyY2UgPCBzb3VyY2VFbmQp IHsKICAgICAgICAgVUNoYXIzMiBjaCA9IDA7Ci0gICAgICAgIGludCBleHRyYUJ5dGVzVG9SZWFk ID0gVVRGOFNlcXVlbmNlTGVuZ3RoKCpzb3VyY2UpIC0gMTsKKyAgICAgICAgaW50IGV4dHJhQnl0 ZXNUb1JlYWQgPSBpbmxpbmVVVEY4U2VxdWVuY2VMZW5ndGgoKnNvdXJjZSkgLSAxOwogICAgICAg ICBpZiAoc291cmNlICsgZXh0cmFCeXRlc1RvUmVhZCA+PSBzb3VyY2VFbmQpIHsKICAgICAgICAg ICAgIHJlc3VsdCA9IHNvdXJjZUV4aGF1c3RlZDsKICAgICAgICAgICAgIGJyZWFrOwo=