Reduce the number of params needed to construct DBIter

Summary:
DBIter, and in-turn NewDBIterator and NewArenaWrappedDBIterator, take a  bunch of params. They can be reduced by passing in ReadOptions directly instead of passing in every new param separately. It also seems much cleaner as a bunch of the params towards the end seem to be optional.

(Recently I introduced max_skippable_internal_keys, which added one more to the already huge count).

Idea courtesy IslamAbdelRahman
Closes https://github.com/facebook/rocksdb/pull/2116

Differential Revision: D4857128

Pulled By: sagar0

fbshipit-source-id: 7d239df094b94bd9ea79d145cdf825478ac037a8

db/db_impl.cc

@@ -1335,13 +1335,10 @@ Iterator* DBImpl::NewIterator(const ReadOptions& read_options,
     SuperVersion* sv = cfd->GetReferencedSuperVersion(&mutex_);
     auto iter = new ForwardIterator(this, read_options, cfd, sv);
     return NewDBIterator(
-        env_, *cfd->ioptions(), cfd->user_comparator(), iter,
+        env_, read_options, *cfd->ioptions(), cfd->user_comparator(), iter,
         kMaxSequenceNumber,
         sv->mutable_cf_options.max_sequential_skip_in_iterations,
-        sv->version_number, read_options.iterate_upper_bound,
-        read_options.prefix_same_as_start, read_options.pin_data,
-        read_options.total_order_seek,
-        read_options.max_skippable_internal_keys);
+        sv->version_number);
 #endif
   } else {
     SequenceNumber latest_snapshot = versions_->LastSequence();
@@ -1396,12 +1393,9 @@ Iterator* DBImpl::NewIterator(const ReadOptions& read_options,
     // likely that any iterator pointer is close to the iterator it points to so
     // that they are likely to be in the same cache line and/or page.
     ArenaWrappedDBIter* db_iter = NewArenaWrappedDbIterator(
-        env_, *cfd->ioptions(), cfd->user_comparator(), snapshot,
+        env_, read_options, *cfd->ioptions(), cfd->user_comparator(), snapshot,
         sv->mutable_cf_options.max_sequential_skip_in_iterations,
-        sv->version_number, read_options.iterate_upper_bound,
-        read_options.prefix_same_as_start, read_options.pin_data,
-        read_options.total_order_seek,
-        read_options.max_skippable_internal_keys);
+        sv->version_number);
 
     InternalIterator* internal_iter =
         NewInternalIterator(read_options, cfd, sv, db_iter->GetArena(),
@@ -1450,12 +1444,10 @@ Status DBImpl::NewIterators(
       SuperVersion* sv = cfd->GetReferencedSuperVersion(&mutex_);
       auto iter = new ForwardIterator(this, read_options, cfd, sv);
       iterators->push_back(NewDBIterator(
-          env_, *cfd->ioptions(), cfd->user_comparator(), iter,
+          env_, read_options, *cfd->ioptions(), cfd->user_comparator(), iter,
           kMaxSequenceNumber,
           sv->mutable_cf_options.max_sequential_skip_in_iterations,
-          sv->version_number, nullptr, false, read_options.pin_data,
-          read_options.total_order_seek,
-          read_options.max_skippable_internal_keys));
+          sv->version_number));
     }
 #endif
   } else {
@@ -1473,11 +1465,9 @@ Status DBImpl::NewIterators(
               : latest_snapshot;
 
       ArenaWrappedDBIter* db_iter = NewArenaWrappedDbIterator(
-          env_, *cfd->ioptions(), cfd->user_comparator(), snapshot,
-          sv->mutable_cf_options.max_sequential_skip_in_iterations,
-          sv->version_number, nullptr, false, read_options.pin_data,
-          read_options.total_order_seek,
-          read_options.max_skippable_internal_keys);
+          env_, read_options, *cfd->ioptions(), cfd->user_comparator(),
+          snapshot, sv->mutable_cf_options.max_sequential_skip_in_iterations,
+          sv->version_number);
       InternalIterator* internal_iter =
           NewInternalIterator(read_options, cfd, sv, db_iter->GetArena(),
                               db_iter->GetRangeDelAggregator());

db/db_impl_readonly.cc

@@ -59,15 +59,13 @@ Iterator* DBImplReadOnly::NewIterator(const ReadOptions& read_options,
   SuperVersion* super_version = cfd->GetSuperVersion()->Ref();
   SequenceNumber latest_snapshot = versions_->LastSequence();
   auto db_iter = NewArenaWrappedDbIterator(
-      env_, *cfd->ioptions(), cfd->user_comparator(),
+      env_, read_options, *cfd->ioptions(), cfd->user_comparator(),
       (read_options.snapshot != nullptr
            ? reinterpret_cast<const SnapshotImpl*>(read_options.snapshot)
                  ->number_
            : latest_snapshot),
       super_version->mutable_cf_options.max_sequential_skip_in_iterations,
-      super_version->version_number, read_options.iterate_upper_bound,
-      read_options.prefix_same_as_start, read_options.pin_data,
-      read_options.total_order_seek, read_options.max_skippable_internal_keys);
+      super_version->version_number);
   auto internal_iter =
       NewInternalIterator(read_options, cfd, super_version, db_iter->GetArena(),
                           db_iter->GetRangeDelAggregator());
@@ -90,16 +88,13 @@ Status DBImplReadOnly::NewIterators(
     auto* cfd = reinterpret_cast<ColumnFamilyHandleImpl*>(cfh)->cfd();
     auto* sv = cfd->GetSuperVersion()->Ref();
     auto* db_iter = NewArenaWrappedDbIterator(
-        env_, *cfd->ioptions(), cfd->user_comparator(),
+        env_, read_options, *cfd->ioptions(), cfd->user_comparator(),
         (read_options.snapshot != nullptr
              ? reinterpret_cast<const SnapshotImpl*>(read_options.snapshot)
                    ->number_
              : latest_snapshot),
         sv->mutable_cf_options.max_sequential_skip_in_iterations,
-        sv->version_number, read_options.iterate_upper_bound,
-        read_options.prefix_same_as_start, read_options.pin_data,
-        read_options.total_order_seek,
-        read_options.max_skippable_internal_keys);
+        sv->version_number);
     auto* internal_iter =
         NewInternalIterator(read_options, cfd, sv, db_iter->GetArena(),
                             db_iter->GetRangeDelAggregator());

db/db_iter.cc

@@ -101,35 +101,32 @@ class DBIter: public Iterator {
     uint64_t bytes_read_;
   };
 
-  DBIter(Env* env, const ImmutableCFOptions& ioptions, const Comparator* cmp,
+  DBIter(Env* env, const ReadOptions& read_options,
+         const ImmutableCFOptions& cf_options, const Comparator* cmp,
          InternalIterator* iter, SequenceNumber s, bool arena_mode,
-         uint64_t max_sequential_skip_in_iterations, uint64_t version_number,
-         const Slice* iterate_upper_bound = nullptr,
-         bool prefix_same_as_start = false, bool pin_data = false,
-         bool total_order_seek = false,
-         uint64_t max_skippable_internal_keys = 0)
+         uint64_t max_sequential_skip_in_iterations, uint64_t version_number)
       : arena_mode_(arena_mode),
         env_(env),
-        logger_(ioptions.info_log),
+        logger_(cf_options.info_log),
         user_comparator_(cmp),
-        merge_operator_(ioptions.merge_operator),
+        merge_operator_(cf_options.merge_operator),
         iter_(iter),
         sequence_(s),
         direction_(kForward),
         valid_(false),
         current_entry_is_merged_(false),
-        statistics_(ioptions.statistics),
+        statistics_(cf_options.statistics),
         version_number_(version_number),
-        iterate_upper_bound_(iterate_upper_bound),
-        prefix_same_as_start_(prefix_same_as_start),
-        pin_thru_lifetime_(pin_data),
-        total_order_seek_(total_order_seek),
-        range_del_agg_(ioptions.internal_comparator, s,
+        iterate_upper_bound_(read_options.iterate_upper_bound),
+        prefix_same_as_start_(read_options.prefix_same_as_start),
+        pin_thru_lifetime_(read_options.pin_data),
+        total_order_seek_(read_options.total_order_seek),
+        range_del_agg_(cf_options.internal_comparator, s,
                        true /* collapse_deletions */) {
     RecordTick(statistics_, NO_ITERATORS);
-    prefix_extractor_ = ioptions.prefix_extractor;
+    prefix_extractor_ = cf_options.prefix_extractor;
     max_skip_ = max_sequential_skip_in_iterations;
-    max_skippable_internal_keys_ = max_skippable_internal_keys;
+    max_skippable_internal_keys_ = read_options.max_skippable_internal_keys;
     if (pin_thru_lifetime_) {
       pinned_iters_mgr_.StartPinning();
     }
@@ -1153,18 +1150,16 @@ void DBIter::SeekToLast() {
   }
 }
 
-Iterator* NewDBIterator(
-    Env* env, const ImmutableCFOptions& ioptions,
-    const Comparator* user_key_comparator, InternalIterator* internal_iter,
-    const SequenceNumber& sequence, uint64_t max_sequential_skip_in_iterations,
-    uint64_t version_number, const Slice* iterate_upper_bound,
-    bool prefix_same_as_start, bool pin_data, bool total_order_seek,
-    uint64_t max_skippable_internal_keys) {
-  DBIter* db_iter =
-      new DBIter(env, ioptions, user_key_comparator, internal_iter, sequence,
-                 false, max_sequential_skip_in_iterations, version_number,
-                 iterate_upper_bound, prefix_same_as_start, pin_data,
-                 total_order_seek, max_skippable_internal_keys);
+Iterator* NewDBIterator(Env* env, const ReadOptions& read_options,
+                        const ImmutableCFOptions& cf_options,
+                        const Comparator* user_key_comparator,
+                        InternalIterator* internal_iter,
+                        const SequenceNumber& sequence,
+                        uint64_t max_sequential_skip_in_iterations,
+                        uint64_t version_number) {
+  DBIter* db_iter = new DBIter(
+      env, read_options, cf_options, user_key_comparator, internal_iter,
+      sequence, false, max_sequential_skip_in_iterations, version_number);
   return db_iter;
 }
 
@@ -1204,19 +1199,16 @@ void ArenaWrappedDBIter::RegisterCleanup(CleanupFunction function, void* arg1,
 }
 
 ArenaWrappedDBIter* NewArenaWrappedDbIterator(
-    Env* env, const ImmutableCFOptions& ioptions,
-    const Comparator* user_key_comparator, const SequenceNumber& sequence,
-    uint64_t max_sequential_skip_in_iterations, uint64_t version_number,
-    const Slice* iterate_upper_bound, bool prefix_same_as_start, bool pin_data,
-    bool total_order_seek, uint64_t max_skippable_internal_keys) {
+    Env* env, const ReadOptions& read_options,
+    const ImmutableCFOptions& cf_options, const Comparator* user_key_comparator,
+    const SequenceNumber& sequence, uint64_t max_sequential_skip_in_iterations,
+    uint64_t version_number) {
   ArenaWrappedDBIter* iter = new ArenaWrappedDBIter();
   Arena* arena = iter->GetArena();
   auto mem = arena->AllocateAligned(sizeof(DBIter));
-  DBIter* db_iter =
-      new (mem) DBIter(env, ioptions, user_key_comparator, nullptr, sequence,
-                       true, max_sequential_skip_in_iterations, version_number,
-                       iterate_upper_bound, prefix_same_as_start, pin_data,
-                       total_order_seek, max_skippable_internal_keys);
+  DBIter* db_iter = new (mem)
+      DBIter(env, read_options, cf_options, user_key_comparator, nullptr,
+             sequence, true, max_sequential_skip_in_iterations, version_number);
 
   iter->SetDBIter(db_iter);
 

db/db_iter.h

@@ -27,13 +27,13 @@ class InternalIterator;
 // Return a new iterator that converts internal keys (yielded by
 // "*internal_iter") that were live at the specified "sequence" number
 // into appropriate user keys.
-extern Iterator* NewDBIterator(
-    Env* env, const ImmutableCFOptions& options,
-    const Comparator* user_key_comparator, InternalIterator* internal_iter,
-    const SequenceNumber& sequence, uint64_t max_sequential_skip_in_iterations,
-    uint64_t version_number, const Slice* iterate_upper_bound = nullptr,
-    bool prefix_same_as_start = false, bool pin_data = false,
-    bool total_order_seek = false, uint64_t max_skippable_internal_keys = 0);
+extern Iterator* NewDBIterator(Env* env, const ReadOptions& read_options,
+                               const ImmutableCFOptions& cf_options,
+                               const Comparator* user_key_comparator,
+                               InternalIterator* internal_iter,
+                               const SequenceNumber& sequence,
+                               uint64_t max_sequential_skip_in_iterations,
+                               uint64_t version_number);
 
 // A wrapper iterator which wraps DB Iterator and the arena, with which the DB
 // iterator is supposed be allocated. This class is used as an entry point of
@@ -77,11 +77,9 @@ class ArenaWrappedDBIter : public Iterator {
 
 // Generate the arena wrapped iterator class.
 extern ArenaWrappedDBIter* NewArenaWrappedDbIterator(
-    Env* env, const ImmutableCFOptions& options,
-    const Comparator* user_key_comparator, const SequenceNumber& sequence,
-    uint64_t max_sequential_skip_in_iterations, uint64_t version_number,
-    const Slice* iterate_upper_bound = nullptr,
-    bool prefix_same_as_start = false, bool pin_data = false,
-    bool total_order_seek = false, uint64_t max_skippable_internal_keys = 0);
+    Env* env, const ReadOptions& read_options,
+    const ImmutableCFOptions& cf_options, const Comparator* user_key_comparator,
+    const SequenceNumber& sequence, uint64_t max_sequential_skip_in_iterations,
+    uint64_t version_number);
 
 }  // namespace rocksdb

utilities/date_tiered/date_tiered_db_impl.cc

@@ -378,8 +378,8 @@ Iterator* DateTieredDBImpl::NewIterator(const ReadOptions& opts) {
   DBImpl* db_impl = reinterpret_cast<DBImpl*>(db_);
 
   auto db_iter = NewArenaWrappedDbIterator(
-      db_impl->GetEnv(), ioptions_, cf_options_.comparator, kMaxSequenceNumber,
-      cf_options_.max_sequential_skip_in_iterations, 0);
+      db_impl->GetEnv(), opts, ioptions_, cf_options_.comparator,
+      kMaxSequenceNumber, cf_options_.max_sequential_skip_in_iterations, 0);
 
   auto arena = db_iter->GetArena();
   MergeIteratorBuilder builder(cf_options_.comparator, arena);