New issue
Advanced search Search tips

Issue 904515 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

RasterDecoderImpl should delete its entries from ServiceTransferCache on destruction

Project Member Reported by piman@chromium.org, Nov 12

Issue description

ServiceTransferCache is shared across all RasterDecoderImpl from multiple clients. It is possible that a client would lock an entry, and then die (e.g. crash) or simply not clean up before destroying its RasterImplementation. Unless I missed something, if that happens, the ServiceTransferCache will hold on to its entries, including locked ones that will never be reaped, leaking memory.

Should we add a ServiceTransferCache::DeleteAllEntriesFromDecoder(decoder_id), called from ~RasterDecoderImpl?
 
Yup, Its possible. We should add that for ServiceTransferCache and ServiceDiscardableManager?
For ServiceDiscardableManager it probably already happens, the texture is removed from the SDM when it's deleted (implicit when the context group goes away, see TextureManager::StopTracking).
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 14

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

commit 57d0e2c16dc0723dba1f26b81fdafbb649c75a48
Author: Khushal <khushalsagar@chromium.org>
Date: Wed Nov 14 02:02:33 2018

gpu: Cleanup transfer cache entries on raster decoder destruction.

If an entry is in locked state when the renderer dies, it will remain in
this state and will never be evicted from the transfer cache in the GPU
process. Avoid this by deleting all entries for a decoder when it dies.
This is in general good for memory use by avoiding keeping unlocked
entries that will never be reused.

R=piman@chromium.org

Bug:  904515 
Change-Id: I4d97d31c7d9ae9d4d51814a874023264a2b06df4
Reviewed-on: https://chromium-review.googlesource.com/c/1334651
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607840}
[modify] https://crrev.com/57d0e2c16dc0723dba1f26b81fdafbb649c75a48/gpu/command_buffer/service/raster_decoder.cc
[modify] https://crrev.com/57d0e2c16dc0723dba1f26b81fdafbb649c75a48/gpu/command_buffer/service/service_transfer_cache.cc
[modify] https://crrev.com/57d0e2c16dc0723dba1f26b81fdafbb649c75a48/gpu/command_buffer/service/service_transfer_cache.h
[modify] https://crrev.com/57d0e2c16dc0723dba1f26b81fdafbb649c75a48/gpu/command_buffer/service/service_transfer_cache_unittest.cc

Status: Fixed (was: Untriaged)

Sign in to add a comment