67739 2011-09-07 15:14:04 -0700 adjustRectForColumns is slow when a huge # of columns are involved 2011-09-07 18:40:21 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 hyatt hyatt rniwa webkit.review.bot oldest_to_newest 463691 0 hyatt 2011-09-07 15:14:04 -0700 adjustRectForColumns is slow when a huge # of columns are involved, since it always loops over every single column regardless of how tiny the repaint rect is. This is a stupid thing to do, since intermediate columns between your beginning and end column always fully repaint anyway. 463703 1 106659 hyatt 2011-09-07 15:31:01 -0700 Created attachment 106659 Patch 463705 2 webkit.review.bot 2011-09-07 15:33:55 -0700 Attachment 106659 did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/rendering/RenderBlock.cpp:4622: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style. 463734 3 106659 mitz 2011-09-07 15:59:13 -0700 Comment on attachment 106659 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=106659&action=review >> Source/WebCore/rendering/RenderBlock.cpp:4622 >> + if (colHeight == 0) > > Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] That’s right. 463793 4 hyatt 2011-09-07 17:51:15 -0700 Fixed in r94737. 463798 5 rniwa 2011-09-07 18:12:06 -0700 This patch broke Windows build: http://build.webkit.org/builders/Windows%20Release%20%28Build%29/builds/20697/steps/compile-webkit/logs/stdio 463808 6 rniwa 2011-09-07 18:40:21 -0700 Fixed Windows build in http://trac.webkit.org/changeset/94739 and http://trac.webkit.org/changeset/94740. 106659 2011-09-07 15:31:01 -0700 2011-09-07 15:59:13 -0700 Patch patch.txt text/plain 3597 hyatt SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDk0NzE3KQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTYgQEAKKzIwMTEtMDktMDcgIERhdmlkIEh5 YXR0ICA8aHlhdHRAYXBwbGUuY29tPgorCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD02NzczOQorCisgICAgICAgIGFkanVzdFJlY3RGb3JDb2x1bW5zIGlz IE8oIyBvZiBjb2x1bW5zKSB3aGVuIGl0IGNhbiBiZSBPKDEpLiBGaXggdGhlIHNsb3cgcGVyZm9y bWFuY2Ugb2YgdGhpcworICAgICAgICBmdW5jdGlvbiBieSByZW1vdmluZyB0aGUgbG9vcCBhbmQg anVzdCBjb21wdXRpbmcgdGhlIHN0YXJ0IGFuZCBlbmQgY29sdW1uIGZvciBhIHJlcGFpbnQgcmVj dAorICAgICAgICBhbmQgdW5pdGluZyBldmVyeXRoaW5nIGluIGJldHdlZW4uCisKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiByZW5kZXJpbmcvUmVuZGVy QmxvY2suY3BwOgorICAgICAgICAoV2ViQ29yZTo6UmVuZGVyQmxvY2s6OmFkanVzdFJlY3RGb3JD b2x1bW5zKToKKwogMjAxMS0wOS0wNyAgQ2hyaXMgRmxlaXphY2ggIDxjZmxlaXphY2hAYXBwbGUu Y29tPgogCiAgICAgICAgIENoYW5nZXMgdG8gYXJpYS1oaWRkZW4gZG9uJ3QgY2hhbmdlIFZPIG5h dmlnYXRpb24KSW5kZXg6IFNvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJCbG9jay5jcHAK PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlckJsb2NrLmNwcAko cmV2aXNpb24gOTQ2OTEpCisrKyBTb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyQmxvY2su Y3BwCSh3b3JraW5nIGNvcHkpCkBAIC00NjA3LDMyICs0NjA3LDQ0IEBAIHZvaWQgUmVuZGVyQmxv Y2s6OmFkanVzdFJlY3RGb3JDb2x1bW5zKEwKICAgICAgICAgcmV0dXJuOwogICAgIAogICAgIENv bHVtbkluZm8qIGNvbEluZm8gPSBjb2x1bW5JbmZvKCk7Ci0KLSAgICAvLyBCZWdpbiB3aXRoIGEg cmVzdWx0IHJlY3QgdGhhdCBpcyBlbXB0eS4KLSAgICBMYXlvdXRSZWN0IHJlc3VsdDsKICAgICAK ICAgICAvLyBEZXRlcm1pbmUgd2hpY2ggY29sdW1ucyB3ZSBpbnRlcnNlY3QuCiAgICAgdW5zaWdu ZWQgY29sQ291bnQgPSBjb2x1bW5Db3VudChjb2xJbmZvKTsKICAgICBpZiAoIWNvbENvdW50KQog ICAgICAgICByZXR1cm47CisKKyAgICAvLyBCZWdpbiB3aXRoIGEgcmVzdWx0IHJlY3QgdGhhdCBp cyBlbXB0eS4KKyAgICBMYXlvdXRSZWN0IHJlc3VsdDsKKworICAgIGJvb2wgaXNIb3Jpem9udGFs ID0gaXNIb3Jpem9udGFsV3JpdGluZ01vZGUoKTsKKyAgICBMYXlvdXRVbml0IGJlZm9yZUJvcmRl clBhZGRpbmcgPSBib3JkZXJCZWZvcmUoKSArIHBhZGRpbmdCZWZvcmUoKTsKKyAgICBMYXlvdXRV bml0IGNvbEhlaWdodCA9IGNvbEluZm8tPmNvbHVtbkhlaWdodCgpOworICAgIGlmIChjb2xIZWln aHQgPT0gMCkKKyAgICAgICAgcmV0dXJuOworCisgICAgTGF5b3V0VW5pdCBzdGFydE9mZnNldCA9 IG1heChpc0hvcml6b250YWwgPyByLnkoKSA6IHIueCgpLCBiZWZvcmVCb3JkZXJQYWRkaW5nKTsK KyAgICBMYXlvdXRVbml0IGVuZE9mZnNldCA9IG1pbjxMYXlvdXRVbml0Pihpc0hvcml6b250YWwg PyByLm1heFkoKSA6IHIubWF4WCgpLCBiZWZvcmVCb3JkZXJQYWRkaW5nICsgY29sQ291bnQgKiBj b2xIZWlnaHQpOwogICAgIAotICAgIExheW91dFVuaXQgbG9naWNhbExlZnQgPSBsb2dpY2FsTGVm dE9mZnNldEZvckNvbnRlbnQoKTsKLSAgICBMYXlvdXRVbml0IGN1cnJMb2dpY2FsT2Zmc2V0ID0g MDsKKyAgICB1bnNpZ25lZCBzdGFydENvbHVtbiA9IChzdGFydE9mZnNldCAtIGJlZm9yZUJvcmRl clBhZGRpbmcpIC8gY29sSGVpZ2h0OworICAgIHVuc2lnbmVkIGVuZENvbHVtbiA9IChlbmRPZmZz ZXQgLSBiZWZvcmVCb3JkZXJQYWRkaW5nKSAvIGNvbEhlaWdodDsKIAotICAgIGZvciAodW5zaWdu ZWQgaSA9IDA7IGkgPCBjb2xDb3VudDsgaSsrKSB7Ci0gICAgICAgIExheW91dFJlY3QgY29sUmVj dCA9IGNvbHVtblJlY3RBdChjb2xJbmZvLCBpKTsKKyAgICBpZiAoc3RhcnRDb2x1bW4gPT0gZW5k Q29sdW1uKSB7CisgICAgICAgIC8vIFRoZSByZWN0IGlzIGZ1bGx5IGNvbnRhaW5lZCB3aXRoaW4g b25lIGNvbHVtbi4gQWRqdXN0IGZvciBvdXIgb2Zmc2V0cworICAgICAgICAvLyBhbmQgcmVwYWlu dCBvbmx5IHRoYXQgcG9ydGlvbi4KKyAgICAgICAgTGF5b3V0VW5pdCBsb2dpY2FsTGVmdE9mZnNl dCA9IGxvZ2ljYWxMZWZ0T2Zmc2V0Rm9yQ29udGVudCgpOworICAgICAgICBMYXlvdXRSZWN0IGNv bFJlY3QgPSBjb2x1bW5SZWN0QXQoY29sSW5mbywgc3RhcnRDb2x1bW4pOwogICAgICAgICBMYXlv dXRSZWN0IHJlcGFpbnRSZWN0ID0gcjsKLSAgICAgICAgaWYgKGlzSG9yaXpvbnRhbFdyaXRpbmdN b2RlKCkpIHsKLSAgICAgICAgICAgIExheW91dFVuaXQgY3VyclhPZmZzZXQgPSBjb2xSZWN0Lngo KSAtIGxvZ2ljYWxMZWZ0OwotICAgICAgICAgICAgcmVwYWludFJlY3QubW92ZShjdXJyWE9mZnNl dCwgY3VyckxvZ2ljYWxPZmZzZXQpOwotICAgICAgICAgICAgY3VyckxvZ2ljYWxPZmZzZXQgLT0g Y29sUmVjdC5oZWlnaHQoKTsKLSAgICAgICAgfSBlbHNlIHsKLSAgICAgICAgICAgIExheW91dFVu aXQgY3VycllPZmZzZXQgPSBjb2xSZWN0LnkoKSAtIGxvZ2ljYWxMZWZ0OwotICAgICAgICAgICAg cmVwYWludFJlY3QubW92ZShjdXJyTG9naWNhbE9mZnNldCwgY3VycllPZmZzZXQpOwotICAgICAg ICAgICAgY3VyckxvZ2ljYWxPZmZzZXQgLT0gY29sUmVjdC53aWR0aCgpOwotICAgICAgICB9Cisg ICAgICAgIGlmIChpc0hvcml6b250YWwpCisgICAgICAgICAgICByZXBhaW50UmVjdC5tb3ZlKGNv bFJlY3QueCgpIC0gbG9naWNhbExlZnRPZmZzZXQsIC1zdGFydENvbHVtbiAqIGNvbEhlaWdodCk7 CisgICAgICAgIGVsc2UKKyAgICAgICAgICAgIHJlcGFpbnRSZWN0Lm1vdmUoLXN0YXJ0Q29sdW1u ICogY29sSGVpZ2h0LCBjb2xSZWN0LnkoKSAtIGxvZ2ljYWxMZWZ0T2Zmc2V0KTsKICAgICAgICAg cmVwYWludFJlY3QuaW50ZXJzZWN0KGNvbFJlY3QpOwogICAgICAgICByZXN1bHQudW5pdGUocmVw YWludFJlY3QpOworICAgIH0gZWxzZSB7CisgICAgICAgIC8vIFdlIHNwYW4gbXVsdGlwbGUgY29s dW1ucy4gV2UgY2FuIGp1c3QgdW5pdGUgdGhlIHN0YXJ0IGFuZCBlbmQgY29sdW1uIHRvIGdldCB0 aGUgZmluYWwKKyAgICAgICAgLy8gcmVwYWludCByZWN0LgorICAgICAgICByZXN1bHQudW5pdGUo Y29sdW1uUmVjdEF0KGNvbEluZm8sIHN0YXJ0Q29sdW1uKSk7CisgICAgICAgIHJlc3VsdC51bml0 ZShjb2x1bW5SZWN0QXQoY29sSW5mbywgZW5kQ29sdW1uKSk7CiAgICAgfQogCiAgICAgciA9IHJl c3VsdDsK