Skip to content

Commit

Permalink
Revert "PinnableSlice"
Browse files Browse the repository at this point in the history
Summary:
This reverts commit 54d94e9.

The pull request was landed by mistake.
Closes #1755

Differential Revision: D4391678

Pulled By: maysamyabandeh

fbshipit-source-id: 36d5149
  • Loading branch information
Maysam Yabandeh authored and facebook-github-bot committed Jan 8, 2017
1 parent 54d94e9 commit d0ba8ec
Show file tree
Hide file tree
Showing 31 changed files with 176 additions and 528 deletions.
13 changes: 5 additions & 8 deletions db/compacted_db_impl.cc
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
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
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
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
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
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
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
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
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
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

0 comments on commit d0ba8ec

Please sign in to comment.