Two small refactoring for better inlining

Summary:
Move uncommon code paths in RangeDelAggregator::ShouldDelete() and IterKey::EnlargeBufferIfNeeded() to a separate function, so that the inlined strcuture can be more optimized.

Optimize it because these places show up in CPU profiling, though minimum. The performance is really hard measure. I ran db_bench with readseq benchmark against in-memory DB many times. The variation is big, but it seems to show 1% improvements.
Closes https://github.com/facebook/rocksdb/pull/2877

Differential Revision: D5828123

Pulled By: siying

fbshipit-source-id: 41a49e229f91e9f8409f85cc6f0dc70e31334e4b

diff --git a/db/dbformat.cc b/db/dbformat.cc
index 20c54495aa02907df5f898145c641abdb664b246..d76b28e603b965a6f3d29087719b7cbdf723aa1d 100644 (file)
--- a/db/dbformat.cc
+++ b/db/dbformat.cc
@@ -174,4 +174,13 @@ LookupKey::LookupKey(const Slice& _user_key, SequenceNumber s) {
   end_ = dst;
 }
 
+void IterKey::EnlargeBuffer(size_t key_size) {
+  // If size is smaller than buffer size, continue using current buffer,
+  // or the static allocated one, as default
+  assert(key_size > buf_size_);
+  // Need to enlarge the buffer.
+  ResetBuffer();
+  buf_ = new char[key_size];
+  buf_size_ = key_size;
+}
 }  // namespace rocksdb

diff --git a/db/dbformat.h b/db/dbformat.h
index 187069cd0ecfdedd027390a43fe8cf908d24138d..8e176807daea3f983d5b57723d2b7fcf3cee153d 100644 (file)
--- a/db/dbformat.h
+++ b/db/dbformat.h
@@ -504,13 +504,12 @@ class IterKey {
     // If size is smaller than buffer size, continue using current buffer,
     // or the static allocated one, as default
     if (key_size > buf_size_) {
-      // Need to enlarge the buffer.
-      ResetBuffer();
-      buf_ = new char[key_size];
-      buf_size_ = key_size;
+      EnlargeBuffer(key_size);
     }
   }
 
+  void EnlargeBuffer(size_t key_size);
+
   // No copying allowed
   IterKey(const IterKey&) = delete;
   void operator=(const IterKey&) = delete;

diff --git a/db/range_del_aggregator.cc b/db/range_del_aggregator.cc
index c83f5a88cd8b1e9da367aef278bc3aaa28e9e314..e80b5349467cab9dc5821570ee2fe599f79c2c10 100644 (file)
--- a/db/range_del_aggregator.cc
+++ b/db/range_del_aggregator.cc
@@ -43,11 +43,9 @@ void RangeDelAggregator::InitRep(const std::vector<SequenceNumber>& snapshots) {
   rep_->pinned_iters_mgr_.StartPinning();
 }
 
-bool RangeDelAggregator::ShouldDelete(
+bool RangeDelAggregator::ShouldDeleteImpl(
     const Slice& internal_key, RangeDelAggregator::RangePositioningMode mode) {
-  if (rep_ == nullptr) {
-    return false;
-  }
+  assert(rep_ != nullptr);
   ParsedInternalKey parsed;
   if (!ParseInternalKey(internal_key, &parsed)) {
     assert(false);
@@ -55,13 +53,11 @@ bool RangeDelAggregator::ShouldDelete(
   return ShouldDelete(parsed, mode);
 }
 
-bool RangeDelAggregator::ShouldDelete(
+bool RangeDelAggregator::ShouldDeleteImpl(
     const ParsedInternalKey& parsed,
     RangeDelAggregator::RangePositioningMode mode) {
   assert(IsValueType(parsed.type));
-  if (rep_ == nullptr) {
-    return false;
-  }
+  assert(rep_ != nullptr);
   auto& positional_tombstone_map = GetPositionalTombstoneMap(parsed.sequence);
   const auto& tombstone_map = positional_tombstone_map.raw_map;
   if (tombstone_map.empty()) {

diff --git a/db/range_del_aggregator.h b/db/range_del_aggregator.h
index 9d4b8ca16832f40e02eccf30e95e3a16da19c0bb..76fe1870a078a11b63d1960962fcfceef1e56816 100644 (file)
--- a/db/range_del_aggregator.h
+++ b/db/range_del_aggregator.h
@@ -74,9 +74,24 @@ class RangeDelAggregator {
   //             the deletion whose interval contains this key. Otherwise, its
   //             value must be kFullScan indicating linear scan from beginning..
   bool ShouldDelete(const ParsedInternalKey& parsed,
-                    RangePositioningMode mode = kFullScan);
+                    RangePositioningMode mode = kFullScan) {
+    if (rep_ == nullptr) {
+      return false;
+    }
+    return ShouldDeleteImpl(parsed, mode);
+  }
   bool ShouldDelete(const Slice& internal_key,
-                    RangePositioningMode mode = kFullScan);
+                    RangePositioningMode mode = kFullScan) {
+    if (rep_ == nullptr) {
+      return false;
+    }
+    return ShouldDeleteImpl(internal_key, mode);
+  }
+  bool ShouldDeleteImpl(const ParsedInternalKey& parsed,
+                        RangePositioningMode mode = kFullScan);
+  bool ShouldDeleteImpl(const Slice& internal_key,
+                        RangePositioningMode mode = kFullScan);
+
   bool ShouldAddTombstones(bool bottommost_level = false);
 
   // Adds tombstones to the tombstone aggregation structure maintained by this