From 692f6a3138720ce70f2ade0247be20fd04b4c5b1 Mon Sep 17 00:00:00 2001
From: sdong <siying.d@fb.com>
Date: Wed, 29 Jul 2020 09:43:56 -0700
Subject: [PATCH] Implement NextAndGetResult() in memtable and level iterator
 (#7179)

Summary:
NextAndGetResult() is not implemented in memtable and is very simply implemented in level iterator. The result is that for a normal leveled iterator, performance regression will be observed for calling PrepareValue() for most iterator Next(). Mitigate the problem by implementing the function for both iterators. In level iterator, the implementation cannot be perfect as when calling file iterator's SeekToFirst() we don't have information about whether the value is prepared. Fortunately, the first key should not cause a big portion of the CPu.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/7179

Test Plan: Run normal crash test for a while.

Reviewed By: anand1976

Differential Revision: D22783840

fbshipit-source-id: c19f45cdf21b756190adef97a3b66ccde3936e05
---
 db/memtable.cc           | 10 ++++++++++
 db/version_set.cc        | 32 ++++++++++++++++++--------------
 table/iterator_wrapper.h |  7 +++++++
 3 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/db/memtable.cc b/db/memtable.cc
index b391fe561..422433459 100644
--- a/db/memtable.cc
+++ b/db/memtable.cc
@@ -377,6 +377,16 @@ class MemTableIterator : public InternalIterator {
     iter_->Next();
     valid_ = iter_->Valid();
   }
+  bool NextAndGetResult(IterateResult* result) override {
+    Next();
+    bool is_valid = valid_;
+    if (is_valid) {
+      result->key = key();
+      result->may_be_out_of_upper_bound = true;
+      result->value_prepared = true;
+    }
+    return is_valid;
+  }
   void Prev() override {
     PERF_COUNTER_ADD(prev_on_memtable_count, 1);
     assert(Valid());
diff --git a/db/version_set.cc b/db/version_set.cc
index 3ac5b0c83..67d1be3ad 100644
--- a/db/version_set.cc
+++ b/db/version_set.cc
@@ -970,13 +970,6 @@ 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;
@@ -1140,15 +1133,26 @@ void LevelIterator::SeekToLast() {
   CheckMayBeOutOfLowerBound();
 }
 
-void LevelIterator::Next() { NextImpl(); }
+void LevelIterator::Next() {
+  assert(Valid());
+  file_iter_.Next();
+  SkipEmptyFileForward();
+}
 
 bool LevelIterator::NextAndGetResult(IterateResult* result) {
-  NextImpl();
-  bool is_valid = Valid();
-  if (is_valid) {
-    result->key = key();
-    result->may_be_out_of_upper_bound = MayBeOutOfUpperBound();
-    result->value_prepared = !allow_unprepared_value_;
+  assert(Valid());
+  bool is_valid = file_iter_.NextAndGetResult(result);
+  if (!is_valid) {
+    SkipEmptyFileForward();
+    is_valid = Valid();
+    if (is_valid) {
+      result->key = key();
+      result->may_be_out_of_upper_bound = MayBeOutOfUpperBound();
+      // Ideally, we should return the real file_iter_.value_prepared but the
+      // information is not here. It would casue an extra PrepareValue()
+      // for the first key of a file.
+      result->value_prepared = !allow_unprepared_value_;
+    }
   }
   return is_valid;
 }
diff --git a/table/iterator_wrapper.h b/table/iterator_wrapper.h
index b4b051ff6..010a23abf 100644
--- a/table/iterator_wrapper.h
+++ b/table/iterator_wrapper.h
@@ -89,6 +89,13 @@ class IteratorWrapperBase {
     valid_ = iter_->NextAndGetResult(&result_);
     assert(!valid_ || iter_->status().ok());
   }
+  bool NextAndGetResult(IterateResult* result) {
+    assert(iter_);
+    valid_ = iter_->NextAndGetResult(&result_);
+    *result = result_;
+    assert(!valid_ || iter_->status().ok());
+    return valid_;
+  }
   void Prev() {
     assert(iter_);
     iter_->Prev();