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

Issue 643722 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

500.6%-502.2% regression in performance_browser_tests at 416001:416084

Project Member Reported by mustaq@chromium.org, Sep 2 2016

Issue description

See the link to graphs below.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=643722

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgkYbqoQkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgke2huwkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgkca7qwoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgkZKSsQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgkazsgQgM


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

chromium-rel-win7-dual
chromium-rel-win7-gpu-ati
chromium-rel-win7-gpu-nvidia
chromium-rel-win7-x64-dual
chromium-rel-win8-dual
Cc: enne@chromium.org
Owner: enne@chromium.org

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

Hi enne@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 : Remove default begin frame sources from the renderer
Author  : enne
Commit description:
  
This changes both cc proxies to no longer create default begin frame
sources.  In cc, it must either be the case that an external begin frame
source is provided (with use_external_begin_frame_source set) or the
output surface provides a begin frame source (with
use_output_surface_begin_frame_source).

This changes the world from three states of BFS creation (default,
output surface provided, and external) to only two (output surface
provided and external).  In the future, external will also be removed.

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/da025c649b0c2339771f2b06c482dcf8e388e889
Review-Url: https://codereview.chromium.org/2083423006
Cr-Original-Commit-Position: refs/heads/master@{#415757}
Cr-Commit-Position: refs/heads/master@{#416033}
Commit  : 5593b84f823ee1517347cf773c6cd1854c0268ba
Date    : Thu Sep 01 20:24:11 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev    N   Good?
chromium@416030  41.6569  0.0189112  5   good
chromium@416032  41.647   0.0268763  5   good
chromium@416033  250.856  0.0579951  20  bad    <--
chromium@416036  250.835  0.0404286  20  bad
chromium@416041  250.858  0.0532357  20  bad
chromium@416051  250.87   0.0532017  20  bad

Bisect job ran on: winx64nvidia_perf_bisect
Bug ID: 643722

Test Command: .\src\out\Release_x64\performance_browser_tests.exe --test-launcher-print-test-stdio=always --enable-gpu
Test Metric: CastV2Performance_gpu_novsync_24fps/time_between_captures
Relative Change: 502.23%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/winx64nvidia_perf_bisect/builds/1851
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9002613509996486688


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

| 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!
Labels: -Pri-2 Pri-1
This is also being flagged as causing failures on some platforms:
https://bugs.chromium.org/p/chromium/issues/detail?id=644370
Labels: -Pri-1 Pri-2
Actually; I'm not convinced this is the same issue as the other bug. I'm kicking off another bisect.
Labels: -Pri-2 Pri-1
Ok, I resolved the other problem with performance_browser_tests. The failure caught by that bisect is real and was just masked by the other issue. Upping the priority to p1 since its both a regression and causing tests to fail.
Labels: Performance-Sheriff-BotHealth
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 15 2016

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

commit e292bdc26487a681a93fab35767adcbaba5d524e
Author: enne <enne@chromium.org>
Date: Thu Sep 15 19:57:15 2016

Make disable vsync run the renderer independently

https://codereview.chromium.org/2083423006 removed the creation of begin
frame sources inside of cc proxies.  When vsync was disabled, the
browser would run unthrottled but the renderer would still tick based
on the browser.  This regressed some perf tests.  This patch reverts
that situation and reimplements unthrottled renderer begin frame sources
in a different way.

When the command line parameter is present, the renderer now runs
unthrottled by creating a separate unthrottled begin frame source inside
of RenderThreadImpl instead of an external one.

As some clean up, the property of being throttled is baked into the
BeginFrameSource interface itself.  This allows for the removal of the
throttling scheduler setting (and the already unused use external bfs
setting).  This should allow for future patches to allow toggling this
property.

The browser compositor and display compositor continue to be tied
together and tick at the same rate.  Therefore, with this patch,
--disable-gpu-vsync=gpu only applies to the display/browser.
--disable-gpu-vsync=beginframe only applies to the renderer.
--disable-gpu-vsync on its own will unthrottle both.

R=brianderson@chromium.org,sunnyps@chromium.org
BUG= 643722 , 644926 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

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

[modify] https://crrev.com/e292bdc26487a681a93fab35767adcbaba5d524e/cc/scheduler/begin_frame_source.cc
[modify] https://crrev.com/e292bdc26487a681a93fab35767adcbaba5d524e/cc/scheduler/begin_frame_source.h
[modify] https://crrev.com/e292bdc26487a681a93fab35767adcbaba5d524e/cc/scheduler/scheduler.cc
[modify] https://crrev.com/e292bdc26487a681a93fab35767adcbaba5d524e/cc/scheduler/scheduler_settings.cc
[modify] https://crrev.com/e292bdc26487a681a93fab35767adcbaba5d524e/cc/scheduler/scheduler_settings.h
[modify] https://crrev.com/e292bdc26487a681a93fab35767adcbaba5d524e/cc/scheduler/scheduler_unittest.cc
[modify] https://crrev.com/e292bdc26487a681a93fab35767adcbaba5d524e/cc/surfaces/surface_manager_unittest.cc
[modify] https://crrev.com/e292bdc26487a681a93fab35767adcbaba5d524e/cc/test/fake_external_begin_frame_source.cc
[modify] https://crrev.com/e292bdc26487a681a93fab35767adcbaba5d524e/cc/test/fake_external_begin_frame_source.h
[modify] https://crrev.com/e292bdc26487a681a93fab35767adcbaba5d524e/cc/trees/layer_tree_host_perftest.cc
[modify] https://crrev.com/e292bdc26487a681a93fab35767adcbaba5d524e/cc/trees/layer_tree_host_unittest.cc
[modify] https://crrev.com/e292bdc26487a681a93fab35767adcbaba5d524e/cc/trees/layer_tree_settings.cc
[modify] https://crrev.com/e292bdc26487a681a93fab35767adcbaba5d524e/cc/trees/layer_tree_settings.h
[modify] https://crrev.com/e292bdc26487a681a93fab35767adcbaba5d524e/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/e292bdc26487a681a93fab35767adcbaba5d524e/content/renderer/gpu/compositor_external_begin_frame_source.cc
[modify] https://crrev.com/e292bdc26487a681a93fab35767adcbaba5d524e/content/renderer/gpu/compositor_external_begin_frame_source.h
[modify] https://crrev.com/e292bdc26487a681a93fab35767adcbaba5d524e/content/renderer/gpu/render_widget_compositor.cc
[modify] https://crrev.com/e292bdc26487a681a93fab35767adcbaba5d524e/content/renderer/render_thread_impl.cc
[modify] https://crrev.com/e292bdc26487a681a93fab35767adcbaba5d524e/ui/compositor/compositor.cc
[modify] https://crrev.com/e292bdc26487a681a93fab35767adcbaba5d524e/ui/gl/gl_switches.cc

Comment 9 by enne@chromium.org, Sep 16 2016

Cc: piman@chromium.org briander...@chromium.org
Status: Fixed (was: Assigned)
This appears to have fixed the problem with the disabled vsync cast tests.  Not running the renderer unthrottled independently of the browser definitely was the culprit here in terms of why these tests regressed.

Sign in to add a comment