Adds gpu cache destroyed callback for each cache.
- By setting the destroyed callback, the browser can notify the GPU
process that the cache is no longer being used on the browser side
and hence safe to stage it for removal on the GPU side. Without this
the GPU process doesn't know when it can cull stale or unused
in-memory caches. Note that we cannot just cull them when they are no
longer used on the GPU side because it is possible for the browser to
create new GPU channels referencing the culled cache since the browser
does not know that they have been destroyed on the GPU side. This
results in the GPU using an empty in-memory cache that will never be
populated.
Bug: dawn:549
Change-Id: Ie4578a44518c4bdafff9466efe0c05b42c89f08d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3827706
Commit-Queue: Loko Kung <lokokung@google.com>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1036526}
diff --git a/components/viz/host/gpu_host_impl.cc b/components/viz/host/gpu_host_impl.cc
index 49364f4..d4085752 100644
--- a/components/viz/host/gpu_host_impl.cc
+++ b/components/viz/host/gpu_host_impl.cc
@@ -293,8 +293,11 @@
if (!cache) {
// Create the cache if necessary and save a reference.
cache = delegate_->GetGpuDiskCacheFactory()->Create(
- handle, base::BindRepeating(&GpuHostImpl::LoadedBlob,
- weak_ptr_factory_.GetWeakPtr()));
+ handle,
+ base::BindRepeating(&GpuHostImpl::LoadedBlob,
+ weak_ptr_factory_.GetWeakPtr()),
+ base::BindOnce(&GpuHostImpl::OnDiskCacheHandleDestoyed,
+ weak_ptr_factory_.GetWeakPtr()));
if (!cache) {
return;
}
@@ -441,6 +444,12 @@
}
}
+void GpuHostImpl::OnDiskCacheHandleDestoyed(
+ const gpu::GpuDiskCacheHandle& handle) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ gpu_service_remote_->OnDiskCacheHandleDestoyed(handle);
+}
+
void GpuHostImpl::OnChannelEstablished(
int client_id,
bool sync,
diff --git a/components/viz/host/gpu_host_impl.h b/components/viz/host/gpu_host_impl.h
index 21c91786..5226bd4 100644
--- a/components/viz/host/gpu_host_impl.h
+++ b/components/viz/host/gpu_host_impl.h
@@ -224,6 +224,7 @@
void LoadedBlob(const gpu::GpuDiskCacheHandle& handle,
const std::string& key,
const std::string& data);
+ void OnDiskCacheHandleDestoyed(const gpu::GpuDiskCacheHandle& handle);
void OnChannelEstablished(int client_id,
bool sync,
diff --git a/components/viz/host/host_gpu_memory_buffer_manager_unittest.cc b/components/viz/host/host_gpu_memory_buffer_manager_unittest.cc
index 6a21045d..c48e5d2 100644
--- a/components/viz/host/host_gpu_memory_buffer_manager_unittest.cc
+++ b/components/viz/host/host_gpu_memory_buffer_manager_unittest.cc
@@ -113,6 +113,8 @@
void SetChannelDiskCacheHandle(
int32_t client_id,
const gpu::GpuDiskCacheHandle& handle) override {}
+ void OnDiskCacheHandleDestoyed(
+ const gpu::GpuDiskCacheHandle& handle) override {}
void CloseChannel(int32_t client_id) override {}
#if BUILDFLAG(IS_CHROMEOS_ASH)
diff --git a/components/viz/service/gl/gpu_service_impl.cc b/components/viz/service/gl/gpu_service_impl.cc
index a8282560..4eab547 100644
--- a/components/viz/service/gl/gpu_service_impl.cc
+++ b/components/viz/service/gl/gpu_service_impl.cc
@@ -1065,6 +1065,17 @@
gpu_channel_manager_->SetChannelDiskCacheHandle(client_id, handle);
}
+void GpuServiceImpl::OnDiskCacheHandleDestoyed(
+ const gpu::GpuDiskCacheHandle& handle) {
+ if (!main_runner_->BelongsToCurrentThread()) {
+ main_runner_->PostTask(
+ FROM_HERE, base::BindOnce(&GpuServiceImpl::OnDiskCacheHandleDestoyed,
+ weak_ptr_, handle));
+ return;
+ }
+ gpu_channel_manager_->OnDiskCacheHandleDestoyed(handle);
+}
+
void GpuServiceImpl::CloseChannel(int32_t client_id) {
if (!main_runner_->BelongsToCurrentThread()) {
main_runner_->PostTask(
diff --git a/components/viz/service/gl/gpu_service_impl.h b/components/viz/service/gl/gpu_service_impl.h
index b36c537..899c4e89 100644
--- a/components/viz/service/gl/gpu_service_impl.h
+++ b/components/viz/service/gl/gpu_service_impl.h
@@ -147,6 +147,8 @@
void SetChannelDiskCacheHandle(
int32_t client_id,
const gpu::GpuDiskCacheHandle& handle) override;
+ void OnDiskCacheHandleDestoyed(
+ const gpu::GpuDiskCacheHandle& handle) override;
void CloseChannel(int32_t client_id) override;
#if BUILDFLAG(IS_CHROMEOS_ASH)
#if BUILDFLAG(USE_CHROMEOS_MEDIA_ACCELERATION)
diff --git a/content/browser/gpu/peak_gpu_memory_tracker_impl_browsertest.cc b/content/browser/gpu/peak_gpu_memory_tracker_impl_browsertest.cc
index 2527ec2..d7eb200 100644
--- a/content/browser/gpu/peak_gpu_memory_tracker_impl_browsertest.cc
+++ b/content/browser/gpu/peak_gpu_memory_tracker_impl_browsertest.cc
@@ -65,6 +65,8 @@
void SetChannelDiskCacheHandle(
int32_t client_id,
const gpu::GpuDiskCacheHandle& handle) override {}
+ void OnDiskCacheHandleDestoyed(
+ const gpu::GpuDiskCacheHandle& handle) override {}
void CloseChannel(int32_t client_id) override {}
#if BUILDFLAG(IS_CHROMEOS_ASH)
#if BUILDFLAG(USE_CHROMEOS_MEDIA_ACCELERATION)
diff --git a/gpu/ipc/host/gpu_disk_cache.cc b/gpu/ipc/host/gpu_disk_cache.cc
index 9fa3f72..2fe57e3 100644
--- a/gpu/ipc/host/gpu_disk_cache.cc
+++ b/gpu/ipc/host/gpu_disk_cache.cc
@@ -507,7 +507,8 @@
scoped_refptr<GpuDiskCache> GpuDiskCacheFactory::Create(
const GpuDiskCacheHandle& handle,
- const BlobLoadedForCacheCallback& blob_loaded_cb) {
+ const BlobLoadedForCacheCallback& blob_loaded_cb,
+ CacheDestroyedCallback cache_destroyed_cb) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
DCHECK(Get(handle) == nullptr);
@@ -515,20 +516,22 @@
if (it == handle_to_path_map_.end()) {
return nullptr;
}
- return GetOrCreateByPath(it->second,
- base::BindRepeating(blob_loaded_cb, handle));
+ return GetOrCreateByPath(
+ it->second, base::BindRepeating(blob_loaded_cb, handle),
+ base::BindOnce(std::move(cache_destroyed_cb), handle));
}
scoped_refptr<GpuDiskCache> GpuDiskCacheFactory::GetOrCreateByPath(
const base::FilePath& path,
- const GpuDiskCache::BlobLoadedCallback& blob_loaded_cb) {
+ const GpuDiskCache::BlobLoadedCallback& blob_loaded_cb,
+ base::OnceClosure cache_destroyed_cb) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
auto iter = gpu_cache_map_.find(path);
if (iter != gpu_cache_map_.end())
return iter->second;
- auto cache =
- base::WrapRefCounted(new GpuDiskCache(this, path, blob_loaded_cb));
+ auto cache = base::WrapRefCounted(new GpuDiskCache(
+ this, path, blob_loaded_cb, std::move(cache_destroyed_cb)));
cache->Init();
return cache;
}
@@ -617,15 +620,18 @@
GpuDiskCache::GpuDiskCache(GpuDiskCacheFactory* factory,
const base::FilePath& cache_path,
- const BlobLoadedCallback& blob_loaded_cb)
+ const BlobLoadedCallback& blob_loaded_cb,
+ base::OnceClosure cache_destroyed_cb)
: factory_(factory),
cache_path_(cache_path),
- blob_loaded_cb_(blob_loaded_cb) {
+ blob_loaded_cb_(blob_loaded_cb),
+ cache_destroyed_cb_(std::move(cache_destroyed_cb)) {
factory_->AddToCache(cache_path_, this);
}
GpuDiskCache::~GpuDiskCache() {
factory_->RemoveFromCache(cache_path_);
+ std::move(cache_destroyed_cb_).Run();
}
void GpuDiskCache::Init() {
diff --git a/gpu/ipc/host/gpu_disk_cache.h b/gpu/ipc/host/gpu_disk_cache.h
index cf8454a9..a4711c2 100644
--- a/gpu/ipc/host/gpu_disk_cache.h
+++ b/gpu/ipc/host/gpu_disk_cache.h
@@ -76,7 +76,8 @@
GpuDiskCache(GpuDiskCacheFactory* factory,
const base::FilePath& cache_path,
- const BlobLoadedCallback& blob_loaded_cb);
+ const BlobLoadedCallback& blob_loaded_cb,
+ base::OnceClosure cache_destroyed_cb);
~GpuDiskCache();
void Init();
@@ -94,6 +95,7 @@
net::CompletionOnceCallback available_callback_;
net::CompletionOnceCallback cache_complete_callback_;
BlobLoadedCallback blob_loaded_cb_;
+ base::OnceClosure cache_destroyed_cb_;
std::unique_ptr<disk_cache::Backend> backend_;
@@ -109,6 +111,8 @@
using HandleToPathMap = base::flat_map<GpuDiskCacheHandle, base::FilePath>;
using BlobLoadedForCacheCallback = base::RepeatingCallback<
void(const GpuDiskCacheHandle&, const std::string&, const std::string&)>;
+ using CacheDestroyedCallback =
+ base::OnceCallback<void(const GpuDiskCacheHandle&)>;
// Constructor allows passing in reserved handles and their corresponding
// paths.
@@ -158,7 +162,8 @@
// path.
scoped_refptr<GpuDiskCache> Create(
const GpuDiskCacheHandle& handle,
- const BlobLoadedForCacheCallback& blob_loaded_cb = base::DoNothing());
+ const BlobLoadedForCacheCallback& blob_loaded_cb = base::DoNothing(),
+ CacheDestroyedCallback cache_destroyed_cb = base::DoNothing());
// Set the provided |cache| into the cache map for the given |path|.
void AddToCache(const base::FilePath& path, GpuDiskCache* cache);
@@ -172,7 +177,8 @@
scoped_refptr<GpuDiskCache> GetOrCreateByPath(
const base::FilePath& path,
const GpuDiskCache::BlobLoadedCallback& blob_loaded_cb =
- base::DoNothing());
+ base::DoNothing(),
+ base::OnceClosure cache_destroyed_cb = base::DoNothing());
void CacheCleared(GpuDiskCache* cache);
diff --git a/gpu/ipc/host/gpu_disk_cache_unittest.cc b/gpu/ipc/host/gpu_disk_cache_unittest.cc
index bf8c200..154db87 100644
--- a/gpu/ipc/host/gpu_disk_cache_unittest.cc
+++ b/gpu/ipc/host/gpu_disk_cache_unittest.cc
@@ -181,4 +181,55 @@
}
}
+TEST_F(GpuDiskCacheTest, DestroyedCallbackCalledOneInstance) {
+ InitCache();
+
+ // Create a cache with a destroy callback set.
+ bool destroyed = false;
+ base::RunLoop run_loop;
+ {
+ scoped_refptr<GpuDiskCache> cache = factory()->Create(
+ handle_, base::DoNothing(),
+ base::BindLambdaForTesting(
+ [&destroyed, &run_loop](const GpuDiskCacheHandle&) {
+ destroyed = true;
+ run_loop.Quit();
+ }));
+ ASSERT_TRUE(cache.get() != nullptr);
+ }
+ // Destroying the last and only reference to the cache should cause the
+ // callback to run.
+ run_loop.Run();
+ EXPECT_TRUE(destroyed);
+}
+
+TEST_F(GpuDiskCacheTest, DestroyedCallbackCalledMultipleInstance) {
+ InitCache();
+
+ // Create a cache with a destroy callback set.
+ bool destroyed = false;
+ base::RunLoop run_loop;
+ scoped_refptr<GpuDiskCache> cache_1 =
+ factory()->Create(handle_, base::DoNothing(),
+ base::BindLambdaForTesting(
+ [&destroyed, &run_loop](const GpuDiskCacheHandle&) {
+ destroyed = true;
+ run_loop.Quit();
+ }));
+ ASSERT_TRUE(cache_1.get() != nullptr);
+
+ // Get another instance of the same cache.
+ scoped_refptr<GpuDiskCache> cache_2 = factory()->Get(handle_);
+ ASSERT_TRUE(cache_2.get() == cache_1.get());
+
+ // Destroying one of the references should not trigger the callback.
+ cache_1 = nullptr;
+ EXPECT_FALSE(destroyed);
+
+ // Destroying the last reference should trigger the callback.
+ cache_2 = nullptr;
+ run_loop.Run();
+ EXPECT_TRUE(destroyed);
+}
+
} // namespace gpu
diff --git a/gpu/ipc/service/gpu_channel_manager.cc b/gpu/ipc/service/gpu_channel_manager.cc
index a1faf51..a835c5c 100644
--- a/gpu/ipc/service/gpu_channel_manager.cc
+++ b/gpu/ipc/service/gpu_channel_manager.cc
@@ -515,6 +515,25 @@
}
}
+void GpuChannelManager::OnDiskCacheHandleDestoyed(
+ const gpu::GpuDiskCacheHandle& handle) {
+ DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+ switch (gpu::GetHandleType(handle)) {
+ case gpu::GpuDiskCacheType::kGlShaders: {
+ // Currently there isn't any handling necessary for when the disk cache is
+ // destroyed for the shader cache because it consists of just 2 massive
+ // caches that are long-living and shared across all channels (i.e.
+ // unfortunately there is currently no access partitioning for it w.r.t
+ // different handles).
+ break;
+ }
+ case gpu::GpuDiskCacheType::kDawnWebGPU: {
+ // TODO(dawn:549) Implement cache destruction for Dawn.
+ break;
+ }
+ }
+}
+
void GpuChannelManager::InternalDestroyGpuMemoryBuffer(
gfx::GpuMemoryBufferId id,
int client_id) {
diff --git a/gpu/ipc/service/gpu_channel_manager.h b/gpu/ipc/service/gpu_channel_manager.h
index 00db0475..136207a0 100644
--- a/gpu/ipc/service/gpu_channel_manager.h
+++ b/gpu/ipc/service/gpu_channel_manager.h
@@ -120,6 +120,7 @@
void SetChannelClientPid(int client_id, base::ProcessId client_pid);
void SetChannelDiskCacheHandle(int client_id,
const gpu::GpuDiskCacheHandle& handle);
+ void OnDiskCacheHandleDestoyed(const gpu::GpuDiskCacheHandle& handle);
void PopulateCache(const gpu::GpuDiskCacheHandle& handle,
const std::string& key,
diff --git a/services/viz/privileged/mojom/gl/gpu_service.mojom b/services/viz/privileged/mojom/gl/gpu_service.mojom
index d1b3582..b7d60ff 100644
--- a/services/viz/privileged/mojom/gl/gpu_service.mojom
+++ b/services/viz/privileged/mojom/gl/gpu_service.mojom
@@ -65,6 +65,10 @@
SetChannelDiskCacheHandle(int32 client_id,
gpu.mojom.GpuDiskCacheHandle cache_handle);
+ // Called by the browser when the last reference to a GPU disk cache handle
+ // is gone. Results in the GPU process purging the in memory copy.
+ OnDiskCacheHandleDestoyed(gpu.mojom.GpuDiskCacheHandle cache_handle);
+
// Tells the GPU process to close the channel identified by |client_id|.
// If no channel can be identified, do nothing.
CloseChannel(int32 client_id);
