From 7289bca018ef21746a3992879d8045a5df9e1b98 Mon Sep 17 00:00:00 2001 From: Loko Kung Date: Fri, 24 Jun 2022 23:39:49 +0000 Subject: [PATCH] Add dawn version hash to cache keys. Motivation is to simplify cache evicting by reusing the LRU to evict stale entries from older Dawn versions since there isn't already a simple cache busting solution. The dawn version will just be pushed into the cache keys instead so old version entries will eventually be retired. - Removes the fingerprint from the GetCachingInterface API on DawnNative. - Adds the "fingerprint", which was just the hash, to the device;s cache key directly instead. Bug: dawn:549 Change-Id: I573aa03a2bb96dfe044293b1176d3a7746725572 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/94140 Reviewed-by: Corentin Wallez Commit-Queue: Loko Kung --- include/dawn/platform/DawnPlatform.h | 8 ++++---- src/dawn/native/BlobCache.cpp | 10 ++++++++++ src/dawn/native/BlobCache.h | 4 ++++ src/dawn/native/CacheKey.cpp | 10 +++++++++- src/dawn/native/CacheKey.h | 1 - src/dawn/native/Device.cpp | 3 ++- src/dawn/native/Instance.cpp | 5 ++--- src/dawn/platform/DawnPlatform.cpp | 3 +-- .../tests/mocks/platform/CachingInterfaceMock.cpp | 4 +--- src/dawn/tests/mocks/platform/CachingInterfaceMock.h | 4 ++-- src/dawn/tests/unittests/native/CacheKeyTests.cpp | 12 +++++++++++- 11 files changed, 46 insertions(+), 18 deletions(-) diff --git a/include/dawn/platform/DawnPlatform.h b/include/dawn/platform/DawnPlatform.h index 5c616d1bbc..0645441d5f 100644 --- a/include/dawn/platform/DawnPlatform.h +++ b/include/dawn/platform/DawnPlatform.h @@ -93,10 +93,10 @@ class DAWN_PLATFORM_EXPORT Platform { const uint64_t* argValues, unsigned char flags); - // The |fingerprint| is provided by Dawn to inform the client to discard the Dawn caches - // when the fingerprint changes. The returned CachingInterface is expected to outlive the - // device which uses it to persistently cache objects. - virtual CachingInterface* GetCachingInterface(const void* fingerprint, size_t fingerprintSize); + // The returned CachingInterface is expected to outlive the device which uses it to persistently + // cache objects. + virtual CachingInterface* GetCachingInterface(); + virtual std::unique_ptr CreateWorkerTaskPool(); private: diff --git a/src/dawn/native/BlobCache.cpp b/src/dawn/native/BlobCache.cpp index 435f12dd49..a919318420 100644 --- a/src/dawn/native/BlobCache.cpp +++ b/src/dawn/native/BlobCache.cpp @@ -14,7 +14,10 @@ #include "dawn/native/BlobCache.h" +#include + #include "dawn/common/Assert.h" +#include "dawn/common/Version_autogen.h" #include "dawn/native/CacheKey.h" #include "dawn/native/Instance.h" #include "dawn/platform/DawnPlatform.h" @@ -39,6 +42,7 @@ void BlobCache::Store(const CacheKey& key, const Blob& value) { } Blob BlobCache::LoadInternal(const CacheKey& key) { + ASSERT(ValidateCacheKey(key)); if (mCache == nullptr) { return Blob(); } @@ -55,6 +59,7 @@ Blob BlobCache::LoadInternal(const CacheKey& key) { } void BlobCache::StoreInternal(const CacheKey& key, size_t valueSize, const void* value) { + ASSERT(ValidateCacheKey(key)); ASSERT(value != nullptr); ASSERT(valueSize > 0); if (mCache == nullptr) { @@ -63,4 +68,9 @@ void BlobCache::StoreInternal(const CacheKey& key, size_t valueSize, const void* mCache->StoreData(key.data(), key.size(), value, valueSize); } +bool BlobCache::ValidateCacheKey(const CacheKey& key) { + return std::search(key.begin(), key.end(), kDawnVersion.begin(), kDawnVersion.end()) != + key.end(); +} + } // namespace dawn::native diff --git a/src/dawn/native/BlobCache.h b/src/dawn/native/BlobCache.h index 615af2f877..61753a88f5 100644 --- a/src/dawn/native/BlobCache.h +++ b/src/dawn/native/BlobCache.h @@ -48,6 +48,10 @@ class BlobCache { Blob LoadInternal(const CacheKey& key); void StoreInternal(const CacheKey& key, size_t valueSize, const void* value); + // Validates the cache key for this version of Dawn. At the moment, this is naively checking + // that the cache key contains the dawn version string in it. + bool ValidateCacheKey(const CacheKey& key); + // Protects thread safety of access to mCache. std::mutex mMutex; dawn::platform::CachingInterface* mCache; diff --git a/src/dawn/native/CacheKey.cpp b/src/dawn/native/CacheKey.cpp index 495b013ed9..414b91560d 100644 --- a/src/dawn/native/CacheKey.cpp +++ b/src/dawn/native/CacheKey.cpp @@ -15,6 +15,8 @@ #include "dawn/native/CacheKey.h" #include +#include +#include namespace dawn::native { @@ -29,7 +31,13 @@ std::ostream& operator<<(std::ostream& os, const CacheKey& key) { template <> void CacheKeySerializer::Serialize(CacheKey* key, const std::string& t) { - key->Record(static_cast(t.length())); + key->Record(t.length()); + key->insert(key->end(), t.begin(), t.end()); +} + +template <> +void CacheKeySerializer::Serialize(CacheKey* key, const std::string_view& t) { + key->Record(t.length()); key->insert(key->end(), t.begin(), t.end()); } diff --git a/src/dawn/native/CacheKey.h b/src/dawn/native/CacheKey.h index c2901db40f..98a3ce7d87 100644 --- a/src/dawn/native/CacheKey.h +++ b/src/dawn/native/CacheKey.h @@ -19,7 +19,6 @@ #include #include #include -#include #include #include #include diff --git a/src/dawn/native/Device.cpp b/src/dawn/native/Device.cpp index 55435d810d..ab133bd7bc 100644 --- a/src/dawn/native/Device.cpp +++ b/src/dawn/native/Device.cpp @@ -20,6 +20,7 @@ #include #include "dawn/common/Log.h" +#include "dawn/common/Version_autogen.h" #include "dawn/native/Adapter.h" #include "dawn/native/AsyncTask.h" #include "dawn/native/AttachmentState.h" @@ -208,7 +209,7 @@ DeviceBase::DeviceBase(AdapterBase* adapter, const DeviceDescriptor* descriptor) // Record the cache key from the properties. Note that currently, if a new extension // descriptor is added (and probably handled here), the cache key recording needs to be // updated. - mDeviceCacheKey.Record(adapterProperties, mEnabledFeatures.featuresBitSet, + mDeviceCacheKey.Record(kDawnVersion, adapterProperties, mEnabledFeatures.featuresBitSet, mEnabledToggles.toggleBitset, cacheDesc); } diff --git a/src/dawn/native/Instance.cpp b/src/dawn/native/Instance.cpp index 3ea134c298..4a402a8432 100644 --- a/src/dawn/native/Instance.cpp +++ b/src/dawn/native/Instance.cpp @@ -20,7 +20,6 @@ #include "dawn/common/GPUInfo.h" #include "dawn/common/Log.h" #include "dawn/common/SystemUtils.h" -#include "dawn/common/Version_autogen.h" #include "dawn/native/ChainUtils_autogen.h" #include "dawn/native/ErrorData.h" #include "dawn/native/Surface.h" @@ -95,8 +94,8 @@ BackendsBitset GetEnabledBackends() { } dawn::platform::CachingInterface* GetCachingInterface(dawn::platform::Platform* platform) { - if (platform != nullptr && dawn::kDawnVersion.size() > 0) { - return platform->GetCachingInterface(dawn::kDawnVersion.data(), dawn::kDawnVersion.size()); + if (platform != nullptr) { + return platform->GetCachingInterface(); } return nullptr; } diff --git a/src/dawn/platform/DawnPlatform.cpp b/src/dawn/platform/DawnPlatform.cpp index 0d52a33f4c..3b22ade0cd 100644 --- a/src/dawn/platform/DawnPlatform.cpp +++ b/src/dawn/platform/DawnPlatform.cpp @@ -53,8 +53,7 @@ uint64_t Platform::AddTraceEvent(char phase, return 0; } -dawn::platform::CachingInterface* Platform::GetCachingInterface(const void* fingerprint, - size_t fingerprintSize) { +dawn::platform::CachingInterface* Platform::GetCachingInterface() { return nullptr; } diff --git a/src/dawn/tests/mocks/platform/CachingInterfaceMock.cpp b/src/dawn/tests/mocks/platform/CachingInterfaceMock.cpp index a52d4c2cec..4622963c34 100644 --- a/src/dawn/tests/mocks/platform/CachingInterfaceMock.cpp +++ b/src/dawn/tests/mocks/platform/CachingInterfaceMock.cpp @@ -79,8 +79,6 @@ void CachingInterfaceMock::StoreDataDefault(const void* key, DawnCachingMockPlatform::DawnCachingMockPlatform(dawn::platform::CachingInterface* cachingInterface) : mCachingInterface(cachingInterface) {} -dawn::platform::CachingInterface* DawnCachingMockPlatform::GetCachingInterface( - const void* fingerprint, - size_t fingerprintSize) { +dawn::platform::CachingInterface* DawnCachingMockPlatform::GetCachingInterface() { return mCachingInterface; } diff --git a/src/dawn/tests/mocks/platform/CachingInterfaceMock.h b/src/dawn/tests/mocks/platform/CachingInterfaceMock.h index 0e9e6aff0e..154957bb10 100644 --- a/src/dawn/tests/mocks/platform/CachingInterfaceMock.h +++ b/src/dawn/tests/mocks/platform/CachingInterfaceMock.h @@ -63,8 +63,8 @@ class CachingInterfaceMock : public dawn::platform::CachingInterface { class DawnCachingMockPlatform : public dawn::platform::Platform { public: explicit DawnCachingMockPlatform(dawn::platform::CachingInterface* cachingInterface); - dawn::platform::CachingInterface* GetCachingInterface(const void* fingerprint, - size_t fingerprintSize) override; + + dawn::platform::CachingInterface* GetCachingInterface() override; private: dawn::platform::CachingInterface* mCachingInterface = nullptr; diff --git a/src/dawn/tests/unittests/native/CacheKeyTests.cpp b/src/dawn/tests/unittests/native/CacheKeyTests.cpp index abd1acc072..615f2ac3ef 100644 --- a/src/dawn/tests/unittests/native/CacheKeyTests.cpp +++ b/src/dawn/tests/unittests/native/CacheKeyTests.cpp @@ -163,7 +163,17 @@ TEST(CacheKeySerializerTests, StdStrings) { std::string str = "string"; CacheKey expected; - expected.Record((size_t)6); + expected.Record(size_t(6)); + expected.insert(expected.end(), str.begin(), str.end()); + + EXPECT_THAT(CacheKey().Record(str), CacheKeyEq(expected)); +} + +TEST(CacheKeySerializerTests, StdStringViews) { + static constexpr std::string_view str("string"); + + CacheKey expected; + expected.Record(size_t(6)); expected.insert(expected.end(), str.begin(), str.end()); EXPECT_THAT(CacheKey().Record(str), CacheKeyEq(expected));