Issue metadata
Sign in to add a comment
|
16%-24.1% regression in memory.top_10_mobile_stress at 415482:415528 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Sep 1 2016
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/9002705132967091328
,
Sep 2 2016
=== Auto-CCing suspected CL author ericrk@chromium.org === Hi ericrk@chromium.org, the bisect results pointed to your CL below as possibly causing a regression. Please have a look at this info and see whether your CL be related. ===== BISECT JOB RESULTS ===== Status: completed ===== SUSPECTED CL(s) ===== Subject : Refactor client visibility handling Author : ericrk Commit description: Currently, ContextSupport::SetClientVisibile relies on the caller remembering to pair visible/not-visible calls. Failure to do so will result in a "leak", where the context support will always think there are visible clients. To avoid this, visibility handling now uses ScopedVisibility objects which will dcheck if visible > not visible updates are not paired. This change also introduces a new ContextCacheController object which handles trimming context caches based on visibility. This simplifies handling in clients of the ContextProvider and removes this logic from ContextSupport. CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel Review-Url: https://codereview.chromium.org/2278283003 Cr-Commit-Position: refs/heads/master@{#415491} Commit : f6c4fb6b0b77f35f094cb73c49b0bfe22d3f4461 Date : Tue Aug 30 23:29:09 2016 ===== TESTED REVISIONS ===== Revision Mean Std Dev N Good? chromium@415481 71242875 6420639 5 good chromium@415487 71290060 4851116 5 good chromium@415490 73733243 6050343 5 good chromium@415491 90772603 3063128 5 bad <-- chromium@415492 91102904 5820671 5 bad chromium@415493 93205299 4658978 5 bad chromium@415505 92418867 4450739 5 bad chromium@415528 89194496 3437387 5 bad Bisect job ran on: android_nexus9_perf_bisect Bug ID: 643251 Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests memory.top_10_mobile_stress Test Metric: background-memory:chrome:all_processes:reported_by_os:gpu_memory:proportional_resident_size_avg/after_http_www_baidu_com_s_word_google Relative Change: 25.20% Score: 99.5 Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus9_perf_bisect/builds/2080 Job details: https://chromeperf.appspot.com/buildbucket_job_status/9002705132967091328 Not what you expected? We'll investigate and get back to you! https://chromeperf.appspot.com/bad_bisect?try_job_id=5777928170766336 | 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 Tests>AutoBisect. Thank you!
,
Sep 9 2016
ericrk, could you comment on whether the bisect above seems correct to blame your CL as the source of the regression?
,
Sep 13 2016
Sorry, am OOO until the 19th. It makes sense that my CL might cause memory regressions, but should only do so in cases where we were incorrectly freeing memory before (and thrashing caches). I'll need to run the test and check whether this regression is good - we were clearing things incorrectly before - or if my CL is causing us to keep resources where we shouldn't. I'll prioritize this once I get back.
,
Sep 20 2016
,
Sep 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d3bd7f0bacd5e2141ddb9f08335a9157da367b14 commit d3bd7f0bacd5e2141ddb9f08335a9157da367b14 Author: ericrk <ericrk@chromium.org> Date: Tue Sep 20 22:29:01 2016 Flush contexts after LayerTreeHostImpl cleanup Currently, we do not flush our GL contexts after cleaning up LayerTreeHostImpl (LTHI) resources. This delays the time it takes for these deletes to reach the GPU process, and can allow driver caches to grow unnecessarily. This CL does a ShallowFlushCHROMIUM after freeing LTHI resources, which reduces cache growth on some platforms (observed on N9). R=vmpstr BUG= 643251 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel Review-Url: https://codereview.chromium.org/2358593002 Cr-Commit-Position: refs/heads/master@{#419874} [modify] https://crrev.com/d3bd7f0bacd5e2141ddb9f08335a9157da367b14/cc/trees/layer_tree_host_impl.cc
,
Sep 21 2016
With the CL in #7, the metric has recovered. Closing out.
,
Sep 21 2016
The CL which caused this regression has been merged to 54 - requesting merge for the fix.
,
Sep 21 2016
,
Sep 21 2016
Your change meets the bar and is auto-approved for M54 (branch: 2840)
,
Sep 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c850e8a84cb5c6effd3d12ab76d85ac5194e3187 commit c850e8a84cb5c6effd3d12ab76d85ac5194e3187 Author: Eric Karl <ericrk@chromium.org> Date: Thu Sep 22 19:38:40 2016 Flush contexts after LayerTreeHostImpl cleanup Currently, we do not flush our GL contexts after cleaning up LayerTreeHostImpl (LTHI) resources. This delays the time it takes for these deletes to reach the GPU process, and can allow driver caches to grow unnecessarily. This CL does a ShallowFlushCHROMIUM after freeing LTHI resources, which reduces cache growth on some platforms (observed on N9). R=vmpstr BUG= 643251 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel Review-Url: https://codereview.chromium.org/2358593002 Cr-Commit-Position: refs/heads/master@{#419874} (cherry picked from commit d3bd7f0bacd5e2141ddb9f08335a9157da367b14) Review URL: https://codereview.chromium.org/2361853003 . Cr-Commit-Position: refs/branch-heads/2840@{#494} Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607} [modify] https://crrev.com/c850e8a84cb5c6effd3d12ab76d85ac5194e3187/cc/trees/layer_tree_host_impl.cc
,
Sep 22 2016
,
Sep 23 2016
,
Oct 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c850e8a84cb5c6effd3d12ab76d85ac5194e3187 commit c850e8a84cb5c6effd3d12ab76d85ac5194e3187 Author: Eric Karl <ericrk@chromium.org> Date: Thu Sep 22 19:38:40 2016 Flush contexts after LayerTreeHostImpl cleanup Currently, we do not flush our GL contexts after cleaning up LayerTreeHostImpl (LTHI) resources. This delays the time it takes for these deletes to reach the GPU process, and can allow driver caches to grow unnecessarily. This CL does a ShallowFlushCHROMIUM after freeing LTHI resources, which reduces cache growth on some platforms (observed on N9). R=vmpstr BUG= 643251 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel Review-Url: https://codereview.chromium.org/2358593002 Cr-Commit-Position: refs/heads/master@{#419874} (cherry picked from commit d3bd7f0bacd5e2141ddb9f08335a9157da367b14) Review URL: https://codereview.chromium.org/2361853003 . Cr-Commit-Position: refs/branch-heads/2840@{#494} Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607} [modify] https://crrev.com/c850e8a84cb5c6effd3d12ab76d85ac5194e3187/cc/trees/layer_tree_host_impl.cc |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by mustaq@chromium.org
, Sep 1 2016