Introduce InternalIteratorBase::NextAndGetResult() (#5197)

Summary:
In long scans, virtual function calls of Next(), Valid(), key() and value() are not trivial. By introducing NextAndGetResult(), Some of the Next(), Valid() and key() calls are consolidated into one virtual function call to reduce CPU.
Also did some inline tricks and add some "final" randomly in some functions. Even without the "final" annotation, most Next() calls are inlined with -O3, but sometimes with a final it is inlined by O2 too. It doesn't hurt to add those final annotations.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/5197

Differential Revision: D14945977

Pulled By: siying

fbshipit-source-id: 7003969f9a5f1d5717f0bda503b91d19ba75ed88

db/db_iter.cc

@@ -231,7 +231,7 @@ class DBIter final: public Iterator {
     return Status::InvalidArgument("Unidentified property.");
   }
 
-  void Next() override;
+  void Next() final override;
   void Prev() override;
   void Seek(const Slice& target) override;
   void SeekForPrev(const Slice& target) override;

db/version_set.cc

@@ -906,7 +906,8 @@ class LevelIterator final : public InternalIterator {
   void SeekForPrev(const Slice& target) override;
   void SeekToFirst() override;
   void SeekToLast() override;
-  void Next() override;
+  void Next() final override;
+  bool NextAndGetResult(Slice* ret_key) override;
   void Prev() override;
 
   bool Valid() const override { return file_iter_.Valid(); }
@@ -942,6 +943,13 @@ class LevelIterator final : public InternalIterator {
   void SetFileIterator(InternalIterator* iter);
   void InitFileIterator(size_t new_file_index);
 
+  // Called by both of Next() and NextAndGetResult(). Force inline.
+  void NextImpl() {
+    assert(Valid());
+    file_iter_.Next();
+    SkipEmptyFileForward();
+  }
+
   const Slice& file_smallest_key(size_t file_index) {
     assert(file_index < flevel_->num_files);
     return flevel_->files[file_index].smallest_key;
@@ -1037,10 +1045,15 @@ void LevelIterator::SeekToLast() {
   SkipEmptyFileBackward();
 }
 
-void LevelIterator::Next() {
-  assert(Valid());
-  file_iter_.Next();
-  SkipEmptyFileForward();
+void LevelIterator::Next() { NextImpl(); }
+
+bool LevelIterator::NextAndGetResult(Slice* ret_key) {
+  NextImpl();
+  bool is_valid = Valid();
+  if (is_valid) {
+    *ret_key = key();
+  }
+  return is_valid;
 }
 
 void LevelIterator::Prev() {

table/block.h

@@ -392,7 +392,7 @@ class DataBlockIter final : public BlockIter<Slice> {
 
   virtual void Prev() override;
 
-  virtual void Next() override;
+  virtual void Next() final override;
 
   // Try to advance to the next entry in the block. If there is data corruption
   // or error, report it to the caller instead of aborting the process. May

table/block_based_table_reader.cc

@@ -2435,6 +2435,17 @@ void BlockBasedTableIterator<TBlockIter, TValue>::Next() {
   FindKeyForward();
 }
 
+template <class TBlockIter, typename TValue>
+bool BlockBasedTableIterator<TBlockIter, TValue>::NextAndGetResult(
+    Slice* ret_key) {
+  Next();
+  bool is_valid = Valid();
+  if (is_valid) {
+    *ret_key = key();
+  }
+  return is_valid;
+}
+
 template <class TBlockIter, typename TValue>
 void BlockBasedTableIterator<TBlockIter, TValue>::Prev() {
   assert(block_iter_points_to_real_block_);
@@ -2493,10 +2504,10 @@ void BlockBasedTableIterator<TBlockIter, TValue>::InitDataBlock() {
 }
 
 template <class TBlockIter, typename TValue>
-void BlockBasedTableIterator<TBlockIter, TValue>::FindKeyForward() {
+void BlockBasedTableIterator<TBlockIter, TValue>::FindBlockForward() {
   // TODO the while loop inherits from two-level-iterator. We don't know
   // whether a block can be empty so it can be replaced by an "if".
-  while (!block_iter_.Valid()) {
+  do {
     if (!block_iter_.status().ok()) {
       return;
     }
@@ -2528,6 +2539,15 @@ void BlockBasedTableIterator<TBlockIter, TValue>::FindKeyForward() {
     } else {
       return;
     }
+  } while (!block_iter_.Valid());
+}
+
+template <class TBlockIter, typename TValue>
+void BlockBasedTableIterator<TBlockIter, TValue>::FindKeyForward() {
+  assert(!is_out_of_bound_);
+
+  if (!block_iter_.Valid()) {
+    FindBlockForward();
   }
 }
 

table/block_based_table_reader.h

@@ -611,7 +611,8 @@ class BlockBasedTableIterator : public InternalIteratorBase<TValue> {
   void SeekForPrev(const Slice& target) override;
   void SeekToFirst() override;
   void SeekToLast() override;
-  void Next() override;
+  void Next() final override;
+  bool NextAndGetResult(Slice* ret_key) override;
   void Prev() override;
   bool Valid() const override {
     return !is_out_of_bound_ && block_iter_points_to_real_block_ &&
@@ -688,7 +689,8 @@ class BlockBasedTableIterator : public InternalIteratorBase<TValue> {
   }
 
   void InitDataBlock();
-  void FindKeyForward();
+  inline void FindKeyForward();
+  void FindBlockForward();
   void FindKeyBackward();
   void CheckOutOfBound();
 

table/internal_iterator.h

@@ -54,6 +54,15 @@ class InternalIteratorBase : public Cleanable {
   // REQUIRES: Valid()
   virtual void Next() = 0;
 
+  virtual bool NextAndGetResult(Slice* ret_key) {
+    Next();
+    bool is_valid = Valid();
+    if (is_valid) {
+      *ret_key = key();
+    }
+    return is_valid;
+  }
+
   // Moves to the previous entry in the source.  After this call, Valid() is
   // true iff the iterator was not positioned at the first entry in source.
   // REQUIRES: Valid()

table/iterator_wrapper.h

@@ -63,7 +63,11 @@ class IteratorWrapperBase {
   }
   // Methods below require iter() != nullptr
   Status status() const     { assert(iter_); return iter_->status(); }
-  void Next()               { assert(iter_); iter_->Next();        Update(); }
+  void Next() {
+    assert(iter_);
+    valid_ = iter_->NextAndGetResult(&key_);
+    assert(!valid_ || iter_->status().ok());
+  }
   void Prev()               { assert(iter_); iter_->Prev();        Update(); }
   void Seek(const Slice& k) { assert(iter_); iter_->Seek(k);       Update(); }
   void SeekForPrev(const Slice& k) {