New issue
Advanced search Search tips

Issue 642107 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

7.5%-588.3% regression in startup.cold.blank_page at 414578:414616

Project Member Reported by mustaq@chromium.org, Aug 29 2016

Issue description

See the link to graphs below.
 
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Aug 29 2016

Cc: reillyg@chromium.org
Owner: reillyg@chromium.org

=== Auto-CCing suspected CL author reillyg@chromium.org ===

Hi reillyg@chromium.org, the bisect results pointed to your CL below as possibly
causing a regression. Please have a look at this info and see whether
your CL be related.


===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : Enable the WebUSB Origin Trial.
Author  : reillyg
Commit description:
  
This patch enables the WebUSB interfaces if a site has an Origin Trial
key for the feature "WebUSB". They can still also be enabled by passing
the --enable-experimental-web-platform-features flag.

BUG= 492204 

Review-Url: https://codereview.chromium.org/2274303002
Cr-Commit-Position: refs/heads/master@{#414591}
Commit  : 2fe96ea775a73ad5dcc05105f57be096d302baa2
Date    : Fri Aug 26 00:30:29 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N  Good?
chromium@414580  541.032  0.439718  5  good
chromium@414589  550.642  1.84188   5  good
chromium@414590  548.958  1.88021   5  good
chromium@414591  3733.28  4.22081   5  bad    <--
chromium@414593  3733.72  1.96034   5  bad
chromium@414597  3728.23  4.01324   5  bad
chromium@414614  3728.82  3.74879   5  bad

Bisect job ran on: mac_10_11_perf_bisect
Bug ID: 642107

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests startup.warm.blank_page
Test Metric: first_main_frame_load_time/first_main_frame_load_time
Relative Change: 589.21%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_10_11_perf_bisect/builds/857
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9002963369794211200


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5883508600340480

| O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq
|  X  | for more information addressing perf regression bugs. For feedback,
| / \ | file a bug with component Tests>AutoBisect.  Thank you!
Cc: haraken@chromium.org cha...@chromium.org
This patch does add additional V8 bindings but I am surprised that there was such a large effect given that previous patches have added other Origin Trial checks. If this is indeed the cause then this will affect M-54 as well.
Cc: iclell...@chromium.org
iclelland@: Do you think this is related to the overhead of navigator wrapper instantiation?

Comment 7 Deleted

Comment 8 Deleted

Comment 9 by mustaq@chromium.org, Aug 30 2016

Cc: -perezju@chromium.org
Ignore my comments #7 and #8 above, seems I mixed up the bugs.
iclelland is OOO this week. I'll try to help out instead.

To comment #4, I would not expect the additional V8 bindings to have such a large effect. The bindings would only take effect if WebUSB is enabled (either via runtime flag or valid origin trial token).

Looking at the CL, the other change is that WebUSB is now enabled by default at browser start:
https://codereview.chromium.org/2274303002/diff/20001/chrome/browser/chrome_browser_main.cc

Before this CL, the USB initialization was behind the experimental features flag. I'm not familiar with the config of the perf benchmarks. If experimental runtime features are not enabled for the perf benchmarks, this means that the WebUSB initialization is now happening on startup, where it would not have before. That seems like a potential cause for the regression.

reillyg@, do you have measurements on the impact of the WebUSB initialization? Is it possible that is causing the regression?

I'm not ruling out the V8 bindings as a cause. Ian is working on  bug 637148 , but appears to affect mostly iframes, and did not seem to result in a perf regression like this issue. So, the bindings seem less likely as the cause here.

Status: Started (was: Assigned)
I have a patch out to delay initializing of the WebUSB device detector and add an UMA histogram to measure the distribution of startup times:

https://codereview.chromium.org/2295023002
Project Member

Comment 12 by bugdroid1@chromium.org, Sep 1 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/478a62181383d2cc7fa545ee6a7a91ab34fec477

commit 478a62181383d2cc7fa545ee6a7a91ab34fec477
Author: reillyg <reillyg@chromium.org>
Date: Thu Sep 01 16:43:00 2016

Delay WebUSB initialization until after browser startup.

Initializing the WebUSB device detector module causes the USB service to
be initialized and enumerate all USB device connected to the system.
This can cause unnecessary delay during browser startup. The purpose of
this module is to detect devices connected after Chrome starts to
display a notification so it doesn't matter if it is initialized after
first page load.

An UMA histogram for the time spent initializing the detector is also
added to aid further investigation of this issue.

BUG= 642107 

Review-Url: https://codereview.chromium.org/2295023002
Cr-Commit-Position: refs/heads/master@{#415971}

[modify] https://crrev.com/478a62181383d2cc7fa545ee6a7a91ab34fec477/chrome/browser/chrome_browser_main.cc
[modify] https://crrev.com/478a62181383d2cc7fa545ee6a7a91ab34fec477/chrome/browser/usb/web_usb_detector.cc
[modify] https://crrev.com/478a62181383d2cc7fa545ee6a7a91ab34fec477/chrome/browser/usb/web_usb_detector.h
[modify] https://crrev.com/478a62181383d2cc7fa545ee6a7a91ab34fec477/chrome/browser/usb/web_usb_detector_unittest.cc
[modify] https://crrev.com/478a62181383d2cc7fa545ee6a7a91ab34fec477/tools/metrics/histograms/histograms.xml

Labels: -M-55 M-54 Merge-Request-54
This feature is enabled in M-54 as well so this change needs to be merged once we validate the regression has been fixed.
Status: Fixed (was: Started)
It appears that these metrics have returned to normal.

Comment 15 by dimu@chromium.org, Sep 2 2016

Labels: -Merge-Request-54 Merge-Approved-54 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M54 (branch: 2840)
Project Member

Comment 16 by bugdroid1@chromium.org, Sep 2 2016

Labels: -merge-approved-54 merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/165965c08079b64a645819ffddc221915891409f

commit 165965c08079b64a645819ffddc221915891409f
Author: Reilly Grant <reillyg@chromium.org>
Date: Fri Sep 02 18:25:38 2016

Delay WebUSB initialization until after browser startup.

Initializing the WebUSB device detector module causes the USB service to
be initialized and enumerate all USB device connected to the system.
This can cause unnecessary delay during browser startup. The purpose of
this module is to detect devices connected after Chrome starts to
display a notification so it doesn't matter if it is initialized after
first page load.

An UMA histogram for the time spent initializing the detector is also
added to aid further investigation of this issue.

BUG= 642107 

Review-Url: https://codereview.chromium.org/2295023002
Cr-Commit-Position: refs/heads/master@{#415971}
(cherry picked from commit 478a62181383d2cc7fa545ee6a7a91ab34fec477)

Review URL: https://codereview.chromium.org/2304253002 .

Cr-Commit-Position: refs/branch-heads/2840@{#127}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/165965c08079b64a645819ffddc221915891409f/chrome/browser/chrome_browser_main.cc
[modify] https://crrev.com/165965c08079b64a645819ffddc221915891409f/chrome/browser/usb/web_usb_detector.cc
[modify] https://crrev.com/165965c08079b64a645819ffddc221915891409f/chrome/browser/usb/web_usb_detector.h
[modify] https://crrev.com/165965c08079b64a645819ffddc221915891409f/chrome/browser/usb/web_usb_detector_unittest.cc
[modify] https://crrev.com/165965c08079b64a645819ffddc221915891409f/tools/metrics/histograms/histograms.xml

Project Member

Comment 17 by bugdroid1@chromium.org, Oct 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/165965c08079b64a645819ffddc221915891409f

commit 165965c08079b64a645819ffddc221915891409f
Author: Reilly Grant <reillyg@chromium.org>
Date: Fri Sep 02 18:25:38 2016

Delay WebUSB initialization until after browser startup.

Initializing the WebUSB device detector module causes the USB service to
be initialized and enumerate all USB device connected to the system.
This can cause unnecessary delay during browser startup. The purpose of
this module is to detect devices connected after Chrome starts to
display a notification so it doesn't matter if it is initialized after
first page load.

An UMA histogram for the time spent initializing the detector is also
added to aid further investigation of this issue.

BUG= 642107 

Review-Url: https://codereview.chromium.org/2295023002
Cr-Commit-Position: refs/heads/master@{#415971}
(cherry picked from commit 478a62181383d2cc7fa545ee6a7a91ab34fec477)

Review URL: https://codereview.chromium.org/2304253002 .

Cr-Commit-Position: refs/branch-heads/2840@{#127}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/165965c08079b64a645819ffddc221915891409f/chrome/browser/chrome_browser_main.cc
[modify] https://crrev.com/165965c08079b64a645819ffddc221915891409f/chrome/browser/usb/web_usb_detector.cc
[modify] https://crrev.com/165965c08079b64a645819ffddc221915891409f/chrome/browser/usb/web_usb_detector.h
[modify] https://crrev.com/165965c08079b64a645819ffddc221915891409f/chrome/browser/usb/web_usb_detector_unittest.cc
[modify] https://crrev.com/165965c08079b64a645819ffddc221915891409f/tools/metrics/histograms/histograms.xml

Sign in to add a comment