Issue metadata
Sign in to add a comment
|
10.3%-13.9% regression in loading.desktop at 501032:501155 |
||||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Sep 13 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8968559399560982672
,
Sep 13 2017
=== Auto-CCing suspected CL author alexmos@chromium.org === Hi alexmos@chromium.org, the bisect results pointed to your CL, please take a look at the results. === BISECT JOB RESULTS === Perf regression found with culprit Suspected Commit Author : Alex Moshchuk Commit : 27caae83cb530daaf49f9a38793e427cdf493a65 Date : Mon Sep 11 23:11:18 2017 Subject: Always create a proxy for cross-process navigations. Bisect Details Configuration: mac_10_11_perf_bisect Benchmark : loading.desktop Metric : cpuTimeToFirstMeaningfulPaint_avg/pcv1-cold/Economist Change : 18.65% | 243.251833333 -> 288.623833333 Revision Result N chromium@501031 243.252 +- 25.5213 6 good chromium@501062 245.763 +- 18.8382 6 good chromium@501078 249.077 +- 25.067 6 good chromium@501080 247.448 +- 18.8126 6 good chromium@501081 287.943 +- 12.1528 6 bad <-- chromium@501082 288.1 +- 11.8342 6 bad chromium@501086 286.867 +- 27.6791 6 bad chromium@501093 291.216 +- 14.8962 6 bad chromium@501155 288.624 +- 24.2394 6 bad To Run This Test src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=Economist loading.desktop More information on addressing performance regressions: http://g.co/ChromePerformanceRegressions Debug information about this bisect: https://chromeperf.appspot.com/buildbucket_job_status/8968559399560982672 For feedback, file a bug with component Speed>Bisection
,
Sep 13 2017
,
Oct 20 2017
alexmos: any update on this bug?
,
Oct 21 2017
I've looked into it. First of all, it is very surprising that my CL would add 40ms to time-to-first-meaningful-paint for the Economist. I'd expect it to have a little bit of overhead, since we'll now create an extra object (RenderFrameProxy) before creating the real RenderFrame, and then we'll also spend a little more time swapping it in at commit time. But this shouldn't cause 40ms of delay. After commit, the code should run identically before and after my change. There were three metrics reported here, just for the Economist page: 1. cpuTimeToFirstMeaningfulPaint_avg/pcv1-cold/Economist 2. cpuTimeToFirstMeaningfulPaint_avg / pcv1-warm / MLB 3. cpuTimeToFirstMeaningfulPaint_avg / pcv1-cold / Economist 1 and 2 didn't increase by much and have since gotten back to lower than they were in the range 502244 - 502328, perhaps due to PlzNavigate being turned on by default (r502251). 3 is the 40ms increase. I'm tried to repro it locally (macbook pro, though with 10.12 instead of 10.11) without finding much. On the browser side, the time from NavigateToEntry (navigation start) to RFHI::OnDidCommitProvisionalLoad (navigation commit) was 199ms without my change and 197ms with it. On the renderer side, the time from NavigateToEntry to receiving the RFI::OnNavigate IPC (which would include proxy creation time, as the proxy would be created before the provisional RenderFrame) was 154ms without my change and 147ms with it. Similarly, the time from RFHM::Navigate to RFI::DidCommitProvisionalLoad was 204ms without my change vs 195ms with it. These numbers are all pretty similar (averaged over a couple of runs, which are also pretty consistent). The one difference I found is that RFI::DidCommitProvisionalLoad itself takes about 7ms of time with my change, as it now has to swap the proxy in, vs. 0.6 ms without my change -- this is the expected overhead, but it's fairly small. In general, this was an architectural change necessary for correctness of cross-process navigations, so a small overhead should be worth it for preventing a fairly large number of crashes (see issue 756790) and race conditions. But moreover, I'm skeptical there's actually a meaningful regression here, given the analysis above. None of the other sites reported an increase, and no other OSes did either. sullivan@: any advice on how to proceed from here?
,
Oct 22 2017
+kouhei and ksakamoto, owners of the loading.desktop benchmark. Any thoughts on how deeply this should be investigated, or what next steps would be, given that this seems to be restricted to one page on one hardware configuration? One thing you could do is look at traces before/after the regression to see if any big surprises jump out. On the perf dashboard, you can click on a data point and click "View trace for this run" to see a trace. Before your CL: https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/trace-file-id_83-2017-09-11_16-13-12-58282.html After your CL: https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/trace-file-id_83-2017-09-11_21-21-40-20724.html
,
Oct 23 2017
Given that this is just one page on single bot, and affected metric is only cpuTimeToFMP (timeToFMP has recovered), I think we shouldn't spend too much time on it. I'm fine with wontfix-ing this.
,
Jan 8 2018
WontFix-ing per # 8. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Sep 13 2017