Avoid calling GetDBOptions() inside GetFromBatchAndDB()

Summary:
MyRocks hit a regression, @mung generated perf reports showing that the reason is the cost of calling `GetDBOptions()` inside `GetFromBatchAndDB()`
This diff avoid calling `GetDBOptions` and use the `ImmutableDBOptions` instead

Test Plan: make check -j64

Reviewers: sdong, yiwu

Reviewed By: yiwu

Subscribers: andrewkr, dhruba, mung

Differential Revision: https://reviews.facebook.net/D65151
This commit is contained in:
Islam AbdelRahman 2016-10-18 13:19:26 -07:00
parent 6fbe96baf8
commit aa09d03381
4 changed files with 23 additions and 14 deletions

View File

@ -396,6 +396,10 @@ class DBImpl : public DB {
const SnapshotList& snapshots() const { return snapshots_; } const SnapshotList& snapshots() const { return snapshots_; }
const ImmutableDBOptions& immutable_db_options() const {
return immutable_db_options_;
}
void CancelAllBackgroundWork(bool wait); void CancelAllBackgroundWork(bool wait);
// Find Super version and reference it. Based on options, it might return // Find Super version and reference it. Based on options, it might return

View File

@ -11,12 +11,14 @@
#include <memory> #include <memory>
#include "db/column_family.h" #include "db/column_family.h"
#include "db/db_impl.h"
#include "db/merge_context.h" #include "db/merge_context.h"
#include "db/merge_helper.h" #include "db/merge_helper.h"
#include "db/skiplist.h" #include "db/skiplist.h"
#include "rocksdb/comparator.h" #include "rocksdb/comparator.h"
#include "rocksdb/iterator.h" #include "rocksdb/iterator.h"
#include "util/arena.h" #include "util/arena.h"
#include "util/db_options.h"
#include "utilities/write_batch_with_index/write_batch_with_index_internal.h" #include "utilities/write_batch_with_index/write_batch_with_index_internal.h"
namespace rocksdb { namespace rocksdb {
@ -680,11 +682,12 @@ Status WriteBatchWithIndex::GetFromBatch(ColumnFamilyHandle* column_family,
const Slice& key, std::string* value) { const Slice& key, std::string* value) {
Status s; Status s;
MergeContext merge_context; MergeContext merge_context;
const ImmutableDBOptions immuable_db_options(options);
WriteBatchWithIndexInternal::Result result = WriteBatchWithIndexInternal::Result result =
WriteBatchWithIndexInternal::GetFromBatch( WriteBatchWithIndexInternal::GetFromBatch(
options, this, column_family, key, &merge_context, &rep->comparator, immuable_db_options, this, column_family, key, &merge_context,
value, rep->overwrite_key, &s); &rep->comparator, value, rep->overwrite_key, &s);
switch (result) { switch (result) {
case WriteBatchWithIndexInternal::Result::kFound: case WriteBatchWithIndexInternal::Result::kFound:
@ -720,13 +723,14 @@ Status WriteBatchWithIndex::GetFromBatchAndDB(DB* db,
std::string* value) { std::string* value) {
Status s; Status s;
MergeContext merge_context; MergeContext merge_context;
const DBOptions& options = db->GetDBOptions(); const ImmutableDBOptions& immuable_db_options =
reinterpret_cast<DBImpl*>(db)->immutable_db_options();
std::string batch_value; std::string batch_value;
WriteBatchWithIndexInternal::Result result = WriteBatchWithIndexInternal::Result result =
WriteBatchWithIndexInternal::GetFromBatch( WriteBatchWithIndexInternal::GetFromBatch(
options, this, column_family, key, &merge_context, &rep->comparator, immuable_db_options, this, column_family, key, &merge_context,
&batch_value, rep->overwrite_key, &s); &rep->comparator, &batch_value, rep->overwrite_key, &s);
if (result == WriteBatchWithIndexInternal::Result::kFound) { if (result == WriteBatchWithIndexInternal::Result::kFound) {
value->assign(batch_value.data(), batch_value.size()); value->assign(batch_value.data(), batch_value.size());
@ -758,9 +762,9 @@ Status WriteBatchWithIndex::GetFromBatchAndDB(DB* db,
auto cfh = reinterpret_cast<ColumnFamilyHandleImpl*>(column_family); auto cfh = reinterpret_cast<ColumnFamilyHandleImpl*>(column_family);
const MergeOperator* merge_operator = const MergeOperator* merge_operator =
cfh->cfd()->ioptions()->merge_operator; cfh->cfd()->ioptions()->merge_operator;
Statistics* statistics = options.statistics.get(); Statistics* statistics = immuable_db_options.statistics.get();
Env* env = options.env; Env* env = immuable_db_options.env;
Logger* logger = options.info_log.get(); Logger* logger = immuable_db_options.info_log.get();
Slice db_slice(*value); Slice db_slice(*value);
Slice* merge_data; Slice* merge_data;

View File

@ -133,7 +133,7 @@ int WriteBatchEntryComparator::CompareKey(uint32_t column_family,
} }
WriteBatchWithIndexInternal::Result WriteBatchWithIndexInternal::GetFromBatch( WriteBatchWithIndexInternal::Result WriteBatchWithIndexInternal::GetFromBatch(
const DBOptions& options, WriteBatchWithIndex* batch, const ImmutableDBOptions& immuable_db_options, WriteBatchWithIndex* batch,
ColumnFamilyHandle* column_family, const Slice& key, ColumnFamilyHandle* column_family, const Slice& key,
MergeContext* merge_context, WriteBatchEntryComparator* cmp, MergeContext* merge_context, WriteBatchEntryComparator* cmp,
std::string* value, bool overwrite_key, Status* s) { std::string* value, bool overwrite_key, Status* s) {
@ -237,9 +237,9 @@ WriteBatchWithIndexInternal::Result WriteBatchWithIndexInternal::GetFromBatch(
result = WriteBatchWithIndexInternal::Result::kError; result = WriteBatchWithIndexInternal::Result::kError;
return result; return result;
} }
Statistics* statistics = options.statistics.get(); Statistics* statistics = immuable_db_options.statistics.get();
Env* env = options.env; Env* env = immuable_db_options.env;
Logger* logger = options.info_log.get(); Logger* logger = immuable_db_options.info_log.get();
if (merge_operator) { if (merge_operator) {
*s = MergeHelper::TimedFullMerge(merge_operator, key, entry_value, *s = MergeHelper::TimedFullMerge(merge_operator, key, entry_value,

View File

@ -10,12 +10,13 @@
#include <string> #include <string>
#include <vector> #include <vector>
#include "port/port.h"
#include "rocksdb/comparator.h" #include "rocksdb/comparator.h"
#include "rocksdb/iterator.h" #include "rocksdb/iterator.h"
#include "rocksdb/slice.h" #include "rocksdb/slice.h"
#include "rocksdb/status.h" #include "rocksdb/status.h"
#include "rocksdb/utilities/write_batch_with_index.h" #include "rocksdb/utilities/write_batch_with_index.h"
#include "port/port.h" #include "util/db_options.h"
namespace rocksdb { namespace rocksdb {
@ -103,7 +104,7 @@ class WriteBatchWithIndexInternal {
// If batch does not contain this key, return kNotFound // If batch does not contain this key, return kNotFound
// Else, return kError on error with error Status stored in *s. // Else, return kError on error with error Status stored in *s.
static WriteBatchWithIndexInternal::Result GetFromBatch( static WriteBatchWithIndexInternal::Result GetFromBatch(
const DBOptions& options, WriteBatchWithIndex* batch, const ImmutableDBOptions& ioptions, WriteBatchWithIndex* batch,
ColumnFamilyHandle* column_family, const Slice& key, ColumnFamilyHandle* column_family, const Slice& key,
MergeContext* merge_context, WriteBatchEntryComparator* cmp, MergeContext* merge_context, WriteBatchEntryComparator* cmp,
std::string* value, bool overwrite_key, Status* s); std::string* value, bool overwrite_key, Status* s);