Fix perf_context.user_key_comparison_count for range scan (#5098)

Summary:
Currently `perf_context.user_key_comparison_count` is bump only in `InternalKeyComparator`. For places user comparator is used directly the counter is not bump. Fixing the majority of it.

Index iterator and filter code also use user comparator directly and don't bump the counter. It is not fixed in this patch.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/5098

Differential Revision: D14603753

Pulled By: siying

fbshipit-source-id: 1cd41035644ca9e49b97a51030a5d1e15f5f3cae
main
Yi Wu 6 years ago committed by Facebook Github Bot
parent 2b4d5ceb47
commit d69241586e
  1. 38
      db/db_iter.cc
  2. 11
      db/dbformat.cc
  3. 23
      db/dbformat.h
  4. 8
      db/version_set.cc
  5. 10
      table/block_based_table_reader.cc
  6. 3
      table/block_based_table_reader.h
  7. 65
      util/user_comparator_wrapper.h

@ -28,6 +28,7 @@
#include "util/mutexlock.h" #include "util/mutexlock.h"
#include "util/string_util.h" #include "util/string_util.h"
#include "util/trace_replay.h" #include "util/trace_replay.h"
#include "util/user_comparator_wrapper.h"
namespace rocksdb { namespace rocksdb {
@ -306,7 +307,7 @@ class DBIter final: public Iterator {
const SliceTransform* prefix_extractor_; const SliceTransform* prefix_extractor_;
Env* const env_; Env* const env_;
Logger* logger_; Logger* logger_;
const Comparator* const user_comparator_; UserComparatorWrapper user_comparator_;
const MergeOperator* const merge_operator_; const MergeOperator* const merge_operator_;
InternalIterator* iter_; InternalIterator* iter_;
ReadCallback* read_callback_; ReadCallback* read_callback_;
@ -455,7 +456,7 @@ bool DBIter::FindNextUserEntryInternal(bool skipping, bool prefix_check) {
} }
if (iterate_upper_bound_ != nullptr && if (iterate_upper_bound_ != nullptr &&
user_comparator_->Compare(ikey_.user_key, *iterate_upper_bound_) >= 0) { user_comparator_.Compare(ikey_.user_key, *iterate_upper_bound_) >= 0) {
break; break;
} }
@ -470,8 +471,8 @@ bool DBIter::FindNextUserEntryInternal(bool skipping, bool prefix_check) {
} }
if (IsVisible(ikey_.sequence)) { if (IsVisible(ikey_.sequence)) {
if (skipping && user_comparator_->Compare(ikey_.user_key, if (skipping && user_comparator_.Compare(ikey_.user_key,
saved_key_.GetUserKey()) <= 0) { saved_key_.GetUserKey()) <= 0) {
num_skipped++; // skip this entry num_skipped++; // skip this entry
PERF_COUNTER_ADD(internal_key_skipped_count, 1); PERF_COUNTER_ADD(internal_key_skipped_count, 1);
} else { } else {
@ -578,7 +579,7 @@ bool DBIter::FindNextUserEntryInternal(bool skipping, bool prefix_check) {
// If this happens too many times in a row for the same user key, we want // If this happens too many times in a row for the same user key, we want
// to seek to the target sequence number. // to seek to the target sequence number.
int cmp = int cmp =
user_comparator_->Compare(ikey_.user_key, saved_key_.GetUserKey()); user_comparator_.Compare(ikey_.user_key, saved_key_.GetUserKey());
if (cmp == 0 || (skipping && cmp <= 0)) { if (cmp == 0 || (skipping && cmp <= 0)) {
num_skipped++; num_skipped++;
} else { } else {
@ -654,7 +655,7 @@ bool DBIter::MergeValuesNewToOld() {
return false; return false;
} }
if (!user_comparator_->Equal(ikey.user_key, saved_key_.GetUserKey())) { if (!user_comparator_.Equal(ikey.user_key, saved_key_.GetUserKey())) {
// hit the next user key, stop right here // hit the next user key, stop right here
break; break;
} else if (kTypeDeletion == ikey.type || kTypeSingleDeletion == ikey.type || } else if (kTypeDeletion == ikey.type || kTypeSingleDeletion == ikey.type ||
@ -774,8 +775,7 @@ bool DBIter::ReverseToForward() {
if (!ParseKey(&ikey)) { if (!ParseKey(&ikey)) {
return false; return false;
} }
if (user_comparator_->Compare(ikey.user_key, saved_key_.GetUserKey()) >= if (user_comparator_.Compare(ikey.user_key, saved_key_.GetUserKey()) >= 0) {
0) {
return true; return true;
} }
iter_->Next(); iter_->Next();
@ -838,8 +838,8 @@ void DBIter::PrevInternal() {
} }
if (iterate_lower_bound_ != nullptr && if (iterate_lower_bound_ != nullptr &&
user_comparator_->Compare(saved_key_.GetUserKey(), user_comparator_.Compare(saved_key_.GetUserKey(),
*iterate_lower_bound_) < 0) { *iterate_lower_bound_) < 0) {
// We've iterated earlier than the user-specified lower bound. // We've iterated earlier than the user-specified lower bound.
valid_ = false; valid_ = false;
return; return;
@ -900,7 +900,7 @@ bool DBIter::FindValueForCurrentKey() {
} }
if (!IsVisible(ikey.sequence) || if (!IsVisible(ikey.sequence) ||
!user_comparator_->Equal(ikey.user_key, saved_key_.GetUserKey())) { !user_comparator_.Equal(ikey.user_key, saved_key_.GetUserKey())) {
break; break;
} }
if (TooManyInternalKeysSkipped()) { if (TooManyInternalKeysSkipped()) {
@ -1053,7 +1053,7 @@ bool DBIter::FindValueForCurrentKeyUsingSeek() {
if (!ParseKey(&ikey)) { if (!ParseKey(&ikey)) {
return false; return false;
} }
if (!user_comparator_->Equal(ikey.user_key, saved_key_.GetUserKey())) { if (!user_comparator_.Equal(ikey.user_key, saved_key_.GetUserKey())) {
// No visible values for this key, even though FindValueForCurrentKey() // No visible values for this key, even though FindValueForCurrentKey()
// has seen some. This is possible if we're using a tailing iterator, and // has seen some. This is possible if we're using a tailing iterator, and
// the entries were discarded in a compaction. // the entries were discarded in a compaction.
@ -1109,7 +1109,7 @@ bool DBIter::FindValueForCurrentKeyUsingSeek() {
if (!ParseKey(&ikey)) { if (!ParseKey(&ikey)) {
return false; return false;
} }
if (!user_comparator_->Equal(ikey.user_key, saved_key_.GetUserKey())) { if (!user_comparator_.Equal(ikey.user_key, saved_key_.GetUserKey())) {
break; break;
} }
@ -1191,7 +1191,7 @@ bool DBIter::FindUserKeyBeforeSavedKey() {
return false; return false;
} }
if (user_comparator_->Compare(ikey.user_key, saved_key_.GetUserKey()) < 0) { if (user_comparator_.Compare(ikey.user_key, saved_key_.GetUserKey()) < 0) {
return true; return true;
} }
@ -1281,8 +1281,8 @@ void DBIter::Seek(const Slice& target) {
#endif // ROCKSDB_LITE #endif // ROCKSDB_LITE
if (iterate_lower_bound_ != nullptr && if (iterate_lower_bound_ != nullptr &&
user_comparator_->Compare(saved_key_.GetUserKey(), user_comparator_.Compare(saved_key_.GetUserKey(), *iterate_lower_bound_) <
*iterate_lower_bound_) < 0) { 0) {
saved_key_.Clear(); saved_key_.Clear();
saved_key_.SetInternalKey(*iterate_lower_bound_, seq); saved_key_.SetInternalKey(*iterate_lower_bound_, seq);
} }
@ -1333,8 +1333,8 @@ void DBIter::SeekForPrev(const Slice& target) {
kValueTypeForSeekForPrev); kValueTypeForSeekForPrev);
if (iterate_upper_bound_ != nullptr && if (iterate_upper_bound_ != nullptr &&
user_comparator_->Compare(saved_key_.GetUserKey(), user_comparator_.Compare(saved_key_.GetUserKey(),
*iterate_upper_bound_) >= 0) { *iterate_upper_bound_) >= 0) {
saved_key_.Clear(); saved_key_.Clear();
saved_key_.SetInternalKey(*iterate_upper_bound_, kMaxSequenceNumber); saved_key_.SetInternalKey(*iterate_upper_bound_, kMaxSequenceNumber);
} }
@ -1428,7 +1428,7 @@ void DBIter::SeekToLast() {
if (iterate_upper_bound_ != nullptr) { if (iterate_upper_bound_ != nullptr) {
// Seek to last key strictly less than ReadOptions.iterate_upper_bound. // Seek to last key strictly less than ReadOptions.iterate_upper_bound.
SeekForPrev(*iterate_upper_bound_); SeekForPrev(*iterate_upper_bound_);
if (Valid() && user_comparator_->Equal(*iterate_upper_bound_, key())) { if (Valid() && user_comparator_.Equal(*iterate_upper_bound_, key())) {
ReleaseTempPinnedData(); ReleaseTempPinnedData();
PrevInternal(); PrevInternal();
} }

@ -116,8 +116,7 @@ int InternalKeyComparator::Compare(const ParsedInternalKey& a,
// increasing user key (according to user-supplied comparator) // increasing user key (according to user-supplied comparator)
// decreasing sequence number // decreasing sequence number
// decreasing type (though sequence# should be enough to disambiguate) // decreasing type (though sequence# should be enough to disambiguate)
int r = user_comparator_->Compare(a.user_key, b.user_key); int r = user_comparator_.Compare(a.user_key, b.user_key);
PERF_COUNTER_ADD(user_key_comparison_count, 1);
if (r == 0) { if (r == 0) {
if (a.sequence > b.sequence) { if (a.sequence > b.sequence) {
r = -1; r = -1;
@ -139,9 +138,9 @@ void InternalKeyComparator::FindShortestSeparator(
Slice user_start = ExtractUserKey(*start); Slice user_start = ExtractUserKey(*start);
Slice user_limit = ExtractUserKey(limit); Slice user_limit = ExtractUserKey(limit);
std::string tmp(user_start.data(), user_start.size()); std::string tmp(user_start.data(), user_start.size());
user_comparator_->FindShortestSeparator(&tmp, user_limit); user_comparator_.FindShortestSeparator(&tmp, user_limit);
if (tmp.size() <= user_start.size() && if (tmp.size() <= user_start.size() &&
user_comparator_->Compare(user_start, tmp) < 0) { user_comparator_.Compare(user_start, tmp) < 0) {
// User key has become shorter physically, but larger logically. // User key has become shorter physically, but larger logically.
// Tack on the earliest possible number to the shortened user key. // Tack on the earliest possible number to the shortened user key.
PutFixed64(&tmp, PackSequenceAndType(kMaxSequenceNumber,kValueTypeForSeek)); PutFixed64(&tmp, PackSequenceAndType(kMaxSequenceNumber,kValueTypeForSeek));
@ -154,9 +153,9 @@ void InternalKeyComparator::FindShortestSeparator(
void InternalKeyComparator::FindShortSuccessor(std::string* key) const { void InternalKeyComparator::FindShortSuccessor(std::string* key) const {
Slice user_key = ExtractUserKey(*key); Slice user_key = ExtractUserKey(*key);
std::string tmp(user_key.data(), user_key.size()); std::string tmp(user_key.data(), user_key.size());
user_comparator_->FindShortSuccessor(&tmp); user_comparator_.FindShortSuccessor(&tmp);
if (tmp.size() <= user_key.size() && if (tmp.size() <= user_key.size() &&
user_comparator_->Compare(user_key, tmp) < 0) { user_comparator_.Compare(user_key, tmp) < 0) {
// User key has become shorter physically, but larger logically. // User key has become shorter physically, but larger logically.
// Tack on the earliest possible number to the shortened user key. // Tack on the earliest possible number to the shortened user key.
PutFixed64(&tmp, PackSequenceAndType(kMaxSequenceNumber,kValueTypeForSeek)); PutFixed64(&tmp, PackSequenceAndType(kMaxSequenceNumber,kValueTypeForSeek));

@ -21,6 +21,7 @@
#include "rocksdb/types.h" #include "rocksdb/types.h"
#include "util/coding.h" #include "util/coding.h"
#include "util/logging.h" #include "util/logging.h"
#include "util/user_comparator_wrapper.h"
namespace rocksdb { namespace rocksdb {
@ -160,13 +161,13 @@ class InternalKeyComparator
#endif #endif
: public Comparator { : public Comparator {
private: private:
const Comparator* user_comparator_; UserComparatorWrapper user_comparator_;
std::string name_; std::string name_;
public: public:
explicit InternalKeyComparator(const Comparator* c) : user_comparator_(c), explicit InternalKeyComparator(const Comparator* c)
name_("rocksdb.InternalKeyComparator:" + : user_comparator_(c),
std::string(user_comparator_->Name())) { name_("rocksdb.InternalKeyComparator:" +
} std::string(user_comparator_.Name())) {}
virtual ~InternalKeyComparator() {} virtual ~InternalKeyComparator() {}
virtual const char* Name() const override; virtual const char* Name() const override;
@ -177,12 +178,14 @@ class InternalKeyComparator
const Slice& limit) const override; const Slice& limit) const override;
virtual void FindShortSuccessor(std::string* key) const override; virtual void FindShortSuccessor(std::string* key) const override;
const Comparator* user_comparator() const { return user_comparator_; } const Comparator* user_comparator() const {
return user_comparator_.user_comparator();
}
int Compare(const InternalKey& a, const InternalKey& b) const; int Compare(const InternalKey& a, const InternalKey& b) const;
int Compare(const ParsedInternalKey& a, const ParsedInternalKey& b) const; int Compare(const ParsedInternalKey& a, const ParsedInternalKey& b) const;
virtual const Comparator* GetRootComparator() const override { virtual const Comparator* GetRootComparator() const override {
return user_comparator_->GetRootComparator(); return user_comparator_.GetRootComparator();
} }
}; };
@ -639,8 +642,7 @@ int InternalKeyComparator::Compare(const Slice& akey, const Slice& bkey) const {
// increasing user key (according to user-supplied comparator) // increasing user key (according to user-supplied comparator)
// decreasing sequence number // decreasing sequence number
// decreasing type (though sequence# should be enough to disambiguate) // decreasing type (though sequence# should be enough to disambiguate)
int r = user_comparator_->Compare(ExtractUserKey(akey), ExtractUserKey(bkey)); int r = user_comparator_.Compare(ExtractUserKey(akey), ExtractUserKey(bkey));
PERF_COUNTER_ADD(user_key_comparison_count, 1);
if (r == 0) { if (r == 0) {
const uint64_t anum = DecodeFixed64(akey.data() + akey.size() - 8); const uint64_t anum = DecodeFixed64(akey.data() + akey.size() - 8);
const uint64_t bnum = DecodeFixed64(bkey.data() + bkey.size() - 8); const uint64_t bnum = DecodeFixed64(bkey.data() + bkey.size() - 8);
@ -659,8 +661,7 @@ int InternalKeyComparator::CompareKeySeq(const Slice& akey,
// Order by: // Order by:
// increasing user key (according to user-supplied comparator) // increasing user key (according to user-supplied comparator)
// decreasing sequence number // decreasing sequence number
int r = user_comparator_->Compare(ExtractUserKey(akey), ExtractUserKey(bkey)); int r = user_comparator_.Compare(ExtractUserKey(akey), ExtractUserKey(bkey));
PERF_COUNTER_ADD(user_key_comparison_count, 1);
if (r == 0) { if (r == 0) {
// Shift the number to exclude the last byte which contains the value type // Shift the number to exclude the last byte which contains the value type
const uint64_t anum = DecodeFixed64(akey.data() + akey.size() - 8) >> 8; const uint64_t anum = DecodeFixed64(akey.data() + akey.size() - 8) >> 8;

@ -51,6 +51,7 @@
#include "util/stop_watch.h" #include "util/stop_watch.h"
#include "util/string_util.h" #include "util/string_util.h"
#include "util/sync_point.h" #include "util/sync_point.h"
#include "util/user_comparator_wrapper.h"
namespace rocksdb { namespace rocksdb {
@ -477,6 +478,7 @@ class LevelIterator final : public InternalIterator {
read_options_(read_options), read_options_(read_options),
env_options_(env_options), env_options_(env_options),
icomparator_(icomparator), icomparator_(icomparator),
user_comparator_(icomparator.user_comparator()),
flevel_(flevel), flevel_(flevel),
prefix_extractor_(prefix_extractor), prefix_extractor_(prefix_extractor),
file_read_hist_(file_read_hist), file_read_hist_(file_read_hist),
@ -541,9 +543,8 @@ class LevelIterator final : public InternalIterator {
bool KeyReachedUpperBound(const Slice& internal_key) { bool KeyReachedUpperBound(const Slice& internal_key) {
return read_options_.iterate_upper_bound != nullptr && return read_options_.iterate_upper_bound != nullptr &&
icomparator_.user_comparator()->Compare( user_comparator_.Compare(ExtractUserKey(internal_key),
ExtractUserKey(internal_key), *read_options_.iterate_upper_bound) >= 0;
*read_options_.iterate_upper_bound) >= 0;
} }
InternalIterator* NewFileIterator() { InternalIterator* NewFileIterator() {
@ -571,6 +572,7 @@ class LevelIterator final : public InternalIterator {
const ReadOptions read_options_; const ReadOptions read_options_;
const EnvOptions& env_options_; const EnvOptions& env_options_;
const InternalKeyComparator& icomparator_; const InternalKeyComparator& icomparator_;
const UserComparatorWrapper user_comparator_;
const LevelFilesBrief* flevel_; const LevelFilesBrief* flevel_;
mutable FileDescriptor current_value_; mutable FileDescriptor current_value_;
const SliceTransform* prefix_extractor_; const SliceTransform* prefix_extractor_;

@ -2353,9 +2353,8 @@ void BlockBasedTableIterator<TBlockIter, TValue>::Seek(const Slice& target) {
assert( assert(
!block_iter_.Valid() || !block_iter_.Valid() ||
(key_includes_seq_ && icomp_.Compare(target, block_iter_.key()) <= 0) || (key_includes_seq_ && icomp_.Compare(target, block_iter_.key()) <= 0) ||
(!key_includes_seq_ && (!key_includes_seq_ && user_comparator_.Compare(ExtractUserKey(target),
icomp_.user_comparator()->Compare(ExtractUserKey(target), block_iter_.key()) <= 0));
block_iter_.key()) <= 0));
} }
template <class TBlockIter, typename TValue> template <class TBlockIter, typename TValue>
@ -2522,9 +2521,8 @@ void BlockBasedTableIterator<TBlockIter, TValue>::FindKeyForward() {
bool reached_upper_bound = bool reached_upper_bound =
(read_options_.iterate_upper_bound != nullptr && (read_options_.iterate_upper_bound != nullptr &&
block_iter_points_to_real_block_ && block_iter_.Valid() && block_iter_points_to_real_block_ && block_iter_.Valid() &&
icomp_.user_comparator()->Compare(ExtractUserKey(block_iter_.key()), user_comparator_.Compare(ExtractUserKey(block_iter_.key()),
*read_options_.iterate_upper_bound) >= *read_options_.iterate_upper_bound) >= 0);
0);
TEST_SYNC_POINT_CALLBACK( TEST_SYNC_POINT_CALLBACK(
"BlockBasedTable::BlockEntryIteratorState::KeyReachedUpperBound", "BlockBasedTable::BlockEntryIteratorState::KeyReachedUpperBound",
&reached_upper_bound); &reached_upper_bound);

@ -33,6 +33,7 @@
#include "table/two_level_iterator.h" #include "table/two_level_iterator.h"
#include "util/coding.h" #include "util/coding.h"
#include "util/file_reader_writer.h" #include "util/file_reader_writer.h"
#include "util/user_comparator_wrapper.h"
namespace rocksdb { namespace rocksdb {
@ -580,6 +581,7 @@ class BlockBasedTableIterator : public InternalIteratorBase<TValue> {
: table_(table), : table_(table),
read_options_(read_options), read_options_(read_options),
icomp_(icomp), icomp_(icomp),
user_comparator_(icomp.user_comparator()),
index_iter_(index_iter), index_iter_(index_iter),
pinned_iters_mgr_(nullptr), pinned_iters_mgr_(nullptr),
block_iter_points_to_real_block_(false), block_iter_points_to_real_block_(false),
@ -676,6 +678,7 @@ class BlockBasedTableIterator : public InternalIteratorBase<TValue> {
BlockBasedTable* table_; BlockBasedTable* table_;
const ReadOptions read_options_; const ReadOptions read_options_;
const InternalKeyComparator& icomp_; const InternalKeyComparator& icomp_;
UserComparatorWrapper user_comparator_;
InternalIteratorBase<BlockHandle>* index_iter_; InternalIteratorBase<BlockHandle>* index_iter_;
PinnedIteratorsManager* pinned_iters_mgr_; PinnedIteratorsManager* pinned_iters_mgr_;
TBlockIter block_iter_; TBlockIter block_iter_;

@ -0,0 +1,65 @@
// Copyright (c) 2011-present, Facebook, Inc. All rights reserved.
// This source code is licensed under both the GPLv2 (found in the
// COPYING file in the root directory) and Apache 2.0 License
// (found in the LICENSE.Apache file in the root directory).
// Copyright (c) 2011 The LevelDB Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. See the AUTHORS file for names of contributors.
#pragma once
#include "monitoring/perf_context_imp.h"
#include "rocksdb/comparator.h"
namespace rocksdb {
// Wrapper of user comparator, with auto increment to
// perf_context.user_key_comparison_count.
class UserComparatorWrapper final : public Comparator {
public:
explicit UserComparatorWrapper(const Comparator* const user_cmp)
: user_comparator_(user_cmp) {}
~UserComparatorWrapper() = default;
const Comparator* user_comparator() const { return user_comparator_; }
int Compare(const Slice& a, const Slice& b) const override {
PERF_COUNTER_ADD(user_key_comparison_count, 1);
return user_comparator_->Compare(a, b);
}
bool Equal(const Slice& a, const Slice& b) const override {
PERF_COUNTER_ADD(user_key_comparison_count, 1);
return user_comparator_->Equal(a, b);
}
const char* Name() const override { return user_comparator_->Name(); }
void FindShortestSeparator(std::string* start,
const Slice& limit) const override {
return user_comparator_->FindShortestSeparator(start, limit);
}
void FindShortSuccessor(std::string* key) const override {
return user_comparator_->FindShortSuccessor(key);
}
const Comparator* GetRootComparator() const override {
return user_comparator_->GetRootComparator();
}
bool IsSameLengthImmediateSuccessor(const Slice& s,
const Slice& t) const override {
return user_comparator_->IsSameLengthImmediateSuccessor(s, t);
}
bool CanKeysWithDifferentByteContentsBeEqual() const override {
return user_comparator_->CanKeysWithDifferentByteContentsBeEqual();
}
private:
const Comparator* user_comparator_;
};
} // namespace rocksdb
Loading…
Cancel
Save