diff --git a/cache/clock_cache.cc b/cache/clock_cache.cc index 8fd0b012f..3d34740a0 100644 --- a/cache/clock_cache.cc +++ b/cache/clock_cache.cc @@ -289,14 +289,11 @@ int ClockCacheShard::CalcHashBits( CacheMetadataChargePolicy metadata_charge_policy) { size_t handle_charge = CalcEstimatedHandleCharge(estimated_value_size, metadata_charge_policy); + assert(handle_charge > 0); uint32_t num_entries = - static_cast(capacity / (kLoadFactor * handle_charge)); - - if (num_entries == 0) { - return 0; - } - int hash_bits = FloorLog2(num_entries); - return hash_bits + (size_t{1} << hash_bits < num_entries ? 1 : 0); + static_cast(capacity / (kLoadFactor * handle_charge)) + 1; + assert(num_entries <= uint32_t{1} << 31); + return FloorLog2((num_entries << 1) - 1); } void ClockCacheShard::SetCapacity(size_t capacity) { @@ -516,6 +513,8 @@ ClockCache::ClockCache(size_t capacity, size_t estimated_value_size, int num_shard_bits, bool strict_capacity_limit, CacheMetadataChargePolicy metadata_charge_policy) : ShardedCache(capacity, num_shard_bits, strict_capacity_limit) { + assert(estimated_value_size > 0 || + metadata_charge_policy != kDontChargeCacheMetadata); num_shards_ = 1 << num_shard_bits; shards_ = reinterpret_cast( port::cacheline_aligned_alloc(sizeof(ClockCacheShard) * num_shards_)); diff --git a/cache/fast_lru_cache.cc b/cache/fast_lru_cache.cc index f6ffefba2..8ef74f56f 100644 --- a/cache/fast_lru_cache.cc +++ b/cache/fast_lru_cache.cc @@ -290,14 +290,11 @@ int LRUCacheShard::CalcHashBits( CacheMetadataChargePolicy metadata_charge_policy) { size_t handle_charge = CalcEstimatedHandleCharge(estimated_value_size, metadata_charge_policy); + assert(handle_charge > 0); uint32_t num_entries = - static_cast(capacity / (kLoadFactor * handle_charge)); - - if (num_entries == 0) { - return 0; - } - int hash_bits = FloorLog2(num_entries); - return hash_bits + (size_t{1} << hash_bits < num_entries ? 1 : 0); + static_cast(capacity / (kLoadFactor * handle_charge)) + 1; + assert(num_entries <= uint32_t{1} << 31); + return FloorLog2((num_entries << 1) - 1); } void LRUCacheShard::SetCapacity(size_t capacity) { @@ -530,6 +527,8 @@ LRUCache::LRUCache(size_t capacity, size_t estimated_value_size, int num_shard_bits, bool strict_capacity_limit, CacheMetadataChargePolicy metadata_charge_policy) : ShardedCache(capacity, num_shard_bits, strict_capacity_limit) { + assert(estimated_value_size > 0 || + metadata_charge_policy != kDontChargeCacheMetadata); num_shards_ = 1 << num_shard_bits; shards_ = reinterpret_cast( port::cacheline_aligned_alloc(sizeof(LRUCacheShard) * num_shards_)); diff --git a/cache/lru_cache_test.cc b/cache/lru_cache_test.cc index 0833e63d6..e611dbd9c 100644 --- a/cache/lru_cache_test.cc +++ b/cache/lru_cache_test.cc @@ -287,51 +287,63 @@ TEST_F(FastLRUCacheTest, ValidateKeySize) { } 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); - int hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, - metadata_charge_policy); - EXPECT_TRUE(TableSizeIsAppropriate(hash_bits, max_occupancy)); - - capacity = 1024; + size_t capacity; + size_t estimated_value_size; + double max_occupancy; + int hash_bits; + CacheMetadataChargePolicy metadata_charge_policy; + + // Vary the cache capacity, fix the element charge. + for (int i = 0; i < 2048; i++) { + capacity = i; + estimated_value_size = 0; + metadata_charge_policy = kFullChargeCacheMetadata; + 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)); + } + + // Fix the cache capacity, vary the element charge. + for (int i = 0; i < 1024; i++) { + capacity = 1024; + estimated_value_size = i; + metadata_charge_policy = kFullChargeCacheMetadata; + 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)); + } + + // Zero-capacity cache, and only values have charge. + capacity = 0; estimated_value_size = 1; - metadata_charge_policy = kFullChargeCacheMetadata; - max_occupancy = - CalcMaxOccupancy(capacity, estimated_value_size, metadata_charge_policy); + metadata_charge_policy = kDontChargeCacheMetadata; hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, metadata_charge_policy); - EXPECT_TRUE(TableSizeIsAppropriate(hash_bits, max_occupancy)); + EXPECT_TRUE(TableSizeIsAppropriate(hash_bits, 0 /* max_occupancy */)); - // No elements fit in cache. + // Zero-capacity cache, and only metadata has charge. capacity = 0; - estimated_value_size = 1; - metadata_charge_policy = kDontChargeCacheMetadata; + estimated_value_size = 0; + metadata_charge_policy = kFullChargeCacheMetadata; 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 < - // 100% at least one handle will fit in the table. - estimated_value_size = 1; - 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); + // Small cache, large elements. + capacity = 1024; + estimated_value_size = 8192; + metadata_charge_policy = kFullChargeCacheMetadata; hash_bits = CalcHashBitsWrapper(capacity, estimated_value_size, metadata_charge_policy); - EXPECT_TRUE(TableSizeIsAppropriate(hash_bits, max_occupancy)); + EXPECT_TRUE(TableSizeIsAppropriate(hash_bits, 0 /* max_occupancy */)); // Large capacity. capacity = 31924172; - estimated_value_size = 321; + estimated_value_size = 8192; metadata_charge_policy = kFullChargeCacheMetadata; max_occupancy = CalcMaxOccupancy(capacity, estimated_value_size, metadata_charge_policy);