Calculate table size of FastLRUCache more accurately

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<uint8_t>(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,8 @@ 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<size_t>(capacity / (kLoadFactor * h.total_charge));
   uint8_t num_hash_bits = 0;
   while (num_entries >>= 1) {
     ++num_hash_bits;

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);
 

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, metadata_charge_policy);
+    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 = 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);
+  ExpectTableSizeRoughlyMaxOccupancy(hash_bits, max_occupancy);
+}
+
 class TestSecondaryCache : public SecondaryCache {
  public:
   // Specifies what action to take on a lookup for a particular key