From 94f76df5e53e73dfbe9ccac94e1696b59086e59c Mon Sep 17 00:00:00 2001 From: Guido Tagliavini Ponce Date: Wed, 22 Jun 2022 15:34:05 -0700 Subject: [PATCH 01/12] Bake load factor in CalcHashBits calculation. --- cache/fast_lru_cache.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cache/fast_lru_cache.cc b/cache/fast_lru_cache.cc index e003673ad99..25b7506f1a9 100644 --- a/cache/fast_lru_cache.cc +++ b/cache/fast_lru_cache.cc @@ -192,8 +192,7 @@ LRUCacheShard::LRUCacheShard(size_t capacity, size_t estimated_value_size, : capacity_(capacity), strict_capacity_limit_(strict_capacity_limit), table_( - CalcHashBits(capacity, estimated_value_size, metadata_charge_policy) + - static_cast(ceil(log2(1.0 / kLoadFactor)))), + CalcHashBits(capacity, estimated_value_size, metadata_charge_policy)), usage_(0), lru_usage_(0) { set_metadata_charge_policy(metadata_charge_policy); @@ -300,7 +299,7 @@ uint8_t LRUCacheShard::CalcHashBits( CacheMetadataChargePolicy metadata_charge_policy) { LRUHandle h; h.CalcTotalCharge(estimated_value_size, metadata_charge_policy); - size_t num_entries = capacity / h.total_charge; + size_t num_entries = static_cast(capacity / (kLoadFactor * h.total_charge)); uint8_t num_hash_bits = 0; while (num_entries >>= 1) { ++num_hash_bits; From 4e52d18f1769bd76fe498e0a686fe10a3604deb8 Mon Sep 17 00:00:00 2001 From: Guido Tagliavini Ponce Date: Wed, 22 Jun 2022 16:05:18 -0700 Subject: [PATCH 02/12] Format. --- cache/fast_lru_cache.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cache/fast_lru_cache.cc b/cache/fast_lru_cache.cc index 25b7506f1a9..15c40b2c7f4 100644 --- a/cache/fast_lru_cache.cc +++ b/cache/fast_lru_cache.cc @@ -299,7 +299,8 @@ uint8_t LRUCacheShard::CalcHashBits( CacheMetadataChargePolicy metadata_charge_policy) { LRUHandle h; h.CalcTotalCharge(estimated_value_size, metadata_charge_policy); - size_t num_entries = static_cast(capacity / (kLoadFactor * h.total_charge)); + size_t num_entries = + static_cast(capacity / (kLoadFactor * h.total_charge)); uint8_t num_hash_bits = 0; while (num_entries >>= 1) { ++num_hash_bits; From fe06d3ea0cca61271e7cb9572a559015ef962100 Mon Sep 17 00:00:00 2001 From: Guido Tagliavini Ponce Date: Mon, 27 Jun 2022 11:30:38 -0700 Subject: [PATCH 03/12] CalcHashBits test. --- cache/fast_lru_cache.h | 2 ++ cache/lru_cache_test.cc | 62 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/cache/fast_lru_cache.h b/cache/fast_lru_cache.h index 9fcd694f26c..a25ddc816e9 100644 --- a/cache/fast_lru_cache.h +++ b/cache/fast_lru_cache.h @@ -365,6 +365,8 @@ class ALIGN_AS(CACHE_LINE_SIZE) LRUCacheShard final : public CacheShard { private: friend class LRUCache; + friend class FastLRUCacheTest; + void LRU_Remove(LRUHandle* e); void LRU_Insert(LRUHandle* e); diff --git a/cache/lru_cache_test.cc b/cache/lru_cache_test.cc index 8af05c5d740..736789c1987 100644 --- a/cache/lru_cache_test.cc +++ b/cache/lru_cache_test.cc @@ -238,6 +238,26 @@ class FastLRUCacheTest : public testing::Test { Status Insert(char key, size_t len) { return Insert(std::string(len, key)); } + uint8_t CalcHashBitsWrapper(size_t capacity, size_t estimated_value_size, CacheMetadataChargePolicy metadata_charge_policy) { + return fast_lru_cache::LRUCacheShard::CalcHashBits(capacity, estimated_value_size, metadata_charge_policy); + } + + size_t CalcHandleCharge(size_t estimated_value_size, CacheMetadataChargePolicy metadata_charge_policy) { + LRUHandle h; + h.CalcTotalCharge(estimated_value_size, kDontChargeCacheMetadata); + return h.total_charge; + } + + double CalcMaxOccupancy(size_t capacity, size_t estimated_value_size, CacheMetadataChargePolicy metadata_charge_policy) { + size_t handle_charge = CalcHandleCharge(estimated_value_size, metadata_charge_policy); + return capacity / (fast_lru_cache::kLoadFactor * handle_charge); + } + + void ExpectTableSizeRoughlyMaxOccupancy(uint8_t hash_bits, double max_occupancy) { + EXPECT_GT(1 << hash_bits, max_occupancy); + EXPECT_LT(1 << (hash_bits - 1), max_occupancy); + } + private: fast_lru_cache::LRUCacheShard* cache_ = nullptr; }; @@ -253,6 +273,48 @@ TEST_F(FastLRUCacheTest, ValidateKeySize) { EXPECT_NOK(Insert('f', 0)); } +TEST_F(FastLRUCacheTest, CalcHashBitsTest) { + size_t capacity = 1024; + size_t estimated_value_size = 1; + CacheMetadataChargePolicy metadata_charge_policy = kDontChargeCacheMetadata; + double max_occupancy = CalcMaxOccupancy(capacity, estimated_value_size, metadata_charge_policy); + uint8_t hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, metadata_charge_policy); + ExpectTableSizeRoughlyMaxOccupancy(hash_bits, max_occupancy); + + capacity = 1024; + estimated_value_size = 1; + metadata_charge_policy = kDefaultCacheMetadataChargePolicy; + max_occupancy = CalcMaxOccupancy(capacity, estimated_value_size, metadata_charge_policy); + hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, metadata_charge_policy); + ExpectTableSizeRoughlyMaxOccupancy(hash_bits, max_occupancy); + + // Capacity below the size of a handle. No elements fit in the cache. + capacity = 1; + estimated_value_size = 1; + metadata_charge_policy = kDontChargeCacheMetadata; + max_occupancy = CalcMaxOccupancy(capacity, estimated_value_size, metadata_charge_policy); + hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, metadata_charge_policy); + ExpectTableSizeRoughlyMaxOccupancy(hash_bits, max_occupancy); + + // Capacity below the size of a handle, but because the load factor is < 100% + // at least one handle will fit. + estimated_value_size = 1; + capacity = CalcHandleCharge(1, kDontChargeCacheMetadata) - 1; + assert(capacity / fast_lru_cache::kLoadFactor > capacity); + metadata_charge_policy = kDontChargeCacheMetadata; + max_occupancy = CalcMaxOccupancy(capacity, estimated_value_size, metadata_charge_policy); + hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, metadata_charge_policy); + ExpectTableSizeRoughlyMaxOccupancy(hash_bits, max_occupancy); + + // Large capacity. + capacity = 31924172; + estimated_value_size = 1; + metadata_charge_policy = kDefaultCacheMetadataChargePolicy; + max_occupancy = CalcMaxOccupancy(capacity, estimated_value_size, metadata_charge_policy); + hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, metadata_charge_policy); + ExpectTableSizeRoughlyMaxOccupancy(hash_bits, max_occupancy); +} + class TestSecondaryCache : public SecondaryCache { public: // Specifies what action to take on a lookup for a particular key From a131b9d774f0d496ba9d45d1a2c595b02141853f Mon Sep 17 00:00:00 2001 From: Guido Tagliavini Ponce Date: Mon, 27 Jun 2022 11:34:37 -0700 Subject: [PATCH 04/12] Fix unused parameter. --- cache/lru_cache_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cache/lru_cache_test.cc b/cache/lru_cache_test.cc index 736789c1987..f2f26e02520 100644 --- a/cache/lru_cache_test.cc +++ b/cache/lru_cache_test.cc @@ -244,7 +244,7 @@ class FastLRUCacheTest : public testing::Test { size_t CalcHandleCharge(size_t estimated_value_size, CacheMetadataChargePolicy metadata_charge_policy) { LRUHandle h; - h.CalcTotalCharge(estimated_value_size, kDontChargeCacheMetadata); + h.CalcTotalCharge(estimated_value_size, metadata_charge_policy); return h.total_charge; } @@ -308,7 +308,7 @@ TEST_F(FastLRUCacheTest, CalcHashBitsTest) { // Large capacity. capacity = 31924172; - estimated_value_size = 1; + estimated_value_size = 321; metadata_charge_policy = kDefaultCacheMetadataChargePolicy; max_occupancy = CalcMaxOccupancy(capacity, estimated_value_size, metadata_charge_policy); hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, metadata_charge_policy); From adf5be207a8021f76cd8e4c646bccda2924bfcd8 Mon Sep 17 00:00:00 2001 From: Guido Tagliavini Ponce Date: Mon, 27 Jun 2022 11:48:15 -0700 Subject: [PATCH 05/12] Fix bug. --- cache/lru_cache_test.cc | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/cache/lru_cache_test.cc b/cache/lru_cache_test.cc index f2f26e02520..708b87c9c76 100644 --- a/cache/lru_cache_test.cc +++ b/cache/lru_cache_test.cc @@ -254,8 +254,12 @@ class FastLRUCacheTest : public testing::Test { } void ExpectTableSizeRoughlyMaxOccupancy(uint8_t hash_bits, double max_occupancy) { - EXPECT_GT(1 << hash_bits, max_occupancy); - EXPECT_LT(1 << (hash_bits - 1), max_occupancy); + if (hash_bits == 0) { + EXPECT_EQ(max_occupancy, 0); + } else { + EXPECT_GE(1 << hash_bits, max_occupancy); + EXPECT_LE(1 << (hash_bits - 1), max_occupancy); + } } private: @@ -283,18 +287,17 @@ TEST_F(FastLRUCacheTest, CalcHashBitsTest) { capacity = 1024; estimated_value_size = 1; - metadata_charge_policy = kDefaultCacheMetadataChargePolicy; + metadata_charge_policy = kFullChargeCacheMetadata; max_occupancy = CalcMaxOccupancy(capacity, estimated_value_size, metadata_charge_policy); hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, metadata_charge_policy); ExpectTableSizeRoughlyMaxOccupancy(hash_bits, max_occupancy); - // Capacity below the size of a handle. No elements fit in the cache. - capacity = 1; + // No elements fit in cache. + capacity = 0; estimated_value_size = 1; metadata_charge_policy = kDontChargeCacheMetadata; - max_occupancy = CalcMaxOccupancy(capacity, estimated_value_size, metadata_charge_policy); hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, metadata_charge_policy); - ExpectTableSizeRoughlyMaxOccupancy(hash_bits, max_occupancy); + ExpectTableSizeRoughlyMaxOccupancy(hash_bits, 0); // Capacity below the size of a handle, but because the load factor is < 100% // at least one handle will fit. @@ -309,7 +312,7 @@ TEST_F(FastLRUCacheTest, CalcHashBitsTest) { // Large capacity. capacity = 31924172; estimated_value_size = 321; - metadata_charge_policy = kDefaultCacheMetadataChargePolicy; + metadata_charge_policy = kFullChargeCacheMetadata; max_occupancy = CalcMaxOccupancy(capacity, estimated_value_size, metadata_charge_policy); hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, metadata_charge_policy); ExpectTableSizeRoughlyMaxOccupancy(hash_bits, max_occupancy); From 09cd0f9a9aac0b28092e7dd972bae814879ff1b7 Mon Sep 17 00:00:00 2001 From: Guido Tagliavini Ponce Date: Mon, 27 Jun 2022 14:11:59 -0700 Subject: [PATCH 06/12] Fix friend class issue. --- cache/fast_lru_cache.cc | 22 +++++++++++++---- cache/fast_lru_cache.h | 10 +++++++- cache/lru_cache_test.cc | 54 ++++++++++++++++++++++++++--------------- 3 files changed, 60 insertions(+), 26 deletions(-) diff --git a/cache/fast_lru_cache.cc b/cache/fast_lru_cache.cc index 15c40b2c7f4..21f22c6c2f3 100644 --- a/cache/fast_lru_cache.cc +++ b/cache/fast_lru_cache.cc @@ -294,18 +294,30 @@ void LRUCacheShard::EvictFromLRU(size_t charge, } } -uint8_t LRUCacheShard::CalcHashBits( - size_t capacity, size_t estimated_value_size, +size_t LRUCacheShard::CalcEstimatedHandleCharge( + size_t estimated_value_size, CacheMetadataChargePolicy metadata_charge_policy) { LRUHandle h; h.CalcTotalCharge(estimated_value_size, metadata_charge_policy); + return h.total_charge; +} + +uint8_t LRUCacheShard::CalcHashBits( + size_t capacity, size_t estimated_value_size, + CacheMetadataChargePolicy metadata_charge_policy) { + size_t handle_charge = + CalcEstimatedHandleCharge(estimated_value_size, metadata_charge_policy); size_t num_entries = - static_cast(capacity / (kLoadFactor * h.total_charge)); + static_cast(capacity / (kLoadFactor * handle_charge)); uint8_t num_hash_bits = 0; - while (num_entries >>= 1) { + size_t num_entries_copy = num_entries; + while (num_entries_copy >>= 1) { ++num_hash_bits; } - return num_hash_bits; + // Compute the ceiling of log2(num_entries). If num_entries == 0, return 1. + return static_cast(1 << num_hash_bits) < num_entries + ? num_hash_bits + 1 + : num_hash_bits; } void LRUCacheShard::SetCapacity(size_t capacity) { diff --git a/cache/fast_lru_cache.h b/cache/fast_lru_cache.h index a25ddc816e9..e6164c1ab82 100644 --- a/cache/fast_lru_cache.h +++ b/cache/fast_lru_cache.h @@ -25,6 +25,9 @@ namespace ROCKSDB_NAMESPACE { namespace fast_lru_cache { +// Forward declaration of friend class. +class FastLRUCacheTest; + // LRU cache implementation using an open-address hash table. // // Every slot in the hash table is an LRUHandle. Because handles can be @@ -365,7 +368,7 @@ class ALIGN_AS(CACHE_LINE_SIZE) LRUCacheShard final : public CacheShard { private: friend class LRUCache; - friend class FastLRUCacheTest; + friend class ROCKSDB_NAMESPACE::fast_lru_cache::FastLRUCacheTest; void LRU_Remove(LRUHandle* e); void LRU_Insert(LRUHandle* e); @@ -376,6 +379,11 @@ class ALIGN_AS(CACHE_LINE_SIZE) LRUCacheShard final : public CacheShard { // holding the mutex_. void EvictFromLRU(size_t charge, autovector* deleted); + // Returns the charge of a single handle. + static size_t CalcEstimatedHandleCharge( + size_t estimated_value_size, + CacheMetadataChargePolicy metadata_charge_policy); + // Returns the number of bits used to hash an element in the hash // table. static uint8_t CalcHashBits(size_t capacity, size_t estimated_value_size, diff --git a/cache/lru_cache_test.cc b/cache/lru_cache_test.cc index 708b87c9c76..7983a53d5dc 100644 --- a/cache/lru_cache_test.cc +++ b/cache/lru_cache_test.cc @@ -206,6 +206,7 @@ TEST_F(LRUCacheTest, EntriesWithPriority) { ValidateLRUList({"e", "f", "g", "Z", "d"}, 2); } +namespace fast_lru_cache { // TODO(guido) Consolidate the following FastLRUCache tests with // that of LRUCache. class FastLRUCacheTest : public testing::Test { @@ -238,27 +239,34 @@ class FastLRUCacheTest : public testing::Test { Status Insert(char key, size_t len) { return Insert(std::string(len, key)); } - uint8_t CalcHashBitsWrapper(size_t capacity, size_t estimated_value_size, CacheMetadataChargePolicy metadata_charge_policy) { - return fast_lru_cache::LRUCacheShard::CalcHashBits(capacity, estimated_value_size, metadata_charge_policy); + size_t CalcEstimatedHandleChargeWrapper( + size_t estimated_value_size, + CacheMetadataChargePolicy metadata_charge_policy) { + return fast_lru_cache::LRUCacheShard::CalcEstimatedHandleCharge( + estimated_value_size, metadata_charge_policy); } - size_t CalcHandleCharge(size_t estimated_value_size, CacheMetadataChargePolicy metadata_charge_policy) { - LRUHandle h; - h.CalcTotalCharge(estimated_value_size, metadata_charge_policy); - return h.total_charge; + uint8_t CalcHashBitsWrapper( + size_t capacity, size_t estimated_value_size, + CacheMetadataChargePolicy metadata_charge_policy) { + return fast_lru_cache::LRUCacheShard::CalcHashBits( + capacity, estimated_value_size, metadata_charge_policy); } + // Maximum number of items that a shard can hold. double CalcMaxOccupancy(size_t capacity, size_t estimated_value_size, CacheMetadataChargePolicy metadata_charge_policy) { - size_t handle_charge = CalcHandleCharge(estimated_value_size, metadata_charge_policy); + size_t handle_charge = + fast_lru_cache::LRUCacheShard::CalcEstimatedHandleCharge( + estimated_value_size, metadata_charge_policy); return capacity / (fast_lru_cache::kLoadFactor * handle_charge); } - void ExpectTableSizeRoughlyMaxOccupancy(uint8_t hash_bits, double max_occupancy) { + bool TableSizeIsAppropriate(uint8_t hash_bits, double max_occupancy) { if (hash_bits == 0) { - EXPECT_EQ(max_occupancy, 0); + return max_occupancy <= 1; } else { - EXPECT_GE(1 << hash_bits, max_occupancy); - EXPECT_LE(1 << (hash_bits - 1), max_occupancy); + return (1 << hash_bits >= max_occupancy) && + (1 << (hash_bits - 1) <= max_occupancy); } } @@ -283,31 +291,35 @@ TEST_F(FastLRUCacheTest, CalcHashBitsTest) { CacheMetadataChargePolicy metadata_charge_policy = kDontChargeCacheMetadata; double max_occupancy = CalcMaxOccupancy(capacity, estimated_value_size, metadata_charge_policy); uint8_t hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, metadata_charge_policy); - ExpectTableSizeRoughlyMaxOccupancy(hash_bits, max_occupancy); + EXPECT_TRUE(TableSizeIsAppropriate(hash_bits, max_occupancy)); capacity = 1024; estimated_value_size = 1; metadata_charge_policy = kFullChargeCacheMetadata; max_occupancy = CalcMaxOccupancy(capacity, estimated_value_size, metadata_charge_policy); hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, metadata_charge_policy); - ExpectTableSizeRoughlyMaxOccupancy(hash_bits, max_occupancy); + EXPECT_TRUE(TableSizeIsAppropriate(hash_bits, max_occupancy)); // No elements fit in cache. capacity = 0; estimated_value_size = 1; metadata_charge_policy = kDontChargeCacheMetadata; hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, metadata_charge_policy); - ExpectTableSizeRoughlyMaxOccupancy(hash_bits, 0); + EXPECT_TRUE(TableSizeIsAppropriate(hash_bits, 0 /* max_occupancy */)); - // Capacity below the size of a handle, but because the load factor is < 100% - // at least one handle will fit. + // Set the capacity just below a single handle. Because the load factor is < + // 100% at least one handle will fit in the table. estimated_value_size = 1; - capacity = CalcHandleCharge(1, kDontChargeCacheMetadata) - 1; - assert(capacity / fast_lru_cache::kLoadFactor > capacity); + size_t handle_charge = CalcEstimatedHandleChargeWrapper( + 8192 /* estimated_value_size */, kDontChargeCacheMetadata); + capacity = handle_charge - 1; + // The load factor should be bounded away from 100%. + assert(static_cast(capacity / fast_lru_cache::kLoadFactor) > + handle_charge); metadata_charge_policy = kDontChargeCacheMetadata; max_occupancy = CalcMaxOccupancy(capacity, estimated_value_size, metadata_charge_policy); hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, metadata_charge_policy); - ExpectTableSizeRoughlyMaxOccupancy(hash_bits, max_occupancy); + EXPECT_TRUE(TableSizeIsAppropriate(hash_bits, max_occupancy)); // Large capacity. capacity = 31924172; @@ -315,9 +327,11 @@ TEST_F(FastLRUCacheTest, CalcHashBitsTest) { metadata_charge_policy = kFullChargeCacheMetadata; max_occupancy = CalcMaxOccupancy(capacity, estimated_value_size, metadata_charge_policy); hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, metadata_charge_policy); - ExpectTableSizeRoughlyMaxOccupancy(hash_bits, max_occupancy); + EXPECT_TRUE(TableSizeIsAppropriate(hash_bits, max_occupancy)); } +} // namespace fast_lru_cache + class TestSecondaryCache : public SecondaryCache { public: // Specifies what action to take on a lookup for a particular key From 6bf135c9a8e393ffd513be12efed8e0482cbbeaf Mon Sep 17 00:00:00 2001 From: Guido Tagliavini Ponce Date: Mon, 27 Jun 2022 14:16:42 -0700 Subject: [PATCH 07/12] Format. --- cache/lru_cache_test.cc | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/cache/lru_cache_test.cc b/cache/lru_cache_test.cc index 7983a53d5dc..5767d9186e8 100644 --- a/cache/lru_cache_test.cc +++ b/cache/lru_cache_test.cc @@ -254,7 +254,8 @@ class FastLRUCacheTest : public testing::Test { } // Maximum number of items that a shard can hold. - double CalcMaxOccupancy(size_t capacity, size_t estimated_value_size, CacheMetadataChargePolicy metadata_charge_policy) { + double CalcMaxOccupancy(size_t capacity, size_t estimated_value_size, + CacheMetadataChargePolicy metadata_charge_policy) { size_t handle_charge = fast_lru_cache::LRUCacheShard::CalcEstimatedHandleCharge( estimated_value_size, metadata_charge_policy); @@ -289,22 +290,27 @@ TEST_F(FastLRUCacheTest, CalcHashBitsTest) { size_t capacity = 1024; size_t estimated_value_size = 1; CacheMetadataChargePolicy metadata_charge_policy = kDontChargeCacheMetadata; - double max_occupancy = CalcMaxOccupancy(capacity, estimated_value_size, metadata_charge_policy); - uint8_t hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, metadata_charge_policy); + double max_occupancy = + CalcMaxOccupancy(capacity, estimated_value_size, metadata_charge_policy); + uint8_t hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, + metadata_charge_policy); EXPECT_TRUE(TableSizeIsAppropriate(hash_bits, max_occupancy)); capacity = 1024; estimated_value_size = 1; metadata_charge_policy = kFullChargeCacheMetadata; - max_occupancy = CalcMaxOccupancy(capacity, estimated_value_size, metadata_charge_policy); - hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, metadata_charge_policy); + max_occupancy = + CalcMaxOccupancy(capacity, estimated_value_size, metadata_charge_policy); + hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, + metadata_charge_policy); EXPECT_TRUE(TableSizeIsAppropriate(hash_bits, max_occupancy)); // No elements fit in cache. capacity = 0; estimated_value_size = 1; metadata_charge_policy = kDontChargeCacheMetadata; - hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, metadata_charge_policy); + hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, + metadata_charge_policy); EXPECT_TRUE(TableSizeIsAppropriate(hash_bits, 0 /* max_occupancy */)); // Set the capacity just below a single handle. Because the load factor is < @@ -317,16 +323,20 @@ TEST_F(FastLRUCacheTest, CalcHashBitsTest) { assert(static_cast(capacity / fast_lru_cache::kLoadFactor) > handle_charge); metadata_charge_policy = kDontChargeCacheMetadata; - max_occupancy = CalcMaxOccupancy(capacity, estimated_value_size, metadata_charge_policy); - hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, metadata_charge_policy); + max_occupancy = + CalcMaxOccupancy(capacity, estimated_value_size, metadata_charge_policy); + hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, + metadata_charge_policy); EXPECT_TRUE(TableSizeIsAppropriate(hash_bits, max_occupancy)); // Large capacity. capacity = 31924172; estimated_value_size = 321; metadata_charge_policy = kFullChargeCacheMetadata; - max_occupancy = CalcMaxOccupancy(capacity, estimated_value_size, metadata_charge_policy); - hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, metadata_charge_policy); + max_occupancy = + CalcMaxOccupancy(capacity, estimated_value_size, metadata_charge_policy); + hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, + metadata_charge_policy); EXPECT_TRUE(TableSizeIsAppropriate(hash_bits, max_occupancy)); } From 97b34220b9452b279b3b6e7558c32388e9a7c12d Mon Sep 17 00:00:00 2001 From: Guido Tagliavini Ponce Date: Mon, 27 Jun 2022 15:05:09 -0700 Subject: [PATCH 08/12] Fix bit shift type. --- cache/fast_lru_cache.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cache/fast_lru_cache.cc b/cache/fast_lru_cache.cc index 21f22c6c2f3..80c891b5141 100644 --- a/cache/fast_lru_cache.cc +++ b/cache/fast_lru_cache.cc @@ -315,9 +315,8 @@ uint8_t LRUCacheShard::CalcHashBits( ++num_hash_bits; } // Compute the ceiling of log2(num_entries). If num_entries == 0, return 1. - return static_cast(1 << num_hash_bits) < num_entries - ? num_hash_bits + 1 - : num_hash_bits; + return size_t{1} << num_hash_bits < num_entries ? num_hash_bits + 1 + : num_hash_bits; } void LRUCacheShard::SetCapacity(size_t capacity) { From 27289dd6a80777eae28e991afa18fd103be7124c Mon Sep 17 00:00:00 2001 From: Guido Tagliavini Ponce Date: Mon, 27 Jun 2022 15:52:12 -0700 Subject: [PATCH 09/12] Remove full namespace qualifier. --- cache/fast_lru_cache.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cache/fast_lru_cache.h b/cache/fast_lru_cache.h index e6164c1ab82..7b0917a5548 100644 --- a/cache/fast_lru_cache.h +++ b/cache/fast_lru_cache.h @@ -368,7 +368,7 @@ class ALIGN_AS(CACHE_LINE_SIZE) LRUCacheShard final : public CacheShard { private: friend class LRUCache; - friend class ROCKSDB_NAMESPACE::fast_lru_cache::FastLRUCacheTest; + friend class FastLRUCacheTest; void LRU_Remove(LRUHandle* e); void LRU_Insert(LRUHandle* e); From b09d9a13a36e9e897b12141ee81de94f1f9dbe06 Mon Sep 17 00:00:00 2001 From: Guido Tagliavini Ponce Date: Mon, 27 Jun 2022 15:55:02 -0700 Subject: [PATCH 10/12] Cleaner computation. --- cache/fast_lru_cache.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cache/fast_lru_cache.cc b/cache/fast_lru_cache.cc index 80c891b5141..5045f9721f6 100644 --- a/cache/fast_lru_cache.cc +++ b/cache/fast_lru_cache.cc @@ -309,14 +309,15 @@ uint8_t LRUCacheShard::CalcHashBits( CalcEstimatedHandleCharge(estimated_value_size, metadata_charge_policy); size_t num_entries = static_cast(capacity / (kLoadFactor * handle_charge)); + + // Compute the ceiling of log2(num_entries). If num_entries == 0, return 1. uint8_t num_hash_bits = 0; size_t num_entries_copy = num_entries; while (num_entries_copy >>= 1) { ++num_hash_bits; } - // Compute the ceiling of log2(num_entries). If num_entries == 0, return 1. - return size_t{1} << num_hash_bits < num_entries ? num_hash_bits + 1 - : num_hash_bits; + num_hash_bits += size_t{1} << num_hash_bits < num_entries ? 1 : 0; + return num_hash_bits; } void LRUCacheShard::SetCapacity(size_t capacity) { From cc80c46a5b196210c65d1a0de2e062ae59178930 Mon Sep 17 00:00:00 2001 From: Guido Tagliavini Ponce Date: Mon, 27 Jun 2022 16:39:08 -0700 Subject: [PATCH 11/12] Remove wrong comment. --- cache/fast_lru_cache.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cache/fast_lru_cache.cc b/cache/fast_lru_cache.cc index 5045f9721f6..0c149ecda9e 100644 --- a/cache/fast_lru_cache.cc +++ b/cache/fast_lru_cache.cc @@ -310,7 +310,7 @@ uint8_t LRUCacheShard::CalcHashBits( size_t num_entries = static_cast(capacity / (kLoadFactor * handle_charge)); - // Compute the ceiling of log2(num_entries). If num_entries == 0, return 1. + // Compute the ceiling of log2(num_entries). uint8_t num_hash_bits = 0; size_t num_entries_copy = num_entries; while (num_entries_copy >>= 1) { From 4900fd051b618db75870c037c626269374980b00 Mon Sep 17 00:00:00 2001 From: Guido Tagliavini Ponce Date: Mon, 27 Jun 2022 16:40:25 -0700 Subject: [PATCH 12/12] Fix comment. --- cache/fast_lru_cache.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cache/fast_lru_cache.cc b/cache/fast_lru_cache.cc index 0c149ecda9e..62821a8752e 100644 --- a/cache/fast_lru_cache.cc +++ b/cache/fast_lru_cache.cc @@ -310,7 +310,7 @@ uint8_t LRUCacheShard::CalcHashBits( size_t num_entries = static_cast(capacity / (kLoadFactor * handle_charge)); - // Compute the ceiling of log2(num_entries). + // Compute the ceiling of log2(num_entries). If num_entries == 0, return 0. uint8_t num_hash_bits = 0; size_t num_entries_copy = num_entries; while (num_entries_copy >>= 1) {