New issue
Advanced search Search tips

Issue 669319 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

7.6%-12.2% regression in media.android.tough_video_cases at 434298:434308

Project Member Reported by jrumm...@chromium.org, Nov 29 2016

Issue description

See the link to graphs below.
 
Project Member

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

Cc: bokan@chromium.org
Owner: bokan@chromium.org

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

Hi bokan@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 : LayoutView's paint layer should clip to the full viewport size.
Author  : bokan
Commit description:
  
This patch makes it so that the LayoutView's painting size is always at least
the size of the layout viewport. Today (without root-layer-scrolls), the
LayoutView's PaintLayer (and its parent root content layer) is sized to contain
all the document's content and is clipped and scrolled by the PaintLayers
belonging to the Frame's PaintLayerCompositor (the Frame Scroll and Clipping
layers).  This is problematic if the content is smaller than the viewport since
position: fixed elements are attached to the viewport but are not counted as
part of the LayoutView's "layout overflow".

This occurs in two situations (both occur only on Android):

1.  Bug 666806  - With inert top controls on and the top controls hidden, a page
whose document is empty (for e.g.) will produce a LayoutView layer whose height
is the height of the viewport minus the height of the top controls. This means
bottom position: fixed elements will get clipped.

2.  Bug 436871  - If the content on a page is wider than the ICB, Chrome will
grow the layout viewport until its width covers all the content (or the minimum
scale is reached), keeping the aspect-ratio fixed. When this happens, the
layout viewport can become taller than the content resulting in clipping of
bottom position: fixed elements. (This is related to  crbug.com/492871 ).

This patch fixes both issues.

BUG= 666806 , 436871 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2519163003
Cr-Commit-Position: refs/heads/master@{#434303}
Commit  : a226f0e367ee7e064cc398d0b76d347d53103295
Date    : Thu Nov 24 05:13:21 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N  Good?
chromium@434297  14.0546  1.04803   8  good
chromium@434300  13.9564  0.951767  8  good
chromium@434302  14.1299  1.0035    8  good
chromium@434303  15.6906  0.687811  5  bad    <--
chromium@434308  16.1664  0.215882  5  bad

Bisect job ran on: android_nexus7_perf_bisect
Bug ID: 669319

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=video.html.src.crowd1080.vp9.webm media.android.tough_video_cases
Test Metric: cpu_utilization/video.html?src_crowd1080_vp9.webm
Relative Change: 15.03%

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus7_perf_bisect/builds/3517
Job details: https://chromeperf.appspot.com/buildbucket_job_status/8994699532653977952


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

| 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!

Comment 4 by bokan@chromium.org, Nov 29 2016

Labels: OS-Android
Status: Assigned (was: Untriaged)
Taking a look
Project Member

Comment 6 by 42576172...@developer.gserviceaccount.com, Nov 30 2016


===== BISECT JOB RESULTS =====
Status: failed


=== Bisection aborted ===
The bisect was aborted because Bisect cannot identify a culprit: No values were found while testing the reference range.
Please contact the the team (see below) if you believe this is in error.

===== TESTED REVISIONS =====
Revision         Mean  Std Dev  N  Good?
chromium@434297  N/A   N/A      0  good
chromium@434308  N/A   N/A      0  bad

Bisect job ran on: android_nexus7_perf_bisect
Bug ID: 669319

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=crowd1080.vp9.webm.gpu media.android.tough_video_cases
Test Metric: cpu_utilization/crowd1080_vp9.webm_gpu
Relative Change: None

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus7_perf_bisect/builds/3522
Job details: https://chromeperf.appspot.com/buildbucket_job_status/8994564220632490144


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

| 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!
Project Member

Comment 8 by 42576172...@developer.gserviceaccount.com, Nov 30 2016


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


===== SUSPECTED CL(s) =====
Subject : LayoutView's paint layer should clip to the full viewport size.
Author  : bokan
Commit description:
  
This patch makes it so that the LayoutView's painting size is always at least
the size of the layout viewport. Today (without root-layer-scrolls), the
LayoutView's PaintLayer (and its parent root content layer) is sized to contain
all the document's content and is clipped and scrolled by the PaintLayers
belonging to the Frame's PaintLayerCompositor (the Frame Scroll and Clipping
layers).  This is problematic if the content is smaller than the viewport since
position: fixed elements are attached to the viewport but are not counted as
part of the LayoutView's "layout overflow".

This occurs in two situations (both occur only on Android):

1.  Bug 666806  - With inert top controls on and the top controls hidden, a page
whose document is empty (for e.g.) will produce a LayoutView layer whose height
is the height of the viewport minus the height of the top controls. This means
bottom position: fixed elements will get clipped.

2.  Bug 436871  - If the content on a page is wider than the ICB, Chrome will
grow the layout viewport until its width covers all the content (or the minimum
scale is reached), keeping the aspect-ratio fixed. When this happens, the
layout viewport can become taller than the content resulting in clipping of
bottom position: fixed elements. (This is related to  crbug.com/492871 ).

This patch fixes both issues.

BUG= 666806 , 436871 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2519163003
Cr-Commit-Position: refs/heads/master@{#434303}
Commit  : a226f0e367ee7e064cc398d0b76d347d53103295
Date    : Thu Nov 24 05:13:21 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N   Good?
chromium@434297  20.8968  0.976078  11  good
chromium@434300  20.8226  0.220449  4   good
chromium@434302  20.9939  0.458426  4   good
chromium@434303  22.3394  1.28735   8   bad    <--
chromium@434308  22.4296  0.872623  6   bad

Bisect job ran on: android_nexus7_perf_bisect
Bug ID: 669319

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=video.html.src.smpte.3840x2160.60fps.vp9.webm media.android.tough_video_cases
Test Metric: cpu_utilization/video.html?src_smpte_3840x2160_60fps_vp9.webm
Relative Change: 7.49%

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus7_perf_bisect/builds/3523
Job details: https://chromeperf.appspot.com/buildbucket_job_status/8994559998607122272


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

| 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!

Comment 9 by bokan@chromium.org, Jan 3 2017

Status: Started (was: Assigned)
Status: WontFix (was: Started)
Managed to reproduce locally with:

run_benchmark -v --browser=android-chromium --pageset-repeat=1 --also-run-disabled-tests --story-filter="video.html.src.crowd1080.vp9.webm&canvas=true" media.android.tough_video_cases

The difference I see locally is about 2-4% of CPU utilization. I've narrowed it down to having to repaint the whole screen for LayoutView's background since my patch. This is a correctness fix in some cases and I don't think the perf hit is very big. The paint doesn't happen on every frame. 

Sign in to add a comment