Skip to content

Commit

Permalink
Optimize about sstableKeyCompare (#11610)
Browse files Browse the repository at this point in the history
Summary:
We observed `CompactionOutputs::UpdateGrandparentBoundaryInfo` consumes much time for `InternalKey::DecodeFrom` and `InternalKey::~InternalKey` in flame graph.

This PR omit the InternalKey object in `CompactionOutputs::UpdateGrandparentBoundaryInfo` .

![image](https://github.com/facebook/rocksdb/assets/1574991/661eaeec-2f46-46c6-a6a8-9738d6c191de)

Pull Request resolved: #11610

Reviewed By: ajkr

Differential Revision: D47426971

Pulled By: cbi42

fbshipit-source-id: f0d3a8186d778294515c0685032f5b395c4d6a62
  • Loading branch information
rockeet authored and facebook-github-bot committed Jul 14, 2023
1 parent c3c84b3 commit bc0db33
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 15 deletions.
9 changes: 4 additions & 5 deletions db/compaction/compaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,13 @@ namespace ROCKSDB_NAMESPACE {
const uint64_t kRangeTombstoneSentinel =
PackSequenceAndType(kMaxSequenceNumber, kTypeRangeDeletion);

int sstableKeyCompare(const Comparator* user_cmp, const InternalKey& a,
const InternalKey& b) {
auto c = user_cmp->CompareWithoutTimestamp(a.user_key(), b.user_key());
int sstableKeyCompare(const Comparator* uc, const Slice& a, const Slice& b) {
auto c = uc->CompareWithoutTimestamp(ExtractUserKey(a), ExtractUserKey(b));
if (c != 0) {
return c;
}
auto a_footer = ExtractInternalKeyFooter(a.Encode());
auto b_footer = ExtractInternalKeyFooter(b.Encode());
auto a_footer = ExtractInternalKeyFooter(a);
auto b_footer = ExtractInternalKeyFooter(b);
if (a_footer == kRangeTombstoneSentinel) {
if (b_footer != kRangeTombstoneSentinel) {
return -1;
Expand Down
15 changes: 13 additions & 2 deletions db/compaction/compaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,19 @@ namespace ROCKSDB_NAMESPACE {
// that key never appears in the database. We don't want adjacent sstables to
// be considered overlapping if they are separated by the range tombstone
// sentinel.
int sstableKeyCompare(const Comparator* user_cmp, const InternalKey& a,
const InternalKey& b);
int sstableKeyCompare(const Comparator* user_cmp, const Slice&, const Slice&);
inline int sstableKeyCompare(const Comparator* user_cmp, const Slice& a,
const InternalKey& b) {
return sstableKeyCompare(user_cmp, a, b.Encode());
}
inline int sstableKeyCompare(const Comparator* user_cmp, const InternalKey& a,
const Slice& b) {
return sstableKeyCompare(user_cmp, a.Encode(), b);
}
inline int sstableKeyCompare(const Comparator* user_cmp, const InternalKey& a,
const InternalKey& b) {
return sstableKeyCompare(user_cmp, a.Encode(), b.Encode());
}
int sstableKeyCompare(const Comparator* user_cmp, const InternalKey* a,
const InternalKey& b);
int sstableKeyCompare(const Comparator* user_cmp, const InternalKey& a,
Expand Down
11 changes: 3 additions & 8 deletions db/compaction/compaction_outputs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -127,19 +127,14 @@ size_t CompactionOutputs::UpdateGrandparentBoundaryInfo(
if (grandparents.empty()) {
return curr_key_boundary_switched_num;
}
assert(!internal_key.empty());
InternalKey ikey;
ikey.DecodeFrom(internal_key);
assert(ikey.Valid());

const Comparator* ucmp = compaction_->column_family_data()->user_comparator();

// Move the grandparent_index_ to the file containing the current user_key.
// If there are multiple files containing the same user_key, make sure the
// index points to the last file containing the key.
while (grandparent_index_ < grandparents.size()) {
if (being_grandparent_gap_) {
if (sstableKeyCompare(ucmp, ikey,
if (sstableKeyCompare(ucmp, internal_key,
grandparents[grandparent_index_]->smallest) < 0) {
break;
}
Expand All @@ -152,13 +147,13 @@ size_t CompactionOutputs::UpdateGrandparentBoundaryInfo(
being_grandparent_gap_ = false;
} else {
int cmp_result = sstableKeyCompare(
ucmp, ikey, grandparents[grandparent_index_]->largest);
ucmp, internal_key, grandparents[grandparent_index_]->largest);
// If it's same key, make sure grandparent_index_ is pointing to the last
// one.
if (cmp_result < 0 ||
(cmp_result == 0 &&
(grandparent_index_ == grandparents.size() - 1 ||
sstableKeyCompare(ucmp, ikey,
sstableKeyCompare(ucmp, internal_key,
grandparents[grandparent_index_ + 1]->smallest) <
0))) {
break;
Expand Down

0 comments on commit bc0db33

Please sign in to comment.