From f92040a3d9e7fc39a77498e4da2b98a02088cf1a Mon Sep 17 00:00:00 2001 From: Loko Kung Date: Wed, 27 Apr 2022 01:25:12 +0000 Subject: [PATCH] Remove PersistentCache and suppress shader cache tests. - Removed to allow for easier development changes to caching interface as it is implemented for pipeline caching without having to keep supporting this incomplete feature. Bug: dawn:549, dawn:1341 Change-Id: Id27deca45ac5607a4a6a7a016b19e3d60693ed72 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/87610 Reviewed-by: Austin Eng Reviewed-by: Shrek Shao Commit-Queue: Loko Kung Kokoro: Kokoro --- src/dawn/native/BUILD.gn | 2 - src/dawn/native/BlobCache.h | 4 - src/dawn/native/CMakeLists.txt | 2 - src/dawn/native/CacheKey.h | 2 +- src/dawn/native/Device.cpp | 8 -- src/dawn/native/Device.h | 6 -- src/dawn/native/PersistentCache.cpp | 64 ------------- src/dawn/native/PersistentCache.h | 94 -------------------- src/dawn/native/d3d12/ShaderModuleD3D12.cpp | 47 ++++------ src/dawn/native/d3d12/ShaderModuleD3D12.h | 2 - src/dawn/tests/end2end/D3D12CachingTests.cpp | 6 ++ 11 files changed, 24 insertions(+), 213 deletions(-) delete mode 100644 src/dawn/native/PersistentCache.cpp delete mode 100644 src/dawn/native/PersistentCache.h diff --git a/src/dawn/native/BUILD.gn b/src/dawn/native/BUILD.gn index a82dc546bd..28dc0727db 100644 --- a/src/dawn/native/BUILD.gn +++ b/src/dawn/native/BUILD.gn @@ -281,8 +281,6 @@ source_set("sources") { "PassResourceUsageTracker.h", "PerStage.cpp", "PerStage.h", - "PersistentCache.cpp", - "PersistentCache.h", "Pipeline.cpp", "Pipeline.h", "PipelineLayout.cpp", diff --git a/src/dawn/native/BlobCache.h b/src/dawn/native/BlobCache.h index 2e31261caf..4e92fba89c 100644 --- a/src/dawn/native/BlobCache.h +++ b/src/dawn/native/BlobCache.h @@ -45,10 +45,6 @@ namespace dawn::native { // This class should always be thread-safe because it may be called asynchronously. Its purpose // is to wrap the CachingInterface provided via a platform. - // TODO(dawn:549): This is a "re-declaration" of the current PersistentCache since there are - // some dependencies on that one for the semi-implemented D3D12 shader cache and some changes - // introduced here are breaking. Eventually the goal is to unify the two, but for development - // purposes, we are splitting these for now and will re-merge them in a later change. class BlobCache { public: explicit BlobCache(dawn::platform::CachingInterface* cachingInterface = nullptr); diff --git a/src/dawn/native/CMakeLists.txt b/src/dawn/native/CMakeLists.txt index 3ef0fa01d0..778b1050be 100644 --- a/src/dawn/native/CMakeLists.txt +++ b/src/dawn/native/CMakeLists.txt @@ -122,8 +122,6 @@ target_sources(dawn_native PRIVATE "PassResourceUsage.h" "PassResourceUsageTracker.cpp" "PassResourceUsageTracker.h" - "PersistentCache.cpp" - "PersistentCache.h" "PerStage.cpp" "PerStage.h" "Pipeline.cpp" diff --git a/src/dawn/native/CacheKey.h b/src/dawn/native/CacheKey.h index 658e79dfc8..8b920b5b21 100644 --- a/src/dawn/native/CacheKey.h +++ b/src/dawn/native/CacheKey.h @@ -46,7 +46,7 @@ namespace dawn::native { public: using std::vector::vector; - enum class Type { ComputePipeline, RenderPipeline }; + enum class Type { ComputePipeline, RenderPipeline, Shader }; template CacheKey& Record(const T& t) { diff --git a/src/dawn/native/Device.cpp b/src/dawn/native/Device.cpp index 4f5d84e914..5bccc64480 100644 --- a/src/dawn/native/Device.cpp +++ b/src/dawn/native/Device.cpp @@ -40,7 +40,6 @@ #include "dawn/native/Instance.h" #include "dawn/native/InternalPipelineStore.h" #include "dawn/native/ObjectType_autogen.h" -#include "dawn/native/PersistentCache.h" #include "dawn/native/QuerySet.h" #include "dawn/native/Queue.h" #include "dawn/native/RenderBundleEncoder.h" @@ -257,7 +256,6 @@ namespace dawn::native { mCallbackTaskManager = std::make_unique(); mDeprecationWarnings = std::make_unique(); mInternalPipelineStore = std::make_unique(this); - mPersistentCache = std::make_unique(this); ASSERT(GetPlatform() != nullptr); mWorkerTaskPool = GetPlatform()->CreateWorkerTaskPool(); @@ -413,7 +411,6 @@ namespace dawn::native { mDynamicUploader = nullptr; mCallbackTaskManager = nullptr; mAsyncTaskManager = nullptr; - mPersistentCache = nullptr; mEmptyBindGroupLayout = nullptr; mInternalPipelineStore = nullptr; mExternalTexturePlaceholderView = nullptr; @@ -574,11 +571,6 @@ namespace dawn::native { return returnValue; } - PersistentCache* DeviceBase::GetPersistentCache() { - ASSERT(mPersistentCache.get() != nullptr); - return mPersistentCache.get(); - } - BlobCache* DeviceBase::GetBlobCache() { return mInstance->GetBlobCache(); } diff --git a/src/dawn/native/Device.h b/src/dawn/native/Device.h index de7e438cee..7a109fd2c1 100644 --- a/src/dawn/native/Device.h +++ b/src/dawn/native/Device.h @@ -50,7 +50,6 @@ namespace dawn::native { class DynamicUploader; class ErrorScopeStack; class OwnedCompilationMessages; - class PersistentCache; struct CallbackTask; struct InternalPipelineStore; struct ShaderModuleParseResult; @@ -276,8 +275,6 @@ namespace dawn::native { MaybeError ValidateIsAlive() const; - // TODO(dawn:549): Deprecate PersistentCache, once it's usage in D3D12 shaders is removed. - PersistentCache* GetPersistentCache(); BlobCache* GetBlobCache(); virtual ResultOrError> CreateStagingBuffer( @@ -550,9 +547,6 @@ namespace dawn::native { std::unique_ptr mInternalPipelineStore; - // TODO(dawn:549): Deprecate PersistentCache, once it's usage in D3D12 shaders is removed. - std::unique_ptr mPersistentCache; - std::unique_ptr mCallbackTaskManager; std::unique_ptr mWorkerTaskPool; std::string mLabel; diff --git a/src/dawn/native/PersistentCache.cpp b/src/dawn/native/PersistentCache.cpp deleted file mode 100644 index ce3ab49232..0000000000 --- a/src/dawn/native/PersistentCache.cpp +++ /dev/null @@ -1,64 +0,0 @@ -// Copyright 2020 The Dawn Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "dawn/native/PersistentCache.h" - -#include "dawn/common/Assert.h" -#include "dawn/native/Device.h" -#include "dawn/platform/DawnPlatform.h" - -namespace dawn::native { - - PersistentCache::PersistentCache(DeviceBase* device) - : mDevice(device), mCache(GetPlatformCache()) { - } - - ScopedCachedBlob PersistentCache::LoadData(const PersistentCacheKey& key) { - ScopedCachedBlob blob = {}; - if (mCache == nullptr) { - return blob; - } - std::lock_guard lock(mMutex); - blob.bufferSize = mCache->LoadData(ToAPI(mDevice), key.data(), key.size(), nullptr, 0); - if (blob.bufferSize > 0) { - blob.buffer.reset(new uint8_t[blob.bufferSize]); - const size_t bufferSize = mCache->LoadData(ToAPI(mDevice), key.data(), key.size(), - blob.buffer.get(), blob.bufferSize); - ASSERT(bufferSize == blob.bufferSize); - return blob; - } - return blob; - } - - void PersistentCache::StoreData(const PersistentCacheKey& key, const void* value, size_t size) { - if (mCache == nullptr) { - return; - } - ASSERT(value != nullptr); - ASSERT(size > 0); - std::lock_guard lock(mMutex); - mCache->StoreData(ToAPI(mDevice), key.data(), key.size(), value, size); - } - - dawn::platform::CachingInterface* PersistentCache::GetPlatformCache() { - // TODO(dawn:549): Create a fingerprint of concatenated version strings (ex. Tint commit - // hash, Dawn commit hash). This will be used by the client so it may know when to discard - // previously cached Dawn objects should this fingerprint change. - dawn::platform::Platform* platform = mDevice->GetPlatform(); - if (platform != nullptr) { - return platform->GetCachingInterface(/*fingerprint*/ nullptr, /*fingerprintSize*/ 0); - } - return nullptr; - } -} // namespace dawn::native diff --git a/src/dawn/native/PersistentCache.h b/src/dawn/native/PersistentCache.h deleted file mode 100644 index 71a76dd915..0000000000 --- a/src/dawn/native/PersistentCache.h +++ /dev/null @@ -1,94 +0,0 @@ -// Copyright 2020 The Dawn Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef SRC_DAWN_NATIVE_PERSISTENTCACHE_H_ -#define SRC_DAWN_NATIVE_PERSISTENTCACHE_H_ - -#include -#include -#include -#include - -#include "dawn/native/Error.h" - -namespace dawn::platform { - class CachingInterface; -} - -namespace dawn::native { - - using PersistentCacheKey = std::vector; - - struct ScopedCachedBlob { - std::unique_ptr buffer; - size_t bufferSize = 0; - }; - - class DeviceBase; - - enum class PersistentKeyType { Shader }; - - // This class should always be thread-safe as it is used in Create*PipelineAsync() where it is - // called asynchronously. - // The thread-safety of any access to mCache (the function LoadData() and StoreData()) is - // protected by mMutex. - class PersistentCache { - public: - explicit PersistentCache(DeviceBase* device); - - // Combines load/store operations into a single call. - // If the load was successful, a non-empty blob is returned to the caller. - // Else, the creation callback |createFn| gets invoked with a callback - // |doCache| to store the newly created blob back in the cache. - // - // Example usage: - // - // ScopedCachedBlob cachedBlob = {}; - // DAWN_TRY_ASSIGN(cachedBlob, GetOrCreate(key, [&](auto doCache)) { - // // Create a new blob to be stored - // doCache(newBlobPtr, newBlobSize); // store - // })); - // - template - ResultOrError GetOrCreate(const PersistentCacheKey& key, - CreateFn&& createFn) { - // Attempt to load an existing blob from the cache. - ScopedCachedBlob blob = LoadData(key); - if (blob.bufferSize > 0) { - return std::move(blob); - } - - // Allow the caller to create a new blob to be stored for the given key. - DAWN_TRY(createFn([this, key](const void* value, size_t size) { - this->StoreData(key, value, size); - })); - - return std::move(blob); - } - - private: - // PersistentCache impl - ScopedCachedBlob LoadData(const PersistentCacheKey& key); - void StoreData(const PersistentCacheKey& key, const void* value, size_t size); - - dawn::platform::CachingInterface* GetPlatformCache(); - - DeviceBase* mDevice = nullptr; - - std::mutex mMutex; - dawn::platform::CachingInterface* mCache = nullptr; - }; -} // namespace dawn::native - -#endif // SRC_DAWN_NATIVE_PERSISTENTCACHE_H_ diff --git a/src/dawn/native/d3d12/ShaderModuleD3D12.cpp b/src/dawn/native/d3d12/ShaderModuleD3D12.cpp index 95469e851b..cffb38db15 100644 --- a/src/dawn/native/d3d12/ShaderModuleD3D12.cpp +++ b/src/dawn/native/d3d12/ShaderModuleD3D12.cpp @@ -28,6 +28,7 @@ #include "dawn/common/BitSetIterator.h" #include "dawn/common/Log.h" #include "dawn/common/WindowsUtils.h" +#include "dawn/native/CacheKey.h" #include "dawn/native/Pipeline.h" #include "dawn/native/TintUtils.h" #include "dawn/native/d3d12/BindGroupLayoutD3D12.h" @@ -328,7 +329,8 @@ namespace dawn::native::d3d12 { return std::move(request); } - ResultOrError CreateCacheKey() const { + // TODO(dawn:1341): Move to use CacheKey instead of the vector. + ResultOrError> CreateCacheKey() const { // Generate the WGSL from the Tint program so it's normalized. // TODO(tint:1180): Consider using a binary serialization of the tint AST for a more // compact representation. @@ -344,7 +346,7 @@ namespace dawn::native::d3d12 { // Prefix the key with the type to avoid collisions from another type that could // have the same key. - stream << static_cast(PersistentKeyType::Shader); + stream << static_cast(CacheKey::Type::Shader); stream << "\n"; stream << result.wgsl.length(); @@ -389,8 +391,8 @@ namespace dawn::native::d3d12 { stream << ")"; stream << "\n"; - return PersistentCacheKey(std::istreambuf_iterator{stream}, - std::istreambuf_iterator{}); + return std::vector(std::istreambuf_iterator{stream}, + std::istreambuf_iterator{}); } }; @@ -803,36 +805,21 @@ namespace dawn::native::d3d12 { programmableStage.entryPoint.c_str(), stage, layout, compileFlags, device, program, GetEntryPoint(programmableStage.entryPoint), programmableStage)); - PersistentCacheKey shaderCacheKey; - DAWN_TRY_ASSIGN(shaderCacheKey, request.CreateCacheKey()); - - DAWN_TRY_ASSIGN( - compiledShader.cachedShader, - device->GetPersistentCache()->GetOrCreate( - shaderCacheKey, [&](auto doCache) -> MaybeError { - DAWN_TRY(CompileShader( - device->GetPlatform(), device->GetFunctions(), - device->IsToggleEnabled(Toggle::UseDXC) ? device->GetDxcLibrary().Get() - : nullptr, - device->IsToggleEnabled(Toggle::UseDXC) ? device->GetDxcCompiler().Get() - : nullptr, - std::move(request), device->IsToggleEnabled(Toggle::DumpShaders), - [&](WGPULoggingType loggingType, const char* message) { - GetDevice()->EmitLog(loggingType, message); - }, - &compiledShader)); - const D3D12_SHADER_BYTECODE shader = compiledShader.GetD3D12ShaderBytecode(); - doCache(shader.pShaderBytecode, shader.BytecodeLength); - return {}; - })); - + // TODO(dawn:1341): Add shader cache key generation and caching for the compiled shader. + DAWN_TRY(CompileShader( + device->GetPlatform(), device->GetFunctions(), + device->IsToggleEnabled(Toggle::UseDXC) ? device->GetDxcLibrary().Get() : nullptr, + device->IsToggleEnabled(Toggle::UseDXC) ? device->GetDxcCompiler().Get() : nullptr, + std::move(request), device->IsToggleEnabled(Toggle::DumpShaders), + [&](WGPULoggingType loggingType, const char* message) { + GetDevice()->EmitLog(loggingType, message); + }, + &compiledShader)); return std::move(compiledShader); } D3D12_SHADER_BYTECODE CompiledShader::GetD3D12ShaderBytecode() const { - if (cachedShader.buffer != nullptr) { - return {cachedShader.buffer.get(), cachedShader.bufferSize}; - } else if (compiledFXCShader != nullptr) { + if (compiledFXCShader != nullptr) { return {compiledFXCShader->GetBufferPointer(), compiledFXCShader->GetBufferSize()}; } else if (compiledDXCShader != nullptr) { return {compiledDXCShader->GetBufferPointer(), compiledDXCShader->GetBufferSize()}; diff --git a/src/dawn/native/d3d12/ShaderModuleD3D12.h b/src/dawn/native/d3d12/ShaderModuleD3D12.h index 090abb7f7b..4a3a30f71c 100644 --- a/src/dawn/native/d3d12/ShaderModuleD3D12.h +++ b/src/dawn/native/d3d12/ShaderModuleD3D12.h @@ -15,7 +15,6 @@ #ifndef SRC_DAWN_NATIVE_D3D12_SHADERMODULED3D12_H_ #define SRC_DAWN_NATIVE_D3D12_SHADERMODULED3D12_H_ -#include "dawn/native/PersistentCache.h" #include "dawn/native/ShaderModule.h" #include "dawn/native/d3d12/d3d12_platform.h" @@ -32,7 +31,6 @@ namespace dawn::native::d3d12 { // Manages a ref to one of the various representations of shader blobs and information used to // emulate vertex/instance index starts struct CompiledShader { - ScopedCachedBlob cachedShader; ComPtr compiledFXCShader; ComPtr compiledDXCShader; D3D12_SHADER_BYTECODE GetD3D12ShaderBytecode() const; diff --git a/src/dawn/tests/end2end/D3D12CachingTests.cpp b/src/dawn/tests/end2end/D3D12CachingTests.cpp index 5a4da5bedf..798787958b 100644 --- a/src/dawn/tests/end2end/D3D12CachingTests.cpp +++ b/src/dawn/tests/end2end/D3D12CachingTests.cpp @@ -27,6 +27,12 @@ namespace { class D3D12CachingTests : public DawnTest { protected: + void SetUp() override { + DawnTest::SetUp(); + // TODO(dawn:1341) Re-enable tests once shader caching is re-implemented. + DAWN_SKIP_TEST_IF_BASE(true, "suppressed", "TODO(dawn:1341)"); + } + std::unique_ptr CreateTestPlatform() override { return std::make_unique(&mMockCache); }