Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use unique_ptr in writebatch hint #5808

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion db/memtable.cc
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,8 @@ MemTable::MemTableStats MemTable::ApproximateStats(const Slice& start_ikey,
bool MemTable::Add(SequenceNumber s, ValueType type,
const Slice& key, /* user key */
const Slice& value, bool allow_concurrent,
MemTablePostProcessInfo* post_process_info, void** hint) {
MemTablePostProcessInfo* post_process_info,
std::unique_ptr<char[]>* hint) {
// Format of an entry is concatenation of:
// key_size : varint32 of internal_key.size()
// key bytes : char[internal_key.size()]
Expand Down
2 changes: 1 addition & 1 deletion db/memtable.h
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ class MemTable {
bool Add(SequenceNumber seq, ValueType type, const Slice& key,
const Slice& value, bool allow_concurrent = false,
MemTablePostProcessInfo* post_process_info = nullptr,
void** hint = nullptr);
std::unique_ptr<char[]>* hint = nullptr);

// Used to Get value associated with key or Get Merge Operands associated
// with key.
Expand Down
5 changes: 1 addition & 4 deletions db/write_batch.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1229,7 +1229,7 @@ class MemTableInserter : public WriteBatch::Handler {
bool hint_per_batch_;
bool hint_created_;
// Hints for this batch
using HintMap = std::unordered_map<MemTable*, void*>;
using HintMap = std::unordered_map<MemTable*, std::unique_ptr<char[]>>;
using HintMapType = std::aligned_storage<sizeof(HintMap)>::type;
HintMapType hint_;

Expand Down Expand Up @@ -1315,9 +1315,6 @@ class MemTableInserter : public WriteBatch::Handler {
(&mem_post_info_map_)->~MemPostInfoMap();
}
if (hint_created_) {
for (auto iter : GetHintMap()) {
delete[] reinterpret_cast<char*>(iter.second);
}
reinterpret_cast<HintMap*>(&hint_)->~HintMap();
}
delete rebuilding_trx_;
Expand Down
6 changes: 4 additions & 2 deletions include/rocksdb/memtablerep.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,15 +129,17 @@ class MemTableRep {
//
// Currently only skip-list based memtable implement the interface. Other
// implementations will fallback to InsertConcurrently() by default.
virtual void InsertWithHintConcurrently(KeyHandle handle, void** /*hint*/) {
virtual void InsertWithHintConcurrently(KeyHandle handle,
std::unique_ptr<char[]>* /*hint*/) {
// Ignore the hint by default.
InsertConcurrently(handle);
}

// Same as ::InsertWithHintConcurrently
// Returns false if MemTableRepFactory::CanHandleDuplicatedKey() is true and
// the <key, seq> already exists.
virtual bool InsertKeyWithHintConcurrently(KeyHandle handle, void** hint) {
virtual bool InsertKeyWithHintConcurrently(KeyHandle handle,
std::unique_ptr<char[]>* hint) {
InsertWithHintConcurrently(handle, hint);
return true;
}
Expand Down
14 changes: 7 additions & 7 deletions memtable/inlineskiplist.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ class InlineSkipList {
//
// REQUIRES: nothing that compares equal to key is currently in the list.
// REQUIRES: no concurrent calls that use same hint
bool InsertWithHintConcurrently(const char* key, void** hint);
bool InsertWithHintConcurrently(const char* key,
std::unique_ptr<char[]>* hint);

// Like Insert, but external synchronization is not required.
bool InsertConcurrently(const char* key);
Expand Down Expand Up @@ -690,14 +691,13 @@ bool InlineSkipList<Comparator>::InsertWithHint(const char* key, void** hint) {
}

template <class Comparator>
bool InlineSkipList<Comparator>::InsertWithHintConcurrently(const char* key,
void** hint) {
bool InlineSkipList<Comparator>::InsertWithHintConcurrently(
const char* key, std::unique_ptr<char[]>* hint) {
assert(hint != nullptr);
Splice* splice = reinterpret_cast<Splice*>(*hint);
if (splice == nullptr) {
splice = AllocateSpliceOnHeap();
*hint = reinterpret_cast<void*>(splice);
if (*hint == nullptr) {
hint->reset(reinterpret_cast<char*>(AllocateSpliceOnHeap()));
}
Splice* splice = reinterpret_cast<Splice*>(hint->get());
return Insert<true>(key, splice, true);
}

Expand Down
3 changes: 1 addition & 2 deletions memtable/inlineskiplist_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -418,9 +418,8 @@ class ConcurrentTest {
char* buf = list_.AllocateKey(sizeof(Key));
memcpy(buf, &new_key, sizeof(Key));
if (use_hint) {
void* hint = nullptr;
std::unique_ptr<char[]> hint = nullptr;
list_.InsertWithHintConcurrently(buf, &hint);
delete[] reinterpret_cast<char*>(hint);
} else {
list_.InsertConcurrently(buf);
}
Expand Down
6 changes: 4 additions & 2 deletions memtable/skiplistrep.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,13 @@ class SkipListRep : public MemTableRep {
return skip_list_.InsertWithHint(static_cast<char*>(handle), hint);
}

void InsertWithHintConcurrently(KeyHandle handle, void** hint) override {
void InsertWithHintConcurrently(KeyHandle handle,
std::unique_ptr<char[]>* hint) override {
skip_list_.InsertWithHintConcurrently(static_cast<char*>(handle), hint);
}

bool InsertKeyWithHintConcurrently(KeyHandle handle, void** hint) override {
bool InsertKeyWithHintConcurrently(KeyHandle handle,
std::unique_ptr<char[]>* hint) override {
return skip_list_.InsertWithHintConcurrently(static_cast<char*>(handle),
hint);
}
Expand Down