Skip to content

Commit d5b5bea

Browse files
igchorfacebook-github-bot
igchor
authored and
facebook-github-bot
committed
Add combined locking support for MMContainer (#182)
Summary: through withEvictionIterator function. Also, expose the config option to enable and disable combined locking. withEvictionIterator is implemented as an extra function, getEvictionIterator() is still there and it's behavior hasn't changed. This is a subset of changes from: #172 Pull Request resolved: #182 Reviewed By: therealgymmy Differential Revision: D42038532 Pulled By: haowu14 fbshipit-source-id: 4c4b0671778c3c59f015bd9d68d6068d24d01f8a
1 parent 96d1cb9 commit d5b5bea

File tree

16 files changed

+327
-87
lines changed

16 files changed

+327
-87
lines changed

‎cachelib/allocator/CacheAllocator.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1660,7 +1660,7 @@ class CacheAllocator : public CacheBase {
16601660
// @return An evicted item or nullptr if there is no suitable candidate.
16611661
Item* findEviction(PoolId pid, ClassId cid);
16621662

1663-
using EvictionIterator = typename MMContainer::Iterator;
1663+
using EvictionIterator = typename MMContainer::LockedIterator;
16641664

16651665
// Advance the current iterator and try to evict a regular item
16661666
//

‎cachelib/allocator/MM2Q-inl.h‎

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -241,29 +241,21 @@ bool MM2Q::Container<T, HookPtr>::add(T& node) noexcept {
241241
}
242242

243243
template <typename T, MM2Q::Hook<T> T::*HookPtr>
244-
typename MM2Q::Container<T, HookPtr>::Iterator
244+
typename MM2Q::Container<T, HookPtr>::LockedIterator
245245
MM2Q::Container<T, HookPtr>::getEvictionIterator() const noexcept {
246-
// we cannot use combined critical sections with folly::DistributedMutex here
247-
// because the lock is held for the lifetime of the eviction iterator. In
248-
// other words, the abstraction of the iterator just does not lend itself well
249-
// to combinable critical sections as the user can hold the lock for an
250-
// arbitrary amount of time outside a lambda-friendly piece of code (eg. they
251-
// can return the iterator from functions, pass it to functions, etc)
252-
//
253-
// it would be theoretically possible to refactor this interface into
254-
// something like the following to allow combining
255-
//
256-
// mm2q.withEvictionIterator([&](auto iterator) {
257-
// // user code
258-
// });
259-
//
260-
// at the time of writing it is unclear if the gains from combining are
261-
// reasonable justification for the codemod required to achieve combinability
262-
// as we don't expect this critical section to be the hotspot in user code.
263-
// This is however subject to change at some time in the future as and when
264-
// this assertion becomes false.
265246
LockHolder l(*lruMutex_);
266-
return Iterator{std::move(l), lru_.rbegin()};
247+
return LockedIterator{std::move(l), lru_.rbegin()};
248+
}
249+
250+
template <typename T, MM2Q::Hook<T> T::*HookPtr>
251+
template <typename F>
252+
void MM2Q::Container<T, HookPtr>::withEvictionIterator(F&& fun) {
253+
if (config_.useCombinedLockForIterators) {
254+
lruMutex_->lock_combine([this, &fun]() { fun(Iterator{lru_.rbegin()}); });
255+
} else {
256+
LockHolder lck{*lruMutex_};
257+
fun(Iterator{lru_.rbegin()});
258+
}
267259
}
268260

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

463455
// Iterator Context Implementation
464456
template <typename T, MM2Q::Hook<T> T::*HookPtr>
465-
MM2Q::Container<T, HookPtr>::Iterator::Iterator(
466-
LockHolder l, const typename LruList::Iterator& iter) noexcept
467-
: LruList::Iterator(iter), l_(std::move(l)) {}
457+
MM2Q::Container<T, HookPtr>::LockedIterator::LockedIterator(
458+
LockHolder l, const Iterator& iter) noexcept
459+
: Iterator(iter), l_(std::move(l)) {}
460+
468461
} // namespace cachelib
469462
} // namespace facebook

‎cachelib/allocator/MM2Q.h‎

Lines changed: 66 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,50 @@ class MM2Q {
214214
size_t hotPercent,
215215
size_t coldPercent,
216216
uint32_t mmReconfigureInterval)
217+
: Config(time,
218+
ratio,
219+
updateOnW,
220+
updateOnR,
221+
tryLockU,
222+
rebalanceOnRecordAccs,
223+
hotPercent,
224+
coldPercent,
225+
mmReconfigureInterval,
226+
false) {}
227+
228+
// @param time the LRU refresh time.
229+
// An item will be promoted only once in each
230+
// lru refresh time depite the
231+
// number of accesses it gets.
232+
// @param ratio the LRU refresh ratio. The ratio times the
233+
// oldest element's lifetime in warm queue
234+
// would be the minimum value of LRU refresh
235+
// time.
236+
// @param udpateOnW whether to promote the item on write
237+
// @param updateOnR whether to promote the item on read
238+
// @param tryLockU whether to use a try lock when doing
239+
// update.
240+
// @param rebalanceOnRecordAccs whether to do rebalance on access. If set
241+
// to false, rebalance only happens when
242+
// items are added or removed to the queue.
243+
// @param hotPercent percentage number for the size of the hot
244+
// queue in the overall size.
245+
// @param coldPercent percentage number for the size of the cold
246+
// queue in the overall size.
247+
// @param mmReconfigureInterval Time interval for recalculating lru
248+
// refresh time according to the ratio.
249+
// useCombinedLockForIterators Whether to use combined locking for
250+
// withEvictionIterator
251+
Config(uint32_t time,
252+
double ratio,
253+
bool updateOnW,
254+
bool updateOnR,
255+
bool tryLockU,
256+
bool rebalanceOnRecordAccs,
257+
size_t hotPercent,
258+
size_t coldPercent,
259+
uint32_t mmReconfigureInterval,
260+
bool useCombinedLockForIterators)
217261
: defaultLruRefreshTime(time),
218262
lruRefreshRatio(ratio),
219263
updateOnWrite(updateOnW),
@@ -223,7 +267,8 @@ class MM2Q {
223267
hotSizePercent(hotPercent),
224268
coldSizePercent(coldPercent),
225269
mmReconfigureIntervalSecs(
226-
std::chrono::seconds(mmReconfigureInterval)) {
270+
std::chrono::seconds(mmReconfigureInterval)),
271+
useCombinedLockForIterators(useCombinedLockForIterators) {
227272
checkLruSizes();
228273
}
229274

@@ -306,6 +351,9 @@ class MM2Q {
306351
// Minimum interval between reconfigurations. If 0, reconfigure is never
307352
// called.
308353
std::chrono::seconds mmReconfigureIntervalSecs{};
354+
355+
// Whether to use combined locking for withEvictionIterator.
356+
bool useCombinedLockForIterators{false};
309357
};
310358

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

398+
using Iterator = typename LruList::Iterator;
399+
350400
// context for iterating the MM container. At any given point of time,
351401
// there can be only one iterator active since we need to lock the LRU for
352402
// iteration. we can support multiple iterators at same time, by using a
353403
// shared ptr in the context for the lock holder in the future.
354-
class Iterator : public LruList::Iterator {
404+
class LockedIterator : public Iterator {
355405
public:
356406
// noncopyable but movable.
357-
Iterator(const Iterator&) = delete;
358-
Iterator& operator=(const Iterator&) = delete;
407+
LockedIterator(const LockedIterator&) = delete;
408+
LockedIterator& operator=(const LockedIterator&) = delete;
359409

360-
Iterator(Iterator&&) noexcept = default;
410+
LockedIterator(LockedIterator&&) noexcept = default;
361411

362412
// 1. Invalidate this iterator
363413
// 2. Unlock
364414
void destroy() {
365-
LruList::Iterator::reset();
415+
Iterator::reset();
366416
if (l_.owns_lock()) {
367417
l_.unlock();
368418
}
@@ -373,15 +423,15 @@ class MM2Q {
373423
if (!l_.owns_lock()) {
374424
l_.lock();
375425
}
376-
LruList::Iterator::resetToBegin();
426+
Iterator::resetToBegin();
377427
}
378428

379429
private:
380430
// private because it's easy to misuse and cause deadlock for MM2Q
381-
Iterator& operator=(Iterator&&) noexcept = default;
431+
LockedIterator& operator=(LockedIterator&&) noexcept = default;
382432

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

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

423473
// same as the above but uses an iterator context. The iterator is updated
424474
// on removal of the corresponding node to point to the next node. The
425-
// iterator context holds the lock on the lru.
475+
// iterator context is responsible for locking.
426476
//
427477
// iterator will be advanced to the next node after removing the node
428478
//
@@ -445,7 +495,12 @@ class MM2Q {
445495
// Obtain an iterator that start from the tail and can be used
446496
// to search for evictions. This iterator holds a lock to this
447497
// container and only one such iterator can exist at a time
448-
Iterator getEvictionIterator() const noexcept;
498+
LockedIterator getEvictionIterator() const noexcept;
499+
500+
// Execute provided function under container lock. Function gets
501+
// Iterator passed as parameter.
502+
template <typename F>
503+
void withEvictionIterator(F&& f);
449504

450505
// get the current config as a copy
451506
Config getConfig() const;

‎cachelib/allocator/MMLru-inl.h‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -212,10 +212,21 @@ bool MMLru::Container<T, HookPtr>::add(T& node) noexcept {
212212
}
213213

214214
template <typename T, MMLru::Hook<T> T::*HookPtr>
215-
typename MMLru::Container<T, HookPtr>::Iterator
215+
typename MMLru::Container<T, HookPtr>::LockedIterator
216216
MMLru::Container<T, HookPtr>::getEvictionIterator() const noexcept {
217217
LockHolder l(*lruMutex_);
218-
return Iterator{std::move(l), lru_.rbegin()};
218+
return LockedIterator{std::move(l), lru_.rbegin()};
219+
}
220+
221+
template <typename T, MMLru::Hook<T> T::*HookPtr>
222+
template <typename F>
223+
void MMLru::Container<T, HookPtr>::withEvictionIterator(F&& fun) {
224+
if (config_.useCombinedLockForIterators) {
225+
lruMutex_->lock_combine([this, &fun]() { fun(Iterator{lru_.rbegin()}); });
226+
} else {
227+
LockHolder lck{*lruMutex_};
228+
fun(Iterator{lru_.rbegin()});
229+
}
219230
}
220231

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

361372
// Iterator Context Implementation
362373
template <typename T, MMLru::Hook<T> T::*HookPtr>
363-
MMLru::Container<T, HookPtr>::Iterator::Iterator(
364-
LockHolder l, const typename LruList::Iterator& iter) noexcept
365-
: LruList::Iterator(iter), l_(std::move(l)) {}
374+
MMLru::Container<T, HookPtr>::LockedIterator::LockedIterator(
375+
LockHolder l, const Iterator& iter) noexcept
376+
: Iterator(iter), l_(std::move(l)) {}
377+
366378
} // namespace cachelib
367379
} // namespace facebook

‎cachelib/allocator/MMLru.h‎

Lines changed: 56 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -145,14 +145,49 @@ class MMLru {
145145
bool tryLockU,
146146
uint8_t ipSpec,
147147
uint32_t mmReconfigureInterval)
148+
: Config(time,
149+
ratio,
150+
updateOnW,
151+
updateOnR,
152+
tryLockU,
153+
ipSpec,
154+
mmReconfigureInterval,
155+
false) {}
156+
157+
// @param time the LRU refresh time in seconds.
158+
// An item will be promoted only once in each lru refresh
159+
// time depite the number of accesses it gets.
160+
// @param ratio the lru refresh ratio. The ratio times the
161+
// oldest element's lifetime in warm queue
162+
// would be the minimum value of LRU refresh time.
163+
// @param udpateOnW whether to promote the item on write
164+
// @param updateOnR whether to promote the item on read
165+
// @param tryLockU whether to use a try lock when doing update.
166+
// @param ipSpec insertion point spec, which is the inverse power of
167+
// length from the end of the queue. For example, value 1
168+
// means the insertion point is 1/2 from the end of LRU;
169+
// value 2 means 1/4 from the end of LRU.
170+
// @param mmReconfigureInterval Time interval for recalculating lru
171+
// refresh time according to the ratio.
172+
// useCombinedLockForIterators Whether to use combined locking for
173+
// withEvictionIterator
174+
Config(uint32_t time,
175+
double ratio,
176+
bool updateOnW,
177+
bool updateOnR,
178+
bool tryLockU,
179+
uint8_t ipSpec,
180+
uint32_t mmReconfigureInterval,
181+
bool useCombinedLockForIterators)
148182
: defaultLruRefreshTime(time),
149183
lruRefreshRatio(ratio),
150184
updateOnWrite(updateOnW),
151185
updateOnRead(updateOnR),
152186
tryLockUpdate(tryLockU),
153187
lruInsertionPointSpec(ipSpec),
154188
mmReconfigureIntervalSecs(
155-
std::chrono::seconds(mmReconfigureInterval)) {}
189+
std::chrono::seconds(mmReconfigureInterval)),
190+
useCombinedLockForIterators(useCombinedLockForIterators) {}
156191

157192
Config() = default;
158193
Config(const Config& rhs) = default;
@@ -198,6 +233,9 @@ class MMLru {
198233
// Minimum interval between reconfigurations. If 0, reconfigure is never
199234
// called.
200235
std::chrono::seconds mmReconfigureIntervalSecs{};
236+
237+
// Whether to use combined locking for withEvictionIterator.
238+
bool useCombinedLockForIterators{false};
201239
};
202240

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

275+
using Iterator = typename LruList::Iterator;
276+
237277
// context for iterating the MM container. At any given point of time,
238278
// there can be only one iterator active since we need to lock the LRU for
239279
// iteration. we can support multiple iterators at same time, by using a
240280
// shared ptr in the context for the lock holder in the future.
241-
class Iterator : public LruList::Iterator {
281+
class LockedIterator : public Iterator {
242282
public:
243283
// noncopyable but movable.
244-
Iterator(const Iterator&) = delete;
245-
Iterator& operator=(const Iterator&) = delete;
284+
LockedIterator(const LockedIterator&) = delete;
285+
LockedIterator& operator=(const LockedIterator&) = delete;
246286

247-
Iterator(Iterator&&) noexcept = default;
287+
LockedIterator(LockedIterator&&) noexcept = default;
248288

249289
// 1. Invalidate this iterator
250290
// 2. Unlock
251291
void destroy() {
252-
LruList::Iterator::reset();
292+
Iterator::reset();
253293
if (l_.owns_lock()) {
254294
l_.unlock();
255295
}
@@ -260,15 +300,15 @@ class MMLru {
260300
if (!l_.owns_lock()) {
261301
l_.lock();
262302
}
263-
LruList::Iterator::resetToBegin();
303+
Iterator::resetToBegin();
264304
}
265305

266306
private:
267307
// private because it's easy to misuse and cause deadlock for MMLru
268-
Iterator& operator=(Iterator&&) noexcept = default;
308+
LockedIterator& operator=(LockedIterator&&) noexcept = default;
269309

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

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

310-
using Iterator = Iterator;
311350
// same as the above but uses an iterator context. The iterator is updated
312351
// on removal of the corresponding node to point to the next node. The
313-
// iterator context holds the lock on the lru.
352+
// iterator context is responsible for locking.
314353
//
315354
// iterator will be advanced to the next node after removing the node
316355
//
@@ -330,7 +369,12 @@ class MMLru {
330369
// Obtain an iterator that start from the tail and can be used
331370
// to search for evictions. This iterator holds a lock to this
332371
// container and only one such iterator can exist at a time
333-
Iterator getEvictionIterator() const noexcept;
372+
LockedIterator getEvictionIterator() const noexcept;
373+
374+
// Execute provided function under container lock. Function gets
375+
// iterator passed as parameter.
376+
template <typename F>
377+
void withEvictionIterator(F&& f);
334378

335379
// get copy of current config
336380
Config getConfig() const;

0 commit comments

Comments
 (0)