Skip to content

Commit 8b6acff

Browse files
igchorigchor
committed
Add combined locking support for MMContainer
through withEvictionIterator function. Also, expose config option to enable and disable combined locking. withEvictionIterator is implemented as en extra function, getEvictionIterator() is still there and it's behavior hasn't changed.
1 parent 227cac0 commit 8b6acff

File tree

16 files changed

+293
-97
lines changed

16 files changed

+293
-97
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: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,8 @@ class MM2Q {
182182
rebalanceOnRecordAccs,
183183
hotPercent,
184184
coldPercent,
185-
0) {}
185+
0,
186+
false) {}
186187

187188
// @param time the LRU refresh time.
188189
// An item will be promoted only once in each
@@ -205,6 +206,8 @@ class MM2Q {
205206
// queue in the overall size.
206207
// @param mmReconfigureInterval Time interval for recalculating lru
207208
// refresh time according to the ratio.
209+
// useCombinedLockForIterators Whether to use combined locking for
210+
// withEvictionIterator
208211
Config(uint32_t time,
209212
double ratio,
210213
bool updateOnW,
@@ -213,7 +216,8 @@ class MM2Q {
213216
bool rebalanceOnRecordAccs,
214217
size_t hotPercent,
215218
size_t coldPercent,
216-
uint32_t mmReconfigureInterval)
219+
uint32_t mmReconfigureInterval,
220+
bool useCombinedLockForIterators)
217221
: defaultLruRefreshTime(time),
218222
lruRefreshRatio(ratio),
219223
updateOnWrite(updateOnW),
@@ -223,7 +227,8 @@ class MM2Q {
223227
hotSizePercent(hotPercent),
224228
coldSizePercent(coldPercent),
225229
mmReconfigureIntervalSecs(
226-
std::chrono::seconds(mmReconfigureInterval)) {
230+
std::chrono::seconds(mmReconfigureInterval)),
231+
useCombinedLockForIterators(useCombinedLockForIterators) {
227232
checkLruSizes();
228233
}
229234

@@ -306,6 +311,9 @@ class MM2Q {
306311
// Minimum interval between reconfigurations. If 0, reconfigure is never
307312
// called.
308313
std::chrono::seconds mmReconfigureIntervalSecs{};
314+
315+
// Whether to use combined locking for withEvictionIterator.
316+
bool useCombinedLockForIterators{false};
309317
};
310318

311319
// The container object which can be used to keep track of objects of type
@@ -347,22 +355,24 @@ class MM2Q {
347355
Container(const Container&) = delete;
348356
Container& operator=(const Container&) = delete;
349357

358+
using Iterator = typename LruList::Iterator;
359+
350360
// context for iterating the MM container. At any given point of time,
351361
// there can be only one iterator active since we need to lock the LRU for
352362
// iteration. we can support multiple iterators at same time, by using a
353363
// shared ptr in the context for the lock holder in the future.
354-
class Iterator : public LruList::Iterator {
364+
class LockedIterator : public Iterator {
355365
public:
356366
// noncopyable but movable.
357-
Iterator(const Iterator&) = delete;
358-
Iterator& operator=(const Iterator&) = delete;
367+
LockedIterator(const LockedIterator&) = delete;
368+
LockedIterator& operator=(const LockedIterator&) = delete;
359369

360-
Iterator(Iterator&&) noexcept = default;
370+
LockedIterator(LockedIterator&&) noexcept = default;
361371

362372
// 1. Invalidate this iterator
363373
// 2. Unlock
364374
void destroy() {
365-
LruList::Iterator::reset();
375+
Iterator::reset();
366376
if (l_.owns_lock()) {
367377
l_.unlock();
368378
}
@@ -373,15 +383,15 @@ class MM2Q {
373383
if (!l_.owns_lock()) {
374384
l_.lock();
375385
}
376-
LruList::Iterator::resetToBegin();
386+
Iterator::resetToBegin();
377387
}
378388

379389
private:
380390
// private because it's easy to misuse and cause deadlock for MM2Q
381-
Iterator& operator=(Iterator&&) noexcept = default;
391+
LockedIterator& operator=(LockedIterator&&) noexcept = default;
382392

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

386396
// only the container can create iterators
387397
friend Container<T, HookPtr>;
@@ -422,7 +432,7 @@ class MM2Q {
422432

423433
// same as the above but uses an iterator context. The iterator is updated
424434
// on removal of the corresponding node to point to the next node. The
425-
// iterator context holds the lock on the lru.
435+
// iterator context is responsible for locking.
426436
//
427437
// iterator will be advanced to the next node after removing the node
428438
//
@@ -445,7 +455,12 @@ class MM2Q {
445455
// Obtain an iterator that start from the tail and can be used
446456
// to search for evictions. This iterator holds a lock to this
447457
// container and only one such iterator can exist at a time
448-
Iterator getEvictionIterator() const noexcept;
458+
LockedIterator getEvictionIterator() const noexcept;
459+
460+
// Execute provided function under container lock. Function gets
461+
// Iterator passed as parameter.
462+
template <typename F>
463+
void withEvictionIterator(F&& f);
449464

450465
// get the current config as a copy
451466
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: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,8 @@ class MMLru {
121121
bool updateOnR,
122122
bool tryLockU,
123123
uint8_t ipSpec)
124-
: Config(time, ratio, updateOnW, updateOnR, tryLockU, ipSpec, 0) {}
124+
: Config(
125+
time, ratio, updateOnW, updateOnR, tryLockU, ipSpec, 0, false) {}
125126

126127
// @param time the LRU refresh time in seconds.
127128
// An item will be promoted only once in each lru refresh
@@ -138,21 +139,25 @@ class MMLru {
138139
// value 2 means 1/4 from the end of LRU.
139140
// @param mmReconfigureInterval Time interval for recalculating lru
140141
// refresh time according to the ratio.
142+
// useCombinedLockForIterators Whether to use combined locking for
143+
// withEvictionIterator
141144
Config(uint32_t time,
142145
double ratio,
143146
bool updateOnW,
144147
bool updateOnR,
145148
bool tryLockU,
146149
uint8_t ipSpec,
147-
uint32_t mmReconfigureInterval)
150+
uint32_t mmReconfigureInterval,
151+
bool useCombinedLockForIterators)
148152
: defaultLruRefreshTime(time),
149153
lruRefreshRatio(ratio),
150154
updateOnWrite(updateOnW),
151155
updateOnRead(updateOnR),
152156
tryLockUpdate(tryLockU),
153157
lruInsertionPointSpec(ipSpec),
154158
mmReconfigureIntervalSecs(
155-
std::chrono::seconds(mmReconfigureInterval)) {}
159+
std::chrono::seconds(mmReconfigureInterval)),
160+
useCombinedLockForIterators(useCombinedLockForIterators) {}
156161

157162
Config() = default;
158163
Config(const Config& rhs) = default;
@@ -195,6 +200,9 @@ class MMLru {
195200
// lruInsertionPointSpec = 2, we insert at a point 1/4th from tail
196201
uint8_t lruInsertionPointSpec{0};
197202

203+
// Whether to use combined locking for withEvictionIterator.
204+
bool useCombinedLockForIterators{false};
205+
198206
// Minimum interval between reconfigurations. If 0, reconfigure is never
199207
// called.
200208
std::chrono::seconds mmReconfigureIntervalSecs{};
@@ -234,22 +242,24 @@ class MMLru {
234242
Container(const Container&) = delete;
235243
Container& operator=(const Container&) = delete;
236244

245+
using Iterator = typename LruList::Iterator;
246+
237247
// context for iterating the MM container. At any given point of time,
238248
// there can be only one iterator active since we need to lock the LRU for
239249
// iteration. we can support multiple iterators at same time, by using a
240250
// shared ptr in the context for the lock holder in the future.
241-
class Iterator : public LruList::Iterator {
251+
class LockedIterator : public Iterator {
242252
public:
243253
// noncopyable but movable.
244-
Iterator(const Iterator&) = delete;
245-
Iterator& operator=(const Iterator&) = delete;
254+
LockedIterator(const LockedIterator&) = delete;
255+
LockedIterator& operator=(const LockedIterator&) = delete;
246256

247-
Iterator(Iterator&&) noexcept = default;
257+
LockedIterator(LockedIterator&&) noexcept = default;
248258

249259
// 1. Invalidate this iterator
250260
// 2. Unlock
251261
void destroy() {
252-
LruList::Iterator::reset();
262+
Iterator::reset();
253263
if (l_.owns_lock()) {
254264
l_.unlock();
255265
}
@@ -260,15 +270,15 @@ class MMLru {
260270
if (!l_.owns_lock()) {
261271
l_.lock();
262272
}
263-
LruList::Iterator::resetToBegin();
273+
Iterator::resetToBegin();
264274
}
265275

266276
private:
267277
// private because it's easy to misuse and cause deadlock for MMLru
268-
Iterator& operator=(Iterator&&) noexcept = default;
278+
LockedIterator& operator=(LockedIterator&&) noexcept = default;
269279

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

273283
// only the container can create iterators
274284
friend Container<T, HookPtr>;
@@ -307,10 +317,9 @@ class MMLru {
307317
// state of node is unchanged.
308318
bool remove(T& node) noexcept;
309319

310-
using Iterator = Iterator;
311320
// same as the above but uses an iterator context. The iterator is updated
312321
// on removal of the corresponding node to point to the next node. The
313-
// iterator context holds the lock on the lru.
322+
// iterator context is responsible for locking.
314323
//
315324
// iterator will be advanced to the next node after removing the node
316325
//
@@ -330,7 +339,12 @@ class MMLru {
330339
// Obtain an iterator that start from the tail and can be used
331340
// to search for evictions. This iterator holds a lock to this
332341
// container and only one such iterator can exist at a time
333-
Iterator getEvictionIterator() const noexcept;
342+
LockedIterator getEvictionIterator() const noexcept;
343+
344+
// Execute provided function under container lock. Function gets
345+
// iterator passed as parameter.
346+
template <typename F>
347+
void withEvictionIterator(F&& f);
334348

335349
// get copy of current config
336350
Config getConfig() const;

‎cachelib/allocator/MMTinyLFU-inl.h

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -214,10 +214,18 @@ bool MMTinyLFU::Container<T, HookPtr>::add(T& node) noexcept {
214214
}
215215

216216
template <typename T, MMTinyLFU::Hook<T> T::*HookPtr>
217-
typename MMTinyLFU::Container<T, HookPtr>::Iterator
217+
typename MMTinyLFU::Container<T, HookPtr>::LockedIterator
218218
MMTinyLFU::Container<T, HookPtr>::getEvictionIterator() const noexcept {
219219
LockHolder l(lruMutex_);
220-
return Iterator{std::move(l), *this};
220+
return LockedIterator{std::move(l), *this};
221+
}
222+
223+
template <typename T, MMTinyLFU::Hook<T> T::*HookPtr>
224+
template <typename F>
225+
void MMTinyLFU::Container<T, HookPtr>::withEvictionIterator(F&& fun) {
226+
// TinyLFU uses spin lock which does not support combined locking
227+
LockHolder lck{lruMutex_};
228+
fun(Iterator{*this});
221229
}
222230

223231
template <typename T, MMTinyLFU::Hook<T> T::*HookPtr>
@@ -347,13 +355,18 @@ void MMTinyLFU::Container<T, HookPtr>::reconfigureLocked(const Time& currTime) {
347355
lruRefreshTime_.store(lruRefreshTime, std::memory_order_relaxed);
348356
}
349357

358+
// Locked Iterator Context Implementation
359+
template <typename T, MMTinyLFU::Hook<T> T::*HookPtr>
360+
MMTinyLFU::Container<T, HookPtr>::LockedIterator::LockedIterator(
361+
LockHolder l, const Container<T, HookPtr>& c) noexcept
362+
: Iterator(c), l_(std::move(l)) {}
363+
350364
// Iterator Context Implementation
351365
template <typename T, MMTinyLFU::Hook<T> T::*HookPtr>
352366
MMTinyLFU::Container<T, HookPtr>::Iterator::Iterator(
353-
LockHolder l, const Container<T, HookPtr>& c) noexcept
367+
const Container<T, HookPtr>& c) noexcept
354368
: c_(c),
355369
tIter_(c.lru_.getList(LruType::Tiny).rbegin()),
356-
mIter_(c.lru_.getList(LruType::Main).rbegin()),
357-
l_(std::move(l)) {}
370+
mIter_(c.lru_.getList(LruType::Main).rbegin()) {}
358371
} // namespace cachelib
359372
} // namespace facebook

0 commit comments

Comments
 (0)