New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 778680 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug-Regression

Blocking:
issue 744658



Sign in to add a comment

36.1% regression in v8.runtimestats.browsing_desktop at 511068:511143

Project Member Reported by petermarshall@chromium.org, Oct 26 2017

Issue description

See the link to graphs below.
 
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Oct 26 2017

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

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


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

chromium-rel-win7-gpu-intel
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Oct 26 2017

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

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

Hi zmo@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 : Zhenyao Mo
  Commit : 0728a85617eae06098373349af80c28ce26b60fd
  Date   : Tue Oct 24 11:16:56 2017
  Subject: One step further into moving GPU feature decisions to GPU process

Bisect Details
  Configuration: winx64intel_perf_bisect
  Benchmark    : v8.runtimestats.browsing_desktop
  Metric       : v8-gc-latency-mark-compactor_sum/browse_tech/browse_tech_discourse_infinite_scroll
  Change       : 27.83% | 14.4175 -> 18.4303333333

Revision             Result                   N
chromium@511067      14.4175 +- 1.50072       6      good
chromium@511086      14.3342 +- 0.738101      6      good
chromium@511088      14.1258 +- 1.73654       6      good
chromium@511089      16.9027 +- 3.16708       6      bad       <--
chromium@511091      18.2555 +- 4.92708       6      bad
chromium@511096      16.928 +- 2.72151        6      bad
chromium@511105      18.081 +- 3.40748        6      bad
chromium@511143      18.4303 +- 5.00465       6      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=browse.tech.discourse.infinite.scroll v8.runtimestats.browsing_desktop

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8964662584834202784


For feedback, file a bug with component Speed>Bisection
Project Member

Comment 4 by 42576172...@developer.gserviceaccount.com, Oct 27 2017

Cc: h...@chromium.org yhirano@chromium.org nedngu...@google.com jbudorick@chromium.org thakis@chromium.org
 Issue 778806  has been merged into this issue.
Labels: -Pri-2 Pri-1
This is P1 because it regressed Telemetry test runtime heavily (see issue  Issue 778806 ). 
I am reverting the CL. If the revert failed, please do a manual revert if possible,  Zhenyao

Comment 6 by thakis@chromium.org, Oct 27 2017

Wait, what cl do you want to revert?

Comment 8 by kbr@chromium.org, Oct 27 2017

Blocking: 744658
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 27 2017

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

commit ed8968af9be02f53c33959dfd32518b36a4f8ffc
Author: Zhenyao Mo <zmo@chromium.org>
Date: Fri Oct 27 21:05:32 2017

Attempt to fix SystemInfoHandler regression.

There is a racing, that when we register an GPUInfo update observer, the GPU process already launched and sent back the GPUInfo and GpuFeatureInfo, so the observer is never called.

I think this will likely fix the regression.

Regardless, this is what we should do.

BUG= 778680 , 744658 
TEST=bots
R=piman@chromium.org,nednguyen@chromium.org

Change-Id: I32d0267fe4df96323b18619febb669c7fe2d5408
Reviewed-on: https://chromium-review.googlesource.com/742281
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512277}
[modify] https://crrev.com/ed8968af9be02f53c33959dfd32518b36a4f8ffc/content/browser/devtools/protocol/system_info_handler.cc

Comment 10 by zmo@chromium.org, Oct 29 2017

The above CL doesn't fix the perf bots regression. I looked further into the issue.

The slow down is triggered by SystemInfoHandler timing out, i.e., no GpuFeatureInfo update happens. When timing out happens, below is the stack that's triggered (otherwise, if no timing out, the path isn't triggered).

2   Chromium Framework                  0x000000010e1c4a35 content::GpuProcessTransportFactory::OnLostMainThreadSharedContext() + 85
3   Chromium Framework                  0x000000010e1c20c3 content::GpuProcessTransportFactory::DisableGpuCompositing(ui::Compositor*) + 51
4   Chromium Framework                  0x000000010e1c2071 content::GpuProcessTransportFactory::GpuProcessTransportFactory(gpu::GpuChannelEstablishFactory*, viz::CompositingModeReporterImpl*, scoped_refptr<base::SingleThreadTaskRunner>) + 833

So it seems to because Gpu Compositing is somehow disabled on certain tests, and that prevent GPU process from launching.

Comment 11 by zmo@chromium.org, Oct 29 2017

Cc: capn@chromium.org danakj@chromium.org sugoi@chromium.org
I thought in the above mentioned situation, at least on Windows/Linux, we still launch the GPU process with SwiftShader? Or the launch is on demand when a WebGL context is requested?

Regardless, I am reverting a line in https://chromium-review.googlesource.com/c/chromium/src/+/732623 to make sure GPU process always launches for SystemInfoHandler. Let's get perf bots back to normal.
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 30 2017

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

commit 77604d4d295b16c91b935c0a3e035a574532a497
Author: Zhenyao Mo <zmo@chromium.org>
Date: Mon Oct 30 03:22:48 2017

Call UpdateGPUInfo and UpdateGpuFeatureInfo always on GPU process launch.

Otherwise event observer like SystemInfoHandler could be stuck and time out.

BUG= 778680 , 744658 
TEST=bots, perf tests
TBR=piman@chromium.org,kbr@chromium.org,nednguyen@chromium.org

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I35dbc51f83ff1f1592e46152ec4a5c47f8b4376a
Reviewed-on: https://chromium-review.googlesource.com/743041
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512435}
[modify] https://crrev.com/77604d4d295b16c91b935c0a3e035a574532a497/content/browser/devtools/protocol/system_info_handler.cc
[modify] https://crrev.com/77604d4d295b16c91b935c0a3e035a574532a497/content/browser/gpu/gpu_process_host.cc
[modify] https://crrev.com/77604d4d295b16c91b935c0a3e035a574532a497/content/test/gpu/gpu_tests/gpu_process_expectations.py

Comment 13 by zmo@chromium.org, Oct 30 2017

The above Cl brought regression back quite a lot, but the regression was not fully recovered. I suspect it's due to switching to feature status computed on GPU process. I'll have to wait until I get my hands on to a local Win bot to debug locally.

At this point, I think I should just revert my CL and the two followup fix attempts.

https://chromium-review.googlesource.com/c/chromium/src/+/743741
Project Member

Comment 14 by bugdroid1@chromium.org, Oct 30 2017

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

commit 39d8a7f229e1330e72724895eb4730b7108e47a0
Author: Zhenyao Mo <zmo@chromium.org>
Date: Mon Oct 30 13:45:42 2017

Revert "One step further into moving GPU feature decisions to GPU process"

This reverts commit 0728a85617eae06098373349af80c28ce26b60fd.

Revert "Attempt to fix SystemInfoHandler regression."

This reverts commit ed8968af9be02f53c33959dfd32518b36a4f8ffc.

Revert "Call UpdateGPUInfo and UpdateGpuFeatureInfo always on GPU process launch."

This reverts commit 77604d4d295b16c91b935c0a3e035a574532a497.

Bug= 778680 , 744658 
TEST=bots,perf bots (win)
TBR=piman@chromium.org,kbr@chromium.org,nednguyen@chromium.org

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I7ebcb6335dff91fc396225800ebe72cb17d07ae7
Reviewed-on: https://chromium-review.googlesource.com/743741
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512485}
[modify] https://crrev.com/39d8a7f229e1330e72724895eb4730b7108e47a0/chrome/browser/extensions/api/webstore_private/webstore_private_apitest.cc
[modify] https://crrev.com/39d8a7f229e1330e72724895eb4730b7108e47a0/content/browser/devtools/protocol/system_info_handler.cc
[modify] https://crrev.com/39d8a7f229e1330e72724895eb4730b7108e47a0/content/browser/gpu/compositor_util.cc
[modify] https://crrev.com/39d8a7f229e1330e72724895eb4730b7108e47a0/content/browser/gpu/compositor_util.h
[modify] https://crrev.com/39d8a7f229e1330e72724895eb4730b7108e47a0/content/browser/gpu/gpu_data_manager_impl.cc
[modify] https://crrev.com/39d8a7f229e1330e72724895eb4730b7108e47a0/content/browser/gpu/gpu_data_manager_impl.h
[modify] https://crrev.com/39d8a7f229e1330e72724895eb4730b7108e47a0/content/browser/gpu/gpu_data_manager_impl_private.cc
[modify] https://crrev.com/39d8a7f229e1330e72724895eb4730b7108e47a0/content/browser/gpu/gpu_data_manager_impl_private.h
[modify] https://crrev.com/39d8a7f229e1330e72724895eb4730b7108e47a0/content/browser/gpu/gpu_data_manager_testing_arrays_and_structs_autogen.h
[modify] https://crrev.com/39d8a7f229e1330e72724895eb4730b7108e47a0/content/browser/gpu/gpu_feature_checker_impl.cc
[modify] https://crrev.com/39d8a7f229e1330e72724895eb4730b7108e47a0/content/browser/gpu/gpu_process_host.cc
[modify] https://crrev.com/39d8a7f229e1330e72724895eb4730b7108e47a0/content/browser/resources/gpu/info_view.js
[modify] https://crrev.com/39d8a7f229e1330e72724895eb4730b7108e47a0/content/public/browser/gpu_data_manager.h
[modify] https://crrev.com/39d8a7f229e1330e72724895eb4730b7108e47a0/content/test/gpu/gpu_tests/gpu_process_expectations.py
[modify] https://crrev.com/39d8a7f229e1330e72724895eb4730b7108e47a0/content/test/gpu/gpu_tests/gpu_process_integration_test.py
[modify] https://crrev.com/39d8a7f229e1330e72724895eb4730b7108e47a0/gpu/config/gpu_blacklist.cc
[modify] https://crrev.com/39d8a7f229e1330e72724895eb4730b7108e47a0/gpu/config/gpu_blacklist_unittest.cc
[modify] https://crrev.com/39d8a7f229e1330e72724895eb4730b7108e47a0/gpu/config/gpu_feature_type.h
[modify] https://crrev.com/39d8a7f229e1330e72724895eb4730b7108e47a0/gpu/config/gpu_util.cc
[modify] https://crrev.com/39d8a7f229e1330e72724895eb4730b7108e47a0/gpu/config/software_rendering_list.json

Comment 15 by zmo@chromium.org, Oct 30 2017

Status: Fixed (was: Assigned)
Seems benchmarks have been recovered.
Project Member

Comment 16 by 42576172...@developer.gserviceaccount.com, Oct 31 2017

Cc: simonhatch@chromium.org kraynov@chromium.org
 Issue 777799  has been merged into this issue.

Sign in to add a comment