Issue metadata
Sign in to add a comment
|
1.3%-36.6% regression in memory.top_10_mobile_stress at 460138:460215 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Mar 30 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8983672535679758736
,
Mar 31 2017
=== Auto-CCing suspected CL author skobes@chromium.org === Hi skobes@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 : skobes Commit : 56451775d90238d9def5e9663771de8d0ca7ef6f Date : Tue Mar 28 18:46:38 2017 Subject: Feed ScrollableArea::showOverlayScrollbars into ScrollbarAnimationController. Bisect Details Configuration: android_nexus5_perf_bisect Benchmark : memory.top_10_mobile_stress Metric : memory:chrome:all_processes:reported_by_chrome:cc:effective_size_avg/foreground/https_mobile_twitter_com_justinbieber_skip_interstitial_true Change : 36.20% | 45108713.3333 -> 61438100.0 Revision Result N chromium@460170 45108713 +- 7326952 6 good chromium@460176 45436393 +- 5710038 6 good chromium@460177 46588735 +- 14353313 6 good chromium@460178 60203839 +- 3829980 6 bad <-- chromium@460179 58925887 +- 10065022 6 bad chromium@460182 61492713 +- 5579933 6 bad chromium@460193 59898004 +- 3255761 6 bad chromium@460215 61438100 +- 5978342 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-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests memory.top_10_mobile_stress Debug Info https://chromeperf.appspot.com/buildbucket_job_status/8983672535679758736 Is this bisect wrong? https://chromeperf.appspot.com/bad_bisect?try_job_id=6624879371091968 | 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!
,
Apr 2 2017
,
Apr 12 2017
aelias: I am stumped by this bug. I can repro the regression locally and it disappears if I comment out the body of ScrollbarAnimationController::DidRequestShowFromMainThread, but there are only a handful of calls to this method and I can't figure out how it's causing an extra ~20 MB of cc renderer memory for story "https_mobile_twitter_com_justinbieber_skip_interstitial_true". Do you have any suggestions?
,
Apr 12 2017
No specific idea, that's pretty weird. I'd suggest running memory tracing and examining what category the extra 20MB is accounted under, I might have a better idea when I have some clue to its nature.
,
Apr 12 2017
How do I see the trace data? When I load tools/perf/results.json in chrome://tracing it's just a blank window.
,
Apr 12 2017
,
Apr 12 2017
I found the trace (it writes an .html file in tools/perf with the story name). I see about twice as many 1.9MB resource_nn objects under cc > tile_memory > provider_0, versus baseline. The extra ones(?) have values in the "free_size" column that are equal to effective_size. Does this mean they are in some sort of intermediate state?
,
Apr 12 2017
Eric, what does it mean for a tile to be in "free_size"?
,
Apr 13 2017
"free_size" indicates that the tile is not currently in use, but is being cached in case we want to use it in the future (partial raster, relies on this, but also it just cuts down allocations, which can be expensive). Note that we very aggressively purge free tiles, so if something is showing up here, it means it has been used within the last 1 second (https://cs.chromium.org/chromium/src/cc/resources/resource_pool.cc?rcl=cceb830695c05acfa0c6a4fbcba8ecf8efb9551e&l=27). Not sure why your change is impacting this... I can take a quick look tomorrow to see if anything jumps out at me.
,
Apr 14 2017
Thanks ericrk, mind if I route this to you since it sounds like an issue with tile management? I can provide more context on the scrollbar changes if necessary.
,
Apr 14 2017
It appears that the change results in more rasterization, which means more tiles are used / kept alive. Still debugging, but can you think of any reason why this change would lead to additional invalidations or raster?
,
Apr 14 2017
ScrollbarAnimationController drives opacity animation on scrollbar layers, and r460178 added a new codepath to trigger the animation. I expected it to be a behavior nop, but maybe there's some edge case where we show scrollbars now and didn't before? Even if there is such a case, I don't think a scrollbar layer should consume 20MB of tile memory.
,
Apr 18 2017
,
Apr 21 2017
It looks like removing the ShowOverlayScrollbars() calls from both ScrollableArea::ContentsResized and FrameView::ViewportSizeChanged fixes this bug and issue 712453 . The first of these was already planned for issue 606395 . We could also remove the show trigger from ViewportSizeChanged, but only for Android. (Desktop should probably retain it for window resizing.) But in the meantime I'd like to land a patch that makes SAC::DidRequestShowFromMainThread a nop, so that we can merge it into 59.
,
Apr 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8a4d4531daca4b9a1f6dc1224bcb7134dc053ff4 commit 8a4d4531daca4b9a1f6dc1224bcb7134dc053ff4 Author: skobes <skobes@chromium.org> Date: Mon Apr 24 19:19:10 2017 Temporarily ignore ScrollbarAnimationController::DidRequestShowFromMainThread. BUG= 706927 , 712453 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2834703004 Cr-Commit-Position: refs/heads/master@{#466712} [modify] https://crrev.com/8a4d4531daca4b9a1f6dc1224bcb7134dc053ff4/cc/input/scrollbar_animation_controller.cc
,
Apr 24 2017
,
Apr 24 2017
Please add applicable OSs.
,
Apr 24 2017
,
Apr 24 2017
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dddc56692b829eb3d12e23d47e82549381557d06 commit dddc56692b829eb3d12e23d47e82549381557d06 Author: Steve Kobes <skobes@chromium.org> Date: Mon Apr 24 23:51:54 2017 Temporarily ignore ScrollbarAnimationController::DidRequestShowFromMainThread. BUG= 706927 , 712453 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2834703004 Cr-Commit-Position: refs/heads/master@{#466712} (cherry picked from commit 8a4d4531daca4b9a1f6dc1224bcb7134dc053ff4) Review-Url: https://codereview.chromium.org/2836243002 . Cr-Commit-Position: refs/branch-heads/3071@{#184} Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641} [modify] https://crrev.com/dddc56692b829eb3d12e23d47e82549381557d06/cc/input/scrollbar_animation_controller.cc
,
Apr 26 2017
It turns out r466712 didn't actually move the graphs from this issue or issue 712453 . The first_gesture_scroll_update_latency metric actually returned to its original levels between 466539 - 466552 (though nothing in the range looks likely): https://chromeperf.appspot.com/report?sid=97e645b929da551dde7f530a4424d4f81dcd6ff1bc8eec73ed76f0acddaba156 The cc memory regression got better and then worse and then even worse and then better and then worse again for some other reasons: https://chromeperf.appspot.com/report?sid=b2bf4017eb609d82437a3edb8f1f723fcdfe51849c8187e2c29a92d5f5eaa228&start_rev=443961&end_rev=467113 There's also issue 715279 which is somewhat mysterious.
,
Apr 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/66aacfa671360982f234c0fbd3b186efe21ee763 commit 66aacfa671360982f234c0fbd3b186efe21ee763 Author: skobes <skobes@chromium.org> Date: Wed Apr 26 23:40:39 2017 Re-enable ScrollbarAnimationController::DidRequestShowFromMainThread. Disabling it didn't fix http://crbug.com/706927 , but caused a new regression in http://crbug.com/715279 , while http://crbug.com/712453 recovered for some other reason. BUG= 606395 , 706927 , 712453 , 715279 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2834393003 Cr-Commit-Position: refs/heads/master@{#467513} [modify] https://crrev.com/66aacfa671360982f234c0fbd3b186efe21ee763/cc/input/scrollbar_animation_controller.cc
,
May 23 2017
The cc memory graph seems to have partially recovered: https://chromeperf.appspot.com/report?sid=b2bf4017eb609d82437a3edb8f1f723fcdfe51849c8187e2c29a92d5f5eaa228&start_rev=443961&end_rev=473864 I'm closing this since I'm not sure what else to do here.
,
May 23 2017
|
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by lanwei@chromium.org
, Mar 30 2017