Fix key size in cache_bench (#10234)

Summary:
cache_bench wasn't generating 16B keys, which are necessary for FastLRUCache. Also:
- Added asserts in cache_bench, which is assuming that inserts never fail. When they fail (for example, if we used keys of the wrong size), memory allocated to the values will becomes leaked, and eventually the program crashes.
- Move kCacheKeySize to the right spot.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/10234

Test Plan:
``make -j24 check``. Also, run cache_bench with FastLRUCache and check that memory usage doesn't blow up:
``./cache_bench -cache_type=fast_lru_cache -num_shard_bits=6 -skewed=true \
                        -lookup_insert_percent=100 -lookup_percent=0 -insert_percent=0 -erase_percent=0 \
                        -populate_cache=true -cache_size=1073741824 -ops_per_thread=10000000 \
                        -value_bytes=8192 -resident_ratio=1 -threads=16``

Reviewed By: pdillinger

Differential Revision: D37382949

Pulled By: guidotag

fbshipit-source-id: b697a942ebb215de5d341f98dc8566763436ba9b
main
Guido Tagliavini Ponce 2 years ago committed by Facebook GitHub Bot
parent f81ea75df7
commit b52620ab0e
  1. 19
      cache/cache_bench_tool.cc
  2. 2
      cache/cache_key.h
  3. 8
      cache/fast_lru_cache.h

@ -3,6 +3,7 @@
// COPYING file in the root directory) and Apache 2.0 License // COPYING file in the root directory) and Apache 2.0 License
// (found in the LICENSE.Apache file in the root directory). // (found in the LICENSE.Apache file in the root directory).
#include "cache_key.h"
#ifdef GFLAGS #ifdef GFLAGS
#include <cinttypes> #include <cinttypes>
#include <cstddef> #include <cstddef>
@ -214,7 +215,8 @@ struct KeyGen {
EncodeFixed64(key_data + 10, key); EncodeFixed64(key_data + 10, key);
key_data[18] = char{4}; key_data[18] = char{4};
EncodeFixed64(key_data + 19, key); EncodeFixed64(key_data + 19, key);
return Slice(&key_data[off], sizeof(key_data) - off); assert(27 >= kCacheKeySize);
return Slice(&key_data[off], kCacheKeySize);
} }
}; };
@ -321,8 +323,9 @@ class CacheBench {
Random64 rnd(1); Random64 rnd(1);
KeyGen keygen; KeyGen keygen;
for (uint64_t i = 0; i < 2 * FLAGS_cache_size; i += FLAGS_value_bytes) { for (uint64_t i = 0; i < 2 * FLAGS_cache_size; i += FLAGS_value_bytes) {
cache_->Insert(keygen.GetRand(rnd, max_key_, max_log_), createValue(rnd), Status s = cache_->Insert(keygen.GetRand(rnd, max_key_, max_log_),
&helper1, FLAGS_value_bytes); createValue(rnd), &helper1, FLAGS_value_bytes);
assert(s.ok());
} }
} }
@ -542,8 +545,9 @@ class CacheBench {
FLAGS_value_bytes); FLAGS_value_bytes);
} else { } else {
// do insert // do insert
cache_->Insert(key, createValue(thread->rnd), &helper2, Status s = cache_->Insert(key, createValue(thread->rnd), &helper2,
FLAGS_value_bytes, &handle); FLAGS_value_bytes, &handle);
assert(s.ok());
} }
} else if (random_op < insert_threshold_) { } else if (random_op < insert_threshold_) {
if (handle) { if (handle) {
@ -551,8 +555,9 @@ class CacheBench {
handle = nullptr; handle = nullptr;
} }
// do insert // do insert
cache_->Insert(key, createValue(thread->rnd), &helper3, Status s = cache_->Insert(key, createValue(thread->rnd), &helper3,
FLAGS_value_bytes, &handle); FLAGS_value_bytes, &handle);
assert(s.ok());
} else if (random_op < lookup_threshold_) { } else if (random_op < lookup_threshold_) {
if (handle) { if (handle) {
cache_->Release(handle); cache_->Release(handle);

2
cache/cache_key.h vendored

@ -65,6 +65,8 @@ class CacheKey {
uint64_t offset_etc64_; uint64_t offset_etc64_;
}; };
constexpr uint8_t kCacheKeySize = static_cast<uint8_t>(sizeof(CacheKey));
// A file-specific generator of cache keys, sometimes referred to as the // A file-specific generator of cache keys, sometimes referred to as the
// "base" cache key for a file because all the cache keys for various offsets // "base" cache key for a file because all the cache keys for various offsets
// within the file are computed using simple arithmetic. The basis for the // within the file are computed using simple arithmetic. The basis for the

@ -22,10 +22,11 @@
#include "util/distributed_mutex.h" #include "util/distributed_mutex.h"
namespace ROCKSDB_NAMESPACE { namespace ROCKSDB_NAMESPACE {
namespace fast_lru_cache { namespace fast_lru_cache {
// LRU cache implementation using an open-address hash table. // LRU cache implementation using an open-address hash table.
//
// Every slot in the hash table is an LRUHandle. Because handles can be // Every slot in the hash table is an LRUHandle. Because handles can be
// referenced externally, we can't discard them immediately once they are // referenced externally, we can't discard them immediately once they are
// deleted (via a delete or an LRU eviction) or replaced by a new version // deleted (via a delete or an LRU eviction) or replaced by a new version
@ -51,7 +52,7 @@ namespace fast_lru_cache {
// - Not R --> R: When an unreferenced element becomes referenced. This can only // - Not R --> R: When an unreferenced element becomes referenced. This can only
// happen if the element is V, since references to an element can only be // happen if the element is V, since references to an element can only be
// created when it's visible. // created when it's visible.
//
// Internally, the cache uses an open-addressed hash table to index the handles. // Internally, the cache uses an open-addressed hash table to index the handles.
// We use tombstone counters to keep track of displacements. // We use tombstone counters to keep track of displacements.
// Because of the tombstones and the two possible visibility states of an // Because of the tombstones and the two possible visibility states of an
@ -70,9 +71,6 @@ namespace fast_lru_cache {
// slot. In any case, the slot becomes available. When a handle is inserted // slot. In any case, the slot becomes available. When a handle is inserted
// into that slot, it becomes a visible element again. // into that slot, it becomes a visible element again.
constexpr uint8_t kCacheKeySize =
static_cast<uint8_t>(sizeof(ROCKSDB_NAMESPACE::CacheKey));
// The load factor p is a real number in (0, 1) such that at all // The load factor p is a real number in (0, 1) such that at all
// times at most a fraction p of all slots, without counting tombstones, // times at most a fraction p of all slots, without counting tombstones,
// are occupied by elements. This means that the probability that a // are occupied by elements. This means that the probability that a

Loading…
Cancel
Save