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 <cwallez@chromium.org>
Commit-Queue: Loko Kung <lokokung@google.com>
diff --git a/include/dawn/platform/DawnPlatform.h b/include/dawn/platform/DawnPlatform.h
index 5c616d1..0645441 100644
--- a/include/dawn/platform/DawnPlatform.h
+++ b/include/dawn/platform/DawnPlatform.h
@@ -93,10 +93,10 @@
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<WorkerTaskPool> CreateWorkerTaskPool();
private:
diff --git a/src/dawn/native/BlobCache.cpp b/src/dawn/native/BlobCache.cpp
index 435f12d..a919318 100644
--- a/src/dawn/native/BlobCache.cpp
+++ b/src/dawn/native/BlobCache.cpp
@@ -14,7 +14,10 @@
#include "dawn/native/BlobCache.h"
+#include <algorithm>
+
#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 @@
}
Blob BlobCache::LoadInternal(const CacheKey& key) {
+ ASSERT(ValidateCacheKey(key));
if (mCache == nullptr) {
return Blob();
}
@@ -55,6 +59,7 @@
}
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 @@
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 615af2f..61753a8 100644
--- a/src/dawn/native/BlobCache.h
+++ b/src/dawn/native/BlobCache.h
@@ -48,6 +48,10 @@
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 495b013..414b915 100644
--- a/src/dawn/native/CacheKey.cpp
+++ b/src/dawn/native/CacheKey.cpp
@@ -15,6 +15,8 @@
#include "dawn/native/CacheKey.h"
#include <iomanip>
+#include <string>
+#include <string_view>
namespace dawn::native {
@@ -29,7 +31,13 @@
template <>
void CacheKeySerializer<std::string>::Serialize(CacheKey* key, const std::string& t) {
- key->Record(static_cast<size_t>(t.length()));
+ key->Record(t.length());
+ key->insert(key->end(), t.begin(), t.end());
+}
+
+template <>
+void CacheKeySerializer<std::string_view>::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 c2901db..98a3ce7 100644
--- a/src/dawn/native/CacheKey.h
+++ b/src/dawn/native/CacheKey.h
@@ -19,7 +19,6 @@
#include <iostream>
#include <limits>
#include <memory>
-#include <string>
#include <type_traits>
#include <utility>
#include <vector>
diff --git a/src/dawn/native/Device.cpp b/src/dawn/native/Device.cpp
index 55435d8..ab133bd 100644
--- a/src/dawn/native/Device.cpp
+++ b/src/dawn/native/Device.cpp
@@ -20,6 +20,7 @@
#include <unordered_set>
#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 @@
// 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..4a402a8 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 @@
}
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 0d52a33..3b22ade 100644
--- a/src/dawn/platform/DawnPlatform.cpp
+++ b/src/dawn/platform/DawnPlatform.cpp
@@ -53,8 +53,7 @@
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 a52d4c2..4622963 100644
--- a/src/dawn/tests/mocks/platform/CachingInterfaceMock.cpp
+++ b/src/dawn/tests/mocks/platform/CachingInterfaceMock.cpp
@@ -79,8 +79,6 @@
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 0e9e6af..154957b 100644
--- a/src/dawn/tests/mocks/platform/CachingInterfaceMock.h
+++ b/src/dawn/tests/mocks/platform/CachingInterfaceMock.h
@@ -63,8 +63,8 @@
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 abd1acc..615f2ac 100644
--- a/src/dawn/tests/unittests/native/CacheKeyTests.cpp
+++ b/src/dawn/tests/unittests/native/CacheKeyTests.cpp
@@ -163,7 +163,17 @@
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));
