New issue
Advanced search Search tips

Issue 628293 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

1% regression in system_health.memory_mobile at 405363:405398

Project Member Reported by petrcermak@chromium.org, Jul 14 2016

Issue description

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

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


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

android-nexus5X
Project Member

Comment 4 by 42576172...@developer.gserviceaccount.com, Aug 16 2016

Cc: ericrk@chromium.org
Owner: ericrk@chromium.org

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

Hi ericrk@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 : Partial raster for GPU
Author  : ericrk
Commit description:
  
Partial raster allows for significant savings during raster by allowing us to
rasterize on top of the previous content, only updating the areas which have
changed.

In order to leverage this optimization, the resource containing the previous
content must be unused and available for writing. Currently, the GPU raster
path isn't able to effectively leverage this optimization, as in nearly all cases,
the previous content is still in use (being displayed by the browser), and
cannot be overwritten with new updates.

To allow GPU raster to better leverage partial raster, this causes us to track
invalidations to a given resource over multiple frames. This allows us to use a
resource from two frames ago in partial raster (as opposed to just the most
recent frame). Resources from >1 frame in the past have a much higher
likelihood of being unused.

While this approach isn't perfect (you can't partial raster on the first change,
only subsequent ones) there is virtually no cost to it, which makes it a nice
starting point. Other approaches, such as copying in-use resources may be
investigated in the future if this technique produces insufficient results
compared to Software's Partial Raster numbers.

CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review-Url: https://codereview.chromium.org/2110083004
Cr-Commit-Position: refs/heads/master@{#405379}
Commit  : 5ac42f32e7b4142cd578082424a85a26794cf0ab
Date    : Thu Jul 14 01:11:16 2016


===== TESTED REVISIONS =====
Revision         Mean      Std Dev  N   Good?
chromium@405362  13828146  3339.79  5   good
chromium@405371  13829639  4090.39  5   good
chromium@405376  13831133  4090.39  5   good
chromium@405378  13829764  3845.49  12  good
chromium@405379  13896544  67906.0  12  bad    <--
chromium@405380  13926823  55902.0  12  bad
chromium@405398  13944141  47616.2  8   bad

Bisect job ran on: android_nexus5X_perf_bisect
Bug ID: 628293

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests system_health.memory_mobile
Test Metric: load_media-memory:chrome:all_processes:reported_by_chrome:skia:effective_size_avg/load_media_flickr
Relative Change: 0.77%
Score: 99.9

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


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

| 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: SystemHealth-Sheriff
Labels: -Performance-Sheriff

Comment 7 by ericrk@chromium.org, Sep 28 2016

Status: WontFix (was: Assigned)
The change causes some differences in Skia rendering patterns, and it seems reasonable it would have a small impact on Skia's overall cache size (either up or down) - the change here is around 130k on this specific page. Overall (on all of load_media), the average regression is only 20k - seems pretty uninteresting.


Especially as the cache cleanup work in crrev.com/2353033003 takes this number down to near zero, I'd vote to close this out.

Sign in to add a comment