Add a return value to BlobCache::set

HWUI's shader cache uses a BlobCache for writing to persistent storage.
We would like to know whether the cache is large enough to hold the
necessary shaders, but BlobCache does not currently provide any way to
inspect it.

Add a new enum with possible outcomes from BlobCache::set and return it
from that method. This way a client can use this information, e.g. in
perfetto traces.

Bug: 231194869
Bug: 225211273
Test: atest libEGL_test (BlobCacheTest)
Change-Id: I9aade9da42cae83da053cc8d5e24999de1936de6

3 files changed, 95 insertions, 30 deletions

diff --git a/opengl/libs/EGL/BlobCache.cpp b/opengl/libs/EGL/BlobCache.cpp
index beca7f191a..86c788d3b3 100644
--- a/opengl/libs/EGL/BlobCache.cpp
+++ b/opengl/libs/EGL/BlobCache.cpp
@@ -52,35 +52,37 @@ BlobCache::BlobCache(size_t maxKeySize, size_t maxValueSize, size_t maxTotalSize
     ALOGV("initializing random seed using %lld", (unsigned long long)now);
 }
 
-void BlobCache::set(const void* key, size_t keySize, const void* value, size_t valueSize) {
+BlobCache::InsertResult BlobCache::set(const void* key, size_t keySize, const void* value,
+                                       size_t valueSize) {
     if (mMaxKeySize < keySize) {
         ALOGV("set: not caching because the key is too large: %zu (limit: %zu)", keySize,
               mMaxKeySize);
-        return;
+        return InsertResult::kKeyTooBig;
     }
     if (mMaxValueSize < valueSize) {
         ALOGV("set: not caching because the value is too large: %zu (limit: %zu)", valueSize,
               mMaxValueSize);
-        return;
+        return InsertResult::kValueTooBig;
     }
     if (mMaxTotalSize < keySize + valueSize) {
         ALOGV("set: not caching because the combined key/value size is too "
               "large: %zu (limit: %zu)",
               keySize + valueSize, mMaxTotalSize);
-        return;
+        return InsertResult::kCombinedTooBig;
     }
     if (keySize == 0) {
         ALOGW("set: not caching because keySize is 0");
-        return;
+        return InsertResult::kInvalidKeySize;
     }
-    if (valueSize <= 0) {
+    if (valueSize == 0) {
         ALOGW("set: not caching because valueSize is 0");
-        return;
+        return InsertResult::kInvalidValueSize;
     }
 
     std::shared_ptr<Blob> cacheKey(new Blob(key, keySize, false));
     CacheEntry cacheEntry(cacheKey, nullptr);
 
+    bool didClean = false;
     while (true) {
         auto index = std::lower_bound(mCacheEntries.begin(), mCacheEntries.end(), cacheEntry);
         if (index == mCacheEntries.end() || cacheEntry < *index) {
@@ -92,13 +94,14 @@ void BlobCache::set(const void* key, size_t keySize, const void* value, size_t v
                 if (isCleanable()) {
                     // Clean the cache and try again.
                     clean();
+                    didClean = true;
                     continue;
                 } else {
                     ALOGV("set: not caching new key/value pair because the "
                           "total cache size limit would be exceeded: %zu "
                           "(limit: %zu)",
                           keySize + valueSize, mMaxTotalSize);
-                    break;
+                    return InsertResult::kNotEnoughSpace;
                 }
             }
             mCacheEntries.insert(index, CacheEntry(keyBlob, valueBlob));
@@ -114,12 +117,13 @@ void BlobCache::set(const void* key, size_t keySize, const void* value, size_t v
                 if (isCleanable()) {
                     // Clean the cache and try again.
                     clean();
+                    didClean = true;
                     continue;
                 } else {
                     ALOGV("set: not caching new value because the total cache "
                           "size limit would be exceeded: %zu (limit: %zu)",
                           keySize + valueSize, mMaxTotalSize);
-                    break;
+                    return InsertResult::kNotEnoughSpace;
                 }
             }
             index->setValue(valueBlob);
@@ -128,7 +132,7 @@ void BlobCache::set(const void* key, size_t keySize, const void* value, size_t v
                   "value",
                   keySize, valueSize);
         }
-        break;
+        return didClean ? InsertResult::kDidClean : InsertResult::kInserted;
     }
 }
 
diff --git a/opengl/libs/EGL/BlobCache.h b/opengl/libs/EGL/BlobCache.h
index 50b4e4c6d0..ff03d30099 100644
--- a/opengl/libs/EGL/BlobCache.h
+++ b/opengl/libs/EGL/BlobCache.h
@@ -39,6 +39,26 @@ public:
     // (key sizes plus value sizes) will not exceed maxTotalSize.
     BlobCache(size_t maxKeySize, size_t maxValueSize, size_t maxTotalSize);
 
+    // Return value from set(), below.
+    enum class InsertResult {
+        // The key is larger than maxKeySize specified in the constructor.
+        kKeyTooBig,
+        // The value is larger than maxValueSize specified in the constructor.
+        kValueTooBig,
+        // The combined key + value is larger than maxTotalSize specified in the constructor.
+        kCombinedTooBig,
+        // keySize is 0
+        kInvalidKeySize,
+        // valueSize is 0
+        kInvalidValueSize,
+        // Unable to free enough space to fit the new entry.
+        kNotEnoughSpace,
+        // The new entry was inserted, but an old entry had to be evicted.
+        kDidClean,
+        // There was enough room in the cache and the new entry was inserted.
+        kInserted,
+
+    };
     // set inserts a new binary value into the cache and associates it with the
     // given binary key.  If the key or value are too large for the cache then
     // the cache remains unchanged.  This includes the case where a different
@@ -54,7 +74,7 @@ public:
     //   0 < keySize
     //   value != NULL
     //   0 < valueSize
-    void set(const void* key, size_t keySize, const void* value, size_t valueSize);
+    InsertResult set(const void* key, size_t keySize, const void* value, size_t valueSize);
 
     // get retrieves from the cache the binary value associated with a given
     // binary key.  If the key is present in the cache then the length of the
diff --git a/opengl/libs/EGL/BlobCache_test.cpp b/opengl/libs/EGL/BlobCache_test.cpp
index d31373b37a..ceea0fb979 100644
--- a/opengl/libs/EGL/BlobCache_test.cpp
+++ b/opengl/libs/EGL/BlobCache_test.cpp
@@ -49,7 +49,7 @@ protected:
 
 TEST_F(BlobCacheTest, CacheSingleValueSucceeds) {
     unsigned char buf[4] = {0xee, 0xee, 0xee, 0xee};
-    mBC->set("abcd", 4, "efgh", 4);
+    ASSERT_EQ(BlobCache::InsertResult::kInserted, mBC->set("abcd", 4, "efgh", 4));
     ASSERT_EQ(size_t(4), mBC->get("abcd", 4, buf, 4));
     ASSERT_EQ('e', buf[0]);
     ASSERT_EQ('f', buf[1]);
@@ -59,8 +59,8 @@ TEST_F(BlobCacheTest, CacheSingleValueSucceeds) {
 
 TEST_F(BlobCacheTest, CacheTwoValuesSucceeds) {
     unsigned char buf[2] = {0xee, 0xee};
-    mBC->set("ab", 2, "cd", 2);
-    mBC->set("ef", 2, "gh", 2);
+    ASSERT_EQ(BlobCache::InsertResult::kInserted, mBC->set("ab", 2, "cd", 2));
+    ASSERT_EQ(BlobCache::InsertResult::kInserted, mBC->set("ef", 2, "gh", 2));
     ASSERT_EQ(size_t(2), mBC->get("ab", 2, buf, 2));
     ASSERT_EQ('c', buf[0]);
     ASSERT_EQ('d', buf[1]);
@@ -71,7 +71,7 @@ TEST_F(BlobCacheTest, CacheTwoValuesSucceeds) {
 
 TEST_F(BlobCacheTest, GetOnlyWritesInsideBounds) {
     unsigned char buf[6] = {0xee, 0xee, 0xee, 0xee, 0xee, 0xee};
-    mBC->set("abcd", 4, "efgh", 4);
+    ASSERT_EQ(BlobCache::InsertResult::kInserted, mBC->set("abcd", 4, "efgh", 4));
     ASSERT_EQ(size_t(4), mBC->get("abcd", 4, buf + 1, 4));
     ASSERT_EQ(0xee, buf[0]);
     ASSERT_EQ('e', buf[1]);
@@ -83,7 +83,7 @@ TEST_F(BlobCacheTest, GetOnlyWritesInsideBounds) {
 
 TEST_F(BlobCacheTest, GetOnlyWritesIfBufferIsLargeEnough) {
     unsigned char buf[3] = {0xee, 0xee, 0xee};
-    mBC->set("abcd", 4, "efgh", 4);
+    ASSERT_EQ(BlobCache::InsertResult::kInserted, mBC->set("abcd", 4, "efgh", 4));
     ASSERT_EQ(size_t(4), mBC->get("abcd", 4, buf, 3));
     ASSERT_EQ(0xee, buf[0]);
     ASSERT_EQ(0xee, buf[1]);
@@ -91,14 +91,14 @@ TEST_F(BlobCacheTest, GetOnlyWritesIfBufferIsLargeEnough) {
 }
 
 TEST_F(BlobCacheTest, GetDoesntAccessNullBuffer) {
-    mBC->set("abcd", 4, "efgh", 4);
+    ASSERT_EQ(BlobCache::InsertResult::kInserted, mBC->set("abcd", 4, "efgh", 4));
     ASSERT_EQ(size_t(4), mBC->get("abcd", 4, nullptr, 0));
 }
 
 TEST_F(BlobCacheTest, MultipleSetsCacheLatestValue) {
     unsigned char buf[4] = {0xee, 0xee, 0xee, 0xee};
-    mBC->set("abcd", 4, "efgh", 4);
-    mBC->set("abcd", 4, "ijkl", 4);
+    ASSERT_EQ(BlobCache::InsertResult::kInserted, mBC->set("abcd", 4, "efgh", 4));
+    ASSERT_EQ(BlobCache::InsertResult::kInserted, mBC->set("abcd", 4, "ijkl", 4));
     ASSERT_EQ(size_t(4), mBC->get("abcd", 4, buf, 4));
     ASSERT_EQ('i', buf[0]);
     ASSERT_EQ('j', buf[1]);
@@ -108,8 +108,8 @@ TEST_F(BlobCacheTest, MultipleSetsCacheLatestValue) {
 
 TEST_F(BlobCacheTest, SecondSetKeepsFirstValueIfTooLarge) {
     unsigned char buf[MAX_VALUE_SIZE + 1] = {0xee, 0xee, 0xee, 0xee};
-    mBC->set("abcd", 4, "efgh", 4);
-    mBC->set("abcd", 4, buf, MAX_VALUE_SIZE + 1);
+    ASSERT_EQ(BlobCache::InsertResult::kInserted, mBC->set("abcd", 4, "efgh", 4));
+    ASSERT_EQ(BlobCache::InsertResult::kValueTooBig, mBC->set("abcd", 4, buf, MAX_VALUE_SIZE + 1));
     ASSERT_EQ(size_t(4), mBC->get("abcd", 4, buf, 4));
     ASSERT_EQ('e', buf[0]);
     ASSERT_EQ('f', buf[1]);
@@ -123,7 +123,7 @@ TEST_F(BlobCacheTest, DoesntCacheIfKeyIsTooBig) {
     for (int i = 0; i < MAX_KEY_SIZE + 1; i++) {
         key[i] = 'a';
     }
-    mBC->set(key, MAX_KEY_SIZE + 1, "bbbb", 4);
+    ASSERT_EQ(BlobCache::InsertResult::kKeyTooBig, mBC->set(key, MAX_KEY_SIZE + 1, "bbbb", 4));
     ASSERT_EQ(size_t(0), mBC->get(key, MAX_KEY_SIZE + 1, buf, 4));
     ASSERT_EQ(0xee, buf[0]);
     ASSERT_EQ(0xee, buf[1]);
@@ -136,7 +136,7 @@ TEST_F(BlobCacheTest, DoesntCacheIfValueIsTooBig) {
     for (int i = 0; i < MAX_VALUE_SIZE + 1; i++) {
         buf[i] = 'b';
     }
-    mBC->set("abcd", 4, buf, MAX_VALUE_SIZE + 1);
+    ASSERT_EQ(BlobCache::InsertResult::kValueTooBig, mBC->set("abcd", 4, buf, MAX_VALUE_SIZE + 1));
     for (int i = 0; i < MAX_VALUE_SIZE + 1; i++) {
         buf[i] = 0xee;
     }
@@ -163,7 +163,8 @@ TEST_F(BlobCacheTest, DoesntCacheIfKeyValuePairIsTooBig) {
         buf[i] = 'b';
     }
 
-    mBC->set(key, MAX_KEY_SIZE, buf, MAX_VALUE_SIZE);
+    ASSERT_EQ(BlobCache::InsertResult::kCombinedTooBig,
+              mBC->set(key, MAX_KEY_SIZE, buf, MAX_VALUE_SIZE));
     ASSERT_EQ(size_t(0), mBC->get(key, MAX_KEY_SIZE, nullptr, 0));
 }
 
@@ -173,7 +174,7 @@ TEST_F(BlobCacheTest, CacheMaxKeySizeSucceeds) {
     for (int i = 0; i < MAX_KEY_SIZE; i++) {
         key[i] = 'a';
     }
-    mBC->set(key, MAX_KEY_SIZE, "wxyz", 4);
+    ASSERT_EQ(BlobCache::InsertResult::kInserted, mBC->set(key, MAX_KEY_SIZE, "wxyz", 4));
     ASSERT_EQ(size_t(4), mBC->get(key, MAX_KEY_SIZE, buf, 4));
     ASSERT_EQ('w', buf[0]);
     ASSERT_EQ('x', buf[1]);
@@ -186,7 +187,7 @@ TEST_F(BlobCacheTest, CacheMaxValueSizeSucceeds) {
     for (int i = 0; i < MAX_VALUE_SIZE; i++) {
         buf[i] = 'b';
     }
-    mBC->set("abcd", 4, buf, MAX_VALUE_SIZE);
+    ASSERT_EQ(BlobCache::InsertResult::kInserted, mBC->set("abcd", 4, buf, MAX_VALUE_SIZE));
     for (int i = 0; i < MAX_VALUE_SIZE; i++) {
         buf[i] = 0xee;
     }
@@ -212,13 +213,45 @@ TEST_F(BlobCacheTest, CacheMaxKeyValuePairSizeSucceeds) {
         buf[i] = 'b';
     }
 
-    mBC->set(key, MAX_KEY_SIZE, buf, bufSize);
+    ASSERT_EQ(BlobCache::InsertResult::kInserted, mBC->set(key, MAX_KEY_SIZE, buf, bufSize));
     ASSERT_EQ(size_t(bufSize), mBC->get(key, MAX_KEY_SIZE, nullptr, 0));
 }
 
+// Verify that kNotEnoughSpace is returned from BlobCache::set when expected.
+// Note: This relies on internal knowledge of how BlobCache works.
+TEST_F(BlobCacheTest, NotEnoughSpace) {
+    // Insert a small entry into the cache.
+    ASSERT_EQ(BlobCache::InsertResult::kInserted, mBC->set("x", 1, "y", 1));
+
+    // Attempt to put a max size entry into the cache. If the cache were empty,
+    // as in CacheMaxKeyValuePairSizeSucceeds, this would succeed. Based on the
+    // current logic of BlobCache, the small entry is not big enough to allow it
+    // to be cleaned to insert the new entry.
+    ASSERT_TRUE(MAX_KEY_SIZE < MAX_TOTAL_SIZE);
+
+    enum { bufSize = MAX_TOTAL_SIZE - MAX_KEY_SIZE };
+
+    char key[MAX_KEY_SIZE];
+    char buf[bufSize];
+    for (int i = 0; i < MAX_KEY_SIZE; i++) {
+        key[i] = 'a';
+    }
+    for (int i = 0; i < bufSize; i++) {
+        buf[i] = 'b';
+    }
+
+    ASSERT_EQ(BlobCache::InsertResult::kNotEnoughSpace, mBC->set(key, MAX_KEY_SIZE, buf, bufSize));
+    ASSERT_EQ(0, mBC->get(key, MAX_KEY_SIZE, nullptr, 0));
+
+    // The original entry remains in the cache.
+    unsigned char buf2[1] = {0xee};
+    ASSERT_EQ(size_t(1), mBC->get("x", 1, buf2, 1));
+    ASSERT_EQ('y', buf2[0]);
+}
+
 TEST_F(BlobCacheTest, CacheMinKeyAndValueSizeSucceeds) {
     unsigned char buf[1] = {0xee};
-    mBC->set("x", 1, "y", 1);
+    ASSERT_EQ(BlobCache::InsertResult::kInserted, mBC->set("x", 1, "y", 1));
     ASSERT_EQ(size_t(1), mBC->get("x", 1, buf, 1));
     ASSERT_EQ('y', buf[0]);
 }
@@ -243,12 +276,12 @@ TEST_F(BlobCacheTest, ExceedingTotalLimitHalvesCacheSize) {
     const int maxEntries = MAX_TOTAL_SIZE / 2;
     for (int i = 0; i < maxEntries; i++) {
         uint8_t k = i;
-        mBC->set(&k, 1, "x", 1);
+        ASSERT_EQ(BlobCache::InsertResult::kInserted, mBC->set(&k, 1, "x", 1));
     }
     // Insert one more entry, causing a cache overflow.
     {
         uint8_t k = maxEntries;
-        mBC->set(&k, 1, "x", 1);
+        ASSERT_EQ(BlobCache::InsertResult::kDidClean, mBC->set(&k, 1, "x", 1));
     }
     // Count the number of entries in the cache.
     int numCached = 0;
@@ -261,6 +294,14 @@ TEST_F(BlobCacheTest, ExceedingTotalLimitHalvesCacheSize) {
     ASSERT_EQ(maxEntries / 2 + 1, numCached);
 }
 
+TEST_F(BlobCacheTest, InvalidKeySize) {
+    ASSERT_EQ(BlobCache::InsertResult::kInvalidKeySize, mBC->set("", 0, "efgh", 4));
+}
+
+TEST_F(BlobCacheTest, InvalidValueSize) {
+    ASSERT_EQ(BlobCache::InsertResult::kInvalidValueSize, mBC->set("abcd", 4, "", 0));
+}
+
 class BlobCacheFlattenTest : public BlobCacheTest {
 protected:
     virtual void SetUp() {