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

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


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

android-one

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

Hi bashi@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 : Use preferred format to allocate memory for decoded images
Author  : bashi
Commit description:
  
We use fixed color format (kN32_SkColorType) to decode images. Since the default
preferred format is 16-bit color on low-end devices, we can reduce memory
footprint by storing decoded image in 16-bit color format when we use the
default settings for the preferred format. This CL makes
SoftwareImageDecodeController allocate memory based on the preferred format.

I ran memory.blink_memory_mobile 20 times. The results can be found at [1].
I removed SimulateMemoryPressureNotification() so that I can observe memory usage in
common patterns (still, memory pressure notifications would be sent if memory
usage is high). According to the results, we could reduce ~40% discardable
memory on average.

Note that Blink still uses fixed 32-bit color for decoding. This means that
skia/cc will convert decoded images when we store decoded image in 16-bit color.
We might want to make Blink's decoding format configurable in the future.

[1] https://drive.google.com/file/d/0B6NYyLPujP4TaWItc3lsZXR6TTg/view?usp=sharing

BUG= 519146 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review URL: https://codereview.chromium.org/1808633002

Cr-Commit-Position: refs/heads/master@{#381859}
Commit  : 0c87a7ea398b77020298f09abe3fd946a0627603
Date    : Fri Mar 18 01:07:19 2016


===== TESTED REVISIONS =====
Revision                Mean Value  Std. Dev.   Num Values  Good?
chromium@381834         58.8084     0.280954    5           good
chromium@381848         58.5656     0.225323    5           good
chromium@381855         57.017      4.042315    5           good
chromium@381857         56.8598     4.284817    5           good
chromium@381858         56.8754     4.361476    5           good
chromium@381859         67.8994     0.187906    5           bad
chromium@381862         67.8558     0.417912    5           bad

Bisect job ran on: android_one_perf_bisect
Bug ID: 596054

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --also-run-disabled-tests rasterize_and_record_micro.top_25_smooth
Test Metric: rasterize_time/ESPN
Relative Change: 15.38%
Score: 99.5

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_one_perf_bisect/builds/1218
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9017847109019241104


| 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 label Cr-Tests-AutoBisect.  Thank you!

cc-ing vmpstr, owner of rasterize_and_record_micro.top_25

bashi, it looks like your CL caused a performance regression. Are you investigating?

This is a trade-off between saving memory vs avoid copying as I described in the description of the CL. I think saving memory is more important than the micro benchmark results on low-end devices because consuming a lot of memory can make OS busy to kill other processes, causing junks. WDYT?

vmpstr, any opinion on #4?

gentle ping..

bashi@, do you have a link to the bots showing the memory improvement? If this is a memory reduction on low end devices, then I think the perf regression is fine. It would just be nice to have graphs to support this.

Also, from the graphs, I assume that the n7v2 memory regression is not actually related to this patch. Is this correct? Can it be separated out?

gentle ping. bashi@

I missed this. Sorry for late response.

Unfortunately the improvement doesn't show up on bots because our benchmarks are stabilized by using explicit memory pressure notifications, which purge memory as much as possible. That said I believe this is a real improvement because renderers less frequently receive pressure notifications in many real usage even on low-end devices (not sure why).

> Also, from the graphs, I assume that the n7v2 memory regression is not actually related to this patch. Is this correct? Can it be separated out?

Yes, I don't think my CL affects n7v2 memory regression. My CL only changed behavior on low-end devices and n7v2 is not.

Bisect re-kicked with wider revision range as suspected CL is not the culprit.

Details on the below link:
=========================
https://chromeperf.appspot.com/group_report?bug_id=596054

Thank you!

Looks like someone separated out the n7v2 regression, I don't see it on the list anymore.

Given the (hopeful) memory improvement, are we accepting the android one regression?

Friendly Perf sheriff ping!
bashi@, could you please comment on #11.

I'm not sure what you want me to comment, but my position doesn't change from #9.