Skip to content

Add combined locking support for MMContainer #182

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Add combined locking support for MMContainer #182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cachelib/allocator/CacheAllocator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1660,7 +1660,7 @@ class CacheAllocator : public CacheBase {
// @return An evicted item or nullptr if there is no suitable candidate.
Item* findEviction(PoolId pid, ClassId cid);

using EvictionIterator = typename MMContainer::Iterator;
using EvictionIterator = typename MMContainer::LockedIterator;

// Advance the current iterator and try to evict a regular item
//
Expand Down
41 changes: 17 additions & 24 deletions cachelib/allocator/MM2Q-inl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,29 +241,21 @@ bool MM2Q::Container<T, HookPtr>::add(T& node) noexcept {
}

template <typename T, MM2Q::Hook<T> T::*HookPtr>
typename MM2Q::Container<T, HookPtr>::Iterator
typename MM2Q::Container<T, HookPtr>::LockedIterator
MM2Q::Container<T, HookPtr>::getEvictionIterator() const noexcept {
// we cannot use combined critical sections with folly::DistributedMutex here
// because the lock is held for the lifetime of the eviction iterator. In
// other words, the abstraction of the iterator just does not lend itself well
// to combinable critical sections as the user can hold the lock for an
// arbitrary amount of time outside a lambda-friendly piece of code (eg. they
// can return the iterator from functions, pass it to functions, etc)
//
// it would be theoretically possible to refactor this interface into
// something like the following to allow combining
//
// mm2q.withEvictionIterator([&](auto iterator) {
// // user code
// });
//
// at the time of writing it is unclear if the gains from combining are
// reasonable justification for the codemod required to achieve combinability
// as we don't expect this critical section to be the hotspot in user code.
// This is however subject to change at some time in the future as and when
// this assertion becomes false.
LockHolder l(*lruMutex_);
return Iterator{std::move(l), lru_.rbegin()};
return LockedIterator{std::move(l), lru_.rbegin()};
}

template <typename T, MM2Q::Hook<T> T::*HookPtr>
template <typename F>
void MM2Q::Container<T, HookPtr>::withEvictionIterator(F&& fun) {
if (config_.useCombinedLockForIterators) {
lruMutex_->lock_combine([this, &fun]() { fun(Iterator{lru_.rbegin()}); });
} else {
LockHolder lck{*lruMutex_};
fun(Iterator{lru_.rbegin()});
}
}

template <typename T, MM2Q::Hook<T> T::*HookPtr>
Expand Down Expand Up @@ -462,8 +454,9 @@ void MM2Q::Container<T, HookPtr>::reconfigureLocked(const Time& currTime) {

// Iterator Context Implementation
template <typename T, MM2Q::Hook<T> T::*HookPtr>
MM2Q::Container<T, HookPtr>::Iterator::Iterator(
LockHolder l, const typename LruList::Iterator& iter) noexcept
: LruList::Iterator(iter), l_(std::move(l)) {}
MM2Q::Container<T, HookPtr>::LockedIterator::LockedIterator(
LockHolder l, const Iterator& iter) noexcept
: Iterator(iter), l_(std::move(l)) {}

} // namespace cachelib
} // namespace facebook
77 changes: 66 additions & 11 deletions cachelib/allocator/MM2Q.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,50 @@ class MM2Q {
size_t hotPercent,
size_t coldPercent,
uint32_t mmReconfigureInterval)
: Config(time,
ratio,
updateOnW,
updateOnR,
tryLockU,
rebalanceOnRecordAccs,
hotPercent,
coldPercent,
mmReconfigureInterval,
false) {}

// @param time the LRU refresh time.
// An item will be promoted only once in each
// lru refresh time depite the
// number of accesses it gets.
// @param ratio the LRU refresh ratio. The ratio times the
// oldest element's lifetime in warm queue
// would be the minimum value of LRU refresh
// time.
// @param udpateOnW whether to promote the item on write
// @param updateOnR whether to promote the item on read
// @param tryLockU whether to use a try lock when doing
// update.
// @param rebalanceOnRecordAccs whether to do rebalance on access. If set
// to false, rebalance only happens when
// items are added or removed to the queue.
// @param hotPercent percentage number for the size of the hot
// queue in the overall size.
// @param coldPercent percentage number for the size of the cold
// queue in the overall size.
// @param mmReconfigureInterval Time interval for recalculating lru
// refresh time according to the ratio.
// useCombinedLockForIterators Whether to use combined locking for
// withEvictionIterator
Config(uint32_t time,
double ratio,
bool updateOnW,
bool updateOnR,
bool tryLockU,
bool rebalanceOnRecordAccs,
size_t hotPercent,
size_t coldPercent,
uint32_t mmReconfigureInterval,
bool useCombinedLockForIterators)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make this optional.
The way we typically do is create a new constructor with all the parameters while keeping the constructor of Config(uint32_t time, double ratio, bool updateOnW, bool updateOnR, bool tryLockU, bool rebalanceOnRecordAccs, size_t hotPercent, size_t coldPercent, uint32_t mmReconfigureInterval)
And have it call the constructor with all the parameters with default value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, done.

: defaultLruRefreshTime(time),
lruRefreshRatio(ratio),
updateOnWrite(updateOnW),
Expand All @@ -223,7 +267,8 @@ class MM2Q {
hotSizePercent(hotPercent),
coldSizePercent(coldPercent),
mmReconfigureIntervalSecs(
std::chrono::seconds(mmReconfigureInterval)) {
std::chrono::seconds(mmReconfigureInterval)),
useCombinedLockForIterators(useCombinedLockForIterators) {
checkLruSizes();
}

Expand Down Expand Up @@ -306,6 +351,9 @@ class MM2Q {
// Minimum interval between reconfigurations. If 0, reconfigure is never
// called.
std::chrono::seconds mmReconfigureIntervalSecs{};

// Whether to use combined locking for withEvictionIterator.
bool useCombinedLockForIterators{false};
};

// The container object which can be used to keep track of objects of type
Expand Down Expand Up @@ -347,22 +395,24 @@ class MM2Q {
Container(const Container&) = delete;
Container& operator=(const Container&) = delete;

using Iterator = typename LruList::Iterator;

// context for iterating the MM container. At any given point of time,
// there can be only one iterator active since we need to lock the LRU for
// iteration. we can support multiple iterators at same time, by using a
// shared ptr in the context for the lock holder in the future.
class Iterator : public LruList::Iterator {
class LockedIterator : public Iterator {
public:
// noncopyable but movable.
Iterator(const Iterator&) = delete;
Iterator& operator=(const Iterator&) = delete;
LockedIterator(const LockedIterator&) = delete;
LockedIterator& operator=(const LockedIterator&) = delete;

Iterator(Iterator&&) noexcept = default;
LockedIterator(LockedIterator&&) noexcept = default;

// 1. Invalidate this iterator
// 2. Unlock
void destroy() {
LruList::Iterator::reset();
Iterator::reset();
if (l_.owns_lock()) {
l_.unlock();
}
Expand All @@ -373,15 +423,15 @@ class MM2Q {
if (!l_.owns_lock()) {
l_.lock();
}
LruList::Iterator::resetToBegin();
Iterator::resetToBegin();
}

private:
// private because it's easy to misuse and cause deadlock for MM2Q
Iterator& operator=(Iterator&&) noexcept = default;
LockedIterator& operator=(LockedIterator&&) noexcept = default;

// create an lru iterator with the lock being held.
Iterator(LockHolder l, const typename LruList::Iterator& iter) noexcept;
LockedIterator(LockHolder l, const Iterator& iter) noexcept;

// only the container can create iterators
friend Container<T, HookPtr>;
Expand Down Expand Up @@ -422,7 +472,7 @@ class MM2Q {

// same as the above but uses an iterator context. The iterator is updated
// on removal of the corresponding node to point to the next node. The
// iterator context holds the lock on the lru.
// iterator context is responsible for locking.
//
// iterator will be advanced to the next node after removing the node
//
Expand All @@ -445,7 +495,12 @@ class MM2Q {
// Obtain an iterator that start from the tail and can be used
// to search for evictions. This iterator holds a lock to this
// container and only one such iterator can exist at a time
Iterator getEvictionIterator() const noexcept;
LockedIterator getEvictionIterator() const noexcept;

// Execute provided function under container lock. Function gets
// Iterator passed as parameter.
template <typename F>
void withEvictionIterator(F&& f);

// get the current config as a copy
Config getConfig() const;
Expand Down
22 changes: 17 additions & 5 deletions cachelib/allocator/MMLru-inl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,10 +212,21 @@ bool MMLru::Container<T, HookPtr>::add(T& node) noexcept {
}

template <typename T, MMLru::Hook<T> T::*HookPtr>
typename MMLru::Container<T, HookPtr>::Iterator
typename MMLru::Container<T, HookPtr>::LockedIterator
MMLru::Container<T, HookPtr>::getEvictionIterator() const noexcept {
LockHolder l(*lruMutex_);
return Iterator{std::move(l), lru_.rbegin()};
return LockedIterator{std::move(l), lru_.rbegin()};
}

template <typename T, MMLru::Hook<T> T::*HookPtr>
template <typename F>
void MMLru::Container<T, HookPtr>::withEvictionIterator(F&& fun) {
if (config_.useCombinedLockForIterators) {
lruMutex_->lock_combine([this, &fun]() { fun(Iterator{lru_.rbegin()}); });
} else {
LockHolder lck{*lruMutex_};
fun(Iterator{lru_.rbegin()});
}
}

template <typename T, MMLru::Hook<T> T::*HookPtr>
Expand Down Expand Up @@ -360,8 +371,9 @@ void MMLru::Container<T, HookPtr>::reconfigureLocked(const Time& currTime) {

// Iterator Context Implementation
template <typename T, MMLru::Hook<T> T::*HookPtr>
MMLru::Container<T, HookPtr>::Iterator::Iterator(
LockHolder l, const typename LruList::Iterator& iter) noexcept
: LruList::Iterator(iter), l_(std::move(l)) {}
MMLru::Container<T, HookPtr>::LockedIterator::LockedIterator(
LockHolder l, const Iterator& iter) noexcept
: Iterator(iter), l_(std::move(l)) {}

} // namespace cachelib
} // namespace facebook
68 changes: 56 additions & 12 deletions cachelib/allocator/MMLru.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,14 +145,49 @@ class MMLru {
bool tryLockU,
uint8_t ipSpec,
uint32_t mmReconfigureInterval)
: Config(time,
ratio,
updateOnW,
updateOnR,
tryLockU,
ipSpec,
mmReconfigureInterval,
false) {}

// @param time the LRU refresh time in seconds.
// An item will be promoted only once in each lru refresh
// time depite the number of accesses it gets.
// @param ratio the lru refresh ratio. The ratio times the
// oldest element's lifetime in warm queue
// would be the minimum value of LRU refresh time.
// @param udpateOnW whether to promote the item on write
// @param updateOnR whether to promote the item on read
// @param tryLockU whether to use a try lock when doing update.
// @param ipSpec insertion point spec, which is the inverse power of
// length from the end of the queue. For example, value 1
// means the insertion point is 1/2 from the end of LRU;
// value 2 means 1/4 from the end of LRU.
// @param mmReconfigureInterval Time interval for recalculating lru
// refresh time according to the ratio.
// useCombinedLockForIterators Whether to use combined locking for
// withEvictionIterator
Config(uint32_t time,
double ratio,
bool updateOnW,
bool updateOnR,
bool tryLockU,
uint8_t ipSpec,
uint32_t mmReconfigureInterval,
bool useCombinedLockForIterators)
: defaultLruRefreshTime(time),
lruRefreshRatio(ratio),
updateOnWrite(updateOnW),
updateOnRead(updateOnR),
tryLockUpdate(tryLockU),
lruInsertionPointSpec(ipSpec),
mmReconfigureIntervalSecs(
std::chrono::seconds(mmReconfigureInterval)) {}
std::chrono::seconds(mmReconfigureInterval)),
useCombinedLockForIterators(useCombinedLockForIterators) {}

Config() = default;
Config(const Config& rhs) = default;
Expand Down Expand Up @@ -198,6 +233,9 @@ class MMLru {
// Minimum interval between reconfigurations. If 0, reconfigure is never
// called.
std::chrono::seconds mmReconfigureIntervalSecs{};

// Whether to use combined locking for withEvictionIterator.
bool useCombinedLockForIterators{false};
};

// The container object which can be used to keep track of objects of type
Expand Down Expand Up @@ -234,22 +272,24 @@ class MMLru {
Container(const Container&) = delete;
Container& operator=(const Container&) = delete;

using Iterator = typename LruList::Iterator;

// context for iterating the MM container. At any given point of time,
// there can be only one iterator active since we need to lock the LRU for
// iteration. we can support multiple iterators at same time, by using a
// shared ptr in the context for the lock holder in the future.
class Iterator : public LruList::Iterator {
class LockedIterator : public Iterator {
public:
// noncopyable but movable.
Iterator(const Iterator&) = delete;
Iterator& operator=(const Iterator&) = delete;
LockedIterator(const LockedIterator&) = delete;
LockedIterator& operator=(const LockedIterator&) = delete;

Iterator(Iterator&&) noexcept = default;
LockedIterator(LockedIterator&&) noexcept = default;

// 1. Invalidate this iterator
// 2. Unlock
void destroy() {
LruList::Iterator::reset();
Iterator::reset();
if (l_.owns_lock()) {
l_.unlock();
}
Expand All @@ -260,15 +300,15 @@ class MMLru {
if (!l_.owns_lock()) {
l_.lock();
}
LruList::Iterator::resetToBegin();
Iterator::resetToBegin();
}

private:
// private because it's easy to misuse and cause deadlock for MMLru
Iterator& operator=(Iterator&&) noexcept = default;
LockedIterator& operator=(LockedIterator&&) noexcept = default;

// create an lru iterator with the lock being held.
Iterator(LockHolder l, const typename LruList::Iterator& iter) noexcept;
LockedIterator(LockHolder l, const Iterator& iter) noexcept;

// only the container can create iterators
friend Container<T, HookPtr>;
Expand Down Expand Up @@ -307,10 +347,9 @@ class MMLru {
// state of node is unchanged.
bool remove(T& node) noexcept;

using Iterator = Iterator;
// same as the above but uses an iterator context. The iterator is updated
// on removal of the corresponding node to point to the next node. The
// iterator context holds the lock on the lru.
// iterator context is responsible for locking.
//
// iterator will be advanced to the next node after removing the node
//
Expand All @@ -330,7 +369,12 @@ class MMLru {
// Obtain an iterator that start from the tail and can be used
// to search for evictions. This iterator holds a lock to this
// container and only one such iterator can exist at a time
Iterator getEvictionIterator() const noexcept;
LockedIterator getEvictionIterator() const noexcept;

// Execute provided function under container lock. Function gets
// iterator passed as parameter.
template <typename F>
void withEvictionIterator(F&& f);

// get copy of current config
Config getConfig() const;
Expand Down
Loading