Skip to content

Commit

Permalink
Revert "gpu: Add disk caching for skia generated shaders in OOP-R."
Browse files Browse the repository at this point in the history
This reverts commit 49b85d7.

Reason for revert: Speculative revert for failures in crbug.com/865138

Original change's description:
> gpu: Add disk caching for skia generated shaders in OOP-R.
> 
> 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.
> 
> R=​piman@chromium.org
> 
> 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: I91fd89ed2c3d2a0bcbcb9b1fdc4ddfbc5b8de147
> Bug: 854416,840559
> Reviewed-on: https://chromium-review.googlesource.com/1116197
> Commit-Queue: Khushal <khushalsagar@chromium.org>
> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
> Reviewed-by: Antoine Labour <piman@chromium.org>
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#575987}

TBR=sadrul@chromium.org,tsepez@chromium.org,khushalsagar@chromium.org,piman@chromium.org,ericrk@chromium.org

Change-Id: I0cde27e30d1ab0c8803539148efc42a24a3e2230
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 854416, 840559
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/1142306
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576228}
  • Loading branch information
khushalsagar authored and Commit Bot committed Jul 18, 2018
1 parent 1ca17c5 commit f9395d7
Show file tree
Hide file tree
Showing 35 changed files with 52 additions and 607 deletions.
7 changes: 0 additions & 7 deletions chrome/browser/chrome_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3212,13 +3212,6 @@ base::FilePath ChromeContentBrowserClient::GetShaderDiskCacheDirectory() {
return user_data_dir.Append(FILE_PATH_LITERAL("ShaderCache"));
}

base::FilePath ChromeContentBrowserClient::GetGrShaderDiskCacheDirectory() {
base::FilePath user_data_dir;
base::PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
DCHECK(!user_data_dir.empty());
return user_data_dir.Append(FILE_PATH_LITERAL("GrShaderCache"));
}

void ChromeContentBrowserClient::DidCreatePpapiPlugin(
content::BrowserPpapiHost* browser_host) {
#if BUILDFLAG(ENABLE_PLUGINS)
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/chrome_content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,6 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
base::FilePath GetDefaultDownloadDirectory() override;
std::string GetDefaultDownloadName() override;
base::FilePath GetShaderDiskCacheDirectory() override;
base::FilePath GetGrShaderDiskCacheDirectory() override;
void DidCreatePpapiPlugin(content::BrowserPpapiHost* browser_host) override;
content::BrowserPpapiHost* GetExternalBrowserPpapiHost(
int plugin_process_id) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ class TestGpuService : public mojom::GpuService {
void EstablishGpuChannel(int32_t client_id,
uint64_t client_tracing_id,
bool is_gpu_host,
bool cache_shaders_on_disk,
EstablishGpuChannelCallback callback) override {}

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

void RequestHDRStatus(RequestHDRStatusCallback callback) override {}

void LoadedShader(int32_t client_id,
const std::string& key,
const std::string& data) override {}
void LoadedShader(const std::string& key, const std::string& data) override {}

void WakeUpGpu() override {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include "components/viz/service/display_embedder/in_process_gpu_memory_buffer_manager.h"

#include "base/bind.h"
#include "gpu/ipc/common/gpu_client_ids.h"
#include "gpu/ipc/common/gpu_memory_buffer_impl.h"
#include "gpu/ipc/common/gpu_memory_buffer_support.h"
#include "gpu/ipc/in_process_command_buffer.h"
Expand All @@ -17,7 +16,7 @@ namespace viz {
InProcessGpuMemoryBufferManager::InProcessGpuMemoryBufferManager(
gpu::GpuChannelManager* channel_manager)
: gpu_memory_buffer_support_(new gpu::GpuMemoryBufferSupport()),
client_id_(gpu::kInProcessCommandBufferClientId),
client_id_(gpu::InProcessCommandBuffer::kGpuClientId),
channel_manager_(channel_manager),
weak_factory_(this) {
weak_ptr_ = weak_factory_.GetWeakPtr();
Expand Down
23 changes: 9 additions & 14 deletions components/viz/service/gl/gpu_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
#include "gpu/config/gpu_info_collector.h"
#include "gpu/config/gpu_switches.h"
#include "gpu/config/gpu_util.h"
#include "gpu/ipc/common/gpu_client_ids.h"
#include "gpu/ipc/common/gpu_memory_buffer_support.h"
#include "gpu/ipc/common/memory_stats.h"
#include "gpu/ipc/in_process_command_buffer.h"
Expand Down Expand Up @@ -703,9 +702,8 @@ void GpuServiceImpl::SetActiveURL(const GURL& url) {
void GpuServiceImpl::EstablishGpuChannel(int32_t client_id,
uint64_t client_tracing_id,
bool is_gpu_host,
bool cache_shaders_on_disk,
EstablishGpuChannelCallback callback) {
if (gpu::IsReservedClientId(client_id)) {
if (oopd_enabled_ && client_id == gpu::InProcessCommandBuffer::kGpuClientId) {
std::move(callback).Run(mojo::ScopedMessagePipeHandle());
return;
}
Expand All @@ -719,15 +717,14 @@ void GpuServiceImpl::EstablishGpuChannel(int32_t client_id,
},
io_runner_, std::move(callback));
main_runner_->PostTask(
FROM_HERE,
base::BindOnce(&GpuServiceImpl::EstablishGpuChannel, weak_ptr_,
client_id, client_tracing_id, is_gpu_host,
cache_shaders_on_disk, std::move(wrap_callback)));
FROM_HERE, base::BindOnce(&GpuServiceImpl::EstablishGpuChannel,
weak_ptr_, client_id, client_tracing_id,
is_gpu_host, std::move(wrap_callback)));
return;
}

gpu::GpuChannel* gpu_channel = gpu_channel_manager_->EstablishChannel(
client_id, client_tracing_id, is_gpu_host, cache_shaders_on_disk);
client_id, client_tracing_id, is_gpu_host);

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

void GpuServiceImpl::LoadedShader(int32_t client_id,
const std::string& key,
void GpuServiceImpl::LoadedShader(const std::string& key,
const std::string& data) {
if (io_runner_->BelongsToCurrentThread()) {
main_runner_->PostTask(FROM_HERE,
base::Bind(&GpuServiceImpl::LoadedShader, weak_ptr_,
client_id, key, data));
main_runner_->PostTask(FROM_HERE, base::Bind(&GpuServiceImpl::LoadedShader,
weak_ptr_, key, data));
return;
}
gpu_channel_manager_->PopulateShaderCache(client_id, key, data);
gpu_channel_manager_->PopulateShaderCache(key, data);
}

void GpuServiceImpl::WakeUpGpu() {
Expand Down
5 changes: 1 addition & 4 deletions components/viz/service/gl/gpu_service_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,6 @@ class VIZ_SERVICE_EXPORT GpuServiceImpl : public gpu::GpuChannelManagerDelegate,
void EstablishGpuChannel(int32_t client_id,
uint64_t client_tracing_id,
bool is_gpu_host,
bool cache_shaders_on_disk,
EstablishGpuChannelCallback callback) override;
void CloseChannel(int32_t client_id) override;
#if defined(OS_CHROMEOS)
Expand Down Expand Up @@ -218,9 +217,7 @@ class VIZ_SERVICE_EXPORT GpuServiceImpl : public gpu::GpuChannelManagerDelegate,
void GetGpuSupportedRuntimeVersion(
GetGpuSupportedRuntimeVersionCallback callback) override;
void RequestHDRStatus(RequestHDRStatusCallback callback) override;
void LoadedShader(int32_t client_id,
const std::string& key,
const std::string& data) override;
void LoadedShader(const std::string& key, const std::string& data) override;
void WakeUpGpu() override;
void GpuSwitched() override;
void DestroyAllChannels() override;
Expand Down
24 changes: 1 addition & 23 deletions content/browser/gpu/browser_gpu_channel_host_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@
#include "content/public/common/content_client.h"
#include "content/public/common/content_switches.h"
#include "gpu/command_buffer/service/gpu_switches.h"
#include "gpu/config/gpu_finch_features.h"
#include "gpu/ipc/common/gpu_client_ids.h"
#include "gpu/ipc/in_process_command_buffer.h"
#include "services/resource_coordinator/public/mojom/memory_instrumentation/constants.mojom.h"
#include "services/service_manager/runner/common/client_util.h"
Expand Down Expand Up @@ -272,19 +270,6 @@ BrowserGpuChannelHostFactory::BrowserGpuChannelHostFactory()
&BrowserGpuChannelHostFactory::InitializeShaderDiskCacheOnIO,
gpu_client_id_, cache_dir));
}

if (base::FeatureList::IsEnabled(
features::kDefaultEnableOopRasterization)) {
base::FilePath gr_cache_dir =
GetContentClient()->browser()->GetGrShaderDiskCacheDirectory();
if (!gr_cache_dir.empty()) {
BrowserThread::PostTask(
BrowserThread::IO, FROM_HERE,
base::BindOnce(
&BrowserGpuChannelHostFactory::InitializeGrShaderDiskCacheOnIO,
gr_cache_dir));
}
}
}
}

Expand Down Expand Up @@ -401,15 +386,8 @@ void BrowserGpuChannelHostFactory::InitializeShaderDiskCacheOnIO(
GetShaderCacheFactorySingleton()->SetCacheInfo(gpu_client_id, cache_dir);
if (base::FeatureList::IsEnabled(features::kVizDisplayCompositor)) {
GetShaderCacheFactorySingleton()->SetCacheInfo(
gpu::kInProcessCommandBufferClientId, cache_dir);
gpu::InProcessCommandBuffer::kGpuClientId, cache_dir);
}
}

// static
void BrowserGpuChannelHostFactory::InitializeGrShaderDiskCacheOnIO(
const base::FilePath& cache_dir) {
GetShaderCacheFactorySingleton()->SetCacheInfo(gpu::kGrShaderCacheClientId,
cache_dir);
}

} // namespace content
1 change: 0 additions & 1 deletion content/browser/gpu/browser_gpu_channel_host_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ class CONTENT_EXPORT BrowserGpuChannelHostFactory

static void InitializeShaderDiskCacheOnIO(int gpu_client_id,
const base::FilePath& cache_dir);
static void InitializeGrShaderDiskCacheOnIO(const base::FilePath& cache_dir);

const int gpu_client_id_;
const uint64_t gpu_client_tracing_id_;
Expand Down
31 changes: 9 additions & 22 deletions content/browser/gpu/gpu_process_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,7 @@
#include "gpu/command_buffer/service/gpu_switches.h"
#include "gpu/config/gpu_driver_bug_list.h"
#include "gpu/config/gpu_driver_bug_workaround_type.h"
#include "gpu/config/gpu_finch_features.h"
#include "gpu/config/gpu_preferences.h"
#include "gpu/ipc/common/gpu_client_ids.h"
#include "gpu/ipc/host/shader_disk_cache.h"
#include "gpu/ipc/in_process_command_buffer.h"
#include "media/base/media_switches.h"
Expand Down Expand Up @@ -996,9 +994,10 @@ void GpuProcessHost::EstablishGpuChannel(
return;
}

if (gpu::IsReservedClientId(client_id)) {
// The display-compositor/GrShaderCache in the gpu process uses these
// special client ids.
bool oopd_enabled =
base::FeatureList::IsEnabled(features::kVizDisplayCompositor);
if (oopd_enabled && client_id == gpu::InProcessCommandBuffer::kGpuClientId) {
// The display-compositor in the gpu process uses this special client id.
callback.Run(mojo::ScopedMessagePipeHandle(), gpu::GPUInfo(),
gpu::GpuFeatureInfo(),
EstablishChannelStatus::GPU_ACCESS_DENIED);
Expand All @@ -1008,28 +1007,18 @@ void GpuProcessHost::EstablishGpuChannel(
DCHECK_EQ(preempts, allow_view_command_buffers);
DCHECK_EQ(preempts, allow_real_time_streams);
bool is_gpu_host = preempts;
bool cache_shaders_on_disk =
GetShaderCacheFactorySingleton()->Get(client_id) != nullptr;

channel_requests_.push(callback);
gpu_service_ptr_->EstablishGpuChannel(
client_id, client_tracing_id, is_gpu_host, cache_shaders_on_disk,
client_id, client_tracing_id, is_gpu_host,
base::BindOnce(&GpuProcessHost::OnChannelEstablished,
weak_ptr_factory_.GetWeakPtr(), client_id, callback));

if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kDisableGpuShaderDiskCache)) {
CreateChannelCache(client_id);

bool oopd_enabled =
base::FeatureList::IsEnabled(features::kVizDisplayCompositor);
if (oopd_enabled)
CreateChannelCache(gpu::kInProcessCommandBufferClientId);

bool oopr_enabled =
base::FeatureList::IsEnabled(features::kDefaultEnableOopRasterization);
if (oopr_enabled)
CreateChannelCache(gpu::kGrShaderCacheClientId);
CreateChannelCache(gpu::InProcessCommandBuffer::kGpuClientId);
}
}

Expand Down Expand Up @@ -1599,16 +1588,15 @@ std::string GpuProcessHost::GetShaderPrefixKey() {
return shader_prefix_key_;
}

void GpuProcessHost::LoadedShader(int32_t client_id,
const std::string& key,
void GpuProcessHost::LoadedShader(const std::string& key,
const std::string& data) {
std::string prefix = GetShaderPrefixKey();
bool prefix_ok = !key.compare(0, prefix.length(), prefix);
UMA_HISTOGRAM_BOOLEAN("GPU.ShaderLoadPrefixOK", prefix_ok);
if (prefix_ok) {
// Remove the prefix from the key before load.
std::string key_no_prefix = key.substr(prefix.length() + 1);
gpu_service_ptr_->LoadedShader(client_id, key_no_prefix, data);
gpu_service_ptr_->LoadedShader(key_no_prefix, data);
}
}

Expand All @@ -1626,8 +1614,7 @@ void GpuProcessHost::CreateChannelCache(int32_t client_id) {
return;

cache->set_shader_loaded_callback(base::Bind(&GpuProcessHost::LoadedShader,
weak_ptr_factory_.GetWeakPtr(),
client_id));
weak_ptr_factory_.GetWeakPtr()));

client_id_to_shader_cache_[client_id] = cache;
}
Expand Down
4 changes: 1 addition & 3 deletions content/browser/gpu/gpu_process_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -172,9 +172,7 @@ class GpuProcessHost : public BrowserChildProcessHostDelegate,
// Forcefully terminates the GPU process.
void ForceShutdown();

void LoadedShader(int32_t client_id,
const std::string& key,
const std::string& data);
void LoadedShader(const std::string& key, const std::string& data);

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

Expand Down
4 changes: 0 additions & 4 deletions content/public/browser/content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -463,10 +463,6 @@ base::FilePath ContentBrowserClient::GetShaderDiskCacheDirectory() {
return base::FilePath();
}

base::FilePath ContentBrowserClient::GetGrShaderDiskCacheDirectory() {
return base::FilePath();
}

BrowserPpapiHost*
ContentBrowserClient::GetExternalBrowserPpapiHost(int plugin_process_id) {
return nullptr;
Expand Down
4 changes: 0 additions & 4 deletions content/public/browser/content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -743,10 +743,6 @@ class CONTENT_EXPORT ContentBrowserClient {
// Returns the path to the browser shader disk cache root.
virtual base::FilePath GetShaderDiskCacheDirectory();

// Returns the path to the shader disk cache root for shaders generated by
// skia.
virtual base::FilePath GetGrShaderDiskCacheDirectory();

// Notification that a pepper plugin has just been spawned. This allows the
// embedder to add filters onto the host to implement interfaces.
// This is called on the IO thread.
Expand Down
1 change: 0 additions & 1 deletion gpu/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,6 @@ test("gpu_unittests") {
"command_buffer/service/gpu_service_test.h",
"command_buffer/service/gpu_tracer_unittest.cc",
"command_buffer/service/gr_cache_controller_unittest.cc",
"command_buffer/service/gr_shader_cache_unittest.cc",
"command_buffer/service/id_manager_unittest.cc",
"command_buffer/service/indexed_buffer_binding_host_unittest.cc",
"command_buffer/service/mailbox_manager_unittest.cc",
Expand Down
2 changes: 0 additions & 2 deletions gpu/command_buffer/service/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,6 @@ target(link_target_type, "gles2_sources") {
"gpu_tracer.h",
"gr_cache_controller.cc",
"gr_cache_controller.h",
"gr_shader_cache.cc",
"gr_shader_cache.h",
"id_manager.cc",
"id_manager.h",
"indexed_buffer_binding_host.cc",
Expand Down
2 changes: 1 addition & 1 deletion gpu/command_buffer/service/gr_cache_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class GrCacheControllerTest : public testing::Test {
context_state_ = new raster::RasterDecoderContextState(
std::move(share_group), std::move(surface), std::move(context),
false /* use_virtualized_gl_contexts */);
context_state_->InitializeGrContext(workarounds, nullptr);
context_state_->InitializeGrContext(workarounds);

controller_ =
std::make_unique<GrCacheController>(context_state_.get(), task_runner_);
Expand Down
Loading

0 comments on commit f9395d7

Please sign in to comment.