Issue metadata
Sign in to add a comment
|
1564.1% regression in memory.top_10_mobile at 1530676910:1530688180 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Jul 5
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8941831112310978784
,
Jul 5
,
Jul 5
,
Jul 6
=== Auto-CCing suspected CL author khushalsagar@chromium.org === Hi khushalsagar@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 : Khushal Commit : db2d4ad73d521275dd1c97a7ebc8effce024f948 Date : Tue Jul 03 22:36:27 2018 Subject: gpu: Share GrContext between RasterDecoders for OOP raster. Bisect Details Configuration: go-phone-1024-perf-bisect Benchmark : memory.top_10_mobile Metric : memory:chrome:all_processes:reported_by_os:gpu_memory:proportional_resident_size_avg/background/after_http_en_m_wikipedia_org_wiki_Science Change : 1573.21% | 570709.333333 -> 9549141.33333 Revision Result N android-chrome@99480dfb2c 570709 +- 8848.37 6 good android-chrome@878f534e9f 571392 +- 4295.92 6 good android-chrome@878f534e9f,chromium@572347 574123 +- 7949.48 6 good android-chrome@878f534e9f,chromium@572376 570027 +- 4027.15 6 good android-chrome@878f534e9f,chromium@572377 9543680 +- 5792.62 6 bad <-- android-chrome@878f534e9f,chromium@572378 9549141 +- 10253.6 6 bad android-chrome@878f534e9f,chromium@572380 9548459 +- 7054.96 6 bad android-chrome@878f534e9f,chromium@572383 9548459 +- 3083.36 6 bad android-chrome@878f534e9f,chromium@572390 9545728 +- 5645.95 6 bad android-chrome@4d11ebe3cf 9542997 +- 9128.35 6 bad android-chrome@22df6667c5 9549141 +- 8054.31 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-chrome --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=http.en.m.wikipedia.org.wiki.Science memory.top_10_mobile More information on addressing performance regressions: http://g.co/ChromePerformanceRegressions Debug information about this bisect: https://chromeperf.appspot.com/buildbucket_job_status/8941831112310978784 For feedback, file a bug with component Speed>Bisection
,
Jul 6
There is a strong suspicion that this CL is causing a large memory leak in Android, c.f. issue 856557. I'm going to speculatively revert r572377 to see if this fixes the leak.
,
Jul 6
Revert landed as: https://chromium.googlesource.com/chromium/src/+/2200b6d949dc8b19a65c3e1a0753479ee68d3d65 Waiting for it to cycle through bots ...
,
Jul 6
,
Jul 6
Issue 860744 has been merged into this issue.
,
Jul 7
Issue 860743 has been merged into this issue.
,
Jul 7
Issue 860745 has been merged into this issue.
,
Jul 7
Issue 860747 has been merged into this issue.
,
Jul 7
Issue 860752 has been merged into this issue.
,
Jul 7
Issue 860756 has been merged into this issue.
,
Jul 9
Issue 856557 has been merged into this issue.
,
Jul 9
Confirmed the memory leak detected in issue 856557 was fixed by the revert in r572933. Before trying to re-land it would be advisable to run some try jobs in http://pinpoint-dot-chromeperf.appspot.com Leaving this open with lower Pri in case you want to investigate further.
,
Jul 9
Thanks for the revert perezju@. I'll diagnose this regression before re-landing the change.
,
Jul 9
Hmmm, so looks like its all cases of low-end on background. There is an optimization for low-end Android that destroys the gpu channels from all renderers and through that releases all Gpu memory after x seconds of being in background. Since this change lifted the GrContext from 1:1 per channel to a shared context on the GpuChannelManager, that optimization isn't releasing this memory anymore. The solution is to simply release this shared context at the same spot where we're destroying the channels.
,
Jul 9
I wanted to do a pinpoint run to confirm on one of the bots that regressed, but its all ClankInternal android-go/low-end ones which are not available for tryjobs. I verified locally by simulating low-end mode on an Android device that the fix mentioned in #18 does address this. I'll still keep an eye on the bots as this relands (https://chromium-review.googlesource.com/c/chromium/src/+/1129548).
,
Jul 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b2c140b85bf6cb4aa81c1a24a0d140770b6f519c commit b2c140b85bf6cb4aa81c1a24a0d140770b6f519c Author: Khushal <khushalsagar@chromium.org> Date: Mon Jul 09 20:21:16 2018 Reland "gpu: Share GrContext between RasterDecoders for OOP raster." This reverts commit 2200b6d949dc8b19a65c3e1a0753479ee68d3d65. The change inadvertantly disabled an optimization for low-end Android devices that releases all GPU memory when the application has been backgrounded. The optimization works by releasing all non-WebGL GpuChannels. Since this change moved the GrContext ownership to a shared object cached in the GpuChannelManager instead of per GpuChannel, it is necessary to explicitly destroy it in the channel manager during cleanup. R=ericrk@chromium.org, piman@chromium.org Bug: 860506 , 854416 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel Change-Id: I3f6754ce939c3f815b13a3f96cd4414f5c8a3449 Reviewed-on: https://chromium-review.googlesource.com/1129548 Reviewed-by: Antoine Labour <piman@chromium.org> Commit-Queue: Khushal <khushalsagar@chromium.org> Cr-Commit-Position: refs/heads/master@{#573428} [modify] https://crrev.com/b2c140b85bf6cb4aa81c1a24a0d140770b6f519c/gpu/BUILD.gn [modify] https://crrev.com/b2c140b85bf6cb4aa81c1a24a0d140770b6f519c/gpu/command_buffer/service/BUILD.gn [modify] https://crrev.com/b2c140b85bf6cb4aa81c1a24a0d140770b6f519c/gpu/command_buffer/service/raster_decoder.cc [modify] https://crrev.com/b2c140b85bf6cb4aa81c1a24a0d140770b6f519c/gpu/command_buffer/service/raster_decoder.h [add] https://crrev.com/b2c140b85bf6cb4aa81c1a24a0d140770b6f519c/gpu/command_buffer/service/raster_decoder_context_state.cc [add] https://crrev.com/b2c140b85bf6cb4aa81c1a24a0d140770b6f519c/gpu/command_buffer/service/raster_decoder_context_state.h [modify] https://crrev.com/b2c140b85bf6cb4aa81c1a24a0d140770b6f519c/gpu/command_buffer/service/raster_decoder_unittest.cc [modify] https://crrev.com/b2c140b85bf6cb4aa81c1a24a0d140770b6f519c/gpu/command_buffer/service/raster_decoder_unittest_base.cc [modify] https://crrev.com/b2c140b85bf6cb4aa81c1a24a0d140770b6f519c/gpu/command_buffer/service/service_utils.cc [modify] https://crrev.com/b2c140b85bf6cb4aa81c1a24a0d140770b6f519c/gpu/command_buffer/service/service_utils.h [modify] https://crrev.com/b2c140b85bf6cb4aa81c1a24a0d140770b6f519c/gpu/command_buffer/tests/fuzzer_main.cc [modify] https://crrev.com/b2c140b85bf6cb4aa81c1a24a0d140770b6f519c/gpu/ipc/in_process_command_buffer.cc [modify] https://crrev.com/b2c140b85bf6cb4aa81c1a24a0d140770b6f519c/gpu/ipc/service/gpu_channel_manager.cc [modify] https://crrev.com/b2c140b85bf6cb4aa81c1a24a0d140770b6f519c/gpu/ipc/service/gpu_channel_manager.h [modify] https://crrev.com/b2c140b85bf6cb4aa81c1a24a0d140770b6f519c/gpu/ipc/service/gpu_channel_manager_unittest.cc [modify] https://crrev.com/b2c140b85bf6cb4aa81c1a24a0d140770b6f519c/gpu/ipc/service/raster_command_buffer_stub.cc
,
Jul 10
Closing this now, the reland did not have any regressions. |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Jul 5