Issue metadata
Sign in to add a comment
|
2.5% regression in system_health.memory_mobile at 454055:454099 |
||||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Mar 2 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8986206717238888784
,
Mar 2 2017
=== Auto-CCing suspected CL author mlamouri@chromium.org === Hi mlamouri@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 : mlamouri Commit : 8daac32c17f494d496fb2ca3768d67d9bd77db5e Date : Wed Mar 01 22:10:03 2017 Subject: Media: fix memory leak because of the document holding on an EventListener. Bisect Details Configuration: android_webview_arm64_aosp_perf_bisect Benchmark : system_health.memory_mobile Metric : memory:webview:all_processes:reported_by_os:system_memory:native_heap:proportional_resident_size_avg/load_media/load_media_dailymotion Change : 2.68% | 57237405.3333 -> 58770504.0 Revision Result N chromium@454054 57237405 +- 1067354 6 good chromium@454057 57762717 +- 998447 6 good chromium@454059 57448349 +- 1080994 6 good chromium@454060 58755144 +- 694903 6 bad <-- chromium@454066 58880072 +- 604226 6 bad chromium@454077 58838088 +- 980235 6 bad chromium@454099 58770504 +- 718719 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.dailymotion system_health.memory_mobile Debug Info https://chromeperf.appspot.com/buildbucket_job_status/8986206717238888784 Is this bisect wrong? https://chromeperf.appspot.com/bad_bisect?try_job_id=5085590590062592 | 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!
,
Mar 2 2017
+haraken, this is actually an interesting regression. mlamouri@ and I have been looking into this for a while. So there is definitely a regression in the blink_gc cagegory, which shows on different devices. So it's not noise. The regression is definitely in the oilpan total effective size, see for instance these charts: https://chromeperf.appspot.com/report?sid=3bb3bf51a3221505b09412715460db104d40f4b14d9c79af230ee6e3d53b5d07&start_rev=451269&end_rev=454289 The interesting part is that, by looking at the traces, there is no regression in the "allocated_objects_size". The entire difference before vs after the CL is in the "free_size" column. Essentially it looks like, after this CL, oilpan retains 700KB more stuff in its freelists without releasing that memory to the system. Can somebody in the oilpan team take a look to the traces and see if they can understand more? trace before: https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/trace-file-id_17-2017-03-01_15-03-52-49817.html trace after: https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/trace-file-id_17-2017-03-01_20-18-52-55589.html
,
Mar 2 2017
,
Mar 3 2017
My theory is that using the DOMInserted* and DOMRemoved* events has a significant overhead. I have a CL up that uses the C++ callbacks instead. Locally, it seems to make things slightly better. Let see if landing this will reverse the free_size increase.
,
Mar 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8e679697d635baa92d4927b2b4e838f125266af1 commit 8e679697d635baa92d4927b2b4e838f125266af1 Author: mlamouri <mlamouri@chromium.org> Date: Sat Mar 04 13:57:19 2017 Media Controls: do not use events but C++ callbacks for insertion/removal from document. For some reasons, the overhead of the events create some oilpan allocated free memory. It's not a memory leak but it increases the visible memory usage of Chrome for some perf bots. Using C++ callbacks resolve the issue and should be fairly similar as far as the functionality goes. BUG= 697980 Review-Url: https://codereview.chromium.org/2724983006 Cr-Commit-Position: refs/heads/master@{#454772} [modify] https://crrev.com/8e679697d635baa92d4927b2b4e838f125266af1/third_party/WebKit/Source/core/html/shadow/MediaControls.cpp [modify] https://crrev.com/8e679697d635baa92d4927b2b4e838f125266af1/third_party/WebKit/Source/core/html/shadow/MediaControls.h [modify] https://crrev.com/8e679697d635baa92d4927b2b4e838f125266af1/third_party/WebKit/Source/core/html/shadow/MediaControlsMediaEventListener.cpp [modify] https://crrev.com/8e679697d635baa92d4927b2b4e838f125266af1/third_party/WebKit/Source/core/html/shadow/MediaControlsOrientationLockDelegate.cpp
,
Mar 7 2017
Sorry, this bug slipped off my mind. I chatted with mlamouri offline. My best guess is that mlamouri's CL *by accident* changed an object allocation workload and thus changed the way Oilpan allocates things on the free list. The fact that the CL is not regression allocated_objects_size indicates that this is not a real regression. If that is the case, it's not really worth digging into the issue. Marking it as wontfix.
,
Mar 11 2017
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by sullivan@chromium.org
, Mar 2 2017