|Issue 691033||3.8%-7.5% regression in system_health.memory_mobile at 449423:449504|
|Starred by 1 user||Reported by email@example.com, Feb 10||Back to list|
See the link to graphs below.
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 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.
|► Sign in to add a comment|