From aa09d03381e75ca6d4cb7190f9aea1f1b6852eae Mon Sep 17 00:00:00 2001 From: Islam AbdelRahman Date: Tue, 18 Oct 2016 13:19:26 -0700 Subject: [PATCH] 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 --- db/db_impl.h | 4 ++++ .../write_batch_with_index.cc | 20 +++++++++++-------- .../write_batch_with_index_internal.cc | 8 ++++---- .../write_batch_with_index_internal.h | 5 +++-- 4 files changed, 23 insertions(+), 14 deletions(-) diff --git a/db/db_impl.h b/db/db_impl.h index b1a4479ce..c7fd9c20e 100644 --- a/db/db_impl.h +++ b/db/db_impl.h @@ -396,6 +396,10 @@ class DBImpl : public DB { const SnapshotList& snapshots() const { return snapshots_; } + const ImmutableDBOptions& immutable_db_options() const { + return immutable_db_options_; + } + void CancelAllBackgroundWork(bool wait); // Find Super version and reference it. Based on options, it might return diff --git a/utilities/write_batch_with_index/write_batch_with_index.cc b/utilities/write_batch_with_index/write_batch_with_index.cc index 96f0459b3..68c8d4c64 100644 --- a/utilities/write_batch_with_index/write_batch_with_index.cc +++ b/utilities/write_batch_with_index/write_batch_with_index.cc @@ -11,12 +11,14 @@ #include #include "db/column_family.h" +#include "db/db_impl.h" #include "db/merge_context.h" #include "db/merge_helper.h" #include "db/skiplist.h" #include "rocksdb/comparator.h" #include "rocksdb/iterator.h" #include "util/arena.h" +#include "util/db_options.h" #include "utilities/write_batch_with_index/write_batch_with_index_internal.h" namespace rocksdb { @@ -680,11 +682,12 @@ Status WriteBatchWithIndex::GetFromBatch(ColumnFamilyHandle* column_family, const Slice& key, std::string* value) { Status s; MergeContext merge_context; + const ImmutableDBOptions immuable_db_options(options); WriteBatchWithIndexInternal::Result result = WriteBatchWithIndexInternal::GetFromBatch( - options, this, column_family, key, &merge_context, &rep->comparator, - value, rep->overwrite_key, &s); + immuable_db_options, this, column_family, key, &merge_context, + &rep->comparator, value, rep->overwrite_key, &s); switch (result) { case WriteBatchWithIndexInternal::Result::kFound: @@ -720,13 +723,14 @@ Status WriteBatchWithIndex::GetFromBatchAndDB(DB* db, std::string* value) { Status s; MergeContext merge_context; - const DBOptions& options = db->GetDBOptions(); + const ImmutableDBOptions& immuable_db_options = + reinterpret_cast(db)->immutable_db_options(); std::string batch_value; WriteBatchWithIndexInternal::Result result = WriteBatchWithIndexInternal::GetFromBatch( - options, this, column_family, key, &merge_context, &rep->comparator, - &batch_value, rep->overwrite_key, &s); + immuable_db_options, this, column_family, key, &merge_context, + &rep->comparator, &batch_value, rep->overwrite_key, &s); if (result == WriteBatchWithIndexInternal::Result::kFound) { value->assign(batch_value.data(), batch_value.size()); @@ -758,9 +762,9 @@ Status WriteBatchWithIndex::GetFromBatchAndDB(DB* db, auto cfh = reinterpret_cast(column_family); const MergeOperator* merge_operator = cfh->cfd()->ioptions()->merge_operator; - Statistics* statistics = options.statistics.get(); - Env* env = options.env; - Logger* logger = options.info_log.get(); + Statistics* statistics = immuable_db_options.statistics.get(); + Env* env = immuable_db_options.env; + Logger* logger = immuable_db_options.info_log.get(); Slice db_slice(*value); Slice* merge_data; diff --git a/utilities/write_batch_with_index/write_batch_with_index_internal.cc b/utilities/write_batch_with_index/write_batch_with_index_internal.cc index aa1237afb..eaf1dde09 100644 --- a/utilities/write_batch_with_index/write_batch_with_index_internal.cc +++ b/utilities/write_batch_with_index/write_batch_with_index_internal.cc @@ -133,7 +133,7 @@ int WriteBatchEntryComparator::CompareKey(uint32_t column_family, } WriteBatchWithIndexInternal::Result WriteBatchWithIndexInternal::GetFromBatch( - const DBOptions& options, WriteBatchWithIndex* batch, + const ImmutableDBOptions& immuable_db_options, WriteBatchWithIndex* batch, ColumnFamilyHandle* column_family, const Slice& key, MergeContext* merge_context, WriteBatchEntryComparator* cmp, std::string* value, bool overwrite_key, Status* s) { @@ -237,9 +237,9 @@ WriteBatchWithIndexInternal::Result WriteBatchWithIndexInternal::GetFromBatch( result = WriteBatchWithIndexInternal::Result::kError; return result; } - Statistics* statistics = options.statistics.get(); - Env* env = options.env; - Logger* logger = options.info_log.get(); + Statistics* statistics = immuable_db_options.statistics.get(); + Env* env = immuable_db_options.env; + Logger* logger = immuable_db_options.info_log.get(); if (merge_operator) { *s = MergeHelper::TimedFullMerge(merge_operator, key, entry_value, diff --git a/utilities/write_batch_with_index/write_batch_with_index_internal.h b/utilities/write_batch_with_index/write_batch_with_index_internal.h index 95fdf5aaa..7279cde92 100644 --- a/utilities/write_batch_with_index/write_batch_with_index_internal.h +++ b/utilities/write_batch_with_index/write_batch_with_index_internal.h @@ -10,12 +10,13 @@ #include #include +#include "port/port.h" #include "rocksdb/comparator.h" #include "rocksdb/iterator.h" #include "rocksdb/slice.h" #include "rocksdb/status.h" #include "rocksdb/utilities/write_batch_with_index.h" -#include "port/port.h" +#include "util/db_options.h" namespace rocksdb { @@ -103,7 +104,7 @@ class WriteBatchWithIndexInternal { // If batch does not contain this key, return kNotFound // Else, return kError on error with error Status stored in *s. static WriteBatchWithIndexInternal::Result GetFromBatch( - const DBOptions& options, WriteBatchWithIndex* batch, + const ImmutableDBOptions& ioptions, WriteBatchWithIndex* batch, ColumnFamilyHandle* column_family, const Slice& key, MergeContext* merge_context, WriteBatchEntryComparator* cmp, std::string* value, bool overwrite_key, Status* s);