get rid of unnecessary cfh pointer in MultiCfIteratorInfo

jaykorean · Apr 1, 2024 · e01434a · e01434a
1 parent b0b9352
commit e01434a
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 37 deletions.
diff --git a/db/attribute_group_iterator_impl.cc b/db/attribute_group_iterator_impl.cc
@@ -9,7 +9,7 @@ namespace ROCKSDB_NAMESPACE {
 
 const AttributeGroups kNoAttributeGroups;
 
-void AttributeGroupIteratorImpl::AddToAttributeGroups(
+void AttributeGroupIteratorImpl::AddToAttributeGroup(
     ColumnFamilyHandle* /*cfh*/, const WideColumns& /*columns*/) {
   // TODO - Implement AttributeGroup population
 }

diff --git a/db/attribute_group_iterator_impl.h b/db/attribute_group_iterator_impl.h
@@ -19,7 +19,7 @@ class AttributeGroupIteratorImpl : public AttributeGroupIterator {
       : impl_(
             comparator, column_families, child_iterators, [this]() { Reset(); },
             [this](ColumnFamilyHandle* cfh, Iterator* iter) {
-              AddToAttributeGroups(cfh, iter->columns());
+              AddToAttributeGroup(cfh, iter->columns());
             }) {}
   ~AttributeGroupIteratorImpl() override {}
 
@@ -48,8 +48,7 @@ class AttributeGroupIteratorImpl : public AttributeGroupIterator {
  private:
   MultiCfIteratorImpl impl_;
   AttributeGroups attribute_groups_;
-  void AddToAttributeGroups(ColumnFamilyHandle* cfh,
-                            const WideColumns& columns);
+  void AddToAttributeGroup(ColumnFamilyHandle* cfh, const WideColumns& columns);
 };
 
 class EmptyAttributeGroupIterator : public AttributeGroupIterator {

diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc
@@ -3748,48 +3748,46 @@ ArenaWrappedDBIter* DBImpl::NewIteratorImpl(
 std::unique_ptr<Iterator> DBImpl::NewCoalescingIterator(
     const ReadOptions& _read_options,
     const std::vector<ColumnFamilyHandle*>& column_families) {
-  std::vector<Iterator*> child_iterators;
-  Status s = GetChildIteratorsForMultiCfIterator(_read_options, column_families,
-                                                 &child_iterators);
-  if (!s.ok()) {
-    return std::unique_ptr<Iterator>(NewErrorIterator(s));
-  }
-  return std::make_unique<CoalescingIterator>(
-      column_families[0]->GetComparator(), column_families,
-      std::move(child_iterators));
+  return NewMultiCfIterator<Iterator, CoalescingIterator>(
+      _read_options, column_families, [](const Status& s) {
+        return std::unique_ptr<Iterator>(NewErrorIterator(s));
+      });
 }
 
 std::unique_ptr<AttributeGroupIterator> DBImpl::NewAttributeGroupIterator(
     const ReadOptions& _read_options,
     const std::vector<ColumnFamilyHandle*>& column_families) {
-  std::vector<Iterator*> child_iterators;
-  Status s = GetChildIteratorsForMultiCfIterator(_read_options, column_families,
-                                                 &child_iterators);
-  if (!s.ok()) {
-    return NewAttributeGroupErrorIterator(s);
-  }
-  return std::make_unique<AttributeGroupIteratorImpl>(
-      column_families[0]->GetComparator(), column_families,
-      std::move(child_iterators));
+  return NewMultiCfIterator<AttributeGroupIterator, AttributeGroupIteratorImpl>(
+      _read_options, column_families,
+      [](const Status& s) { return NewAttributeGroupErrorIterator(s); });
 }
 
-Status DBImpl::GetChildIteratorsForMultiCfIterator(
-    const ReadOptions& read_options,
+template <typename IterType, typename ImplType, typename ErrorIteratorFuncType>
+std::unique_ptr<IterType> DBImpl::NewMultiCfIterator(
+    const ReadOptions& _read_options,
     const std::vector<ColumnFamilyHandle*>& column_families,
-    std::vector<Iterator*>* child_iterators) {
+    ErrorIteratorFuncType error_iterator_func) {
   if (column_families.size() == 0) {
-    return Status::InvalidArgument("No Column Family was provided");
+    return error_iterator_func(
+        Status::InvalidArgument("No Column Family was provided"));
   }
   const Comparator* first_comparator = column_families[0]->GetComparator();
   for (size_t i = 1; i < column_families.size(); ++i) {
     const Comparator* cf_comparator = column_families[i]->GetComparator();
     if (first_comparator != cf_comparator &&
         first_comparator->GetId().compare(cf_comparator->GetId()) != 0) {
-      return Status::InvalidArgument(
-          "Different comparators are being used across CFs");
+      return error_iterator_func(Status::InvalidArgument(
+          "Different comparators are being used across CFs"));
     }
   }
-  return NewIterators(read_options, column_families, child_iterators);
+  std::vector<Iterator*> child_iterators;
+  Status s = NewIterators(_read_options, column_families, &child_iterators);
+  if (!s.ok()) {
+    return error_iterator_func(s);
+  }
+  return std::make_unique<ImplType>(column_families[0]->GetComparator(),
+                                    column_families,
+                                    std::move(child_iterators));
 }
 
 Status DBImpl::NewIterators(

diff --git a/db/db_impl/db_impl.h b/db/db_impl/db_impl.h
@@ -2416,10 +2416,12 @@ class DBImpl : public DB {
 
   bool ShouldReferenceSuperVersion(const MergeContext& merge_context);
 
-  Status GetChildIteratorsForMultiCfIterator(
-      const ReadOptions& read_options,
+  template <typename IterType, typename ImplType,
+            typename ErrorIteratorFuncType>
+  std::unique_ptr<IterType> NewMultiCfIterator(
+      const ReadOptions& _read_options,
       const std::vector<ColumnFamilyHandle*>& column_families,
-      std::vector<Iterator*>* child_iterators);
+      ErrorIteratorFuncType error_iterator_func);
 
   // Lock over the persistent DB state.  Non-nullptr iff successfully acquired.
   FileLock* db_lock_;

diff --git a/db/multi_cf_iterator_impl.h b/db/multi_cf_iterator_impl.h
@@ -111,7 +111,6 @@ class MultiCfIteratorImpl {
 
   struct MultiCfIteratorInfo {
     Iterator* iterator;
-    ColumnFamilyHandle* cfh;
     int order;
   };
 
@@ -189,11 +188,12 @@ class MultiCfIteratorImpl {
     reset_func_();
     heap.clear();
     int i = 0;
-    for (auto& [cfh, iter] : cfh_iter_pairs_) {
+    for (auto& cfh_iter_pair : cfh_iter_pairs_) {
+      auto& iter = cfh_iter_pair.second;
       child_seek_func(iter.get());
       if (iter->Valid()) {
         assert(iter->status().ok());
-        heap.push(MultiCfIteratorInfo{iter.get(), cfh, i});
+        heap.push(MultiCfIteratorInfo{iter.get(), i});
       } else {
         considerStatus(iter->status());
         if (!status_.ok()) {
@@ -244,14 +244,16 @@ class MultiCfIteratorImpl {
     // 3. Add the top iterator back without advancing it
     assert(!heap.empty());
     auto top = heap.top();
-    populate_func_(top.cfh, top.iterator);
+    auto& [top_cfh, top_iter] = cfh_iter_pairs_[top.order];
+    populate_func_(top_cfh, top_iter.get());
     heap.pop();
     if (!heap.empty()) {
       auto* current = heap.top().iterator;
       while (current->Valid() &&
              comparator_->Compare(top.iterator->key(), current->key()) == 0) {
         assert(current->status().ok());
-        populate_func_(heap.top().cfh, heap.top().iterator);
+        auto& [curr_cfh, curr_iter] = cfh_iter_pairs_[heap.top().order];
+        populate_func_(curr_cfh, curr_iter.get());
         advance_func(current);
         if (current->Valid()) {
           heap.replace_top(heap.top());