Bug 102822 - 2.5% regression on page cycler moz
Summary: 2.5% regression on page cycler moz
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antti Koivisto
URL:
Keywords: PerfRegression
: 102696 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-11-20 10:16 PST by Ojan Vafai
Modified: 2014-05-17 21:14 PDT (History)
14 users (show)

See Also:


Attachments
test case (234 bytes, text/html)
2012-11-26 18:55 PST, Ojan Vafai
no flags Details
patch (1.92 KB, patch)
2012-12-01 08:26 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch2 (3.34 KB, patch)
2012-12-05 16:37 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
simpler safer patch (5.08 KB, patch)
2013-08-07 04:40 PDT, Antti Koivisto
kling: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion (566.58 KB, application/zip)
2013-08-07 05:12 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ojan Vafai 2012-11-20 10:16:25 PST
It's a bit hard to see the regression, but it's definitely there. The way to see it is to compare the difference between the reference trace (t_ref), which does not have the new patches, to the non-reference traces, which do have the new patches. 

http://build.chromium.org/f/chromium/perf/linux-release-webkit-latest/moz-http/report.html?rev=168773&graph=times&history=50
http://build.chromium.org/f/chromium/perf/linux-release-webkit-latest/moz/report.html?rev=168773&graph=times&history=50

Regression range: http://trac.webkit.org/log/?verbose=on&rev=135087&stop_rev=135070

Hard to be sure which of those patches is most likely. I would guess one of the antti/kling patches. But it could also be smfr. Or maybe one of haraken's V8 patches, although those were supposed to just be cleanups with no other effects.
Comment 2 Ojan Vafai 2012-11-26 10:59:37 PST
Looks like the www.nytimes.com_Table load time went from 77ms to 87ms and that accounts for the entirety of the regression.
Comment 3 Ojan Vafai 2012-11-26 12:02:35 PST
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
Comment 5 Ojan Vafai 2012-11-26 14:19:10 PST
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.
Comment 6 Ojan Vafai 2012-11-26 14:48:46 PST
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.
Comment 7 Ojan Vafai 2012-11-26 16:26:57 PST
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
Comment 8 Simon Fraser (smfr) 2012-11-26 16:32:18 PST
Looks like http://trac.webkit.org/changeset/135082 then.
Comment 9 Ojan Vafai 2012-11-26 17:17:53 PST
*** Bug 102696 has been marked as a duplicate of this bug. ***
Comment 10 Ojan Vafai 2012-11-26 18:55:25 PST
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.
Comment 11 Ojan Vafai 2012-11-26 18:58:09 PST
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.
Comment 12 Andreas Kling 2012-11-29 21:28:18 PST
Passing to Antti for comment, yo.
Comment 13 Antti Koivisto 2012-11-30 09:57:02 PST
I think there is an unnecessary style invalidation from that patch...
Comment 14 Antti Koivisto 2012-12-01 08:26:50 PST
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 15 Andreas Kling 2012-12-01 09:55:44 PST
Comment on attachment 177106 [details]
patch

Makes sense, r=me
Comment 16 Build Bot 2012-12-01 16:45:51 PST
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 17 WebKit Review Bot 2012-12-03 08:00:18 PST
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
Comment 18 Antti Koivisto 2012-12-05 16:37:06 PST
Created attachment 177860 [details]
patch2
Comment 19 Build Bot 2012-12-05 18:14:34 PST
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 20 WebKit Review Bot 2012-12-05 19:48:17 PST
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 21 Eric Seidel (no email) 2013-01-04 00:41:42 PST
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.
Comment 22 Antti Koivisto 2013-08-07 04:40:44 PDT
Created attachment 208251 [details]
simpler safer patch
Comment 23 Andreas Kling 2013-08-07 04:43:24 PDT
Comment on attachment 208251 [details]
simpler safer patch

Seems like a genuinely ok thing to do.
Comment 24 Build Bot 2013-08-07 05:12:06 PDT
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
Comment 25 Build Bot 2013-08-07 05:12:10 PDT
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
Comment 26 Antti Koivisto 2013-08-07 08:08:00 PDT
https://trac.webkit.org/r153788
 
with rebase of inspector/timeline/timeline-script-tag-1.html. It is a progression.
Comment 27 Csaba Osztrogonác 2014-02-13 04:09:35 PST
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.