Skip to content

Commit

Permalink
Assume fixed size key (#10137)
Browse files Browse the repository at this point in the history 
Summary:
FastLRUCache now only supports 16B keys. The tests have changed to reflect this.

Because the unit tests were designed for caches that accept any string as keys, some tests are no longer compatible with FastLRUCache. We have disabled those for runs with FastLRUCache. (We could potentially change all tests to use 16B keys, but we don't because the cache public API does not require this.)

Pull Request resolved: facebook/rocksdb#10137

Test Plan: make -j24 check

Reviewed By: gitbw95

Differential Revision: D37083934

Pulled By: guidotag

fbshipit-source-id: be1719cf5f8364a9a32bc4555bce1a0de3833b0d
  • Loading branch information
@facebook-github-bot
Guido Tagliavini Ponce authored and facebook-github-bot committed Jun 11, 2022
1 parent 80afa77 commit 415200d
Show file tree
Hide file tree
Showing 4 changed files with 157 additions and 33 deletions.
131 changes: 100 additions & 31 deletions cache/cache_test.cc
View file
Expand Up @@ -25,18 +25,36 @@

namespace ROCKSDB_NAMESPACE {

namespace {

// Conversions between numeric keys/values and the types expected by Cache.
static std::string EncodeKey(int k) {
std::string EncodeKey16Bytes(int k) {
std::string result;
PutFixed32(&result, k);
result.append(std::string(12, 'a')); // Because we need a 16B output, we
// add a 12-byte padding.
return result;
}

int DecodeKey16Bytes(const Slice& k) {
assert(k.size() == 16);
return DecodeFixed32(k.data()); // Decodes only the first 4 bytes of k.
}

std::string EncodeKey32Bits(int k) {
std::string result;
PutFixed32(&result, k);
return result;
}
static int DecodeKey(const Slice& k) {

int DecodeKey32Bits(const Slice& k) {
assert(k.size() == 4);
return DecodeFixed32(k.data());
}
static void* EncodeValue(uintptr_t v) { return reinterpret_cast<void*>(v); }
static int DecodeValue(void* v) {

void* EncodeValue(uintptr_t v) { return reinterpret_cast<void*>(v); }

int DecodeValue(void* v) {
return static_cast<int>(reinterpret_cast<uintptr_t>(v));
}

Expand All @@ -51,12 +69,19 @@ void eraseDeleter(const Slice& /*key*/, void* value) {
cache->Erase("foo");
}

} // anonymous namespace

class CacheTest : public testing::TestWithParam<std::string> {
public:
static CacheTest* current_;
static std::string type_;

static void Deleter(const Slice& key, void* v) {
current_->deleted_keys_.push_back(DecodeKey(key));
if (type_ == kFast) {
current_->deleted_keys_.push_back(DecodeKey16Bytes(key));
} else {
current_->deleted_keys_.push_back(DecodeKey32Bits(key));
}
current_->deleted_values_.push_back(DecodeValue(v));
}

Expand All @@ -75,6 +100,7 @@ class CacheTest : public testing::TestWithParam<std::string> {
: cache_(NewCache(kCacheSize, kNumShardBits, false)),
cache2_(NewCache(kCacheSize2, kNumShardBits2, false)) {
current_ = this;
type_ = GetParam();
}

~CacheTest() override {}
Expand Down Expand Up @@ -117,6 +143,27 @@ class CacheTest : public testing::TestWithParam<std::string> {
return nullptr;
}

// These functions encode/decode keys in tests cases that use
// int keys.
// Currently, FastLRUCache requires keys to be 16B long, whereas
// LRUCache and ClockCache don't, so the encoding depends on
// the cache type.
std::string EncodeKey(int k) {
if (GetParam() == kFast) {
return EncodeKey16Bytes(k);
} else {
return EncodeKey32Bits(k);
}
}

int DecodeKey(const Slice& k) {
if (GetParam() == kFast) {
return DecodeKey16Bytes(k);
} else {
return DecodeKey32Bits(k);
}
}

int Lookup(std::shared_ptr<Cache> cache, int key) {
Cache::Handle* handle = cache->Lookup(EncodeKey(key));
const int r = (handle == nullptr) ? -1 : DecodeValue(cache->Value(handle));
Expand Down Expand Up @@ -160,11 +207,18 @@ class CacheTest : public testing::TestWithParam<std::string> {
Erase(cache2_, key);
}
};

CacheTest* CacheTest::current_;
std::string CacheTest::type_;

class LRUCacheTest : public CacheTest {};

TEST_P(CacheTest, UsageTest) {
if (GetParam() == kFast) {
ROCKSDB_GTEST_BYPASS("FastLRUCache requires 16 byte keys.");
return;
}

// cache is std::shared_ptr and will be automatically cleaned up.
const uint64_t kCapacity = 100000;
auto cache = NewCache(kCapacity, 8, false, kDontChargeCacheMetadata);
Expand Down Expand Up @@ -209,6 +263,11 @@ TEST_P(CacheTest, UsageTest) {
}

TEST_P(CacheTest, PinnedUsageTest) {
if (GetParam() == kFast) {
ROCKSDB_GTEST_BYPASS("FastLRUCache requires 16 byte keys.");
return;
}

// cache is std::shared_ptr and will be automatically cleaned up.
const uint64_t kCapacity = 200000;
auto cache = NewCache(kCapacity, 8, false, kDontChargeCacheMetadata);
Expand Down Expand Up @@ -461,12 +520,22 @@ TEST_P(CacheTest, EvictionPolicyRef) {
}

TEST_P(CacheTest, EvictEmptyCache) {
if (GetParam() == kFast) {
ROCKSDB_GTEST_BYPASS("FastLRUCache requires 16 byte keys.");
return;
}

// Insert item large than capacity to trigger eviction on empty cache.
auto cache = NewCache(1, 0, false);
ASSERT_OK(cache->Insert("foo", nullptr, 10, dumbDeleter));
}

TEST_P(CacheTest, EraseFromDeleter) {
if (GetParam() == kFast) {
ROCKSDB_GTEST_BYPASS("FastLRUCache requires 16 byte keys.");
return;
}

// Have deleter which will erase item from cache, which will re-enter
// the cache at that point.
std::shared_ptr<Cache> cache = NewCache(10, 0, false);
Expand Down Expand Up @@ -535,9 +604,9 @@ TEST_P(CacheTest, NewId) {

class Value {
public:
explicit Value(size_t v) : v_(v) { }
explicit Value(int v) : v_(v) {}

size_t v_;
int v_;
};

namespace {
Expand Down Expand Up @@ -585,8 +654,8 @@ TEST_P(CacheTest, SetCapacity) {
std::shared_ptr<Cache> cache = NewCache(5, 0, false);
std::vector<Cache::Handle*> handles(10);
// Insert 5 entries, but not releasing.
for (size_t i = 0; i < 5; i++) {
std::string key = std::to_string(i + 1);
for (int i = 0; i < 5; i++) {
std::string key = EncodeKey(i + 1);
Status s = cache->Insert(key, new Value(i + 1), 1, &deleter, &handles[i]);
ASSERT_TRUE(s.ok());
}
Expand All @@ -600,14 +669,14 @@ TEST_P(CacheTest, SetCapacity) {
// insert 5 more elements to cache, then release 5,
// then decrease capacity to 7, final capacity should be 7
// and usage should be 7
for (size_t i = 5; i < 10; i++) {
std::string key = std::to_string(i + 1);
for (int i = 5; i < 10; i++) {
std::string key = EncodeKey(i + 1);
Status s = cache->Insert(key, new Value(i + 1), 1, &deleter, &handles[i]);
ASSERT_TRUE(s.ok());
}
ASSERT_EQ(10U, cache->GetCapacity());
ASSERT_EQ(10U, cache->GetUsage());
for (size_t i = 0; i < 5; i++) {
for (int i = 0; i < 5; i++) {
cache->Release(handles[i]);
}
ASSERT_EQ(10U, cache->GetCapacity());
Expand All @@ -617,7 +686,7 @@ TEST_P(CacheTest, SetCapacity) {
ASSERT_EQ(7, cache->GetUsage());

// release remaining 5 to keep valgrind happy
for (size_t i = 5; i < 10; i++) {
for (int i = 5; i < 10; i++) {
cache->Release(handles[i]);
}

Expand All @@ -631,16 +700,16 @@ TEST_P(LRUCacheTest, SetStrictCapacityLimit) {
std::shared_ptr<Cache> cache = NewCache(5, 0, false);
std::vector<Cache::Handle*> handles(10);
Status s;
for (size_t i = 0; i < 10; i++) {
std::string key = std::to_string(i + 1);
for (int i = 0; i < 10; i++) {
std::string key = EncodeKey(i + 1);
s = cache->Insert(key, new Value(i + 1), 1, &deleter, &handles[i]);
ASSERT_OK(s);
ASSERT_NE(nullptr, handles[i]);
}
ASSERT_EQ(10, cache->GetUsage());

// test2: set the flag to true. Insert and check if it fails.
std::string extra_key = "extra";
std::string extra_key = EncodeKey(100);
Value* extra_value = new Value(0);
cache->SetStrictCapacityLimit(true);
Cache::Handle* handle;
Expand All @@ -649,14 +718,14 @@ TEST_P(LRUCacheTest, SetStrictCapacityLimit) {
ASSERT_EQ(nullptr, handle);
ASSERT_EQ(10, cache->GetUsage());

for (size_t i = 0; i < 10; i++) {
for (int i = 0; i < 10; i++) {
cache->Release(handles[i]);
}

// test3: init with flag being true.
std::shared_ptr<Cache> cache2 = NewCache(5, 0, true);
for (size_t i = 0; i < 5; i++) {
std::string key = std::to_string(i + 1);
for (int i = 0; i < 5; i++) {
std::string key = EncodeKey(i + 1);
s = cache2->Insert(key, new Value(i + 1), 1, &deleter, &handles[i]);
ASSERT_OK(s);
ASSERT_NE(nullptr, handles[i]);
Expand All @@ -671,7 +740,7 @@ TEST_P(LRUCacheTest, SetStrictCapacityLimit) {
ASSERT_EQ(5, cache2->GetUsage());
ASSERT_EQ(nullptr, cache2->Lookup(extra_key));

for (size_t i = 0; i < 5; i++) {
for (int i = 0; i < 5; i++) {
cache2->Release(handles[i]);
}
}
Expand All @@ -685,23 +754,23 @@ TEST_P(CacheTest, OverCapacity) {
std::vector<Cache::Handle*> handles(n+1);

// Insert n+1 entries, but not releasing.
for (size_t i = 0; i < n + 1; i++) {
std::string key = std::to_string(i + 1);
for (int i = 0; i < static_cast<int>(n + 1); i++) {
std::string key = EncodeKey(i + 1);
Status s = cache->Insert(key, new Value(i + 1), 1, &deleter, &handles[i]);
ASSERT_TRUE(s.ok());
}

// Guess what's in the cache now?
for (size_t i = 0; i < n + 1; i++) {
std::string key = std::to_string(i + 1);
for (int i = 0; i < static_cast<int>(n + 1); i++) {
std::string key = EncodeKey(i + 1);
auto h = cache->Lookup(key);
ASSERT_TRUE(h != nullptr);
if (h) cache->Release(h);
}

// the cache is over capacity since nothing could be evicted
ASSERT_EQ(n + 1U, cache->GetUsage());
for (size_t i = 0; i < n + 1; i++) {
for (int i = 0; i < static_cast<int>(n + 1); i++) {
cache->Release(handles[i]);
}
// Make sure eviction is triggered.
Expand All @@ -713,14 +782,14 @@ TEST_P(CacheTest, OverCapacity) {
// element 0 is evicted and the rest is there
// This is consistent with the LRU policy since the element 0
// was released first
for (size_t i = 0; i < n + 1; i++) {
std::string key = std::to_string(i + 1);
for (int i = 0; i < static_cast<int>(n + 1); i++) {
std::string key = EncodeKey(i + 1);
auto h = cache->Lookup(key);
if (h) {
ASSERT_NE(i, 0U);
ASSERT_NE(static_cast<size_t>(i), 0U);
cache->Release(h);
} else {
ASSERT_EQ(i, 0U);
ASSERT_EQ(static_cast<size_t>(i), 0U);
}
}
}
Expand All @@ -746,7 +815,7 @@ TEST_P(CacheTest, ApplyToAllCacheEntriesTest) {
std::sort(inserted.begin(), inserted.end());
std::sort(legacy_callback_state.begin(), legacy_callback_state.end());
ASSERT_EQ(inserted.size(), legacy_callback_state.size());
for (size_t i = 0; i < inserted.size(); ++i) {
for (int i = 0; i < static_cast<int>(inserted.size()); ++i) {
EXPECT_EQ(inserted[i], legacy_callback_state[i]);
}
}
Expand Down Expand Up @@ -774,7 +843,7 @@ TEST_P(CacheTest, ApplyToAllEntriesTest) {
std::sort(inserted.begin(), inserted.end());
std::sort(callback_state.begin(), callback_state.end());
ASSERT_EQ(inserted.size(), callback_state.size());
for (size_t i = 0; i < inserted.size(); ++i) {
for (int i = 0; i < static_cast<int>(inserted.size()); ++i) {
EXPECT_EQ(inserted[i], callback_state[i]);
}
}
Expand Down
4 changes: 4 additions & 0 deletions cache/fast_lru_cache.cc
View file
Expand Up @@ -368,6 +368,10 @@ Status LRUCacheShard::Insert(const Slice& key, uint32_t hash, void* value,
size_t charge, Cache::DeleterFn deleter,
Cache::Handle** handle,
Cache::Priority /*priority*/) {
if (key.size() != 16) {
return Status::NotSupported("FastLRUCache only supports key size 16B.");
}

// Allocate the memory here outside of the mutex.
// If the cache is full, we'll have to release it.
// It shouldn't happen very often though.
Expand Down
48 changes: 48 additions & 0 deletions cache/lru_cache_test.cc
View file
Expand Up @@ -9,6 +9,7 @@
#include <vector>

#include "cache/cache_key.h"
#include "cache/fast_lru_cache.h"
#include "db/db_test_util.h"
#include "file/sst_file_manager_impl.h"
#include "port/port.h"
Expand Down Expand Up @@ -205,6 +206,53 @@ TEST_F(LRUCacheTest, EntriesWithPriority) {
ValidateLRUList({"e", "f", "g", "Z", "d"}, 2);
}

// TODO(guido) Consolidate the following FastLRUCache tests with
// that of LRUCache.
class FastLRUCacheTest : public testing::Test {
public:
FastLRUCacheTest() {}
~FastLRUCacheTest() override { DeleteCache(); }

void DeleteCache() {
if (cache_ != nullptr) {
cache_->~LRUCacheShard();
port::cacheline_aligned_free(cache_);
cache_ = nullptr;
}
}

void NewCache(size_t capacity) {
DeleteCache();
cache_ = reinterpret_cast<fast_lru_cache::LRUCacheShard*>(
port::cacheline_aligned_alloc(sizeof(fast_lru_cache::LRUCacheShard)));
new (cache_) fast_lru_cache::LRUCacheShard(
capacity, false /*strict_capcity_limit*/, kDontChargeCacheMetadata,
24 /*max_upper_hash_bits*/);
}

Status Insert(const std::string& key) {
return cache_->Insert(key, 0 /*hash*/, nullptr /*value*/, 1 /*charge*/,
nullptr /*deleter*/, nullptr /*handle*/,
Cache::Priority::LOW);
}

Status Insert(char key, size_t len) { return Insert(std::string(len, key)); }

private:
fast_lru_cache::LRUCacheShard* cache_ = nullptr;
};

TEST_F(FastLRUCacheTest, ValidateKeySize) {
NewCache(3);
EXPECT_OK(Insert('a', 16));
EXPECT_NOK(Insert('b', 15));
EXPECT_OK(Insert('b', 16));
EXPECT_NOK(Insert('c', 17));
EXPECT_NOK(Insert('d', 1000));
EXPECT_NOK(Insert('e', 11));
EXPECT_NOK(Insert('f', 0));
}

class TestSecondaryCache : public SecondaryCache {
public:
// Specifies what action to take on a lookup for a particular key
Expand Down

0 comments on commit 415200d

Please sign in to comment.