diff --git a/cachelib/allocator/MM2Q-inl.h b/cachelib/allocator/MM2Q-inl.h index e791d6c6c..2f1d53861 100644 --- a/cachelib/allocator/MM2Q-inl.h +++ b/cachelib/allocator/MM2Q-inl.h @@ -252,6 +252,15 @@ MM2Q::Container::withEvictionIterator(F&& fun) { }); } +// returns the head of the hot queue for promotion +template T::*HookPtr> +template +void +MM2Q::Container::withPromotionIterator(F&& fun) { + lruMutex_->lock_combine([this, &fun]() { + fun(Iterator{LockHolder{}, lru_.begin(LruType::Hot)}); + }); +} template T::*HookPtr> void MM2Q::Container::removeLocked(T& node, diff --git a/cachelib/allocator/MM2Q.h b/cachelib/allocator/MM2Q.h index 5138a7842..c7310ee04 100644 --- a/cachelib/allocator/MM2Q.h +++ b/cachelib/allocator/MM2Q.h @@ -442,6 +442,11 @@ class MM2Q { // iterator passed as parameter. template void withEvictionIterator(F&& f); + + // Execute provided function under container lock. Function gets + // iterator passed as parameter. + template + void withPromotionIterator(F&& f); // get the current config as a copy Config getConfig() const; diff --git a/cachelib/allocator/datastruct/DList.h b/cachelib/allocator/datastruct/DList.h index d882eb1bc..3a4093447 100644 --- a/cachelib/allocator/datastruct/DList.h +++ b/cachelib/allocator/datastruct/DList.h @@ -216,6 +216,10 @@ class DList { curr_ = dir_ == Direction::FROM_HEAD ? dlist_->head_ : dlist_->tail_; } + Direction getDirection() noexcept { + return dir_; + } + protected: void goForward() noexcept; void goBackward() noexcept; diff --git a/cachelib/allocator/datastruct/MultiDList-inl.h b/cachelib/allocator/datastruct/MultiDList-inl.h index 861eb5e2d..4cbd58481 100644 --- a/cachelib/allocator/datastruct/MultiDList-inl.h +++ b/cachelib/allocator/datastruct/MultiDList-inl.h @@ -25,12 +25,26 @@ void MultiDList::Iterator::goForward() noexcept { } // Move iterator forward ++currIter_; - // If we land at the rend of this list, move to the previous list. - while (index_ != kInvalidIndex && - currIter_ == mlist_.lists_[index_]->rend()) { - --index_; - if (index_ != kInvalidIndex) { - currIter_ = mlist_.lists_[index_]->rbegin(); + + if (currIter_.getDirection() == DListIterator::Direction::FROM_HEAD) { + // If we land at the rend of this list, move to the previous list. + while (index_ != kInvalidIndex && index_ != mlist_.lists_.size() && + currIter_ == mlist_.lists_[index_]->end()) { + ++index_; + if (index_ != kInvalidIndex && index_ != mlist_.lists_.size()) { + currIter_ = mlist_.lists_[index_]->begin(); + } else { + return; + } + } + } else { + // If we land at the rend of this list, move to the previous list. + while (index_ != kInvalidIndex && + currIter_ == mlist_.lists_[index_]->rend()) { + --index_; + if (index_ != kInvalidIndex) { + currIter_ = mlist_.lists_[index_]->rbegin(); + } } } } @@ -71,6 +85,25 @@ void MultiDList::Iterator::initToValidRBeginFrom( : mlist_.lists_[index_]->rbegin(); } +template T::*HookPtr> +void MultiDList::Iterator::initToValidBeginFrom( + size_t listIdx) noexcept { + // Find the first non-empty list. + index_ = listIdx; + while (index_ != mlist_.lists_.size() && + mlist_.lists_[index_]->size() == 0) { + ++index_; + } + if (index_ == mlist_.lists_.size()) { + //we reached the end - we should get set to + //invalid index + index_ = std::numeric_limits::max(); + } + currIter_ = index_ == std::numeric_limits::max() + ? mlist_.lists_[0]->begin() + : mlist_.lists_[index_]->begin(); +} + template T::*HookPtr> typename MultiDList::Iterator& MultiDList::Iterator::operator++() noexcept { @@ -97,7 +130,16 @@ typename MultiDList::Iterator MultiDList::rbegin( if (listIdx >= lists_.size()) { throw std::invalid_argument("Invalid list index for MultiDList iterator."); } - return MultiDList::Iterator(*this, listIdx); + return MultiDList::Iterator(*this, listIdx, false); +} + +template T::*HookPtr> +typename MultiDList::Iterator MultiDList::begin( + size_t listIdx) const { + if (listIdx >= lists_.size()) { + throw std::invalid_argument("Invalid list index for MultiDList iterator."); + } + return MultiDList::Iterator(*this, listIdx, true); } template T::*HookPtr> diff --git a/cachelib/allocator/datastruct/MultiDList.h b/cachelib/allocator/datastruct/MultiDList.h index fd309614a..8063cd547 100644 --- a/cachelib/allocator/datastruct/MultiDList.h +++ b/cachelib/allocator/datastruct/MultiDList.h @@ -110,14 +110,18 @@ class MultiDList { } explicit Iterator(const MultiDList& mlist, - size_t listIdx) noexcept + size_t listIdx, bool head) noexcept : currIter_(mlist.lists_[mlist.lists_.size() - 1]->rbegin()), mlist_(mlist) { XDCHECK_LT(listIdx, mlist.lists_.size()); - initToValidRBeginFrom(listIdx); + if (head) { + initToValidBeginFrom(listIdx); + } else { + initToValidRBeginFrom(listIdx); + } // We should either point to an element or the end() iterator // which has an invalid index_. - XDCHECK(index_ == kInvalidIndex || currIter_.get() != nullptr); + XDCHECK(index_ == kInvalidIndex || index_ == mlist.lists_.size() || currIter_.get() != nullptr); } virtual ~Iterator() = default; @@ -169,6 +173,9 @@ class MultiDList { // reset iterator to the beginning of a speicific queue void initToValidRBeginFrom(size_t listIdx) noexcept; + + // reset iterator to the head of a specific queue + void initToValidBeginFrom(size_t listIdx) noexcept; // Index of current list size_t index_{0}; @@ -184,6 +191,9 @@ class MultiDList { // provides an iterator starting from the tail of a specific list. Iterator rbegin(size_t idx) const; + + // provides an iterator starting from the head of a specific list. + Iterator begin(size_t idx) const; // Iterator to compare against for the end. Iterator rend() const noexcept; diff --git a/cachelib/allocator/tests/MM2QTest.cpp b/cachelib/allocator/tests/MM2QTest.cpp index a4862c222..17e2eb264 100644 --- a/cachelib/allocator/tests/MM2QTest.cpp +++ b/cachelib/allocator/tests/MM2QTest.cpp @@ -218,6 +218,19 @@ void MMTypeTest::testIterate(std::vector>& nodes, } } +template +void MMTypeTest::testIterateHot(std::vector>& nodes, + Container& c) { + auto it = nodes.rbegin(); + c.withPromotionIterator([&it,&c](auto &&it2q) { + while (it2q && c.isHot(*it2q)) { + ASSERT_EQ(it2q->getId(), (*it)->getId()); + ++it2q; + ++it; + } + }); +} + template void MMTypeTest::testMatch(std::string expected, MMTypeTest::Container& c) { @@ -234,6 +247,23 @@ void MMTypeTest::testMatch(std::string expected, ASSERT_EQ(expected, actual); } +template +void MMTypeTest::testMatchHot(std::string expected, + MMTypeTest::Container& c) { + int index = -1; + std::string actual; + c.withPromotionIterator([&c,&actual,&index](auto &&it2q) { + while (it2q) { + ++index; + actual += folly::stringPrintf( + "%d:%s, ", it2q->getId(), + (c.isHot(*it2q) ? "H" : (c.isCold(*it2q) ? "C" : "W"))); + ++it2q; + } + }); + ASSERT_EQ(expected, actual); +} + TEST_F(MM2QTest, DetailedTest) { MM2Q::Config config; config.lruRefreshTime = 0; @@ -255,8 +285,11 @@ TEST_F(MM2QTest, DetailedTest) { } testIterate(nodes, c); + testIterateHot(nodes, c); testMatch("0:C, 1:C, 2:C, 3:C, 4:H, 5:H, ", c); + testMatchHot("5:H, 4:H, 3:C, 2:C, 1:C, 0:C, ", c); + // Move 3 to top of the hot cache c.recordAccess(*(nodes[4]), AccessMode::kRead); testMatch("0:C, 1:C, 2:C, 3:C, 5:H, 4:H, ", c); diff --git a/cachelib/allocator/tests/MMTypeTest.h b/cachelib/allocator/tests/MMTypeTest.h index 5c421cf4c..6376750b3 100644 --- a/cachelib/allocator/tests/MMTypeTest.h +++ b/cachelib/allocator/tests/MMTypeTest.h @@ -147,7 +147,9 @@ class MMTypeTest : public testing::Test { void testRecordAccessBasic(Config c); void testSerializationBasic(Config c); void testIterate(std::vector>& nodes, Container& c); + void testIterateHot(std::vector>& nodes, Container& c); void testMatch(std::string expected, Container& c); + void testMatchHot(std::string expected, Container& c); size_t getListSize(const Container& c, typename MMType::LruType list); };