Skip to content

Commit

Permalink
MemTableRep::KeyValuePair: Remove virtual functions and related changes
Browse files Browse the repository at this point in the history 
1. Now KeyValuePair is a struct{ikey,value}
2. MemTableRep::Iterator does not derive from KeyValuePair
3. MemTableRep::Get: arg `callback` proto change
4. All related changes
  • Loading branch information
@rockeet
rockeet committed Apr 24, 2023
1 parent d34c705 commit d2ab1a6
Show file tree
Hide file tree
Showing 9 changed files with 34 additions and 61 deletions.
2 changes: 1 addition & 1 deletion db/db_memtable_test.cc
View file
Expand Up @@ -56,7 +56,7 @@ class MockMemTableRep : public MemTableRep {
bool Contains(const Slice& key) const override { return rep_->Contains(key); }

void Get(const ReadOptions& ro, const LookupKey& k, void* callback_args,
bool (*callback_func)(void* arg, const KeyValuePair*)) override {
bool (*callback_func)(void* arg, const KeyValuePair&)) override {
rep_->Get(ro, k, callback_args, callback_func);
}

Expand Down
22 changes: 6 additions & 16 deletions db/memtable.cc
View file
Expand Up @@ -801,7 +801,7 @@ struct Saver {
};
} // anonymous namespace

static bool SaveValue(void* arg, const MemTableRep::KeyValuePair* pair) {
static bool SaveValue(void* arg, const MemTableRep::KeyValuePair& pair) {
Saver* s = reinterpret_cast<Saver*>(arg);
assert(s != nullptr);
assert(!s->value || !s->columns);
Expand All @@ -815,7 +815,8 @@ static bool SaveValue(void* arg, const MemTableRep::KeyValuePair* pair) {
// Check that it belongs to same user key. We do not check the
// sequence number since the Seek() call above should have skipped
// all entries with overly large sequence numbers.
auto [ikey, v] = pair->GetKeyValue();
const Slice ikey = pair.ikey;
Slice v = pair.value;
const size_t key_length = ikey.size();
const char* key_ptr = ikey.data();
assert(key_length >= 8);
Expand Down Expand Up @@ -1530,20 +1531,9 @@ size_t MemTable::CountSuccessiveMergeEntries(const LookupKey& key) {
return num_successive_merges;
}

Slice MemTableRep::EncodedKeyValuePair::GetKey() const {
return GetLengthPrefixedSlice(key_);
}

Slice MemTableRep::EncodedKeyValuePair::GetValue() const {
Slice k = GetLengthPrefixedSlice(key_);
return GetLengthPrefixedSlice(k.data() + k.size());
}

std::pair<Slice, Slice> MemTableRep::EncodedKeyValuePair::GetKeyValue() const {
Slice k = GetLengthPrefixedSlice(key_);
Slice v = GetLengthPrefixedSlice(k.data() + k.size());
return {k, v};
}
MemTableRep::KeyValuePair::KeyValuePair(const char* key)
: ikey(GetLengthPrefixedSlice(key)),
value(GetLengthPrefixedSlice(ikey.end())) {}

Slice MemTableRep::Iterator::GetKey() const {
assert(Valid());
Expand Down
38 changes: 12 additions & 26 deletions include/rocksdb/memtablerep.h
View file
Expand Up @@ -201,27 +201,13 @@ class MemTableRep {
// of time. Otherwise, RocksDB may be blocked.
virtual void MarkFlushed() {}

class KeyValuePair {
public:
virtual Slice GetKey() const = 0;
virtual Slice GetValue() const = 0;
virtual std::pair<Slice, Slice> GetKeyValue() const = 0;
virtual ~KeyValuePair() {}
};

class EncodedKeyValuePair : public KeyValuePair {
public:
virtual Slice GetKey() const override;
virtual Slice GetValue() const override;
virtual std::pair<Slice, Slice> GetKeyValue() const override;

KeyValuePair* SetKey(const char* key) {
key_ = key;
return this;
}

private:
const char* key_ = nullptr;
struct KeyValuePair {
Slice ikey;
Slice value;
explicit KeyValuePair(const char* key); ///< cons from varlen prefixed kv
KeyValuePair(Slice ik, Slice v) : ikey(ik), value(v) {}
KeyValuePair(const std::pair<Slice, Slice>& kv) // implicit cons
: ikey(kv.first), value(kv.second) {}
};

template <class Legacy>
Expand Down Expand Up @@ -252,7 +238,7 @@ class MemTableRep {
// seek and call the call back function.
virtual void Get(const struct ReadOptions&,
const LookupKey&, void* callback_args,
bool (*callback_func)(void* arg, const KeyValuePair*)) = 0;
bool (*callback_func)(void* arg, const KeyValuePair&)) = 0;

virtual uint64_t ApproximateNumEntries(const Slice& /*start_ikey*/,
const Slice& /*end_key*/) {
Expand All @@ -277,7 +263,7 @@ class MemTableRep {
virtual ~MemTableRep() {}

// Iteration over the contents of a skip collection
class Iterator : public KeyValuePair {
class Iterator {
public:
// Initialize an iterator over the specified collection.
// The returned iterator is not valid.
Expand All @@ -293,15 +279,15 @@ class MemTableRep {

// Returns the key at the current position.
// REQUIRES: Valid()
virtual Slice GetKey() const override;
virtual Slice GetKey() const;

// Returns the value at the current position.
// REQUIRES: Valid()
virtual Slice GetValue() const override;
virtual Slice GetValue() const;

// Returns the key & value at the current position.
// REQUIRES: Valid()
virtual std::pair<Slice, Slice> GetKeyValue() const override;
virtual std::pair<Slice, Slice> GetKeyValue() const;

// Advances to the next position.
// REQUIRES: Valid()
Expand Down
9 changes: 4 additions & 5 deletions memtable/hash_linklist_rep.cc
View file
Expand Up @@ -176,7 +176,7 @@ class HashLinkListRep : public MemTableRep {
size_t ApproximateMemoryUsage() override;

void Get(const ReadOptions&, const LookupKey& k, void* callback_args,
bool (*callback_func)(void* arg, const KeyValuePair*)) override;
bool (*callback_func)(void* arg, const KeyValuePair&)) override;

~HashLinkListRep() override;

Expand Down Expand Up @@ -729,11 +729,10 @@ size_t HashLinkListRep::ApproximateMemoryUsage() {

void HashLinkListRep::Get(const ReadOptions&,
const LookupKey& k, void* callback_args,
bool (*callback_func)(void*, const KeyValuePair*)) {
bool (*callback_func)(void*, const KeyValuePair&)) {
auto transformed = transform_->Transform(k.user_key());
Pointer& bucket = GetBucket(transformed);

EncodedKeyValuePair kv;
if (IsEmptyBucket(bucket)) {
return;
}
Expand All @@ -742,7 +741,7 @@ void HashLinkListRep::Get(const ReadOptions&,
if (link_list_head != nullptr) {
LinkListIterator iter(this, link_list_head);
for (iter.Seek(k.internal_key(), nullptr);
iter.Valid() && callback_func(callback_args, kv.SetKey(iter.key()));
iter.Valid() && callback_func(callback_args, KeyValuePair(iter.key()));
iter.Next()) {
}
} else {
Expand All @@ -751,7 +750,7 @@ void HashLinkListRep::Get(const ReadOptions&,
// Is a skip list
MemtableSkipList::Iterator iter(&skip_list_header->skip_list);
for (iter.Seek(k.memtable_key_data());
iter.Valid() && callback_func(callback_args, kv.SetKey(iter.key()));
iter.Valid() && callback_func(callback_args, KeyValuePair(iter.key()));
iter.Next()) {
}
}
Expand Down
7 changes: 3 additions & 4 deletions memtable/hash_skiplist_rep.cc
View file
Expand Up @@ -34,7 +34,7 @@ class HashSkipListRep : public MemTableRep {
size_t ApproximateMemoryUsage() override;

void Get(const ReadOptions&, const LookupKey& k, void* callback_args,
bool (*callback_func)(void* arg, const KeyValuePair*)) override;
bool (*callback_func)(void* arg, const KeyValuePair&)) override;

~HashSkipListRep() override;

Expand Down Expand Up @@ -286,14 +286,13 @@ size_t HashSkipListRep::ApproximateMemoryUsage() { return 0; }

void HashSkipListRep::Get(const ReadOptions&,
const LookupKey& k, void* callback_args,
bool (*callback_func)(void*, const KeyValuePair*)) {
bool (*callback_func)(void*, const KeyValuePair&)) {
auto transformed = transform_->Transform(k.user_key());
auto bucket = GetBucket(transformed);
if (bucket != nullptr) {
EncodedKeyValuePair kv;
Bucket::Iterator iter(bucket);
for (iter.Seek(k.memtable_key_data());
iter.Valid() && callback_func(callback_args, kv.SetKey(iter.key()));
iter.Valid() && callback_func(callback_args, KeyValuePair(iter.key()));
iter.Next()) {
}
}
Expand Down
4 changes: 2 additions & 2 deletions memtable/memtablerep_bench.cc
View file
Expand Up @@ -314,14 +314,14 @@ class ReadBenchmarkThread : public BenchmarkThread {
: BenchmarkThread(table, key_gen, bytes_written, bytes_read, sequence,
num_ops, read_hits) {}

static bool callback(void* arg, const MemTableRep::KeyValuePair* kv) {
static bool callback(void* arg, const MemTableRep::KeyValuePair& kv) {
CallbackVerifyArgs* callback_args = static_cast<CallbackVerifyArgs*>(arg);
assert(callback_args != nullptr);
if (FLAGS_skip_read_cmp) {
callback_args->found = true;
return true;
}
Slice internal_key = kv->GetKey();
Slice internal_key = kv.ikey;
size_t key_length = internal_key.size();
const char* key_ptr = internal_key.data();
if (FLAGS_strict_verify) {
Expand Down
5 changes: 2 additions & 3 deletions memtable/skiplistrep.cc
View file
Expand Up @@ -83,12 +83,11 @@ class SkipListRep : public MemTableRep {
}

void Get(const ReadOptions&, const LookupKey& k, void* callback_args,
bool (*callback_func)(void* arg, const KeyValuePair*)) override {
bool (*callback_func)(void* arg, const KeyValuePair&)) override {
SkipListRep::Iterator iter(&skip_list_);
EncodedKeyValuePair kv;
Slice dummy_slice;
for (iter.Seek(dummy_slice, k.memtable_key_data());
iter.Valid() && callback_func(callback_args, kv.SetKey(iter.key()));
iter.Valid() && callback_func(callback_args, KeyValuePair(iter.key()));
iter.Next()) {
}
}
Expand Down
6 changes: 3 additions & 3 deletions memtable/vectorrep.cc
View file
Expand Up @@ -39,7 +39,7 @@ class VectorRep : public MemTableRep {
size_t ApproximateMemoryUsage() override;

void Get(const ReadOptions&, const LookupKey& k, void* callback_args,
bool (*callback_func)(void* arg, const KeyValuePair*)) override;
bool (*callback_func)(void* arg, const KeyValuePair&)) override;

~VectorRep() override {}

Expand Down Expand Up @@ -253,7 +253,7 @@ void VectorRep::Iterator::SeekToLast() {

void VectorRep::Get(const ReadOptions&,
const LookupKey& k, void* callback_args,
bool (*callback_func)(void* arg, const KeyValuePair*)) {
bool (*callback_func)(void* arg, const KeyValuePair&)) {
rwlock_.ReadLock();
VectorRep* vector_rep;
std::shared_ptr<Bucket> bucket;
Expand All @@ -267,7 +267,7 @@ void VectorRep::Get(const ReadOptions&,
rwlock_.ReadUnlock();

for (iter.Seek(k.user_key(), k.memtable_key_data());
iter.Valid() && callback_func(callback_args, &iter); iter.Next()) {
iter.Valid() && callback_func(callback_args, iter.GetKeyValue()); iter.Next()) {
}
}

Expand Down
2 changes: 1 addition & 1 deletion test_util/testutil.cc
View file
Expand Up @@ -598,7 +598,7 @@ class SpecialMemTableRep : public MemTableRep {

virtual void Get(const ReadOptions& ro, const LookupKey& k, void* callback_args,
bool (*callback_func)(void* arg,
const KeyValuePair*)) override {
const KeyValuePair&)) override {
memtable_->Get(ro, k, callback_args, callback_func);
}

Expand Down

0 comments on commit d2ab1a6

Please sign in to comment.