New issue
Advanced search Search tips

Issue 859419 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Task



Sign in to add a comment

Implement Ganesh cache purging logic in the GPU process for OOP raster.

Project Member Reported by khushals...@chromium.org, Jul 2

Issue description

For GPU raster the ContextCacheController in the renderer process purges GPU resources held by the GrContext with 2 strategies:

1) Visibility: It keeps a count of visible clients (compositors) using a Context and once there are no visible clients, the cache is purged.

2) Busy notifications: Whenever the context lock is acquired, the context is marked busy and a task with a 1 second delay is posted to purge the cache. The aim is to use a second of inactivity as an indicator for idle period and purge the cache once a context becomes idle.

Since we're moving towards sharing a single context across renderers in the GPU, per renderer visibility for cache clearing doesn't make sense anymore. We should probably purge this cache when the app becomes invisible/backgrounded though.

Also analogous to the busy notification logic, we should probably purge this cache in the GPU after a period of inactivity from any raster decoder that supports OOP raster. ericrk@, did I miss any detail here?
 
Owner: khushals...@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2c905214c7eb050e574c337751d1d156c8ba1f27

commit 2c905214c7eb050e574c337751d1d156c8ba1f27
Author: Khushal <khushalsagar@chromium.org>
Date: Mon Jul 09 22:11:46 2018

gpu: Add memory tracing for GrContext in GPU process.

Include memory dump from GrContext used for OOP raster in the GPU
process in traces.

R=bsalomon@chromium.org, ericrk@chromium.org, sadrul@chromium.org
TBR=bsalomon@google.com

Bug:  859419 
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: Ie010e1c567626343d5698b02c45776d215640da0
Reviewed-on: https://chromium-review.googlesource.com/1123894
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Eric Karl <ericrk@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573479}
[modify] https://crrev.com/2c905214c7eb050e574c337751d1d156c8ba1f27/gpu/command_buffer/common/BUILD.gn
[modify] https://crrev.com/2c905214c7eb050e574c337751d1d156c8ba1f27/gpu/command_buffer/common/DEPS
[modify] https://crrev.com/2c905214c7eb050e574c337751d1d156c8ba1f27/gpu/command_buffer/common/skia_utils.cc
[modify] https://crrev.com/2c905214c7eb050e574c337751d1d156c8ba1f27/gpu/command_buffer/common/skia_utils.h
[modify] https://crrev.com/2c905214c7eb050e574c337751d1d156c8ba1f27/gpu/command_buffer/service/raster_decoder_context_state.cc
[modify] https://crrev.com/2c905214c7eb050e574c337751d1d156c8ba1f27/gpu/command_buffer/service/raster_decoder_context_state.h
[modify] https://crrev.com/2c905214c7eb050e574c337751d1d156c8ba1f27/services/ui/public/cpp/gpu/context_provider_command_buffer.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Jul 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/81a71217973e595b574783c2496629311051450f

commit 81a71217973e595b574783c2496629311051450f
Author: Khushal <khushalsagar@chromium.org>
Date: Mon Jul 09 22:48:23 2018

gpu: Add memory tracing for ServiceTransferCache.

Add memory dumps for ServiceTransferCache. Note that this is expected to
come up as a regression in memory reported by the GPU process since this
data was not being reported until now.

R=ericrk@chromium.org

Bug:  859419 
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;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I03de3eef4ef688b62b4714897c1f301f98d63e74
Reviewed-on: https://chromium-review.googlesource.com/1125524
Reviewed-by: Eric Karl <ericrk@chromium.org>
Commit-Queue: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573496}
[modify] https://crrev.com/81a71217973e595b574783c2496629311051450f/cc/paint/image_transfer_cache_entry.cc
[modify] https://crrev.com/81a71217973e595b574783c2496629311051450f/cc/paint/image_transfer_cache_entry.h
[modify] https://crrev.com/81a71217973e595b574783c2496629311051450f/gpu/command_buffer/service/service_transfer_cache.cc
[modify] https://crrev.com/81a71217973e595b574783c2496629311051450f/gpu/command_buffer/service/service_transfer_cache.h

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cbaac9b2e020db5321282bb4397fe9acbc65b891

commit cbaac9b2e020db5321282bb4397fe9acbc65b891
Author: Khushal <khushalsagar@chromium.org>
Date: Tue Jul 10 06:00:45 2018

gpu: Drop GrContext cache when application is backgrounded.

This matches the behaviour in the renderer which drops the cache once
all compositors are invisible. Since we have a shared context across all
renderers now, per-renderer visibility doesn't make apply anymore. So we
do this when the application is backgrounded.

Also this only does it for Android which plumbs background/foreground
notifications to the GPU. We need these notifications plumbed for
desktop too.

R=ericrk@chromium.org

Bug:  859419 
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: Ife08e37bc77e5fcf81ce5a79361b74c34ff9b7bb
Reviewed-on: https://chromium-review.googlesource.com/1123366
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: Eric Karl <ericrk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573629}
[modify] https://crrev.com/cbaac9b2e020db5321282bb4397fe9acbc65b891/components/viz/service/gl/gpu_service_impl.cc
[modify] https://crrev.com/cbaac9b2e020db5321282bb4397fe9acbc65b891/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/cbaac9b2e020db5321282bb4397fe9acbc65b891/gpu/command_buffer/service/raster_decoder_context_state.cc
[modify] https://crrev.com/cbaac9b2e020db5321282bb4397fe9acbc65b891/gpu/command_buffer/service/raster_decoder_context_state.h
[modify] https://crrev.com/cbaac9b2e020db5321282bb4397fe9acbc65b891/gpu/ipc/service/gpu_channel_manager.cc
[modify] https://crrev.com/cbaac9b2e020db5321282bb4397fe9acbc65b891/gpu/ipc/service/gpu_channel_manager.h
[modify] https://crrev.com/cbaac9b2e020db5321282bb4397fe9acbc65b891/gpu/ipc/service/gpu_channel_manager_unittest.cc

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/14ea54b8a40000
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7324ec4b07a7c91aec1fb4e65d2c34964c4a8ab4

commit 7324ec4b07a7c91aec1fb4e65d2c34964c4a8ab4
Author: Khushal <khushalsagar@chromium.org>
Date: Tue Jul 10 20:01:45 2018

gpu: Drop GrContext cache after an idle period.

The renderer drops the GrContext cache after a second of idle period,
where idle period is identified as a continous period where the
ContextProvider backing a GrContext is not used. Implement the same
optimization for OOP raster in the GPU process.

R=ericrk@chromium.org

Bug:  859419 , 859635 
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: Ic2ae533212b94320cd8c1b8b1149cf5ba4ce686e
Reviewed-on: https://chromium-review.googlesource.com/1123775
Reviewed-by: Eric Karl <ericrk@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573871}
[modify] https://crrev.com/7324ec4b07a7c91aec1fb4e65d2c34964c4a8ab4/gpu/BUILD.gn
[modify] https://crrev.com/7324ec4b07a7c91aec1fb4e65d2c34964c4a8ab4/gpu/command_buffer/service/BUILD.gn
[modify] https://crrev.com/7324ec4b07a7c91aec1fb4e65d2c34964c4a8ab4/gpu/command_buffer/service/command_buffer_direct.h
[modify] https://crrev.com/7324ec4b07a7c91aec1fb4e65d2c34964c4a8ab4/gpu/command_buffer/service/decoder_client.h
[modify] https://crrev.com/7324ec4b07a7c91aec1fb4e65d2c34964c4a8ab4/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.h
[add] https://crrev.com/7324ec4b07a7c91aec1fb4e65d2c34964c4a8ab4/gpu/command_buffer/service/gr_cache_controller.cc
[add] https://crrev.com/7324ec4b07a7c91aec1fb4e65d2c34964c4a8ab4/gpu/command_buffer/service/gr_cache_controller.h
[add] https://crrev.com/7324ec4b07a7c91aec1fb4e65d2c34964c4a8ab4/gpu/command_buffer/service/gr_cache_controller_unittest.cc
[modify] https://crrev.com/7324ec4b07a7c91aec1fb4e65d2c34964c4a8ab4/gpu/command_buffer/service/memory_program_cache_unittest.cc
[modify] https://crrev.com/7324ec4b07a7c91aec1fb4e65d2c34964c4a8ab4/gpu/command_buffer/service/program_manager_unittest.cc
[modify] https://crrev.com/7324ec4b07a7c91aec1fb4e65d2c34964c4a8ab4/gpu/command_buffer/service/raster_decoder.cc
[modify] https://crrev.com/7324ec4b07a7c91aec1fb4e65d2c34964c4a8ab4/gpu/command_buffer/service/raster_decoder_unittest.cc
[modify] https://crrev.com/7324ec4b07a7c91aec1fb4e65d2c34964c4a8ab4/gpu/command_buffer/service/raster_decoder_unittest_base.h
[modify] https://crrev.com/7324ec4b07a7c91aec1fb4e65d2c34964c4a8ab4/gpu/ipc/in_process_command_buffer.cc
[modify] https://crrev.com/7324ec4b07a7c91aec1fb4e65d2c34964c4a8ab4/gpu/ipc/in_process_command_buffer.h
[modify] https://crrev.com/7324ec4b07a7c91aec1fb4e65d2c34964c4a8ab4/gpu/ipc/service/command_buffer_stub.cc
[modify] https://crrev.com/7324ec4b07a7c91aec1fb4e65d2c34964c4a8ab4/gpu/ipc/service/command_buffer_stub.h
[modify] https://crrev.com/7324ec4b07a7c91aec1fb4e65d2c34964c4a8ab4/gpu/ipc/service/gpu_channel_manager.cc
[modify] https://crrev.com/7324ec4b07a7c91aec1fb4e65d2c34964c4a8ab4/gpu/ipc/service/gpu_channel_manager.h

Status: Fixed (was: Assigned)
Project Member

Comment 9 by bugdroid1@chromium.org, Jul 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/441bb2d8704de8d4ae008d2fc212674d8b6d5670

commit 441bb2d8704de8d4ae008d2fc212674d8b6d5670
Author: Khushal <khushalsagar@chromium.org>
Date: Wed Jul 11 17:42:39 2018

gpu: Follow up on GrContext idle cleanup patch.

Follow up for remaining comments on:
https://chromium-review.googlesource.com/c/chromium/src/+/1123775

R=ericrk@chromium.org

Bug:  859419 , 859635 
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: I78577f183d4050d6380024dc15aad2573c24714e
Reviewed-on: https://chromium-review.googlesource.com/1132350
Reviewed-by: Eric Karl <ericrk@chromium.org>
Commit-Queue: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574228}
[modify] https://crrev.com/441bb2d8704de8d4ae008d2fc212674d8b6d5670/gpu/command_buffer/service/gr_cache_controller.cc
[modify] https://crrev.com/441bb2d8704de8d4ae008d2fc212674d8b6d5670/gpu/ipc/service/gpu_channel_manager.cc

Sign in to add a comment