From 01cfea66373d6ac7ee65c323fe7c2755d5d5dd7b Mon Sep 17 00:00:00 2001
From: Siying Dong <siying.d@fb.com>
Date: Thu, 18 Apr 2019 12:07:48 -0700
Subject: [PATCH] Some small code changes to improve Next() (#5200)

Summary:
Several small changes for Next():
1. Reducing branching by always update local_stats_.next_count_++ even if statistics is null. This should be faster than a branching.
2. Replacing ResetInternalKeysSkippedCounter() in Next() because the valid_ check is not needed in this case.
3. iter_->Valid() should always be true for non merge case. Remove this check.
4. Adding an inline annotation. It ends up with not picked up by my compiler, but it shouldn't hurt.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/5200

Differential Revision: D15000391

Pulled By: siying

fbshipit-source-id: be97f61c708968234fb8e5cf272b5c2ac07dc4dd
---
 db/db_iter.cc | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/db/db_iter.cc b/db/db_iter.cc
index df2be1b7c..0fce486f8 100644
--- a/db/db_iter.cc
+++ b/db/db_iter.cc
@@ -257,7 +257,7 @@ class DBIter final: public Iterator {
   bool FindValueForCurrentKeyUsingSeek();
   bool FindUserKeyBeforeSavedKey();
   inline bool FindNextUserEntry(bool skipping, bool prefix_check);
-  bool FindNextUserEntryInternal(bool skipping, bool prefix_check);
+  inline bool FindNextUserEntryInternal(bool skipping, bool prefix_check);
   bool ParseKey(ParsedInternalKey* key);
   bool MergeValuesNewToOld();
 
@@ -379,25 +379,26 @@ void DBIter::Next() {
   PERF_CPU_TIMER_GUARD(iter_next_cpu_nanos, env_);
   // Release temporarily pinned blocks from last operation
   ReleaseTempPinnedData();
-  ResetInternalKeysSkippedCounter();
+  local_stats_.skip_count_ += num_internal_keys_skipped_;
+  local_stats_.skip_count_--;
+  num_internal_keys_skipped_ = 0;
   bool ok = true;
   if (direction_ == kReverse) {
     if (!ReverseToForward()) {
       ok = false;
     }
-  } else if (iter_->Valid() && !current_entry_is_merged_) {
+  } else if (!current_entry_is_merged_) {
     // If the current value is not a merge, the iter position is the
     // current key, which is already returned. We can safely issue a
     // Next() without checking the current key.
     // If the current key is a merge, very likely iter already points
     // to the next internal position.
+    assert(iter_->Valid());
     iter_->Next();
     PERF_COUNTER_ADD(internal_key_skipped_count, 1);
   }
 
-  if (statistics_ != nullptr) {
-    local_stats_.next_count_++;
-  }
+  local_stats_.next_count_++;
   if (ok && iter_->Valid()) {
     FindNextUserEntry(true /* skipping the current user key */,
                       prefix_same_as_start_);
@@ -430,7 +431,7 @@ inline bool DBIter::FindNextUserEntry(bool skipping, bool prefix_check) {
 }
 
 // Actual implementation of DBIter::FindNextUserEntry()
-bool DBIter::FindNextUserEntryInternal(bool skipping, bool prefix_check) {
+inline bool DBIter::FindNextUserEntryInternal(bool skipping, bool prefix_check) {
   // Loop until we hit an acceptable entry to yield
   assert(iter_->Valid());
   assert(status_.ok());