New issue
Advanced search Search tips

Issue 607962 link

Starred by 0 users

Issue metadata

Status: WontFix
Owner:
Closed: May 2016
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

2.5% regression in memory.top_7_stress at 390051:390085

Project Member Reported by qyears...@chromium.org, Apr 29 2016

Issue description

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

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICglMDRpQoM


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

android-nexus5X
Still waiting for bisect results from Nexus 5x; started new bisect jobs as well.
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 : Fix main thread top controls scrolling to mirror CC.
Author  : bokan
Commit description:
  
There's a couple of issues I found where our behavior on the main thread is
divergent from the compositor. The major one is that we don't anchor the
viewport when top controls change the viewport bounds. At the document extents,
the changing bounds can cause the viewport's scroll offset to be clamped causing
unwanted scrolling. I've added an anchoring in didUpdateTopControls that should
match CC's anchoring down to the sub-pixel level. This also required some
rearranging of operations in applyViewportDeltas.

Now that we anchor the top control resizes on main thread as well, we can't have
a separate calls to change the "shrinks_blink_layout_size" flag and resize the
Blink size so I've added an overload of resize() in WebWidget that does both at
the same time.

In addition, there were other minor issues that I found but weren't causing an
obvoius noticable effect. CC's MaxScrollOffset method |ceil|s the top controls
adjustment so I made the main thread do this too. CC's anchoring logic still
scrolled the outer viewport first. This was changed a while back in viewport.cc
so I changed it here too.

BUG= 600507 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review-Url: https://codereview.chromium.org/1844013002
Cr-Commit-Position: refs/heads/master@{#390085}
Commit  : c63441cc9941c223e2f2d311085903c00da85938
Date    : Wed Apr 27 15:50:07 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@390050  30580.9  588.54   8  good
chromium@390068  30828.9  258.121  8  good
chromium@390077  30986.8  346.353  5  good
chromium@390081  31659.4  169.005  5  good
chromium@390083  32066.4  430.373  5  good
chromium@390084  32447.6  180.231  5  good
chromium@390085  30215.2  296.044  5  bad    <--

Bisect job ran on: android_nexus5X_perf_bisect
Bug ID: 607962

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests memory.top_7_stress
Test Metric: vm_private_dirty_final_browser/vm_private_dirty_final_browser
Relative Change: 2.34%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5X_perf_bisect/builds/143
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9013387481797666608


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

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

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


=== Bisection aborted ===
The bisect was aborted because The metric values for the initial "good" and "bad" revisions do not represent a clear regression.
Please contact the the team (see below) if you believe this is in error.

=== Warnings ===
The following warnings were raised by the bisect job:

 * Bisect failed to reproduce the regression with enough confidence.

===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N   Good?
chromium@390050  25866.9  542.732  18  good
chromium@390085  26418.9  523.621  18  bad

Bisect job ran on: android_nexus5X_perf_bisect
Bug ID: 607962

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests memory.top_7_stress
Test Metric: vm_private_dirty_final_browser/https___www.google.com__hl_en_q_barack+obama
Relative Change: 1.34%
Score: 0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5X_perf_bisect/builds/144
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9013387224250810800


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

| 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!
Status: WontFix (was: Assigned)
Only one alert; no ref results; bisect didn't clearly reproduce.

Comment 6 by bokan@chromium.org, May 6 2016

Just FYI: My CL is unlikely to have had any measurable effect on memory.
Yep, thanks for confirming! Not sure why that first bisect got a score of "99.9".

Sign in to add a comment