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

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=1b9a21dd90998f831a8e3f1efaf303cca12ae205ac276e0591d2808cc5c310b2


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

android-webview-nexus5X

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

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

Hi tguilbert@chromium.org, the bisect results pointed to your CL, please take a look at the
results.


=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : Thomas Guilbert
  Commit : 6b6be3d570fd3f83666aa26b3067d0af51841aa2
  Date   : Fri Aug 18 03:17:27 2017
  Subject: Add demuxer based HLS detection

Bisect Details
  Configuration: android_webview_arm64_aosp_perf_bisect
  Benchmark    : system_health.memory_mobile
  Metric       : memory:webview:all_processes:reported_by_chrome:gpu:effective_size_avg/background_news/background_news_nytimes
  Change       : 6.25% | 2097152.0 -> 2228224.0

Revision             Result              N
chromium@495248      2097152 +- 0.0      6      good
chromium@495422      2097152 +- 0.0      6      good
chromium@495444      2097152 +- 0.0      6      good
chromium@495447      2097152 +- 0.0      6      good
chromium@495448      2228224 +- 0.0      6      bad       <--
chromium@495449      2228224 +- 0.0      6      bad
chromium@495450      2228224 +- 0.0      6      bad
chromium@495455      2228224 +- 0.0      6      bad
chromium@495466      2228224 +- 0.0      6      bad
chromium@495509      2228224 +- 0.0      6      bad
chromium@495596      2228224 +- 0.0      6      bad

Please refer to the following doc on diagnosing memory regressions:
  https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/memory_benchmarks.md

To Run This Test
  src/tools/perf/run_benchmark -v --browser=android-webview --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=background.news.nytimes system_health.memory_mobile

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8970370017386509712


For feedback, file a bug with component Speed>Bisection

 Issue 759063  has been merged into this issue.

tguilbert: ping on this. bisect reproduced multiple memory over 100kib. can you investigate?

It seems like there are 2 extra buffers (one transfer_buffer_memory and one command_buffer_memory), each 64kib. I am looking deeper into where they may have come from/why they aren't being released.

Issue 759733 has been merged into this issue.

Whenever we create a media::Renderer using the DefaultRendererFactory, we force initialize the creation of the media context provider. When using HLS, we do not use the media context provider. The regression is caused by the fact that we now create a default Renderer that we immediately discard before switching to HLS/MediaPlayerRendererClient (confirmed by adding an early "return nullptr;" in RenderThreadImpl::GetGpuFactories(), and not creating the extra context provider).

tguilbert: Is the default renderer a real regression, or is the memory recovered when it is discarded?

Closing as won't fix with the following reasoning:

The test is measuring a case for which we had a lucky optimization. Some code changes have removed this "lucky optimization", which would not occur in almost all real-world user scenarios. The test did regress, but the product (very probably) did not. Here is why:

The GpuVideoAcceleratorFactoriesImpl is lazily initialized, and its creation leads to the creation of the media context provider. The media context provider is responsible for the extra memory via the two extra transfer buffers (64kib each).

Once the GpuVideoAcceleratorFactoriesImpl is created, we do not clean it up, and we share it across media player instances. IIUC, this memory is reclaimed on process shutdown.

In every case, the media context is instantiated (and kept alive) as soon as a non HLS video is encountered.

Previously, when playing HLS videos, we directly instantiated a MediaPlayerRendererClient, bypassing the creation of the media context. The CL in C#3 made it so that a temporary default renderer is created, which forces the instantiation of the media context when encountering an HLS video.

HLS accounts for a very small portion of our total playbacks (~2-5% on Android). This means that the likelihood of a user encountering a non HLS video, before or after encountering an HLS video, is very very high, and that the media context would be initialized in any case. This means that 1) in most cases, there is no change OR 2) in a handful of cases (such as in the test case, if someone were to only navigate to an HLS only page) the same, fixed amount of memory used by non HLS videos will be used in HLS videos.

From the quick investigation I had done, there was no easy way to further delay the media context instantiation, without significantly complicating the creation of our default renderer (which accounts for 95%+ of playbacks). This, and the fact that the memory gains would only be useful on a few (AFAICT) lucky cases on Android only, leads me to close this as won't fix.

I am happy to reconsider and discuss this issue further if need be.