Skip to content

Commit 971df0f

Browse files
committed
Revert "Reland gpu: Add disk caching for skia generated shaders in OOP-R."
This reverts commit c2667e3. Reason for revert: Only the first part made it to M69, without the subsequent patches this is just unnecessary overhead. Original change's description: > Reland gpu: Add disk caching for skia generated shaders in OOP-R. > > This reverts commit f9395d7. > > Use the GrContextOptions::PersistentCache API provided by skia to > persist shaders generated internally by skia for OOP raster to disk. > This requires using a special client id to namespace these shaders, > similar to the one used by the InProcessCommandBuffer for viz. > > While the shaders for different sources are stored seperately on disk, > they are finally merged into a single memory cache in the GPU process. In > order to maintain a seperate cache for skia generated shaders, this also > plumbs the client id for a loaded shader to the GPU process. > > [email protected] > > Bug: 854416,840559, 865138 > 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: I65544ccaff96c3154a822dbc2500468fbcac8a0b > Reviewed-on: https://chromium-review.googlesource.com/1142829 > Commit-Queue: Khushal <[email protected]> > Reviewed-by: Antoine Labour <[email protected]> > Cr-Commit-Position: refs/heads/master@{#576742} [email protected],[email protected],[email protected] # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 865138 Change-Id: Ia38b186475f802efb81d0764bf0a24350103f868 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 Reviewed-on: https://chromium-review.googlesource.com/1147041 Reviewed-by: Khushal <[email protected]> Cr-Commit-Position: refs/branch-heads/3497@{#61} Cr-Branched-From: 271eaf5-refs/heads/master@{#576753}
1 parent dba741d commit 971df0f

30 files changed

+49
-557
lines changed

components/viz/host/host_gpu_memory_buffer_manager_unittest.cc

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ class TestGpuService : public mojom::GpuService {
5858
void EstablishGpuChannel(int32_t client_id,
5959
uint64_t client_tracing_id,
6060
bool is_gpu_host,
61-
bool cache_shaders_on_disk,
6261
EstablishGpuChannelCallback callback) override {}
6362

6463
void CloseChannel(int32_t client_id) override {}
@@ -112,9 +111,7 @@ class TestGpuService : public mojom::GpuService {
112111

113112
void RequestHDRStatus(RequestHDRStatusCallback callback) override {}
114113

115-
void LoadedShader(int32_t client_id,
116-
const std::string& key,
117-
const std::string& data) override {}
114+
void LoadedShader(const std::string& key, const std::string& data) override {}
118115

119116
void WakeUpGpu() override {}
120117

components/viz/service/display_embedder/in_process_gpu_memory_buffer_manager.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
#include "components/viz/service/display_embedder/in_process_gpu_memory_buffer_manager.h"
66

77
#include "base/bind.h"
8-
#include "gpu/ipc/common/gpu_client_ids.h"
98
#include "gpu/ipc/common/gpu_memory_buffer_impl.h"
109
#include "gpu/ipc/common/gpu_memory_buffer_support.h"
1110
#include "gpu/ipc/in_process_command_buffer.h"
@@ -17,7 +16,7 @@ namespace viz {
1716
InProcessGpuMemoryBufferManager::InProcessGpuMemoryBufferManager(
1817
gpu::GpuChannelManager* channel_manager)
1918
: gpu_memory_buffer_support_(new gpu::GpuMemoryBufferSupport()),
20-
client_id_(gpu::kInProcessCommandBufferClientId),
19+
client_id_(gpu::InProcessCommandBuffer::kGpuClientId),
2120
channel_manager_(channel_manager),
2221
weak_factory_(this) {
2322
weak_ptr_ = weak_factory_.GetWeakPtr();

components/viz/service/gl/gpu_service_impl.cc

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
#include "gpu/config/gpu_info_collector.h"
2929
#include "gpu/config/gpu_switches.h"
3030
#include "gpu/config/gpu_util.h"
31-
#include "gpu/ipc/common/gpu_client_ids.h"
3231
#include "gpu/ipc/common/gpu_memory_buffer_support.h"
3332
#include "gpu/ipc/common/memory_stats.h"
3433
#include "gpu/ipc/in_process_command_buffer.h"
@@ -703,9 +702,8 @@ void GpuServiceImpl::SetActiveURL(const GURL& url) {
703702
void GpuServiceImpl::EstablishGpuChannel(int32_t client_id,
704703
uint64_t client_tracing_id,
705704
bool is_gpu_host,
706-
bool cache_shaders_on_disk,
707705
EstablishGpuChannelCallback callback) {
708-
if (gpu::IsReservedClientId(client_id)) {
706+
if (oopd_enabled_ && client_id == gpu::InProcessCommandBuffer::kGpuClientId) {
709707
std::move(callback).Run(mojo::ScopedMessagePipeHandle());
710708
return;
711709
}
@@ -719,15 +717,14 @@ void GpuServiceImpl::EstablishGpuChannel(int32_t client_id,
719717
},
720718
io_runner_, std::move(callback));
721719
main_runner_->PostTask(
722-
FROM_HERE,
723-
base::BindOnce(&GpuServiceImpl::EstablishGpuChannel, weak_ptr_,
724-
client_id, client_tracing_id, is_gpu_host,
725-
cache_shaders_on_disk, std::move(wrap_callback)));
720+
FROM_HERE, base::BindOnce(&GpuServiceImpl::EstablishGpuChannel,
721+
weak_ptr_, client_id, client_tracing_id,
722+
is_gpu_host, std::move(wrap_callback)));
726723
return;
727724
}
728725

729726
gpu::GpuChannel* gpu_channel = gpu_channel_manager_->EstablishChannel(
730-
client_id, client_tracing_id, is_gpu_host, cache_shaders_on_disk);
727+
client_id, client_tracing_id, is_gpu_host);
731728

732729
mojo::MessagePipe pipe;
733730
gpu_channel->Init(std::make_unique<gpu::SyncChannelFilteredSender>(
@@ -747,16 +744,14 @@ void GpuServiceImpl::CloseChannel(int32_t client_id) {
747744
gpu_channel_manager_->RemoveChannel(client_id);
748745
}
749746

750-
void GpuServiceImpl::LoadedShader(int32_t client_id,
751-
const std::string& key,
747+
void GpuServiceImpl::LoadedShader(const std::string& key,
752748
const std::string& data) {
753749
if (io_runner_->BelongsToCurrentThread()) {
754-
main_runner_->PostTask(FROM_HERE,
755-
base::Bind(&GpuServiceImpl::LoadedShader, weak_ptr_,
756-
client_id, key, data));
750+
main_runner_->PostTask(FROM_HERE, base::Bind(&GpuServiceImpl::LoadedShader,
751+
weak_ptr_, key, data));
757752
return;
758753
}
759-
gpu_channel_manager_->PopulateShaderCache(client_id, key, data);
754+
gpu_channel_manager_->PopulateShaderCache(key, data);
760755
}
761756

762757
void GpuServiceImpl::WakeUpGpu() {

components/viz/service/gl/gpu_service_impl.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,6 @@ class VIZ_SERVICE_EXPORT GpuServiceImpl : public gpu::GpuChannelManagerDelegate,
182182
void EstablishGpuChannel(int32_t client_id,
183183
uint64_t client_tracing_id,
184184
bool is_gpu_host,
185-
bool cache_shaders_on_disk,
186185
EstablishGpuChannelCallback callback) override;
187186
void CloseChannel(int32_t client_id) override;
188187
#if defined(OS_CHROMEOS)
@@ -218,9 +217,7 @@ class VIZ_SERVICE_EXPORT GpuServiceImpl : public gpu::GpuChannelManagerDelegate,
218217
void GetGpuSupportedRuntimeVersion(
219218
GetGpuSupportedRuntimeVersionCallback callback) override;
220219
void RequestHDRStatus(RequestHDRStatusCallback callback) override;
221-
void LoadedShader(int32_t client_id,
222-
const std::string& key,
223-
const std::string& data) override;
220+
void LoadedShader(const std::string& key, const std::string& data) override;
224221
void WakeUpGpu() override;
225222
void GpuSwitched() override;
226223
void DestroyAllChannels() override;

content/browser/gpu/browser_gpu_channel_host_factory.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
#include "content/public/common/content_client.h"
2929
#include "content/public/common/content_switches.h"
3030
#include "gpu/command_buffer/service/gpu_switches.h"
31-
#include "gpu/ipc/common/gpu_client_ids.h"
3231
#include "gpu/ipc/in_process_command_buffer.h"
3332
#include "services/resource_coordinator/public/mojom/memory_instrumentation/constants.mojom.h"
3433
#include "services/service_manager/runner/common/client_util.h"
@@ -387,7 +386,7 @@ void BrowserGpuChannelHostFactory::InitializeShaderDiskCacheOnIO(
387386
GetShaderCacheFactorySingleton()->SetCacheInfo(gpu_client_id, cache_dir);
388387
if (base::FeatureList::IsEnabled(features::kVizDisplayCompositor)) {
389388
GetShaderCacheFactorySingleton()->SetCacheInfo(
390-
gpu::kInProcessCommandBufferClientId, cache_dir);
389+
gpu::InProcessCommandBuffer::kGpuClientId, cache_dir);
391390
}
392391
}
393392

content/browser/gpu/gpu_process_host.cc

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@
6464
#include "gpu/config/gpu_driver_bug_list.h"
6565
#include "gpu/config/gpu_driver_bug_workaround_type.h"
6666
#include "gpu/config/gpu_preferences.h"
67-
#include "gpu/ipc/common/gpu_client_ids.h"
6867
#include "gpu/ipc/host/shader_disk_cache.h"
6968
#include "gpu/ipc/in_process_command_buffer.h"
7069
#include "media/base/media_switches.h"
@@ -997,7 +996,7 @@ void GpuProcessHost::EstablishGpuChannel(
997996

998997
bool oopd_enabled =
999998
base::FeatureList::IsEnabled(features::kVizDisplayCompositor);
1000-
if (oopd_enabled && client_id == gpu::kInProcessCommandBufferClientId) {
999+
if (oopd_enabled && client_id == gpu::InProcessCommandBuffer::kGpuClientId) {
10011000
// The display-compositor in the gpu process uses this special client id.
10021001
callback.Run(mojo::ScopedMessagePipeHandle(), gpu::GPUInfo(),
10031002
gpu::GpuFeatureInfo(),
@@ -1010,17 +1009,16 @@ void GpuProcessHost::EstablishGpuChannel(
10101009
bool is_gpu_host = preempts;
10111010

10121011
channel_requests_.push(callback);
1013-
const bool cache_shaders_on_disk = true;
10141012
gpu_service_ptr_->EstablishGpuChannel(
1015-
client_id, client_tracing_id, is_gpu_host, cache_shaders_on_disk,
1013+
client_id, client_tracing_id, is_gpu_host,
10161014
base::BindOnce(&GpuProcessHost::OnChannelEstablished,
10171015
weak_ptr_factory_.GetWeakPtr(), client_id, callback));
10181016

10191017
if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
10201018
switches::kDisableGpuShaderDiskCache)) {
10211019
CreateChannelCache(client_id);
10221020
if (oopd_enabled)
1023-
CreateChannelCache(gpu::kInProcessCommandBufferClientId);
1021+
CreateChannelCache(gpu::InProcessCommandBuffer::kGpuClientId);
10241022
}
10251023
}
10261024

@@ -1590,16 +1588,15 @@ std::string GpuProcessHost::GetShaderPrefixKey() {
15901588
return shader_prefix_key_;
15911589
}
15921590

1593-
void GpuProcessHost::LoadedShader(int32_t client_id,
1594-
const std::string& key,
1591+
void GpuProcessHost::LoadedShader(const std::string& key,
15951592
const std::string& data) {
15961593
std::string prefix = GetShaderPrefixKey();
15971594
bool prefix_ok = !key.compare(0, prefix.length(), prefix);
15981595
UMA_HISTOGRAM_BOOLEAN("GPU.ShaderLoadPrefixOK", prefix_ok);
15991596
if (prefix_ok) {
16001597
// Remove the prefix from the key before load.
16011598
std::string key_no_prefix = key.substr(prefix.length() + 1);
1602-
gpu_service_ptr_->LoadedShader(client_id, key_no_prefix, data);
1599+
gpu_service_ptr_->LoadedShader(key_no_prefix, data);
16031600
}
16041601
}
16051602

@@ -1617,8 +1614,7 @@ void GpuProcessHost::CreateChannelCache(int32_t client_id) {
16171614
return;
16181615

16191616
cache->set_shader_loaded_callback(base::Bind(&GpuProcessHost::LoadedShader,
1620-
weak_ptr_factory_.GetWeakPtr(),
1621-
client_id));
1617+
weak_ptr_factory_.GetWeakPtr()));
16221618

16231619
client_id_to_shader_cache_[client_id] = cache;
16241620
}

content/browser/gpu/gpu_process_host.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,7 @@ class GpuProcessHost : public BrowserChildProcessHostDelegate,
172172
// Forcefully terminates the GPU process.
173173
void ForceShutdown();
174174

175-
void LoadedShader(int32_t client_id,
176-
const std::string& key,
177-
const std::string& data);
175+
void LoadedShader(const std::string& key, const std::string& data);
178176

179177
CONTENT_EXPORT viz::mojom::GpuService* gpu_service();
180178

gpu/BUILD.gn

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,6 @@ test("gpu_unittests") {
350350
"command_buffer/service/gpu_service_test.h",
351351
"command_buffer/service/gpu_tracer_unittest.cc",
352352
"command_buffer/service/gr_cache_controller_unittest.cc",
353-
"command_buffer/service/gr_shader_cache_unittest.cc",
354353
"command_buffer/service/id_manager_unittest.cc",
355354
"command_buffer/service/indexed_buffer_binding_host_unittest.cc",
356355
"command_buffer/service/mailbox_manager_unittest.cc",

gpu/command_buffer/service/BUILD.gn

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,8 +161,6 @@ target(link_target_type, "gles2_sources") {
161161
"gpu_tracer.h",
162162
"gr_cache_controller.cc",
163163
"gr_cache_controller.h",
164-
"gr_shader_cache.cc",
165-
"gr_shader_cache.h",
166164
"id_manager.cc",
167165
"id_manager.h",
168166
"indexed_buffer_binding_host.cc",

gpu/command_buffer/service/gr_cache_controller_unittest.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class GrCacheControllerTest : public testing::Test {
3636
context_state_ = new raster::RasterDecoderContextState(
3737
std::move(share_group), std::move(surface), std::move(context),
3838
false /* use_virtualized_gl_contexts */);
39-
context_state_->InitializeGrContext(workarounds, nullptr);
39+
context_state_->InitializeGrContext(workarounds);
4040

4141
controller_ =
4242
std::make_unique<GrCacheController>(context_state_.get(), task_runner_);

gpu/command_buffer/service/gr_shader_cache.cc

Lines changed: 0 additions & 164 deletions
This file was deleted.

0 commit comments

Comments
 (0)