EGL Multifile Blobcache: Clean up key reuse
This CL contains multiple fixes that allow key reuse.
* set() now checks whether an entry is already present, and if so
removes it from hotcache and tracking before adding them.
* trackEntry and removeEntry now update the total cache size.
* applyLRU now correctly removes entries from all tracking.
Additional tests:
* SameKeyDifferentValues
* SameKeyLargeValues
Based on work by: Igor Nazarov <i.nazarov@samsung.com>
Test: libEGL_test, EGL_test, ANGLE trace tests, apps
Bug: b/351867582, b/380483358
Flag: com.android.graphics.egl.flags.multifile_blobcache_advanced_usage
Change-Id: I66fe9cde18e468e541a80296c80f2234ac61acb0
diff --git a/opengl/libs/EGL/MultifileBlobCache.cpp b/opengl/libs/EGL/MultifileBlobCache.cpp
index ebbb8c4..917671d 100644
--- a/opengl/libs/EGL/MultifileBlobCache.cpp
+++ b/opengl/libs/EGL/MultifileBlobCache.cpp
@@ -246,12 +246,9 @@
ALOGV("INIT: Entry %u is good, tracking it now.", entryHash);
- // Track details for rapid lookup later
+ // Track details for rapid lookup later and update total size
trackEntry(entryHash, header.valueSize, fileSize, st.st_atime);
- // Track the total size
- increaseTotalCacheSize(fileSize);
-
// Preload the entry for fast retrieval
if ((mHotCacheSize + fileSize) < mHotCacheLimit) {
ALOGV("INIT: Populating hot cache with fd = %i, cacheEntry = %p for "
@@ -326,6 +323,15 @@
// Generate a hash of the key and use it to track this entry
uint32_t entryHash = android::JenkinsHashMixBytes(0, static_cast<const uint8_t*>(key), keySize);
+ // See if we already have this file
+ if (flags::multifile_blobcache_advanced_usage() && contains(entryHash)) {
+ // Remove previous entry from hot cache
+ removeFromHotCache(entryHash);
+
+ // Remove previous entry and update the overall cache size
+ removeEntry(entryHash);
+ }
+
size_t fileSize = sizeof(MultifileHeader) + keySize + valueSize;
// If we're going to be over the cache limit, kick off a trim to clear space
@@ -350,12 +356,9 @@
std::string fullPath = mMultifileDirName + "/" + std::to_string(entryHash);
- // Track the size and access time for quick recall
+ // Track the size and access time for quick recall and update the overall cache size
trackEntry(entryHash, valueSize, fileSize, time(0));
- // Update the overall cache size
- increaseTotalCacheSize(fileSize);
-
// Keep the entry in hot cache for quick retrieval
ALOGV("SET: Adding %u to hot cache.", entryHash);
@@ -638,6 +641,27 @@
time_t accessTime) {
mEntries.insert(entryHash);
mEntryStats[entryHash] = {valueSize, fileSize, accessTime};
+
+ increaseTotalCacheSize(fileSize);
+}
+
+bool MultifileBlobCache::removeEntry(uint32_t entryHash) {
+ auto entryIter = mEntries.find(entryHash);
+ if (entryIter == mEntries.end()) {
+ return false;
+ }
+
+ auto entryStatsIter = mEntryStats.find(entryHash);
+ if (entryStatsIter == mEntryStats.end()) {
+ ALOGE("Failed to remove entryHash (%u) from mEntryStats", entryHash);
+ return false;
+ }
+ decreaseTotalCacheSize(entryStatsIter->second.fileSize);
+
+ mEntryStats.erase(entryStatsIter);
+ mEntries.erase(entryIter);
+
+ return true;
}
bool MultifileBlobCache::contains(uint32_t hashEntry) const {
@@ -728,13 +752,10 @@
// Walk through our map of sorted last access times and remove files until under the limit
for (auto cacheEntryIter = mEntryStats.begin(); cacheEntryIter != mEntryStats.end();) {
uint32_t entryHash = cacheEntryIter->first;
+ const MultifileEntryStats& entryStats = cacheEntryIter->second;
ALOGV("LRU: Removing entryHash %u", entryHash);
- // Track the overall size
- MultifileEntryStats entryStats = getEntryStats(entryHash);
- decreaseTotalCacheSize(entryStats.fileSize);
-
// Remove it from hot cache if present
removeFromHotCache(entryHash);
@@ -748,10 +769,9 @@
// Increment the iterator before clearing the entry
cacheEntryIter++;
- // Delete the entry from our tracking
- size_t count = mEntryStats.erase(entryHash);
- if (count != 1) {
- ALOGE("LRU: Failed to remove entryHash (%u) from mEntryStats", entryHash);
+ // Delete the entry from our tracking and update the overall cache size
+ if (!removeEntry(entryHash)) {
+ ALOGE("LRU: Failed to remove entryHash %u", entryHash);
return false;
}
diff --git a/opengl/libs/EGL/MultifileBlobCache_test.cpp b/opengl/libs/EGL/MultifileBlobCache_test.cpp
index 8222498..c95bf28 100644
--- a/opengl/libs/EGL/MultifileBlobCache_test.cpp
+++ b/opengl/libs/EGL/MultifileBlobCache_test.cpp
@@ -509,4 +509,102 @@
ASSERT_EQ(getCacheEntries().size(), 0);
}
+// Ensure cache is correct when a key is reused
+TEST_F(MultifileBlobCacheTest, SameKeyDifferentValues) {
+ if (!flags::multifile_blobcache_advanced_usage()) {
+ GTEST_SKIP() << "Skipping test that requires multifile_blobcache_advanced_usage flag";
+ }
+
+ unsigned char buf[4] = {0xee, 0xee, 0xee, 0xee};
+
+ size_t startingSize = mMBC->getTotalSize();
+
+ // New cache should be empty
+ ASSERT_EQ(startingSize, 0);
+
+ // Set an initial value
+ mMBC->set("ab", 2, "cdef", 4);
+
+ // Grab the new size
+ size_t firstSize = mMBC->getTotalSize();
+
+ // Ensure the size went up
+ // Note: Checking for an exact size is challenging, as the
+ // file size can differ between platforms.
+ ASSERT_GT(firstSize, startingSize);
+
+ // Verify the cache is correct
+ ASSERT_EQ(size_t(4), mMBC->get("ab", 2, buf, 4));
+ ASSERT_EQ('c', buf[0]);
+ ASSERT_EQ('d', buf[1]);
+ ASSERT_EQ('e', buf[2]);
+ ASSERT_EQ('f', buf[3]);
+
+ // Now reuse the key with a smaller value
+ mMBC->set("ab", 2, "gh", 2);
+
+ // Grab the new size
+ size_t secondSize = mMBC->getTotalSize();
+
+ // Ensure it decreased in size
+ ASSERT_LT(secondSize, firstSize);
+
+ // Verify the cache is correct
+ ASSERT_EQ(size_t(2), mMBC->get("ab", 2, buf, 2));
+ ASSERT_EQ('g', buf[0]);
+ ASSERT_EQ('h', buf[1]);
+
+ // Now put back the original value
+ mMBC->set("ab", 2, "cdef", 4);
+
+ // And we should get back a stable size
+ size_t finalSize = mMBC->getTotalSize();
+ ASSERT_EQ(firstSize, finalSize);
+}
+
+// Ensure cache is correct when a key is reused with large value size
+TEST_F(MultifileBlobCacheTest, SameKeyLargeValues) {
+ if (!flags::multifile_blobcache_advanced_usage()) {
+ GTEST_SKIP() << "Skipping test that requires multifile_blobcache_advanced_usage flag";
+ }
+
+ // Create the cache with larger limits to stress test reuse
+ constexpr uint32_t kLocalMaxKeySize = 1 * 1024 * 1024;
+ constexpr uint32_t kLocalMaxValueSize = 4 * 1024 * 1024;
+ constexpr uint32_t kLocalMaxTotalSize = 32 * 1024 * 1024;
+ mMBC.reset(new MultifileBlobCache(kLocalMaxKeySize, kLocalMaxValueSize, kLocalMaxTotalSize,
+ kMaxTotalEntries, &mTempFile->path[0]));
+
+ constexpr uint32_t kLargeValueCount = 8;
+ constexpr uint32_t kLargeValueSize = 64 * 1024;
+
+ // Create a several really large values
+ unsigned char largeValue[kLargeValueCount][kLargeValueSize];
+ for (int i = 0; i < kLargeValueCount; i++) {
+ for (int j = 0; j < kLargeValueSize; j++) {
+ // Fill the value with the index for uniqueness
+ largeValue[i][j] = i;
+ }
+ }
+
+ size_t startingSize = mMBC->getTotalSize();
+
+ // New cache should be empty
+ ASSERT_EQ(startingSize, 0);
+
+ // Cycle through the values and set them all in sequence
+ for (int i = 0; i < kLargeValueCount; i++) {
+ mMBC->set("abcd", 4, largeValue[i], kLargeValueSize);
+ }
+
+ // Ensure we get the last one back
+ unsigned char outBuf[kLargeValueSize];
+ mMBC->get("abcd", 4, outBuf, kLargeValueSize);
+
+ for (int i = 0; i < kLargeValueSize; i++) {
+ // Buffer should contain highest index value
+ ASSERT_EQ(kLargeValueCount - 1, outBuf[i]);
+ }
+}
+
} // namespace android