New issue
Advanced search Search tips

Issue 764714 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

10.3%-13.9% regression in loading.desktop at 501032:501155

Project Member Reported by mustaq@chromium.org, Sep 13 2017

Issue description

See the link to graphs below.
 
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Sep 13 2017

All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=764714

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=1ad63a93231132fa96b3461a32ed4da802535663981520facb02e37cf5f162f1


Bot(s) for this bug's original alert(s):

chromium-rel-mac11
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Sep 13 2017

Cc: alex...@chromium.org
Owner: alex...@chromium.org
Status: Assigned (was: Untriaged)

=== 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
Cc: creis@chromium.org
Components: Internals>Sandbox>SiteIsolation
alexmos: any update on this bug?
Cc: sullivan@chromium.org
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?
Cc: ksakamoto@chromium.org kouhei@chromium.org
+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
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.

Status: WontFix (was: Assigned)
WontFix-ing per # 8.

Sign in to add a comment