Merge "Add a return value to BlobCache::set" into tm-dev
diff --git a/opengl/libs/EGL/BlobCache.cpp b/opengl/libs/EGL/BlobCache.cpp
index beca7f1..86c788d 100644
--- a/opengl/libs/EGL/BlobCache.cpp
+++ b/opengl/libs/EGL/BlobCache.cpp
@@ -52,35 +52,37 @@
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 @@
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 @@
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 @@
"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 50b4e4c..ff03d30 100644
--- a/opengl/libs/EGL/BlobCache.h
+++ b/opengl/libs/EGL/BlobCache.h
@@ -39,6 +39,26 @@
// (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 @@
// 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 d31373b..ceea0fb 100644
--- a/opengl/libs/EGL/BlobCache_test.cpp
+++ b/opengl/libs/EGL/BlobCache_test.cpp
@@ -49,7 +49,7 @@
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, 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, 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, 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, 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, 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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() {