|3.8%-7.5% regression in system_health.memory_mobile at 449423:449504|
|Reported by email@example.com, Feb 10 2017||Back to list|
See the link to graphs below.
Feb 10 2017,
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
Feb 10 2017,
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8988018991448462768
Feb 11 2017,
=== Auto-CCing suspected CL author firstname.lastname@example.org === Hi email@example.com, 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: : https://chromeperf.appspot.com/report?sid=4c4f9ad55a2fe122857f117af42151bffed4b0c46cf785ae1a980785d6548321&rev=449546 : 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?
Sep 18 (3 days ago),
Sep 18 (3 days ago),
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.
Sep 18 (3 days ago),
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.
|► Sign in to add a comment|