From 07b11534bc324f73488da6d8d037dc941bead7d6 Mon Sep 17 00:00:00 2001 From: kssenii Date: Thu, 23 Nov 2023 16:36:18 +0100 Subject: [PATCH] Tiny refactoring --- src/Interpreters/Cache/FileCache.cpp | 3 +- src/Interpreters/Cache/FileCacheSettings.cpp | 4 +++ .../Cache/LRUFileCachePriority.cpp | 31 +++++++++++++++---- src/Interpreters/Cache/LRUFileCachePriority.h | 9 ++++-- .../Cache/SLRUFileCachePriority.cpp | 30 +++++++++--------- .../Cache/SLRUFileCachePriority.h | 4 +-- 6 files changed, 53 insertions(+), 28 deletions(-) diff --git a/src/Interpreters/Cache/FileCache.cpp b/src/Interpreters/Cache/FileCache.cpp index 2e12d6ef9bf..da996443e68 100644 --- a/src/Interpreters/Cache/FileCache.cpp +++ b/src/Interpreters/Cache/FileCache.cpp @@ -771,8 +771,7 @@ bool FileCache::tryReserve(FileSegment & file_segment, const size_t size, FileCa reserve_stat.stat_by_kind.clear(); } - /// A file_segment_metadata acquires a LRUQueue iterator on first successful space reservation attempt, - /// e.g. queue_iteratir is std::nullopt here if no space has been reserved yet. + /// A file_segment_metadata acquires a priority iterator on first successful space reservation attempt, auto queue_iterator = file_segment.getQueueIterator(); chassert(!queue_iterator || file_segment.getReservedSize() > 0); diff --git a/src/Interpreters/Cache/FileCacheSettings.cpp b/src/Interpreters/Cache/FileCacheSettings.cpp index 6055cec5ae5..564a0f2aacd 100644 --- a/src/Interpreters/Cache/FileCacheSettings.cpp +++ b/src/Interpreters/Cache/FileCacheSettings.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include namespace DB @@ -63,7 +64,10 @@ void FileCacheSettings::loadImpl(FuncHas has, FuncGetUInt get_uint, FuncGetStrin throw Exception(ErrorCodes::BAD_ARGUMENTS, "Setting `boundary_alignment` cannot exceed `max_file_segment_size`"); if (has("cache_policy")) + { cache_policy = get_string("cache_policy"); + boost::to_upper(cache_policy); + } if (has("slru_size_ratio")) slru_size_ratio = get_double("slru_size_ratio"); diff --git a/src/Interpreters/Cache/LRUFileCachePriority.cpp b/src/Interpreters/Cache/LRUFileCachePriority.cpp index 625be890cd3..f9b0ddfce15 100644 --- a/src/Interpreters/Cache/LRUFileCachePriority.cpp +++ b/src/Interpreters/Cache/LRUFileCachePriority.cpp @@ -32,10 +32,10 @@ IFileCachePriority::IteratorPtr LRUFileCachePriority::add( size_t size, const CacheGuard::Lock & lock) { - return add(Entry(key_metadata->key, offset, size, key_metadata), lock); + return std::make_shared(add(Entry(key_metadata->key, offset, size, key_metadata), lock)); } -std::unique_ptr LRUFileCachePriority::add(Entry && entry, const CacheGuard::Lock &) +LRUFileCachePriority::LRUIterator LRUFileCachePriority::add(Entry && entry, const CacheGuard::Lock &) { if (entry.size == 0) { @@ -76,7 +76,7 @@ std::unique_ptr LRUFileCachePriority::add(Ent log, "Added entry into LRU queue, key: {}, offset: {}, size: {}", entry.key, entry.offset, entry.size); - return std::make_unique(this, iterator); + return LRUIterator(this, iterator); } LRUFileCachePriority::LRUQueue::iterator LRUFileCachePriority::remove(LRUQueue::iterator it, const CacheGuard::Lock &) @@ -115,6 +115,26 @@ LRUFileCachePriority::LRUIterator::LRUIterator( { } +LRUFileCachePriority::LRUIterator::LRUIterator(const LRUIterator & other) +{ + *this = other; +} + +LRUFileCachePriority::LRUIterator & LRUFileCachePriority::LRUIterator::operator =(const LRUIterator & other) +{ + if (this == &other) + return *this; + + cache_priority = other.cache_priority; + iterator = other.iterator; + return *this; +} + +bool LRUFileCachePriority::LRUIterator::operator ==(const LRUIterator & other) const +{ + return cache_priority == other.cache_priority && iterator == other.iterator; +} + void LRUFileCachePriority::iterate(IterateFunc && func, const CacheGuard::Lock & lock) { for (auto it = queue.begin(); it != queue.end();) @@ -223,8 +243,7 @@ bool LRUFileCachePriority::collectCandidatesForEviction( return can_fit(); } -std::unique_ptr -LRUFileCachePriority::move(LRUIterator & it, LRUFileCachePriority & other, const CacheGuard::Lock &) +LRUFileCachePriority::LRUIterator LRUFileCachePriority::move(LRUIterator & it, LRUFileCachePriority & other, const CacheGuard::Lock &) { const auto & entry = it.getEntry(); if (entry.size == 0) @@ -254,7 +273,7 @@ LRUFileCachePriority::move(LRUIterator & it, LRUFileCachePriority & other, const other.updateSize(-entry.size); other.updateElementsCount(-1); - return std::make_unique(this, it.iterator); + return LRUIterator(this, it.iterator); } FileSegments LRUFileCachePriority::dump(const CacheGuard::Lock & lock) diff --git a/src/Interpreters/Cache/LRUFileCachePriority.h b/src/Interpreters/Cache/LRUFileCachePriority.h index 63b93de76e4..289968602ca 100644 --- a/src/Interpreters/Cache/LRUFileCachePriority.h +++ b/src/Interpreters/Cache/LRUFileCachePriority.h @@ -67,17 +67,22 @@ private: using IterateFunc = std::function; void iterate(IterateFunc && func, const CacheGuard::Lock &); - std::unique_ptr move(LRUIterator & it, LRUFileCachePriority & other, const CacheGuard::Lock &); - std::unique_ptr add(Entry && entry, const CacheGuard::Lock &); + LRUIterator move(LRUIterator & it, LRUFileCachePriority & other, const CacheGuard::Lock &); + LRUIterator add(Entry && entry, const CacheGuard::Lock &); }; class LRUFileCachePriority::LRUIterator : public IFileCachePriority::Iterator { friend class LRUFileCachePriority; friend class SLRUFileCachePriority; + public: LRUIterator(LRUFileCachePriority * cache_priority_, LRUQueue::iterator iterator_); + LRUIterator(const LRUIterator & other); + LRUIterator & operator =(const LRUIterator & other); + bool operator ==(const LRUIterator & other) const; + const Entry & getEntry() const override { return *iterator; } size_t increasePriority(const CacheGuard::Lock &) override; diff --git a/src/Interpreters/Cache/SLRUFileCachePriority.cpp b/src/Interpreters/Cache/SLRUFileCachePriority.cpp index a9e017c62e4..dfc3686683d 100644 --- a/src/Interpreters/Cache/SLRUFileCachePriority.cpp +++ b/src/Interpreters/Cache/SLRUFileCachePriority.cpp @@ -109,7 +109,7 @@ bool SLRUFileCachePriority::collectCandidatesForEviction( for (const auto & candidate : key_candidates.candidates) { auto * candidate_it = assert_cast(candidate->getQueueIterator().get()); - candidate_it->lru_iterator = probationary_queue.move(*candidate_it->lru_iterator, protected_queue, lk); + candidate_it->lru_iterator = probationary_queue.move(candidate_it->lru_iterator, protected_queue, lk); candidate_it->is_protected = false; } } @@ -124,7 +124,7 @@ void SLRUFileCachePriority::increasePriority(SLRUIterator & iterator, const Cach /// we only need to increase its priority within the protected queue. if (iterator.is_protected) { - iterator.lru_iterator->increasePriority(lock); + iterator.lru_iterator.increasePriority(lock); return; } @@ -137,7 +137,7 @@ void SLRUFileCachePriority::increasePriority(SLRUIterator & iterator, const Cach /// Entry size is bigger than the whole protected queue limit. /// This is only possible if protected_queue_size_limit is less than max_file_segment_size, /// which is not possible in any realistic cache configuration. - iterator.lru_iterator->increasePriority(lock); + iterator.lru_iterator.increasePriority(lock); return; } @@ -153,7 +153,7 @@ void SLRUFileCachePriority::increasePriority(SLRUIterator & iterator, const Cach /// We cannot make space for entry to be moved to protected queue /// (not enough releasable file segments). /// Then just increase its priority within probationary queue. - iterator.lru_iterator->increasePriority(lock); + iterator.lru_iterator.increasePriority(lock); return; } @@ -176,7 +176,7 @@ void SLRUFileCachePriority::increasePriority(SLRUIterator & iterator, const Cach /// "downgrade" candidates cannot be moved to probationary queue, /// so entry cannot be moved to protected queue as well. /// Then just increase its priority within probationary queue. - iterator.lru_iterator->increasePriority(lock); + iterator.lru_iterator.increasePriority(lock); return; } /// Make space for "downgrade" candidates. @@ -186,14 +186,14 @@ void SLRUFileCachePriority::increasePriority(SLRUIterator & iterator, const Cach /// All checks passed, now we can move downgrade candidates to /// probationary queue and our entry to protected queue. Entry entry_copy = iterator.getEntry(); - iterator.lru_iterator->remove(lock); + iterator.lru_iterator.remove(lock); for (const auto & [key, key_candidates] : downgrade_candidates) { for (const auto & candidate : key_candidates.candidates) { auto * candidate_it = assert_cast(candidate->getQueueIterator().get()); - candidate_it->lru_iterator = probationary_queue.move(*candidate_it->lru_iterator, protected_queue, lock); + candidate_it->lru_iterator = probationary_queue.move(candidate_it->lru_iterator, protected_queue, lock); candidate_it->is_protected = false; } } @@ -218,10 +218,10 @@ void SLRUFileCachePriority::shuffle(const CacheGuard::Lock & lock) SLRUFileCachePriority::SLRUIterator::SLRUIterator( SLRUFileCachePriority * cache_priority_, - std::unique_ptr lru_iterator_, + LRUFileCachePriority::LRUIterator && lru_iterator_, bool is_protected_) : cache_priority(cache_priority_) - , lru_iterator(std::move(lru_iterator_)) + , lru_iterator(lru_iterator_) , is_protected(is_protected_) { } @@ -229,7 +229,7 @@ SLRUFileCachePriority::SLRUIterator::SLRUIterator( const SLRUFileCachePriority::Entry & SLRUFileCachePriority::SLRUIterator::getEntry() const { assertValid(); - return lru_iterator->getEntry(); + return lru_iterator.getEntry(); } size_t SLRUFileCachePriority::SLRUIterator::increasePriority(const CacheGuard::Lock & lock) @@ -242,26 +242,24 @@ size_t SLRUFileCachePriority::SLRUIterator::increasePriority(const CacheGuard::L void SLRUFileCachePriority::SLRUIterator::updateSize(int64_t size) { assertValid(); - lru_iterator->updateSize(size); + lru_iterator.updateSize(size); } void SLRUFileCachePriority::SLRUIterator::invalidate() { assertValid(); - lru_iterator->invalidate(); + lru_iterator.invalidate(); } void SLRUFileCachePriority::SLRUIterator::remove(const CacheGuard::Lock & lock) { assertValid(); - lru_iterator->remove(lock); - lru_iterator = nullptr; + lru_iterator.remove(lock); } void SLRUFileCachePriority::SLRUIterator::assertValid() const { - if (!lru_iterator) - throw Exception(ErrorCodes::LOGICAL_ERROR, "Attempt to use invalid iterator"); + lru_iterator.assertValid(); } } diff --git a/src/Interpreters/Cache/SLRUFileCachePriority.h b/src/Interpreters/Cache/SLRUFileCachePriority.h index 46e8f37819f..9dad6c15fee 100644 --- a/src/Interpreters/Cache/SLRUFileCachePriority.h +++ b/src/Interpreters/Cache/SLRUFileCachePriority.h @@ -52,7 +52,7 @@ class SLRUFileCachePriority::SLRUIterator : public IFileCachePriority::Iterator public: SLRUIterator( SLRUFileCachePriority * cache_priority_, - std::unique_ptr lru_iterator_, + LRUIterator && lru_iterator_, bool is_protected_); const Entry & getEntry() const override; @@ -71,7 +71,7 @@ private: void assertValid() const; SLRUFileCachePriority * cache_priority; - mutable std::unique_ptr lru_iterator; + mutable LRUIterator lru_iterator; bool is_protected; };