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

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgpobW9AgM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg5vL2sQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgpoWuvQkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg5qzupgoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg5qy5twoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg5piivwoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg5qaduwoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgpsfJoAoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg5qyjrgoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgpt7GqgsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgppK6rAsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgpr-bvAoM


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

chromium-rel-mac-retina
chromium-rel-win10
chromium-rel-win7-gpu-ati
chromium-rel-win7-gpu-intel
chromium-rel-win7-gpu-nvidia
chromium-rel-win8-dual
linux-release

Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/9005355316985931200

===== 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@409186  252327  17809.3  12  good
chromium@409194  716759  497714   8   bad

Bisect job ran on: winx64_10_perf_bisect
Bug ID: 633941

Test Command: src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --also-run-disabled-tests system_health.memory_desktop
Test Metric: load_news-memory:chrome:all_processes:reported_by_chrome:skia:effective_size_avg/load_news_bbc
Relative Change: 74.74%
Score: 0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/winx64_10_perf_bisect/builds/639
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9005355316985931200


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

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

Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/9005330475343512672

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


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@409186  367256   18091.0  5  good
chromium@409189  352228   13319.2  5  good
chromium@409190  356095   7726.37  5  good
chromium@409191  1292418  17641.7  5  bad
chromium@409192  1285835  18057.3  5  bad
chromium@409198  1297418  14585.0  5  bad
chromium@409210  1289450  8382.58  5  bad

Bisect job ran on: winx64ati_perf_bisect
Bug ID: 633941

Test Command: src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --also-run-disabled-tests system_health.memory_desktop
Test Metric: load_news-memory:chrome:all_processes:reported_by_chrome:skia:effective_size_avg/load_news_bbc
Relative Change: 251.10%
Score: 0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/winx64ati_perf_bisect/builds/1485
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9005330475343512672


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

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

reed: The bisect identified http://crrev.com/409191 as the culprit. It is a Skia roll that contains a single patch, https://codereview.chromium.org/2195893002. Please investigate this issue (+0.89 MiB regression in Skia memory on BBC) and decide on the course of action.

@ssid can you shed some light on what's included under skia:effective_size_avg in these benchmarks?  And is there a way to get (some approximation) of these numbers other than running the full Telemetry benchmark?

It looks like one extra bitmap of 650Kib got added to the resource cache. I have added screenshots of trace from before and after the change.
It is really hard to understand the difference without actually running the benchmark, since the number could be very noisy.

Deleted: skia_bitmap_without.png 63.8 KB

	skia_bitmap_without.png 63.8 KB View Download

Deleted: skia_bitmap_1.png 61.4 KB

	skia_bitmap_1.png 61.4 KB View Download

Thanks, that's a great clue, we weren't sure whether the resource cache was included in the telemetry number.

I was just instrumenting SkResourceCache::DumpMemoryStatistics, and came to the same conclusion: we are now caching (leaving cached) an extra bitmap.

(looking at the stackoverflow page)

Before:
  ** sk_trace_dump_visitor[skia/sk_resource_cache/rects-blur_0x1df7d3ff2d0]: 376

After:
  ** sk_trace_dump_visitor[skia/sk_resource_cache/rects-blur_0x1df7d3ff2d0]: 376
  ** sk_trace_dump_visitor[skia/sk_resource_cache/bitmap_0x7f5e2a9963c0]: 100856

Looks like we're now (inadvertently) caching the result of SkPixmap::scalePixels():

* SkPixmap::scalePixels wraps the pixels into a SkBitmap, and marks it as volatile, to prevent internal caching
* then calls drawBitmapRect to perform the actual scaling
* SkDraw::drawBitmap uses a bitmap shader to perform the draw op
* the new impl turns that into SkImageShader - but by converting from SkBitmap to SkImage we lose the volatile hint
* we end up caching the result

 Issue 633945  has been merged into this issue.

are we caching because its upscaling in kHigh_Quality?

Yes, the test I looked at calls SkPixmap::scalePixels w/ kHigh_Quality.

(which means that image decode controller is caching the result also, but that's presumably accounted for in the baseline numbers)

The following revision refers to this bug:
  https://skia.googlesource.com/skia.git/+/5d2befe0062c7c8dfc8760d3b3c02846988e9a4e

commit 5d2befe0062c7c8dfc8760d3b3c02846988e9a4e
Author: fmalita <fmalita@chromium.org>
Date: Mon Aug 08 14:08:37 2016

Avoid caching resources for volatile bitmap shaders

SkBitmapProvider::isVolatile() treats all SkImages as non-volatile,
which is not what we want for temp SkImage wrappers of volatile bitmaps.

R=reed@google.com
BUG= chromium:633941 
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2222783002

Review-Url: https://codereview.chromium.org/2222783002

[modify] https://crrev.com/5d2befe0062c7c8dfc8760d3b3c02846988e9a4e/src/core/SkBitmapProvider.cpp

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

commit 29af7ea5e344065e3bc49d60fa8aab05d507da29
Author: skia-deps-roller <skia-deps-roller@chromium.org>
Date: Mon Aug 08 16:09:05 2016

Roll src/third_party/skia/ f21cd1622..9be372041 (2 commits).

https://chromium.googlesource.com/skia.git/+log/f21cd16228c2..9be372041ec3

$ git log f21cd1622..9be372041 --date=short --no-merges --format='%ad %ae %s'
2016-08-08 halcanary std::move(SkTDArray)
2016-08-08 fmalita Avoid caching resources for volatile bitmap shaders

BUG= 633941 

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
TBR=halcanary@google.com

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

[modify] https://crrev.com/29af7ea5e344065e3bc49d60fa8aab05d507da29/DEPS

The graphs have recovered, marking as fixed.