From ddd41146c4795c3aa482438f31c51ec45f9729c1 Mon Sep 17 00:00:00 2001 From: sdong Date: Thu, 8 May 2014 13:32:45 -0700 Subject: [PATCH] MergingIterator uses autovector instead of vector Summary: Use autovector in MergingIterator so that if there are 4 or less child iterators in it, iterator wrappers are inline, which is more likely to be cache friendly. Based on one test run with a shadow traffic of one product, it reduces CPU of MergingIterator::Seek() by half. Test Plan: make all check Reviewers: haobo, yhchiang, igor, dhruba Reviewed By: igor CC: leveldb Differential Revision: https://reviews.facebook.net/D18531 --- table/iterator_wrapper.h | 8 +++++++- table/merger.cc | 7 +++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/table/iterator_wrapper.h b/table/iterator_wrapper.h index cb8520be8..6793bfa0a 100644 --- a/table/iterator_wrapper.h +++ b/table/iterator_wrapper.h @@ -20,6 +20,13 @@ class IteratorWrapper { explicit IteratorWrapper(Iterator* iter): iter_(nullptr) { Set(iter); } + IteratorWrapper(const IteratorWrapper&) { + // Iterator wrapper exclusively owns iter_ so it cannot be copied. + // Didn't delete the function because vector requires + // this function to compile. + assert(false); + } + void operator=(const IteratorWrapper&) = delete; ~IteratorWrapper() { delete iter_; } Iterator* iter() const { return iter_; } @@ -35,7 +42,6 @@ class IteratorWrapper { } } - // Iterator interface methods bool Valid() const { return valid_; } Slice key() const { assert(Valid()); return key_; } diff --git a/table/merger.cc b/table/merger.cc index b829f7133..6c51f7fa1 100644 --- a/table/merger.cc +++ b/table/merger.cc @@ -19,6 +19,7 @@ #include "table/iterator_wrapper.h" #include "util/stop_watch.h" #include "util/perf_context_imp.h" +#include "util/autovector.h" namespace rocksdb { namespace { @@ -43,16 +44,18 @@ MinIterHeap NewMinIterHeap(const Comparator* comparator) { return MinIterHeap(MinIteratorComparator(comparator)); } +const size_t kNumIterReserve = 4; + class MergingIterator : public Iterator { public: MergingIterator(const Comparator* comparator, Iterator** children, int n) : comparator_(comparator), - children_(n), current_(nullptr), use_heap_(true), direction_(kForward), maxHeap_(NewMaxIterHeap(comparator_)), minHeap_(NewMinIterHeap(comparator_)) { + children_.resize(n); for (int i = 0; i < n; i++) { children_[i].Set(children[i]); } @@ -240,7 +243,7 @@ class MergingIterator : public Iterator { void ClearHeaps(); const Comparator* comparator_; - std::vector children_; + autovector children_; IteratorWrapper* current_; // If the value is true, both of iterators in the heap and current_ // contain valid rows. If it is false, only current_ can possibly contain