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

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg9YXgqAoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg9YW0uQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgjZ6LuwoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgjbqetwoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg9cvaogkM


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

android-nexus5

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

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

Hi danakj@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 : Move memory observer off OutputSurface/CompositorFrameSink
Author  : danakj
Commit description:
  
Instead make ContextProviderCommandBuffer be a memory observer, and
avoid interfaces inheriting other interfaces.

R=ericrk@chromium.org
BUG= 606056 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Review-Url: https://codereview.chromium.org/2408513002
Cr-Commit-Position: refs/heads/master@{#424039}
Commit  : 2871a8c3516e495b9e9acedfe9c0fd43dc9b7b06
Date    : Sat Oct 08 01:55:53 2016


===== TESTED REVISIONS =====
Revision         Mean      Std Dev  N  Good?
chromium@423994  81135206  750958   5  good
chromium@424025  81167974  765687   5  good
chromium@424033  81074586  716815   5  good
chromium@424037  81108992  736739   5  good
chromium@424038  81167974  765687   5  good
chromium@424039  91601306  739289   5  bad    <--
chromium@424040  91627520  753174   5  bad
chromium@424055  91586560  732961   5  bad

Bisect job ran on: android_nexus5_perf_bisect
Bug ID: 654461

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests memory.top_10_mobile
Test Metric: memory:chrome:all_processes:reported_by_os:gpu_memory:proportional_resident_size_avg/foreground/http_m_youtube_com_results_q_science
Relative Change: 12.88%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5_perf_bisect/builds/4219
Job details: https://chromeperf.appspot.com/buildbucket_job_status/8999170880316627664


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

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

Hm, well. This only gets called when tracing is enabled, which I guess it is in perftests, but doesn't seem super problematic to change the amount of memory used in traces. +ericrk to confirm analysis/conclusion.

So, what's changing here is the amount of GPU memory that we're measuring as being used, which shouldn't be impacted by tracing.

Note that your change shows a big improvement in many SW raster benchmarks - this part is understandable - you stopped us from calling GrContext() and creating a GrContext when we didn't need one. In SW, we typically don't need a GrContext, so your change lowered memory usage. Yay!

The regressions are a bit more mysterious. These show that on one device (N5) your change results in GPU memory (as measured by the OS) increasing. This is pretty weird.

My current guess is that, now that we no longer lock the context using the idle handler, we're losing a random idle cleanup that happened later and was being picked up on the N5 - will track one down to look into it.

Adding a bit more to the confusion, the regression in reported_by_os:gpu reproduces clearly on system_health:
https://chromeperf.appspot.com/report?sid=7c591a7f12322a8c87429e64feb4916d00caebdcb9f17b1b97e96be8015269e2&start_rev=421538&end_rev=424655

But does *not* reproduce on the downstream version:
https://chromeperf.appspot.com/report?sid=4787cca795babe072e0deedc270f16b8ef092ee2c8443897cd8dfbeafce6bdac&start_rev=1475900897&end_rev=1476178945

Also, on low end devices, issue 654736 found that this CL also *improves* reported_by_chrome:gpu by nearly 2MB:
https://chrome-health.googleplex.com/health-plan/android-chrome/memory/nexus4-svelte/?form_commit=423795&to_commit=424378

Thanks for the additional info!

The improvement is expected on low end - we no longer create a GrContext (and the stuff that comes with it) when tracing.

The regression is weird - I think this is probably a timing issue re. when we clear caches.

The difference in bots is weird, but somewhat explainable by the system health bot being on L and the normal N5 bot being on K.

There was a progression about a month after this originally occurred, but it wasn't due to a revert of this CL. Did an investigation every happen for this bug? Juan - do we still need to track this if we're making improvements in other areas on these metrics?

Re: system health, I don't think there is any particular need to follow up on this; particularly after the improvements from issue 667258.

Eric, on https://bugs.chromium.org/p/chromium/issues/detail?id=667258#c10 you mentioned working on a CL to make those improvements "stick". Is there a bug to track that work? Maybe we can close this in favor of that one?

Sure. Filed a bug for that.