| Summary: | 2.5% regression on page cycler moz | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Ojan Vafai <ojan> | ||||||||||||
| Component: | Page Loading | Assignee: | Antti Koivisto <koivisto> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | abarth, buildbot, cmp, commit-queue, dglazkov, esprehn+autocc, haraken, kangil.han, kling, koivisto, rniwa, simon.fraser, slewis, webkit.review.bot | ||||||||||||
| Priority: | P2 | Keywords: | PerfRegression | ||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Ojan Vafai
2012-11-20 10:16:25 PST
FWIW, I considered the possibility that r135082 was just undoing performance gains from r129644, but these graphs didn't show any performance gains at r129644. http://build.chromium.org/f/chromium/perf/linux-release-webkit-latest/moz/report.html?rev=158973&graph=times&history=50 http://build.chromium.org/f/chromium/perf/linux-release-webkit-latest/moz/report.html?rev=158973&graph=times&history=50 Looks like the www.nytimes.com_Table load time went from 77ms to 87ms and that accounts for the entirety of the regression. Further evidence that it was a WebKit side patch is that the regression happened on another bot at a non-overlapping chromium revision, but an overlapping webkit revision: http://build.chromium.org/f/chromium/perf/linux-release/moz/report.html?rev=168773&graph=times&history=50 Actually, this seems to show up on some other bots as well: http://build.chromium.org/f/chromium/perf/chromium-rel-win7-webkit/moz/report.html?rev=168773&graph=times&history=50 http://build.chromium.org/f/chromium/perf/mac-release-10.6-webkit-latest/moz/report.html?rev=168773&graph=times&history=50 And this point at the regression being in the following range: http://trac.webkit.org/log/?verbose=on&rev=135077&stop_rev=135068 Sigh. I'm not sure I trust any of these regression ranges except the large one from the chromium main waterfall (http://trac.webkit.org/log/?verbose=on&rev=135094&stop_rev=135047). Something clearly went wrong on the other bots here where the data from some of the runs is repeated. I suppose we can safely look at the webkit revision before the repetition and after and use that as the range. I'll try piecing this together. Bug with the Chromium bots for anyone that wants to follow along: http://crbug.com/162722. In the meantime, I think I can get a better (accurate) regression range for this bug. OK, I looked at manually at the bot output for the linux release bot and we get more reasonable data out of it. At r135081 there was no regression and at r135087 there was. Regression range: http://trac.webkit.org/log/?rev=135087&stop_rev=135081&verbose=on Here's my raw output out of R of the median times for each run. test times135066 times135067 times135068 times135069 times135077 times135081 times135087 times135094 www.nytimes.com_Table 78.4 79.4 79.8 77.5 80.2 78.3 87.6 87.2 Looks like http://trac.webkit.org/changeset/135082 then. *** Bug 102696 has been marked as a duplicate of this bug. *** Created attachment 176147 [details] test case Took forever, but I think this testcase accurately represents the regression. There's a ~10% regression on this test between r134994 and r135540. Those were just the revisions it was easy for me to test. I still think the regression range is the one listed in comment 7. Since we only measure the time of a document.write of a bunch of tables, I think that rules out the V8 changes as well. So, I think that does in fact only leave http://trac.webkit.org/changeset/135082 as a possible culprit. Passing to Antti for comment, yo. I think there is an unnecessary style invalidation from that patch... Created attachment 177106 [details]
patch
Based on the traces this should fix the page cycler test.
The attached reduction does show something that has regressed but I think the case is not a significant component of the page cycler number. Trace of that points to a layout related regression. Might be worth separate bug.
Comment on attachment 177106 [details]
patch
Makes sense, r=me
Comment on attachment 177106 [details] patch Attachment 177106 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15055984 New failing tests: fast/events/xsl-onload.xhtml fast/parser/external-entities-in-xslt.xml fast/xsl/transform-to-html.xml fast/xsl/xslt-doc-noenc.xml fast/xsl/sort-locale.xml fast/xsl/utf8-chunks.xml fast/xsl/xslt-nested-stylesheets.xml fast/xsl/exslt-node-set.xml fast/xsl/xslt-doc-enc.xml fast/xsl/xslt-enc16to16.xml fast/parser/xslt-with-html.xml http/tests/misc/location-test-xsl-style-sheet.xml fast/events/overflow-scroll-fake-mouse-move.html fast/xsl/import-after-comment.xml fast/xsl/xslt-entity-enc.xml fast/xsl/xslt-enc16.xml fast/xsl/sort-unicode.xml fast/xsl/dtd-in-source-document.xml fast/xsl/subframe-location.html fast/xsl/xslt-missing-namespace-in-xslt.xml fast/xsl/xslt-extra-content-at-end.xml fast/xsl/xslt-enc-cyr.xml fast/xsl/mozilla-tests.xml fast/loader/javascript-url-in-object.html fast/xsl/xslt-import-depth.xml fast/xsl/xslt-enc.xml fast/xsl/xslt-entity.xml fast/xsl/document-function.xml Comment on attachment 177106 [details] patch Attachment 177106 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15121268 New failing tests: fast/events/xsl-onload.xhtml fast/forms/range/slider-delete-while-dragging-thumb.html http/tests/security/contentSecurityPolicy/xsl-allowed.php fast/parser/external-entities-in-xslt.xml http/tests/security/xss-DENIED-xsl-external-entity.xml fast/xsl/sort-locale.xml fast/xsl/exslt-node-set.xml fast/xsl/mozilla-tests.xml http/tests/security/contentSecurityPolicy/xsl-unaffected-by-style-src-2.php http/tests/security/xss-DENIED-xsl-document-securityOrigin.xml fast/parser/xslt-with-html.xml http/tests/misc/location-test-xsl-style-sheet.xml fast/loader/text-document-wrapping.html fast/text/international/hindi-spacing.html http/tests/security/cookies/first-party-cookie-allow-xslt.xml http/tests/security/local-user-CSS-from-remote.html http/tests/security/contentSecurityPolicy/xsl-img-blocked.php fast/xsl/import-after-comment.xml fast/forms/range/slider-mouse-events.html fast/xsl/subframe-location.html fast/xsl/sort-unicode.xml fast/xsl/dtd-in-source-document.xml http/tests/xmlviewer/dumpAsText/xsl-stylesheet.xml http/tests/security/xss-DENIED-xsl-external-entity-redirect.xml http/tests/security/cookies/third-party-cookie-blocking-xslt.xml fast/xsl/document-function.xml fast/loader/javascript-url-in-object.html http/tests/security/xss-DENIED-xsl-document-redirect.xml http/tests/security/xss-DENIED-xsl-document.xml fast/forms/range/slider-onchange-event.html Created attachment 177860 [details]
patch2
Comment on attachment 177860 [details] patch2 Attachment 177860 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15144729 New failing tests: inspector/timeline/timeline-script-tag-1.html Comment on attachment 177860 [details] patch2 Attachment 177860 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15151782 New failing tests: inspector/timeline/timeline-script-tag-1.html userscripts/user-style-all-frames.html Comment on attachment 177106 [details] patch Cleared Andreas Kling's review+ from obsolete attachment 177106 [details] so that this bug does not appear in http://webkit.org/pending-commit. Created attachment 208251 [details]
simpler safer patch
Comment on attachment 208251 [details]
simpler safer patch
Seems like a genuinely ok thing to do.
Comment on attachment 208251 [details] simpler safer patch Attachment 208251 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1399053 New failing tests: inspector/timeline/timeline-script-tag-1.html Created attachment 208253 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04 Port: mac-mountainlion Platform: Mac OS X 10.8.4
https://trac.webkit.org/r153788 with rebase of inspector/timeline/timeline-script-tag-1.html. It is a progression. Comment on attachment 177860 [details] patch2 Cleared Ojan Vafai's review+ from obsolete attachment 177860 [details] so that this bug does not appear in http://webkit.org/pending-commit. |