98625 2012-10-07 15:19:00 -0700 http://www.robohornet.org/tests/svgresize.html spends all its time in StyleResolver::sweepMatchedPropertiesCache 2012-10-16 17:05:29 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 98798 1 eric webkit-unassigned cmarcelo kling koivisto macpherson menard webkit.review.bot oldest_to_newest 736437 0 eric 2012-10-07 15:19:00 -0700 http://www.robohornet.org/tests/svgresize.html spends all (90%) of its time in StyleResolver::sweepMatchedPropertiesCache It must be hitting some pathological case? 736438 1 eric 2012-10-07 15:21:15 -0700 Does the cache grow without bound? 738838 2 koivisto 2012-10-10 04:32:05 -0700 Maybe the sweep needs be done by a short timer instead of being synchronous. 742225 3 168748 kling 2012-10-15 11:33:58 -0700 Created attachment 168748 GC the matched properties cache on a timer instead of every 100 additions. 742233 4 168748 eric 2012-10-15 11:43:59 -0700 Comment on attachment 168748 GC the matched properties cache on a timer instead of every 100 additions. Should we be capping the size of this cache? 742235 5 168748 eric 2012-10-15 11:46:53 -0700 Comment on attachment 168748 GC the matched properties cache on a timer instead of every 100 additions. I now see that I asked the same question earlier in this bug. :) Anyway, the change itself looks fine. I still have some mild concern about the seemingly unbounded nature of this cache. :) 742236 6 eric 2012-10-15 11:47:40 -0700 Perhaps we should add svgresize or similar to PerformanceTests to catch this? 742240 7 kling 2012-10-15 11:50:07 -0700 (In reply to comment #4) > (From update of attachment 168748 [details]) > Should we be capping the size of this cache? That sounds like the sane thing to do. Thoughts, Antti? (In reply to comment #6) > Perhaps we should add svgresize or similar to PerformanceTests to catch this? Could we import the test suite wholesale? If so, that would be the best thing to do. It might have a negative impact on cycle times (though that'll certainly motivate us to make the tests run faster!) 742241 8 eric 2012-10-15 11:51:50 -0700 (In reply to comment #7) > (In reply to comment #6) > > Perhaps we should add svgresize or similar to PerformanceTests to catch this? > > Could we import the test suite wholesale? If so, that would be the best thing to do. It might have a negative impact on cycle times (though that'll certainly motivate us to make the tests run faster!) Bug 97507 is about just that. I think the current prevailing view is that the suite is in-flux and thus not worth importing. shrug. 742644 9 168748 webkit.review.bot 2012-10-15 17:36:41 -0700 Comment on attachment 168748 GC the matched properties cache on a timer instead of every 100 additions. Clearing flags on attachment: 168748 Committed r131388: <http://trac.webkit.org/changeset/131388> 742645 10 webkit.review.bot 2012-10-15 17:36:45 -0700 All reviewed patches have been landed. Closing bug. 743715 11 eric 2012-10-16 17:05:29 -0700 Good times. :) Now the test spends 50% of time in StyleResolver::canShareStyleWithElement (which has shown up hot in other samples before). 168748 2012-10-15 11:33:58 -0700 2012-10-15 17:36:41 -0700 GC the matched properties cache on a timer instead of every 100 additions. bug-98625.diff text/plain 4695 kling ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA4ZmVkNjIzLi5jMzBlNDAzIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjEg QEAKKzIwMTItMTAtMTUgIEFuZHJlYXMgS2xpbmcgIDxrbGluZ0B3ZWJraXQub3JnPgorCisgICAg ICAgIFN0eWxlUmVzb2x2ZXI6IEdhcmJhZ2UgY29sbGVjdCB0aGUgbWF0Y2hlZCBwcm9wZXJ0aWVz IGNhY2hlIG9uIGEgdGltZXIuCisgICAgICAgIDxodHRwOi8vd2Via2l0Lm9yZy9iLzk4NjI1Pgor CisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFN3ZWVwaW5n IHRoZSBtYXRjaGVkIHByb3BlcnRpZXMgY2FjaGUgb25jZSBldmVyeSAxMDAgYWRkaXRpb25zIGVu ZGVkIHVwIGNob2tpbmcgUm9ib0hvcm5ldCdzCisgICAgICAgIHN2Z3Jlc2l6ZS5odG1sIGJlbmNo bWFyay4gTW92ZSBpdCB0byBhIHNpbmdsZS1zaG90IHRpbWVyIHRoYXQncyByZWZyZXNoZWQgZXZl cnkgMTAwIGFkZGl0aW9ucworICAgICAgICBhbmQgZGVmZXJzIHRoZSBhY3R1YWwgc3dlZXAgZm9y IDYwIHNlY29uZHMuCisKKyAgICAgICAgKiBjc3MvU3R5bGVSZXNvbHZlci5jcHA6CisgICAgICAg IChXZWJDb3JlOjpTdHlsZVJlc29sdmVyOjpTdHlsZVJlc29sdmVyKToKKyAgICAgICAgKFdlYkNv cmU6OlN0eWxlUmVzb2x2ZXI6OnN3ZWVwTWF0Y2hlZFByb3BlcnRpZXNDYWNoZSk6CisgICAgICAg IChXZWJDb3JlOjpTdHlsZVJlc29sdmVyOjphZGRUb01hdGNoZWRQcm9wZXJ0aWVzQ2FjaGUpOgor ICAgICAgICAqIGNzcy9TdHlsZVJlc29sdmVyLmg6CisgICAgICAgIChTdHlsZVJlc29sdmVyKToK KwogMjAxMi0xMC0xNSAgVnNldm9sb2QgVmxhc292ICA8dnNldmlrQGNocm9taXVtLm9yZz4KIAog ICAgICAgICBXZWIgSW5zcGVjdG9yOiBTY3JpcHRzIGZvciBkeW5hbWljYWxseSBhZGRlZCBzY3Jp cHQgZWxlbWVudHMgYXJlIG5vdCBzaG93biBpbiBzb3VyY2VzIHBhbmVsLgpkaWZmIC0tZ2l0IGEv U291cmNlL1dlYkNvcmUvY3NzL1N0eWxlUmVzb2x2ZXIuY3BwIGIvU291cmNlL1dlYkNvcmUvY3Nz L1N0eWxlUmVzb2x2ZXIuY3BwCmluZGV4IDAwZWQyNTAuLjA2NzFmMDQgMTAwNjQ0Ci0tLSBhL1Nv dXJjZS9XZWJDb3JlL2Nzcy9TdHlsZVJlc29sdmVyLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9j c3MvU3R5bGVSZXNvbHZlci5jcHAKQEAgLTI2NCw2ICsyNjQsNyBAQCBTdHlsZVJlc29sdmVyOjpT dHlsZVJlc29sdmVyKERvY3VtZW50KiBkb2N1bWVudCwgYm9vbCBtYXRjaEF1dGhvckFuZFVzZXJT dHlsZXMpCiAgICAgOiBtX2hhc1VBQXBwZWFyYW5jZShmYWxzZSkKICAgICAsIG1fYmFja2dyb3Vu ZERhdGEoQmFja2dyb3VuZEZpbGxMYXllcikKICAgICAsIG1fbWF0Y2hlZFByb3BlcnRpZXNDYWNo ZUFkZGl0aW9uc1NpbmNlTGFzdFN3ZWVwKDApCisgICAgLCBtX21hdGNoZWRQcm9wZXJ0aWVzQ2Fj aGVTd2VlcFRpbWVyKHRoaXMsICZTdHlsZVJlc29sdmVyOjpzd2VlcE1hdGNoZWRQcm9wZXJ0aWVz Q2FjaGUpCiAgICAgLCBtX2NoZWNrZXIoZG9jdW1lbnQsICFkb2N1bWVudC0+aW5RdWlya3NNb2Rl KCkpCiAgICAgLCBtX3BhcmVudFN0eWxlKDApCiAgICAgLCBtX3Jvb3RFbGVtZW50U3R5bGUoMCkK QEAgLTQ5MCwxMSArNDkxLDExIEBAIFN0eWxlUmVzb2x2ZXI6On5TdHlsZVJlc29sdmVyKCkKICAg ICBtX2ZvbnRTZWxlY3Rvci0+Y2xlYXJEb2N1bWVudCgpOwogfQogCi12b2lkIFN0eWxlUmVzb2x2 ZXI6OnN3ZWVwTWF0Y2hlZFByb3BlcnRpZXNDYWNoZSgpCit2b2lkIFN0eWxlUmVzb2x2ZXI6OnN3 ZWVwTWF0Y2hlZFByb3BlcnRpZXNDYWNoZShUaW1lcjxTdHlsZVJlc29sdmVyPiopCiB7CiAgICAg Ly8gTG9vayBmb3IgY2FjaGUgZW50cmllcyBjb250YWluaW5nIGEgc3R5bGUgZGVjbGFyYXRpb24g d2l0aCBhIHNpbmdsZSByZWYgYW5kIHJlbW92ZSB0aGVtLgotICAgIC8vIFRoaXMgbWF5IGhhcHBl biB3aGVuIGFuIGVsZW1lbnQgYXR0cmlidXRlIG11dGF0aW9uIGNhdXNlcyBpdCB0byBzd2FwIG91 dCBpdHMgQXR0cmlidXRlOjpkZWNsKCkKLSAgICAvLyBmb3IgYW5vdGhlciBDU1NNYXBwZWRBdHRy aWJ1dGVEZWNsYXJhdGlvbiwgcG90ZW50aWFsbHkgbGVhdmluZyB0aGlzIGNhY2hlIHdpdGggdGhl IGxhc3QgcmVmLgorICAgIC8vIFRoaXMgbWF5IGhhcHBlbiB3aGVuIGFuIGVsZW1lbnQgYXR0cmli dXRlIG11dGF0aW9uIGNhdXNlcyBpdCB0byBnZW5lcmF0ZSBhIG5ldyBhdHRyaWJ1dGVTdHlsZSgp LAorICAgIC8vIHBvdGVudGlhbGx5IGxlYXZpbmcgdGhpcyBjYWNoZSB3aXRoIHRoZSBsYXN0IHJl ZiBvbiB0aGUgb2xkIG9uZS4KICAgICBWZWN0b3I8dW5zaWduZWQsIDE2PiB0b1JlbW92ZTsKICAg ICBNYXRjaGVkUHJvcGVydGllc0NhY2hlOjppdGVyYXRvciBpdCA9IG1fbWF0Y2hlZFByb3BlcnRp ZXNDYWNoZS5iZWdpbigpOwogICAgIE1hdGNoZWRQcm9wZXJ0aWVzQ2FjaGU6Oml0ZXJhdG9yIGVu ZCA9IG1fbWF0Y2hlZFByb3BlcnRpZXNDYWNoZS5lbmQoKTsKQEAgLTUwOSw2ICs1MTAsOCBAQCB2 b2lkIFN0eWxlUmVzb2x2ZXI6OnN3ZWVwTWF0Y2hlZFByb3BlcnRpZXNDYWNoZSgpCiAgICAgfQog ICAgIGZvciAoc2l6ZV90IGkgPSAwOyBpIDwgdG9SZW1vdmUuc2l6ZSgpOyArK2kpCiAgICAgICAg IG1fbWF0Y2hlZFByb3BlcnRpZXNDYWNoZS5yZW1vdmUodG9SZW1vdmVbaV0pOworCisgICAgbV9t YXRjaGVkUHJvcGVydGllc0NhY2hlQWRkaXRpb25zU2luY2VMYXN0U3dlZXAgPSAwOwogfQogCiBz dGF0aWMgU3R5bGVTaGVldENvbnRlbnRzKiBwYXJzZVVBU2hlZXQoY29uc3QgU3RyaW5nJiBzdHIp CkBAIC0yMzc3LDEwICsyMzgwLDEwIEBAIGNvbnN0IFN0eWxlUmVzb2x2ZXI6Ok1hdGNoZWRQcm9w ZXJ0aWVzQ2FjaGVJdGVtKiBTdHlsZVJlc29sdmVyOjpmaW5kRnJvbU1hdGNoZWRQCiAKIHZvaWQg U3R5bGVSZXNvbHZlcjo6YWRkVG9NYXRjaGVkUHJvcGVydGllc0NhY2hlKGNvbnN0IFJlbmRlclN0 eWxlKiBzdHlsZSwgY29uc3QgUmVuZGVyU3R5bGUqIHBhcmVudFN0eWxlLCB1bnNpZ25lZCBoYXNo LCBjb25zdCBNYXRjaFJlc3VsdCYgbWF0Y2hSZXN1bHQpCiB7Ci0gICAgc3RhdGljIHVuc2lnbmVk IG1hdGNoZWREZWNsYXJhdGlvbkNhY2hlQWRkaXRpb25zQmV0d2VlblN3ZWVwcyA9IDEwMDsKKyAg ICBzdGF0aWMgY29uc3QgdW5zaWduZWQgbWF0Y2hlZERlY2xhcmF0aW9uQ2FjaGVBZGRpdGlvbnNC ZXR3ZWVuU3dlZXBzID0gMTAwOwogICAgIGlmICgrK21fbWF0Y2hlZFByb3BlcnRpZXNDYWNoZUFk ZGl0aW9uc1NpbmNlTGFzdFN3ZWVwID49IG1hdGNoZWREZWNsYXJhdGlvbkNhY2hlQWRkaXRpb25z QmV0d2VlblN3ZWVwcykgewotICAgICAgICBzd2VlcE1hdGNoZWRQcm9wZXJ0aWVzQ2FjaGUoKTsK LSAgICAgICAgbV9tYXRjaGVkUHJvcGVydGllc0NhY2hlQWRkaXRpb25zU2luY2VMYXN0U3dlZXAg PSAwOworICAgICAgICBzdGF0aWMgY29uc3QgdW5zaWduZWQgbWF0Y2hlZERlY2xhcmF0aW9uQ2Fj aGVTd2VlcFRpbWVJblNlY29uZHMgPSA2MDsKKyAgICAgICAgbV9tYXRjaGVkUHJvcGVydGllc0Nh Y2hlU3dlZXBUaW1lci5zdGFydE9uZVNob3QobWF0Y2hlZERlY2xhcmF0aW9uQ2FjaGVTd2VlcFRp bWVJblNlY29uZHMpOwogICAgIH0KIAogICAgIEFTU0VSVChoYXNoKTsKZGlmZiAtLWdpdCBhL1Nv dXJjZS9XZWJDb3JlL2Nzcy9TdHlsZVJlc29sdmVyLmggYi9Tb3VyY2UvV2ViQ29yZS9jc3MvU3R5 bGVSZXNvbHZlci5oCmluZGV4IDEyMDk4YWEuLjhjOTU4NzggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9X ZWJDb3JlL2Nzcy9TdHlsZVJlc29sdmVyLmgKKysrIGIvU291cmNlL1dlYkNvcmUvY3NzL1N0eWxl UmVzb2x2ZXIuaApAQCAtNDI5LDEzICs0MjksMTUgQEAgcHJpdmF0ZToKIAogICAgIC8vIEV2ZXJ5 IE4gYWRkaXRpb25zIHRvIHRoZSBtYXRjaGVkIGRlY2xhcmF0aW9uIGNhY2hlIHRyaWdnZXIgYSBz d2VlcCB3aGVyZSBlbnRyaWVzIGhvbGRpbmcKICAgICAvLyB0aGUgbGFzdCByZWZlcmVuY2UgdG8g YSBzdHlsZSBkZWNsYXJhdGlvbiBhcmUgZ2FyYmFnZSBjb2xsZWN0ZWQuCi0gICAgdm9pZCBzd2Vl cE1hdGNoZWRQcm9wZXJ0aWVzQ2FjaGUoKTsKKyAgICB2b2lkIHN3ZWVwTWF0Y2hlZFByb3BlcnRp ZXNDYWNoZShUaW1lcjxTdHlsZVJlc29sdmVyPiopOwogCiAgICAgdW5zaWduZWQgbV9tYXRjaGVk UHJvcGVydGllc0NhY2hlQWRkaXRpb25zU2luY2VMYXN0U3dlZXA7CiAKICAgICB0eXBlZGVmIEhh c2hNYXA8dW5zaWduZWQsIE1hdGNoZWRQcm9wZXJ0aWVzQ2FjaGVJdGVtPiBNYXRjaGVkUHJvcGVy dGllc0NhY2hlOwogICAgIE1hdGNoZWRQcm9wZXJ0aWVzQ2FjaGUgbV9tYXRjaGVkUHJvcGVydGll c0NhY2hlOwogCisgICAgVGltZXI8U3R5bGVSZXNvbHZlcj4gbV9tYXRjaGVkUHJvcGVydGllc0Nh Y2hlU3dlZXBUaW1lcjsKKwogICAgIC8vIEEgYnVmZmVyIHVzZWQgdG8gaG9sZCB0aGUgc2V0IG9m IG1hdGNoZWQgcnVsZXMgZm9yIGFuIGVsZW1lbnQsIGFuZCBhIHRlbXBvcmFyeSBidWZmZXIgdXNl ZCBmb3IKICAgICAvLyBtZXJnZSBzb3J0aW5nLgogICAgIFZlY3Rvcjxjb25zdCBSdWxlRGF0YSos IDMyPiBtX21hdGNoZWRSdWxlczsK