Skip to content

Commit

Permalink
Add a return value to BlobCache::set
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@LeonScroggins
LeonScroggins committed May 3, 2022
1 parent f7f5aac commit 2337796
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 30 deletions.
24 changes: 14 additions & 10 deletions opengl/libs/EGL/BlobCache.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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));
Expand All @@ -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);
Expand All @@ -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;
}
}

Expand Down
22 changes: 21 additions & 1 deletion opengl/libs/EGL/BlobCache.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,26 @@ class BlobCache {
// (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
Expand All @@ -54,7 +74,7 @@ class BlobCache {
// 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
Expand Down
79 changes: 60 additions & 19 deletions opengl/libs/EGL/BlobCache_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class BlobCacheTest : public ::testing::Test {

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]);
Expand All @@ -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]);
Expand All @@ -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]);
Expand All @@ -83,22 +83,22 @@ 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]);
ASSERT_EQ(0xee, buf[2]);
}

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]);
Expand All @@ -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]);
Expand All @@ -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]);
Expand All @@ -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;
}
Expand All @@ -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));
}

Expand All @@ -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]);
Expand All @@ -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;
}
Expand All @@ -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]);
}
Expand All @@ -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;
Expand All @@ -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() {
Expand Down

0 comments on commit 2337796

Please sign in to comment.