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.

Sign up for GitHub

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

Jump to bottom

Revert "PinnableSlice" #1755

Closed
wants to merge 1 commit into from
Closed

Revert "PinnableSlice" #1755

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 5 additions & 8 deletions db/compacted_db_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ size_t CompactedDBImpl::FindFile(const Slice& key) {
return right;
}

Status CompactedDBImpl::Get(const ReadOptions& options, ColumnFamilyHandle*,
const Slice& key, PinnableSlice* pSlice) {
Status CompactedDBImpl::Get(const ReadOptions& options,
ColumnFamilyHandle*, const Slice& key, std::string* value) {
GetContext get_context(user_comparator_, nullptr, nullptr, nullptr,
GetContext::kNotFound, key, pSlice, nullptr, nullptr,
GetContext::kNotFound, key, value, nullptr, nullptr,
nullptr, nullptr);
LookupKey lkey(key, kMaxSequenceNumber);
files_.files[FindFile(key)].fd.table_reader->Get(
Expand Down Expand Up @@ -75,14 +75,11 @@ std::vector<Status> CompactedDBImpl::MultiGet(const ReadOptions& options,
int idx = 0;
for (auto* r : reader_list) {
if (r != nullptr) {
PinnableSlice pSlice;
std::string& value = (*values)[idx];
GetContext get_context(user_comparator_, nullptr, nullptr, nullptr,
GetContext::kNotFound, keys[idx], &pSlice, nullptr,
nullptr, nullptr, nullptr);
GetContext::kNotFound, keys[idx], &(*values)[idx],
nullptr, nullptr, nullptr, nullptr);
LookupKey lkey(keys[idx], kMaxSequenceNumber);
r->Get(options, lkey.internal_key(), &get_context);
value.assign(pSlice.data(), pSlice.size());
if (get_context.State() == GetContext::kFound) {
statuses[idx] = Status::OK();
}
Expand Down
2 changes: 1 addition & 1 deletion db/compacted_db_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class CompactedDBImpl : public DBImpl {
using DB::Get;
virtual Status Get(const ReadOptions& options,
ColumnFamilyHandle* column_family, const Slice& key,
PinnableSlice* value) override;
std::string* value) override;
using DB::MultiGet;
virtual std::vector<Status> MultiGet(
const ReadOptions& options,
Expand Down
43 changes: 16 additions & 27 deletions db/db_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3907,8 +3907,8 @@ ColumnFamilyHandle* DBImpl::DefaultColumnFamily() const {

Status DBImpl::Get(const ReadOptions& read_options,
ColumnFamilyHandle* column_family, const Slice& key,
PinnableSlice* pSlice) {
return GetImpl(read_options, column_family, key, pSlice);
std::string* value) {
return GetImpl(read_options, column_family, key, value);
}

// JobContext gets created and destructed outside of the lock --
Expand Down Expand Up @@ -3965,7 +3965,7 @@ SuperVersion* DBImpl::InstallSuperVersionAndScheduleWork(

Status DBImpl::GetImpl(const ReadOptions& read_options,
ColumnFamilyHandle* column_family, const Slice& key,
PinnableSlice* pSlice, bool* value_found) {
std::string* value, bool* value_found) {
StopWatch sw(env_, stats_, DB_GET);
PERF_TIMER_GUARD(get_snapshot_time);

Expand Down Expand Up @@ -3996,12 +3996,12 @@ Status DBImpl::GetImpl(const ReadOptions& read_options,
(read_options.read_tier == kPersistedTier && has_unpersisted_data_);
bool done = false;
if (!skip_memtable) {
if (sv->mem->Get(lkey, pSlice, &s, &merge_context, &range_del_agg,
if (sv->mem->Get(lkey, value, &s, &merge_context, &range_del_agg,
read_options)) {
done = true;
RecordTick(stats_, MEMTABLE_HIT);
} else if ((s.ok() || s.IsMergeInProgress()) &&
sv->imm->Get(lkey, pSlice, &s, &merge_context, &range_del_agg,
sv->imm->Get(lkey, value, &s, &merge_context, &range_del_agg,
read_options)) {
done = true;
RecordTick(stats_, MEMTABLE_HIT);
Expand All @@ -4012,7 +4012,7 @@ Status DBImpl::GetImpl(const ReadOptions& read_options,
}
if (!done) {
PERF_TIMER_GUARD(get_from_output_files_time);
sv->current->Get(read_options, lkey, pSlice, &s, &merge_context,
sv->current->Get(read_options, lkey, value, &s, &merge_context,
&range_del_agg, value_found);
RecordTick(stats_, MEMTABLE_MISS);
}
Expand All @@ -4023,9 +4023,8 @@ Status DBImpl::GetImpl(const ReadOptions& read_options,
ReturnAndCleanupSuperVersion(cfd, sv);

RecordTick(stats_, NUMBER_KEYS_READ);
size_t size = pSlice->size();
RecordTick(stats_, BYTES_READ, size);
MeasureTime(stats_, BYTES_PER_READ, size);
RecordTick(stats_, BYTES_READ, value->size());
MeasureTime(stats_, BYTES_PER_READ, value->size());
}
return s;
}
Expand Down Expand Up @@ -4101,30 +4100,26 @@ std::vector<Status> DBImpl::MultiGet(
bool skip_memtable =
(read_options.read_tier == kPersistedTier && has_unpersisted_data_);
bool done = false;
PinnableSlice pSlice;
if (!skip_memtable) {
if (super_version->mem->Get(lkey, &pSlice, &s, &merge_context,
if (super_version->mem->Get(lkey, value, &s, &merge_context,
&range_del_agg, read_options)) {
value->assign(pSlice.data(), pSlice.size());
done = true;
// TODO(?): RecordTick(stats_, MEMTABLE_HIT)?
} else if (super_version->imm->Get(lkey, &pSlice, &s, &merge_context,
} else if (super_version->imm->Get(lkey, value, &s, &merge_context,
&range_del_agg, read_options)) {
value->assign(pSlice.data(), pSlice.size());
done = true;
// TODO(?): RecordTick(stats_, MEMTABLE_HIT)?
}
}
if (!done) {
PERF_TIMER_GUARD(get_from_output_files_time);
super_version->current->Get(read_options, lkey, &pSlice, &s,
&merge_context, &range_del_agg);
value->assign(pSlice.data(), pSlice.size());
super_version->current->Get(read_options, lkey, value, &s, &merge_context,
&range_del_agg);
// TODO(?): RecordTick(stats_, MEMTABLE_MISS)?
}

if (s.ok()) {
bytes_read += pSlice.size();
bytes_read += value->size();
}
}

Expand Down Expand Up @@ -4337,12 +4332,7 @@ bool DBImpl::KeyMayExist(const ReadOptions& read_options,
}
ReadOptions roptions = read_options;
roptions.read_tier = kBlockCacheTier; // read from block cache only
PinnableSlice pSlice;
PinnableSlice* pSlicePtr = value != nullptr ? &pSlice : nullptr;
auto s = GetImpl(roptions, column_family, key, pSlicePtr, value_found);
if (value != nullptr) {
value->assign(pSlice.data(), pSlice.size());
}
auto s = GetImpl(roptions, column_family, key, value, value_found);

// If block_cache is enabled and the index block of the table didn't
// not present in block_cache, the return value will be Status::Incomplete.
Expand Down Expand Up @@ -6405,8 +6395,7 @@ Status DBImpl::GetLatestSequenceForKey(SuperVersion* sv, const Slice& key,
*found_record_for_key = false;

// Check if there is a record for this key in the latest memtable
PinnableSlice* pSliceNullPtr = nullptr;
sv->mem->Get(lkey, pSliceNullPtr, &s, &merge_context, &range_del_agg, seq,
sv->mem->Get(lkey, nullptr, &s, &merge_context, &range_del_agg, seq,
read_options);

if (!(s.ok() || s.IsNotFound() || s.IsMergeInProgress())) {
Expand All @@ -6425,7 +6414,7 @@ Status DBImpl::GetLatestSequenceForKey(SuperVersion* sv, const Slice& key,
}

// Check if there is a record for this key in the immutable memtables
sv->imm->Get(lkey, pSliceNullPtr, &s, &merge_context, &range_del_agg, seq,
sv->imm->Get(lkey, nullptr, &s, &merge_context, &range_del_agg, seq,
read_options);

if (!(s.ok() || s.IsNotFound() || s.IsMergeInProgress())) {
Expand Down
4 changes: 2 additions & 2 deletions db/db_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class DBImpl : public DB {
using DB::Get;
virtual Status Get(const ReadOptions& options,
ColumnFamilyHandle* column_family, const Slice& key,
PinnableSlice* value) override;
std::string* value) override;
using DB::MultiGet;
virtual std::vector<Status> MultiGet(
const ReadOptions& options,
Expand Down Expand Up @@ -1088,7 +1088,7 @@ class DBImpl : public DB {
// Function that Get and KeyMayExist call with no_io true or false
// Note: 'value_found' from KeyMayExist propagates here
Status GetImpl(const ReadOptions& options, ColumnFamilyHandle* column_family,
const Slice& key, PinnableSlice* pSlice,
const Slice& key, std::string* value,
bool* value_found = nullptr);

bool GetIntPropertyInternal(ColumnFamilyData* cfd,
Expand Down
6 changes: 3 additions & 3 deletions db/db_impl_readonly.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ DBImplReadOnly::~DBImplReadOnly() {
// Implementations of the DB interface
Status DBImplReadOnly::Get(const ReadOptions& read_options,
ColumnFamilyHandle* column_family, const Slice& key,
PinnableSlice* pSlice) {
std::string* value) {
Status s;
SequenceNumber snapshot = versions_->LastSequence();
auto cfh = reinterpret_cast<ColumnFamilyHandleImpl*>(column_family);
Expand All @@ -40,11 +40,11 @@ Status DBImplReadOnly::Get(const ReadOptions& read_options,
MergeContext merge_context;
RangeDelAggregator range_del_agg(cfd->internal_comparator(), snapshot);
LookupKey lkey(key, snapshot);
if (super_version->mem->Get(lkey, pSlice, &s, &merge_context, &range_del_agg,
if (super_version->mem->Get(lkey, value, &s, &merge_context, &range_del_agg,
read_options)) {
} else {
PERF_TIMER_GUARD(get_from_output_files_time);
super_version->current->Get(read_options, lkey, pSlice, &s, &merge_context,
super_version->current->Get(read_options, lkey, value, &s, &merge_context,
&range_del_agg);
}
return s;
Expand Down
2 changes: 1 addition & 1 deletion db/db_impl_readonly.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class DBImplReadOnly : public DBImpl {
using DB::Get;
virtual Status Get(const ReadOptions& options,
ColumnFamilyHandle* column_family, const Slice& key,
PinnableSlice* value) override;
std::string* value) override;

// TODO: Implement ReadOnly MultiGet?

Expand Down
2 changes: 1 addition & 1 deletion db/db_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2691,7 +2691,7 @@ class ModelDB : public DB {
}
using DB::Get;
virtual Status Get(const ReadOptions& options, ColumnFamilyHandle* cf,
const Slice& key, PinnableSlice* pSlice) override {
const Slice& key, std::string* value) override {
return Status::NotSupported(key);
}

Expand Down
41 changes: 14 additions & 27 deletions db/memtable.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ struct Saver {
const LookupKey* key;
bool* found_final_value; // Is value set correctly? Used by KeyMayExist
bool* merge_in_progress;
PinnableSlice* pSlice;
std::string* value;
SequenceNumber seq;
const MergeOperator* merge_operator;
// the merge operations encountered;
Expand All @@ -534,10 +534,6 @@ struct Saver {
};
} // namespace

//static void UnrefMemTable(void* s, void*) {
// reinterpret_cast<MemTable*>(s)->Unref();
//}

static bool SaveValue(void* arg, const char* entry) {
Saver* s = reinterpret_cast<Saver*>(arg);
MergeContext* merge_context = s->merge_context;
Expand Down Expand Up @@ -575,18 +571,13 @@ static bool SaveValue(void* arg, const char* entry) {
}
Slice v = GetLengthPrefixedSlice(key_ptr + key_length);
*(s->status) = Status::OK();
if (LIKELY(s->pSlice != nullptr)) {
if (*(s->merge_in_progress)) {
*(s->status) = MergeHelper::TimedFullMerge(
merge_operator, s->key->user_key(), &v,
merge_context->GetOperands(), s->pSlice->GetSelf(), s->logger,
s->statistics, s->env_);
s->pSlice->PinSelf();
} else {
//s->mem->Ref();
//s->pSlice->PinSlice(v, UnrefMemTable, s->mem, nullptr);
s->pSlice->PinSelf(v);
}
if (*(s->merge_in_progress)) {
*(s->status) = MergeHelper::TimedFullMerge(
merge_operator, s->key->user_key(), &v,
merge_context->GetOperands(), s->value, s->logger, s->statistics,
s->env_);
} else if (s->value != nullptr) {
s->value->assign(v.data(), v.size());
}
if (s->inplace_update_support) {
s->mem->GetLock(s->key->user_key())->ReadUnlock();
Expand All @@ -598,14 +589,10 @@ static bool SaveValue(void* arg, const char* entry) {
case kTypeSingleDeletion:
case kTypeRangeDeletion: {
if (*(s->merge_in_progress)) {
*(s->status) = Status::OK();
if (s->pSlice != nullptr) {
*(s->status) = MergeHelper::TimedFullMerge(
merge_operator, s->key->user_key(), nullptr,
merge_context->GetOperands(), s->pSlice->GetSelf(), s->logger,
s->statistics, s->env_);
s->pSlice->PinSelf();
}
*(s->status) = MergeHelper::TimedFullMerge(
merge_operator, s->key->user_key(), nullptr,
merge_context->GetOperands(), s->value, s->logger, s->statistics,
s->env_);
} else {
*(s->status) = Status::NotFound();
}
Expand Down Expand Up @@ -639,7 +626,7 @@ static bool SaveValue(void* arg, const char* entry) {
return false;
}

bool MemTable::Get(const LookupKey& key, PinnableSlice* pSlice, Status* s,
bool MemTable::Get(const LookupKey& key, std::string* value, Status* s,
MergeContext* merge_context,
RangeDelAggregator* range_del_agg, SequenceNumber* seq,
const ReadOptions& read_opts) {
Expand Down Expand Up @@ -677,7 +664,7 @@ bool MemTable::Get(const LookupKey& key, PinnableSlice* pSlice, Status* s,
saver.found_final_value = &found_final_value;
saver.merge_in_progress = &merge_in_progress;
saver.key = &key;
saver.pSlice = pSlice;
saver.value = value;
saver.seq = kMaxSequenceNumber;
saver.mem = this;
saver.merge_context = merge_context;
Expand Down
20 changes: 5 additions & 15 deletions db/memtable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,26 +186,16 @@ class MemTable {
// returned). Otherwise, *seq will be set to kMaxSequenceNumber.
// On success, *s may be set to OK, NotFound, or MergeInProgress. Any other
// status returned indicates a corruption or other unexpected error.
bool Get(const LookupKey& key, PinnableSlice* pSlice, Status* s,
MergeContext* merge_context, RangeDelAggregator* range_del_agg,
SequenceNumber* seq, const ReadOptions& read_opts);

inline bool Get(const LookupKey& key, PinnableSlice* pSlice, Status* s,
MergeContext* merge_context,
RangeDelAggregator* range_del_agg,
const ReadOptions& read_opts) {
SequenceNumber seq;
return Get(key, pSlice, s, merge_context, range_del_agg, &seq, read_opts);
}

// deprecated. Use Get with PinnableSlice
bool Get(const LookupKey& key, std::string* value, Status* s,
MergeContext* merge_context, RangeDelAggregator* range_del_agg,
SequenceNumber* seq, const ReadOptions& read_opts);
// deprecated. Use Get with PinnableSlice

bool Get(const LookupKey& key, std::string* value, Status* s,
MergeContext* merge_context, RangeDelAggregator* range_del_agg,
const ReadOptions& read_opts);
const ReadOptions& read_opts) {
SequenceNumber seq;
return Get(key, value, s, merge_context, range_del_agg, &seq, read_opts);
}

// Attempts to update the new_value inplace, else does normal Add
// Pseudocode
Expand Down
26 changes: 11 additions & 15 deletions db/memtable_list.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,12 @@ int MemTableList::NumFlushed() const {
// Search all the memtables starting from the most recent one.
// Return the most recent value found, if any.
// Operands stores the list of merge operations to apply, so far.
bool MemTableListVersion::Get(const LookupKey& key, PinnableSlice* pSlice,
bool MemTableListVersion::Get(const LookupKey& key, std::string* value,
Status* s, MergeContext* merge_context,
RangeDelAggregator* range_del_agg,
SequenceNumber* seq,
const ReadOptions& read_opts) {
return GetFromList(&memlist_, key, pSlice, s, merge_context, range_del_agg,
return GetFromList(&memlist_, key, value, s, merge_context, range_del_agg,
seq, read_opts);
}

Expand All @@ -115,26 +115,22 @@ bool MemTableListVersion::GetFromHistory(const LookupKey& key,
RangeDelAggregator* range_del_agg,
SequenceNumber* seq,
const ReadOptions& read_opts) {
PinnableSlice pSlice;
PinnableSlice* pSlicePtr = value != nullptr ? &pSlice : nullptr;
auto res = GetFromList(&memlist_history_, key, pSlicePtr, s, merge_context,
range_del_agg, seq, read_opts);
if (value != nullptr) {
value->assign(pSlice.data(), pSlice.size());
}
return res;
return GetFromList(&memlist_history_, key, value, s, merge_context,
range_del_agg, seq, read_opts);
}

bool MemTableListVersion::GetFromList(
std::list<MemTable*>* list, const LookupKey& key, PinnableSlice* pSlice,
Status* s, MergeContext* merge_context, RangeDelAggregator* range_del_agg,
SequenceNumber* seq, const ReadOptions& read_opts) {
bool MemTableListVersion::GetFromList(std::list<MemTable*>* list,
const LookupKey& key, std::string* value,
Status* s, MergeContext* merge_context,
RangeDelAggregator* range_del_agg,
SequenceNumber* seq,
const ReadOptions& read_opts) {
*seq = kMaxSequenceNumber;

for (auto& memtable : *list) {
SequenceNumber current_seq = kMaxSequenceNumber;

bool done = memtable->Get(key, pSlice, s, merge_context, range_del_agg,
bool done = memtable->Get(key, value, s, merge_context, range_del_agg,
&current_seq, read_opts);
if (*seq == kMaxSequenceNumber) {
// Store the most recent sequence number of any operation on this key.
Expand Down
Loading