New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 860506 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 10
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug-Regression


Show other hotlists

Hotlists containing this issue:
Hotlist-1


Sign in to add a comment

1564.1% regression in memory.top_10_mobile at 1530676910:1530688180

Project Member Reported by perezju@google.com, Jul 5

Issue description

See the link to graphs below.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=860506

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=00a6af843f90f1855c55d2d801ee114af580199319b9f8f8846d1ab7da2ccd1a


Bot(s) for this bug's original alert(s):

perf-go-phone-1024
Labels: -Pri-2 ReleaseBlock-Beta Pri-1
Labels: OS-Android
Cc: khushals...@chromium.org
Owner: khushals...@chromium.org
Status: Assigned (was: Untriaged)

=== 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
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.
Revert landed as:
https://chromium.googlesource.com/chromium/src/+/2200b6d949dc8b19a65c3e1a0753479ee68d3d65

Waiting for it to cycle through bots ...
Cc: primiano@google.com
 Issue 860742  has been merged into this issue.
 Issue 860744  has been merged into this issue.
 Issue 860743  has been merged into this issue.
 Issue 860745  has been merged into this issue.
 Issue 860747  has been merged into this issue.
 Issue 860752  has been merged into this issue.
 Issue 860756  has been merged into this issue.
Issue 856557 has been merged into this issue.
Labels: -Pri-1 -ReleaseBlock-Beta Pri-2
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.
Thanks for the revert perezju@. I'll diagnose this regression before re-landing the change.
Cc: ericrk@chromium.org
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.
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).
Project Member

Comment 20 by bugdroid1@chromium.org, 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

Status: Fixed (was: Assigned)
Closing this now, the reland did not have any regressions.

Sign in to add a comment