112125 2013-03-12 03:10:53 -0700 REGRESSION(r145305) Performance: 1.3% mac-release-10.6-webkit-latest/intl2/times/t change after rev 145300 2013-03-15 00:34:23 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P1 Normal --- 1 nbarth robert eric esprehn+autocc jchaffraix ojan.autocc robert webkit.review.bot oldest_to_newest 853268 0 nbarth 2013-03-12 03:10:53 -0700 There looks like there's a performance regression caused by r145305 fixing Bug 108357 REGRESSION(r140907): Incorrect baseline for cells with media content during load Concretely: 1.3% change in mac-release-10.6-webkit-latest/intl2/times/t after revision 187133 (WebKit revision 145300) Graph: http://chromium-perf.appspot.com/?details=true&tab=mac-release-10.6-webkit-latest&testSuite=intl2&rev=187433&graph=times&trace=t&history=150&master=ChromiumWebkit (Pretty clear regression, narrow range, this revision changes layout, hence likely cause.) I understand that this revision fixes a layout bug and is thus necessary in some form, but it looks like it has performance impact, perhaps due to additional computation and layout being necessary in some cases. Could you take a look at it and see that any regressions are necessary and expected? (Perhaps there's another way of doing it with less impact.) 853271 1 nbarth 2013-03-12 03:12:39 -0700 Link added below: (In reply to comment #0) > There looks like there's a performance regression caused by r145305 fixing > Bug 108357 REGRESSION(r140907): Incorrect baseline for cells with media content during load r145305: http://trac.webkit.org/changeset/145305 Bug 108357 REGRESSION(r140907): Incorrect baseline for cells with media content during load 853514 2 robert 2013-03-12 08:56:19 -0700 How do I access the source of the test that observed the regression? 853567 3 robert 2013-03-12 09:52:18 -0700 When I access the results page I can't seem to get anything other than a bunch of tabs. I don't get a graph or any other information. I'm using Chrome on Linux. Any suggestions on what I need to do to view the regression? 853985 4 nbarth 2013-03-12 18:52:28 -0700 Added Julien to CC list, in response to discussion at Chromium Issue 173154: https://code.google.com/p/chromium/issues/detail?id=173154 854017 5 nbarth 2013-03-12 19:41:50 -0700 Opened bug at Chromium: Issue 189102: Performance regression 1.3% in table layout baseline fix http://crbug.com/189102 854074 6 nbarth 2013-03-12 23:08:24 -0700 (In reply to comment #2) > How do I access the source of the test that observed the regression? Command line looks to be: /b/build/slave/Mac10_6_Perf/build/src/out/Release/performance_ui_tests --gtest_filter=PageCycler*.Intl2File:PageCycler*.*_Intl2File So (on Mac 10.6) should be able to reproduce by running: out/Release/performance_ui_tests --gtest_filter=PageCycler*.Intl2File:PageCycler*.*_Intl2File 854075 7 nbarth 2013-03-12 23:10:54 -0700 (In reply to comment #3) > When I access the results page I can't seem to get anything other than a bunch of tabs. I don't get a graph or any other information. I'm using Chrome on Linux. > > Any suggestions on what I need to do to view the regression? The graph takes a little while to load, and there's no feedback that it's loading. I'm using Chrome on Linux as well, and this takes about 7 seconds to load: http://chromium-perf.appspot.com/?tab=mac-release-10.6-webkit-latest&graph=times&trace=t&rev=187433&history=150&master=ChromiumWebkit&testSuite=page_cycler_intl2&details=true Could you try viewing again? The regression is about 80% of the way across; if you mouseover you'll see the Chromium and WebKit revision numbers. 854076 8 nbarth 2013-03-12 23:12:49 -0700 (In reply to comment #6) > (In reply to comment #2) > > How do I access the source of the test that observed the regression? > > Command line looks to be: > /b/build/slave/Mac10_6_Perf/build/src/out/Release/performance_ui_tests --gtest_filter=PageCycler*.Intl2File:PageCycler*.*_Intl2File BTW, this is determined by: * clicking through to build bot waterfall (from graph page), * search for test suite (on waterfall page) * click on stdio * eyeballing the stdio for a likely command line 854115 9 192880 robert 2013-03-13 01:25:42 -0700 Created attachment 192880 Patch 854126 10 robert 2013-03-13 01:46:07 -0700 I'm pretty sure this will solve the regression and feel a bit dumb for letting it through in the first place. My attempt to detect cases where the delayed load of an image would push the baseline of the row down incorrectly unless it is allowed to move up into the headroom above did not consider the case where we're laying out the row for the first time so the row doesn't have a baseline yet. In that situation it was laying out the cell a second time unnecessarily. The page cycler test suite is a set of popular websites as far as I can tell, so there's no specific use-case to test. If Julien is OK with the patch I guess we just watch the Performance bots after it lands. 855271 11 192880 webkit.review.bot 2013-03-14 10:04:01 -0700 Comment on attachment 192880 Patch Clearing flags on attachment: 192880 Committed r145822: <http://trac.webkit.org/changeset/145822> 855273 12 webkit.review.bot 2013-03-14 10:04:07 -0700 All reviewed patches have been landed. Closing bug. 855775 13 robert 2013-03-15 00:34:23 -0700 Fingers crossed, that appears to have done the trick: http://chromium-perf.appspot.com/?tab=mac-release-10.6-webkit-latest&graph=times&trace=t&rev=188265&history=150&master=ChromiumWebkit&testSuite=page_cycler_intl2&details=true 192880 2013-03-13 01:25:42 -0700 2013-03-14 10:04:00 -0700 Patch bug-112125-20130313082113.patch text/plain 2196 robert U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ1MzA1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggODk1OTA3YmU2ZDEyNzMw ZTMzOGZlN2E5MGQ4MDI1ZDU4MDY3OTY1Ny4uYmFlOWNlNDZjOGQxZmZhMTJjOGU0ZWQ1MzYwNWQ0 NDkyM2VlOTNlMCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDEzLTAzLTEzICBSb2Jl cnQgSG9nYW4gIDxyb2JlcnRAd2Via2l0Lm9yZz4KKworICAgICAgICBSRUdSRVNTSU9OKHIxNDUz MDUpIFBlcmZvcm1hbmNlOiAxLjMlIG1hYy1yZWxlYXNlLTEwLjYtd2Via2l0LWxhdGVzdC9pbnRs Mi90aW1lcy90IGNoYW5nZSBhZnRlciByZXYgMTQ1MzAwCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMTIxMjUKKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICBXaGVuIGRldGVjdGluZyBjYXNlcyB3aGVyZSBhIGxv YWRlZCBpbWFnZSBtYXkgbmVlZCB0byBtb3ZlIHVwIGludG8gdGhlIHBhZGRpbmcgY3JlYXRlZCBi eSB0aGUgcm93J3MgYmFzZWxpbmUKKyAgICAgICAgd2UgZG9uJ3QgbmVlZCB0byBkbyBhbnl0aGlu ZyBpZiB0aGUgcm93IGRvZXNuJ3QgaGF2ZSBhIGJhc2VsaW5lIHlldC4KKworICAgICAgICAqIHJl bmRlcmluZy9SZW5kZXJUYWJsZUNlbGwuY3BwOgorICAgICAgICAoV2ViQ29yZTo6UmVuZGVyVGFi bGVDZWxsOjpsYXlvdXQpOgorCiAyMDEzLTAzLTA3ICBSb2JlcnQgSG9nYW4gIDxyb2JlcnRAd2Vi a2l0Lm9yZz4KIAogICAgICAgICBSRUdSRVNTSU9OKHIxNDA5MDcpOiBJbmNvcnJlY3QgYmFzZWxp bmUgZm9yIGNlbGxzIHdpdGggbWVkaWEgY29udGVudCBkdXJpbmcgbG9hZApkaWZmIC0tZ2l0IGEv U291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclRhYmxlQ2VsbC5jcHAgYi9Tb3VyY2UvV2Vi Q29yZS9yZW5kZXJpbmcvUmVuZGVyVGFibGVDZWxsLmNwcAppbmRleCBlYzdmMGIxMzIyODFlMWUy ZGYxZTNhNmY3MTg5OTU5Mzk4YWI4NWM1Li40YzAyYzMxMTIyY2EyMGIzMjBmYmY1MjI1YzM0NTBh ODBkOWExY2RiIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyVGFi bGVDZWxsLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyVGFibGVDZWxs LmNwcApAQCAtMjUxLDcgKzI1MSw3IEBAIHZvaWQgUmVuZGVyVGFibGVDZWxsOjpsYXlvdXQoKQog ICAgIC8vIGZvciBsYXlvdXQgKHRoZSBwYWRkaW5nIHJlcXVpcmVkIHRvIHB1c2ggdGhlIGNvbnRl bnRzIG9mIHRoZSBjZWxsIGRvd24gdG8gdGhlIHJvdydzIGJhc2VsaW5lKSBpcyBpbmNsdWRlZCBp biBvdXIgbmV3IGhlaWdodCBhbmQgYmFzZWxpbmUgYW5kIG1ha2VzIGJvdGgKICAgICAvLyBvZiB0 aGVtIHdyb25nLiBTbyBpZiBvdXIgY29udGVudCdzIGludHJpbnNpYyBoZWlnaHQgaGFzIGNoYW5n ZWQgcHVzaCB0aGUgbmV3IGNvbnRlbnQgdXAgaW50byB0aGUgaW50cmluc2ljIHBhZGRpbmcgYW5k IHJlbGF5b3V0IHNvIHRoYXQgdGhlIHJlc3Qgb2YKICAgICAvLyB0YWJsZSBhbmQgcm93IGxheW91 dCBjYW4gdXNlIHRoZSBjb3JyZWN0IGJhc2VsaW5lIGFuZCBoZWlnaHQgZm9yIHRoaXMgY2VsbC4K LSAgICBpZiAoaXNCYXNlbGluZUFsaWduZWQoKSAmJiBjZWxsQmFzZWxpbmVQb3NpdGlvbigpID4g c2VjdGlvbigpLT5yb3dCYXNlbGluZShyb3dJbmRleCgpKSkgeworICAgIGlmIChpc0Jhc2VsaW5l QWxpZ25lZCgpICYmIHNlY3Rpb24oKS0+cm93QmFzZWxpbmUocm93SW5kZXgoKSkgJiYgY2VsbEJh c2VsaW5lUG9zaXRpb24oKSA+IHNlY3Rpb24oKS0+cm93QmFzZWxpbmUocm93SW5kZXgoKSkpIHsK ICAgICAgICAgaW50IG5ld0ludHJpbnNpY1BhZGRpbmdCZWZvcmUgPSBtYXg8TGF5b3V0VW5pdD4o MCwgaW50cmluc2ljUGFkZGluZ0JlZm9yZSgpIC0gbWF4PExheW91dFVuaXQ+KDAsIGNlbGxCYXNl bGluZVBvc2l0aW9uKCkgLSBvbGRDZWxsQmFzZWxpbmUpKTsKICAgICAgICAgc2V0SW50cmluc2lj UGFkZGluZ0JlZm9yZShuZXdJbnRyaW5zaWNQYWRkaW5nQmVmb3JlKTsKICAgICAgICAgc2V0TmVl ZHNMYXlvdXQodHJ1ZSwgTWFya09ubHlUaGlzKTsK