109354 2013-02-09 06:52:09 -0800 RenderText::isAllCollapsibleWhitespace() shouldn't upconvert string to 16-bit. 2013-02-11 15:30:24 -0800 1 1 1 Unclassified WebKit Text 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED Performance P2 Normal --- 1 kling webkit-unassigned benjamin buildbot eric kling koivisto msaboff ojan.autocc rniwa webkit.review.bot oldest_to_newest 829013 0 kling 2013-02-09 06:52:09 -0800 Small win here. 829014 1 187429 kling 2013-02-09 06:52:52 -0800 Created attachment 187429 Patch 829279 2 187429 buildbot 2013-02-10 06:07:27 -0800 Comment on attachment 187429 Patch Attachment 187429 did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/16470772 New failing tests: compositing/checkerboard.html accessibility/anchor-linked-anonymous-block-crash.html http/tests/cache/cancel-multiple-post-xhrs.html animations/3d/state-at-end-event-transform.html animations/3d/replace-filling-transform.html http/tests/cache/history-only-cached-subresource-loads.html compositing/bounds-in-flipped-writing-mode.html accessibility/accessibility-node-reparent.html animations/animation-border-overflow.html accessibility/accessibility-object-detached.html accessibility/anonymous-render-block-in-continuation-causes-crash.html animations/animation-controller-drt-api.html compositing/absolute-position-changed-with-composited-parent-layer.html compositing/absolute-position-changed-in-composited-layer.html http/tests/cache/iframe-304-crash.html animations/3d/transform-perspective.html http/tests/cache/cancel-during-failure-crash.html canvas/philip/tests/2d.clearRect+fillRect.alpha0.5.html canvas/philip/tests/2d.clearRect.basic.html animations/3d/transform-origin-vs-functions.html animations/animation-css-rule-types.html canvas/philip/tests/2d.clearRect+fillRect.basic.html canvas/philip/tests/2d.clearRect+fillRect.alpha0.html 829352 3 187429 eric 2013-02-10 13:51:45 -0800 Comment on attachment 187429 Patch Fantastic. Thank you. Mac-WK2 looks sad, but I'm sure you'll check before landing. 829368 4 187429 benjamin 2013-02-10 16:24:41 -0800 Comment on attachment 187429 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=187429&action=review > Source/WebCore/rendering/RenderText.cpp:1193 > + for (unsigned i = 0; i < textLength(); i++) { > + if (!style()->isCollapsibleWhiteSpace(characterAt(i))) I would store textLength() in a temporary variable. You may want to split this in 2 cases, 8bits and 16bits, instead of doing that in the loop. 829674 5 187429 darin 2013-02-11 07:40:48 -0800 Comment on attachment 187429 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=187429&action=review >> Source/WebCore/rendering/RenderText.cpp:1193 >> + if (!style()->isCollapsibleWhiteSpace(characterAt(i))) > > I would store textLength() in a temporary variable. > > You may want to split this in 2 cases, 8bits and 16bits, instead of doing that in the loop. Ben has a good point about the two cases. The new code is going to be considerably slower than two separate loops. You could prove we need only a single loop with performance testing, or you could write the two loops. 830153 6 187681 kling 2013-02-11 14:45:35 -0800 Created attachment 187681 Patch for landing Hoisted the 8/16 bit checks out of the loop as requested. 830219 7 187681 webkit.review.bot 2013-02-11 15:30:20 -0800 Comment on attachment 187681 Patch for landing Clearing flags on attachment: 187681 Committed r142529: <http://trac.webkit.org/changeset/142529> 830220 8 webkit.review.bot 2013-02-11 15:30:24 -0800 All reviewed patches have been landed. Closing bug. 187429 2013-02-09 06:52:52 -0800 2013-02-11 14:45:35 -0800 Patch bug-109354.diff text/plain 1355 kling ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCAxNzNmOWM2Li4yZmI3N2VlIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUg QEAKKzIwMTMtMDItMDkgIEFuZHJlYXMgS2xpbmcgIDxha2xpbmdAYXBwbGUuY29tPgorCisgICAg ICAgIFJlbmRlclRleHQ6OmlzQWxsQ29sbGFwc2libGVXaGl0ZXNwYWNlKCkgc2hvdWxkbid0IHVw Y29udmVydCBzdHJpbmcgdG8gMTYtYml0LgorICAgICAgICA8aHR0cDovL3dlYmtpdC5vcmcvYi8x MDkzNTQ+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg MjU0IEtCIHByb2dyZXNzaW9uIG9uIE1lbWJ1c3RlcjMuCisKKyAgICAgICAgKiByZW5kZXJpbmcv UmVuZGVyVGV4dC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpSZW5kZXJUZXh0Ojppc0FsbENvbGxh cHNpYmxlV2hpdGVzcGFjZSk6CisKIDIwMTMtMDItMDggIENocmlzIEZsZWl6YWNoICA8Y2ZsZWl6 YWNoQGFwcGxlLmNvbT4KIAogICAgICAgICBSZWZhY3RvciBwbGF0Zm9ybS1zcGVjaWZpYyBjb2Rl IGluIFNwZWVjaFN5bnRoZXNpcwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5n L1JlbmRlclRleHQuY3BwIGIvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclRleHQuY3Bw CmluZGV4IDE5NjI0MjAuLmJmNTg1NzIgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3JlbmRl cmluZy9SZW5kZXJUZXh0LmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVy VGV4dC5jcHAKQEAgLTExODksMTAgKzExODksOCBAQCB2b2lkIFJlbmRlclRleHQ6OmNvbXB1dGVQ cmVmZXJyZWRMb2dpY2FsV2lkdGhzKGZsb2F0IGxlYWRXaWR0aCwgSGFzaFNldDxjb25zdCBTaQog CiBib29sIFJlbmRlclRleHQ6OmlzQWxsQ29sbGFwc2libGVXaGl0ZXNwYWNlKCkKIHsKLSAgICBp bnQgbGVuZ3RoID0gdGV4dExlbmd0aCgpOwotICAgIGNvbnN0IFVDaGFyKiB0ZXh0ID0gY2hhcmFj dGVycygpOwotICAgIGZvciAoaW50IGkgPSAwOyBpIDwgbGVuZ3RoOyBpKyspIHsKLSAgICAgICAg aWYgKCFzdHlsZSgpLT5pc0NvbGxhcHNpYmxlV2hpdGVTcGFjZSh0ZXh0W2ldKSkKKyAgICBmb3Ig KHVuc2lnbmVkIGkgPSAwOyBpIDwgdGV4dExlbmd0aCgpOyBpKyspIHsKKyAgICAgICAgaWYgKCFz dHlsZSgpLT5pc0NvbGxhcHNpYmxlV2hpdGVTcGFjZShjaGFyYWN0ZXJBdChpKSkpCiAgICAgICAg ICAgICByZXR1cm4gZmFsc2U7CiAgICAgfQogICAgIHJldHVybiB0cnVlOwo= 187681 2013-02-11 14:45:35 -0800 2013-02-11 15:30:20 -0800 Patch for landing land-109354.diff text/plain 1664 kling ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCBjNTMzYzQyLi44NzhhZWJiIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUg QEAKKzIwMTMtMDItMTEgIEFuZHJlYXMgS2xpbmcgIDxha2xpbmdAYXBwbGUuY29tPgorCisgICAg ICAgIFJlbmRlclRleHQ6OmlzQWxsQ29sbGFwc2libGVXaGl0ZXNwYWNlKCkgc2hvdWxkbid0IHVw Y29udmVydCBzdHJpbmcgdG8gMTYtYml0LgorICAgICAgICA8aHR0cDovL3dlYmtpdC5vcmcvYi8x MDkzNTQ+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgRXJpYyBTZWlkZWwuCisKKyAgICAgICAgMjU0 IEtCIHByb2dyZXNzaW9uIG9uIE1lbWJ1c3RlcjMuCisKKyAgICAgICAgKiByZW5kZXJpbmcvUmVu ZGVyVGV4dC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpSZW5kZXJUZXh0Ojppc0FsbENvbGxhcHNp YmxlV2hpdGVzcGFjZSk6CisKIDIwMTMtMDItMTAgIFRpbSBIb3J0b24gIDx0aW1vdGh5X2hvcnRv bkBhcHBsZS5jb20+CiAKICAgICAgICAgUkVHUkVTU0lPTiAocjEzMjQyMik6IFBhZ2UgY29udGVu dCBhbmQgc2Nyb2xsYmFycyBhcmUgaW5jb3JyZWN0bHkgb2Zmc2V0IGFmdGVyIHJlc3RvcmluZyBh IHBhZ2UgZnJvbSB0aGUgcGFnZSBjYWNoZQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcmVu ZGVyaW5nL1JlbmRlclRleHQuY3BwIGIvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclRl eHQuY3BwCmluZGV4IGM4ZTgwMzYuLmFhNmIwMzQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3Jl L3JlbmRlcmluZy9SZW5kZXJUZXh0LmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcv UmVuZGVyVGV4dC5jcHAKQEAgLTExODQsMTAgKzExODQsMTYgQEAgdm9pZCBSZW5kZXJUZXh0Ojpj b21wdXRlUHJlZmVycmVkTG9naWNhbFdpZHRocyhmbG9hdCBsZWFkV2lkdGgsIEhhc2hTZXQ8Y29u c3QgU2kKIAogYm9vbCBSZW5kZXJUZXh0Ojppc0FsbENvbGxhcHNpYmxlV2hpdGVzcGFjZSgpCiB7 Ci0gICAgaW50IGxlbmd0aCA9IHRleHRMZW5ndGgoKTsKLSAgICBjb25zdCBVQ2hhciogdGV4dCA9 IGNoYXJhY3RlcnMoKTsKLSAgICBmb3IgKGludCBpID0gMDsgaSA8IGxlbmd0aDsgaSsrKSB7Ci0g ICAgICAgIGlmICghc3R5bGUoKS0+aXNDb2xsYXBzaWJsZVdoaXRlU3BhY2UodGV4dFtpXSkpCisg ICAgdW5zaWduZWQgbGVuZ3RoID0gdGV4dExlbmd0aCgpOworICAgIGlmIChpczhCaXQoKSkgewor ICAgICAgICBmb3IgKHVuc2lnbmVkIGkgPSAwOyBpIDwgbGVuZ3RoOyArK2kpIHsKKyAgICAgICAg ICAgIGlmICghc3R5bGUoKS0+aXNDb2xsYXBzaWJsZVdoaXRlU3BhY2UoY2hhcmFjdGVyczgoKVtp XSkpCisgICAgICAgICAgICAgICAgcmV0dXJuIGZhbHNlOworICAgICAgICB9CisgICAgICAgIHJl dHVybiB0cnVlOworICAgIH0KKyAgICBmb3IgKHVuc2lnbmVkIGkgPSAwOyBpIDwgbGVuZ3RoOyAr K2kpIHsKKyAgICAgICAgaWYgKCFzdHlsZSgpLT5pc0NvbGxhcHNpYmxlV2hpdGVTcGFjZShjaGFy YWN0ZXJzMTYoKVtpXSkpCiAgICAgICAgICAgICByZXR1cm4gZmFsc2U7CiAgICAgfQogICAgIHJl dHVybiB0cnVlOwo=