diff --git a/HISTORY.md b/HISTORY.md index 2030c4cf6..7e689a855 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,5 +1,8 @@ # Rocksdb Change Log ## Unreleased +### Bug fixes +* Fix a performance regression introduced in 6.4 that makes a upper bound check for every Next() even if keys are within a data block that is within the upper bound. + ## 6.12 (2020-07-28) ### Public API Change diff --git a/table/block_based/block_based_table_iterator.cc b/table/block_based/block_based_table_iterator.cc index aa398766b..af81799a0 100644 --- a/table/block_based/block_based_table_iterator.cc +++ b/table/block_based/block_based_table_iterator.cc @@ -300,7 +300,8 @@ void BlockBasedTableIterator::FindBlockForward() { // Whether next data block is out of upper bound, if there is one. const bool next_block_is_out_of_bound = read_options_.iterate_upper_bound != nullptr && - block_iter_points_to_real_block_ && !data_block_within_upper_bound_; + block_iter_points_to_real_block_ && + block_upper_bound_check_ == BlockUpperBound::kUpperBoundInCurBlock; assert(!next_block_is_out_of_bound || user_comparator_.CompareWithoutTimestamp( *read_options_.iterate_upper_bound, /*a_has_ts=*/false, @@ -358,7 +359,9 @@ void BlockBasedTableIterator::FindKeyBackward() { } void BlockBasedTableIterator::CheckOutOfBound() { - if (read_options_.iterate_upper_bound != nullptr && Valid()) { + if (read_options_.iterate_upper_bound != nullptr && + block_upper_bound_check_ != BlockUpperBound::kUpperBoundBeyondCurBlock && + Valid()) { is_out_of_bound_ = user_comparator_.CompareWithoutTimestamp( *read_options_.iterate_upper_bound, /*a_has_ts=*/false, user_key(), @@ -369,11 +372,12 @@ void BlockBasedTableIterator::CheckOutOfBound() { void BlockBasedTableIterator::CheckDataBlockWithinUpperBound() { if (read_options_.iterate_upper_bound != nullptr && block_iter_points_to_real_block_) { - data_block_within_upper_bound_ = - (user_comparator_.CompareWithoutTimestamp( - *read_options_.iterate_upper_bound, /*a_has_ts=*/false, - index_iter_->user_key(), - /*b_has_ts=*/true) > 0); + block_upper_bound_check_ = (user_comparator_.CompareWithoutTimestamp( + *read_options_.iterate_upper_bound, + /*a_has_ts=*/false, index_iter_->user_key(), + /*b_has_ts=*/true) > 0) + ? BlockUpperBound::kUpperBoundBeyondCurBlock + : BlockUpperBound::kUpperBoundInCurBlock; } } } // namespace ROCKSDB_NAMESPACE diff --git a/table/block_based/block_based_table_iterator.h b/table/block_based/block_based_table_iterator.h index 026ac9ee7..ac3f47b68 100644 --- a/table/block_based/block_based_table_iterator.h +++ b/table/block_based/block_based_table_iterator.h @@ -104,7 +104,8 @@ class BlockBasedTableIterator : public InternalIteratorBase { inline bool MayBeOutOfUpperBound() override { assert(Valid()); - return !data_block_within_upper_bound_; + return block_upper_bound_check_ != + BlockUpperBound::kUpperBoundBeyondCurBlock; } void SetPinnedItersMgr(PinnedIteratorsManager* pinned_iters_mgr) override { @@ -134,6 +135,7 @@ class BlockBasedTableIterator : public InternalIteratorBase { block_iter_.Invalidate(Status::OK()); block_iter_points_to_real_block_ = false; } + block_upper_bound_check_ = BlockUpperBound::kUnknown; } void SavePrevIndexValue() { @@ -149,6 +151,34 @@ class BlockBasedTableIterator : public InternalIteratorBase { kForward, kBackward, }; + // This enum indicates whether the upper bound falls into current block + // or beyond. + // +-------------+ + // | cur block | <-- (1) + // +-------------+ + // <-- (2) + // --- --- + // <-- (3) + // +-------------+ + // | next block | <-- (4) + // ...... + // + // When the block is smaller than , kUpperBoundInCurBlock + // is the value to use. The examples are (1) or (2) in the graph. It means + // all keys in the next block or beyond will be out of bound. Keys within + // the current block may or may not be out of bound. + // When the block is larger or equal to , + // kUpperBoundBeyondCurBlock is to be used. The examples are (3) and (4) + // in the graph. It means that all keys in the current block is within the + // upper bound and keys in the next block may or may not be within the uppder + // bound. + // If the boundary key hasn't been checked against the upper bound, + // kUnknown can be used. + enum class BlockUpperBound { + kUpperBoundInCurBlock, + kUpperBoundBeyondCurBlock, + kUnknown, + }; const BlockBasedTable* table_; const ReadOptions& read_options_; @@ -169,8 +199,9 @@ class BlockBasedTableIterator : public InternalIteratorBase { bool block_iter_points_to_real_block_; // See InternalIteratorBase::IsOutOfBound(). bool is_out_of_bound_ = false; - // Whether current data block being fully within iterate upper bound. - bool data_block_within_upper_bound_ = false; + // How current data block's boundary key with the next block is compared with + // iterate upper bound. + BlockUpperBound block_upper_bound_check_ = BlockUpperBound::kUnknown; // True if we're standing at the first key of a block, and we haven't loaded // that block yet. A call to PrepareValue() will trigger loading the block. bool is_at_first_key_from_index_ = false;