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

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


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

android-webview-nexus6

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

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

Hi japhet@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 : japhet
  Commit : ffefd4173f178d76449fc51f5dc22bfd7e120e03
  Date   : Thu Feb 09 22:25:56 2017
  Subject: Actually handle the Vary header in blink, rather than punting on sight.

Bisect Details
  Configuration: android_webview_nexus6_aosp_perf_bisect
  Benchmark    : system_health.memory_mobile
  Metric       : memory:webview:all_processes:reported_by_chrome:v8:effective_size_avg/load_media/load_media_9gag
  Change       : 6.54% | 14134364.8889 -> 15091096.0

Revision             Result                   N
chromium@449422      14134365 +- 272606       9       good
chromium@449430      14102072 +- 354500       9       good
chromium@449434      14035520 +- 377421       9       good
chromium@449436      14226880 +- 1122881      14      good
chromium@449437      14057307 +- 548933       9       good
chromium@449438      14917434 +- 1250773      9       bad       <--
chromium@449504      15091096 +- 211403       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=load.media.9gag system_health.memory_mobile

Debug Info
  https://chromeperf.appspot.com/buildbucket_job_status/8988018991448462768

Is this bisect wrong?
  https://chromeperf.appspot.com/bad_bisect?try_job_id=4984018078007296


| 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 Speed>Bisection.  Thank you!

I verified that https://crrev.com/ffefd4173f178d76449fc51f5dc22bfd7e120e03 is responsible for the change, and that it's specifically related to the change in caching behavior, not to the header parsing portion of my change. I'm fairly certain this is a measurement/timing change, not an actual memory regression.

Reading through the test harness, (third_party/catapult/telemetry/telemetry/internal/actions/navigate.py and MeasureMemory() in third_party/catapult/telemetry/telemetry/internal/actions/action_runner.py) I believe the flow of the test is;
* Wait for document.readyState to be at least interactive
* Wait for rAF
* GC
* Measure memory

My patch causes us to use more resources from blink's in-memory cache, rather than going to the browser process cache. It's entirely possible, e.g., if images are in the cache, that we will load them immediately from the cache, and this will in turn lead to additional memory allocations. This would happen immediately rather than requiring an IPC roundtrip to the browser process, and could certainly happen before document.readyState == interactive. Images are irrelevant to document.readyState, so if more images load before readyState == interactive, this would lead to more memory in use at the time of measurement.

Given this, I'm inclined to declare this WontFix. Feel free to reopen if there are objections.

The explanation is plausible. Have you looked to see if there is improvement elsewhere in other metrics? I would have imagined this change to perhaps improve metrics that measure first meaningfull paint. (Though I am not sure if we have any such metric)

perezju@: Are you happy with this resolution? See comment #4. 

+benhenry, +primiano

I think we should make sure we understand better what is going on. The largest increases caught by these alerts are in the order of ~20 MB, so that will be hard to explain if these affect system health measurements.

A couple of large clean regressions found here:

[1]: https://chromeperf.appspot.com/report?sid=4c4f9ad55a2fe122857f117af42151bffed4b0c46cf785ae1a980785d6548321&rev=449546

[2]: https://chromeperf.appspot.com/report?sid=c49f2707110896c1e4a99767cd0e6bf24e7ea4506dce663d80f621e547650c70&rev=449638

I think that the WontFix was a bit premature.
First of all, memory is not the only thing regressing here. As clearly visible by the link in #1 there are regressions in v8 gc time as well as regressions in timeToFirstContentfulPaint_avg .

As regards memory, the explanation is plausible, but I would like to see a trace on the pages that are regressing that proves it.
You should be able inject another memory measurement after some seconds by tweaking the code you found.

Friendly perf sheriff ping, any update on this?

japhet: ping?

I'm willing to look at this again, but I'm probably going to need quite a bit of hand-holding, as my previous attempts to figure out what I would need to do in the telemetry code were not productive.

Sounds good. Can you start with the things primiano asked for in #7 and let us know where you get stuck? You can look up test owners in go/chrome-benchmarks and cc them for help.

japhet: ping on #7?

Any updates on this? I think this has been open for more than a year now. Would it be still useful to investigate this?

perf sheriff backlog triage:

japhet@, do you still have a plan to do further investigation on this?

Ping! This bug has been flagged as stale by Chrome Speed Metrics bug triage, and it has an owner. Any update on this issue?