Skip to content

Commit 785ce7b

Browse files
igchorbyrnedj
igchor
authored and
byrnedj
committed
wakeUpWaiters in SlabRelease code
1 parent 68e6639 commit 785ce7b

File tree

3 files changed

+84
-38
lines changed

3 files changed

+84
-38
lines changed

‎cachelib/allocator/CacheAllocator-inl.h‎

Lines changed: 49 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1286,7 +1286,7 @@ CacheAllocator<CacheTrait>::handleWithWaitContextForMovingItem(Item& item) {
12861286
}
12871287

12881288
template <typename CacheTrait>
1289-
size_t CacheAllocator<CacheTrait>::wakeUpWaiters(folly::StringPiece key,
1289+
size_t CacheAllocator<CacheTrait>::wakeUpWaitersLocked(folly::StringPiece key,
12901290
WriteHandle&& handle) {
12911291
std::unique_ptr<MoveCtx> ctx;
12921292
auto shard = getShardForKey(key);
@@ -1657,7 +1657,7 @@ CacheAllocator<CacheTrait>::findEviction(TierId tid, PoolId pid, ClassId cid) {
16571657
// wake up any readers that wait for the move to complete
16581658
// it's safe to do now, as we have the item marked exclusive and
16591659
// no other reader can be added to the waiters list
1660-
wakeUpWaiters(candidate->getKey(), WriteHandle{});
1660+
wakeUpWaiters(*candidate, {});
16611661

16621662
if (token.isValid() && shouldWriteToNvmCacheExclusive(*candidate)) {
16631663
nvmCache_->put(*candidate, std::move(token));
@@ -1668,7 +1668,7 @@ CacheAllocator<CacheTrait>::findEviction(TierId tid, PoolId pid, ClassId cid) {
16681668
XDCHECK(!candidate->isAccessible());
16691669
XDCHECK(candidate->getKey() == evictedToNext->getKey());
16701670

1671-
wakeUpWaiters(candidate->getKey(), std::move(evictedToNext));
1671+
wakeUpWaiters(*candidate, std::move(evictedToNext));
16721672
}
16731673

16741674
XDCHECK(!candidate->isExclusive() && !candidate->isMoving());
@@ -1757,7 +1757,7 @@ CacheAllocator<CacheTrait>::tryEvictToNextMemoryTier(
17571757
if(item.isExpired()) {
17581758
accessContainer_->remove(item);
17591759
item.unmarkMoving();
1760-
return acquire(&item); // TODO: wakeUpWaiters with null handle?
1760+
return acquire(&item);
17611761
}
17621762

17631763
TierId nextTier = tid; // TODO - calculate this based on some admission policy
@@ -2984,6 +2984,14 @@ void CacheAllocator<CacheTrait>::throttleWith(util::Throttler& t,
29842984
}
29852985
}
29862986

2987+
template <typename CacheTrait>
2988+
typename RefcountWithFlags::Value CacheAllocator<CacheTrait>::unmarkMovingAndWakeUpWaiters(Item &item, WriteHandle handle)
2989+
{
2990+
auto ret = item.unmarkMoving();
2991+
wakeUpWaiters(item, std::move(handle));
2992+
return ret;
2993+
}
2994+
29872995
template <typename CacheTrait>
29882996
bool CacheAllocator<CacheTrait>::moveForSlabRelease(
29892997
const SlabReleaseContext& ctx, Item& oldItem, util::Throttler& throttler) {
@@ -3002,7 +3010,8 @@ bool CacheAllocator<CacheTrait>::moveForSlabRelease(
30023010

30033011
// Nothing to move and the key is likely also bogus for chained items.
30043012
if (oldItem.isOnlyMoving()) {
3005-
oldItem.unmarkMoving();
3013+
auto ret = unmarkMovingAndWakeUpWaiters(oldItem, {});
3014+
XDCHECK(ret == 0);
30063015
const auto res =
30073016
releaseBackToAllocator(oldItem, RemoveContext::kNormal, false);
30083017
XDCHECK(res == ReleaseRes::kReleased);
@@ -3051,8 +3060,8 @@ bool CacheAllocator<CacheTrait>::moveForSlabRelease(
30513060
}
30523061

30533062
auto tid = getTierId(oldItem);
3054-
auto ref = oldItem.unmarkMoving();
3055-
XDCHECK(ref, 0);
3063+
auto ref = unmarkMovingAndWakeUpWaiters(oldItem, std::move(newItemHdl));
3064+
XDCHECK(ref == 0);
30563065

30573066
const auto allocInfo = allocator_[tid]->getAllocInfo(oldItem.getMemory());
30583067
allocator_[tid]->free(&oldItem);
@@ -3172,9 +3181,26 @@ bool CacheAllocator<CacheTrait>::tryMovingForSlabRelease(
31723181
}
31733182
}
31743183

3175-
return oldItem.isChainedItem()
3176-
? moveChainedItem(oldItem.asChainedItem(), newItemHdl)
3177-
: moveRegularItem(oldItem, newItemHdl);
3184+
// TODO: we can unify move*Item and move*ItemWithSync by always
3185+
// using the moving bit to block readers.
3186+
if (getNumTiers() == 1) {
3187+
return oldItem.isChainedItem()
3188+
? moveChainedItem(oldItem.asChainedItem(), newItemHdl)
3189+
: moveRegularItem(oldItem, newItemHdl);
3190+
} else {
3191+
// TODO: add support for chained items
3192+
moveRegularItemWithSync(oldItem, newItemHdl);
3193+
return true;
3194+
}
3195+
}
3196+
3197+
template <typename CacheTrait>
3198+
void CacheAllocator<CacheTrait>::wakeUpWaiters(Item& item, WriteHandle handle)
3199+
{
3200+
// readers do not block on 'moving' items in case there is only one tier
3201+
if (getNumTiers() > 1) {
3202+
wakeUpWaitersLocked(item.getKey(), std::move(handle));
3203+
}
31783204
}
31793205

31803206
template <typename CacheTrait>
@@ -3187,7 +3213,7 @@ void CacheAllocator<CacheTrait>::evictForSlabRelease(
31873213
stats_.numEvictionAttempts.inc();
31883214

31893215
if (shutDownInProgress_) {
3190-
item.unmarkMoving();
3216+
auto ref = unmarkMovingAndWakeUpWaiters(item, {});
31913217
allocator_[getTierId(item)]->abortSlabRelease(ctx);
31923218
throw exception::SlabReleaseAborted(
31933219
folly::sformat("Slab Release aborted while trying to evict"
@@ -3212,7 +3238,8 @@ void CacheAllocator<CacheTrait>::evictForSlabRelease(
32123238
// nothing needs to be done. We simply need to unmark moving bit and free
32133239
// the item.
32143240
if (item.isOnlyMoving()) {
3215-
item.unmarkMoving();
3241+
auto ref = unmarkMovingAndWakeUpWaiters(item, {});
3242+
XDCHECK(ref == 0);
32163243
const auto res =
32173244
releaseBackToAllocator(item, RemoveContext::kNormal, false);
32183245
XDCHECK(ReleaseRes::kReleased == res);
@@ -3268,13 +3295,13 @@ void CacheAllocator<CacheTrait>::evictForSlabRelease(
32683295
if (evicted->markExclusive()) {
32693296
// unmark the child so it will be freed
32703297
item.unmarkMoving();
3298+
32713299
unlinkItemExclusive(*evicted);
3272-
if (getNumTiers() > 1) {
3273-
// wake up any readers that wait for the move to complete
3274-
// it's safe to do now, as we have the item marked exclusive and
3275-
// no other reader can be added to the waiters list
3276-
wakeUpWaiters(evicted->getKey(), WriteHandle{});
3277-
}
3300+
3301+
// wake up any readers that wait for the move to complete
3302+
// it's safe to do now, as we have the item marked exclusive and
3303+
// no other reader can be added to the waiters list
3304+
wakeUpWaiters(*evicted, {});
32783305
} else {
32793306
continue;
32803307
}
@@ -3284,12 +3311,10 @@ void CacheAllocator<CacheTrait>::evictForSlabRelease(
32843311
token = createPutToken(*evicted);
32853312
if (evicted->markExclusiveWhenMoving()) {
32863313
unlinkItemExclusive(*evicted);
3287-
if (getNumTiers() > 1) {
3288-
// wake up any readers that wait for the move to complete
3289-
// it's safe to do now, as we have the item marked exclusive and
3290-
// no other reader can be added to the waiters list
3291-
wakeUpWaiters(evicted->getKey(), WriteHandle{});
3292-
}
3314+
// wake up any readers that wait for the move to complete
3315+
// it's safe to do now, as we have the item marked exclusive and
3316+
// no other reader can be added to the waiters list
3317+
wakeUpWaiters(*evicted, {});
32933318
} else {
32943319
continue;
32953320
}

‎cachelib/allocator/CacheAllocator.h‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1797,6 +1797,18 @@ class CacheAllocator : public CacheBase {
17971797

17981798
WriteHandle tryPromoteToNextMemoryTier(Item& item, bool fromBgThread);
17991799

1800+
// Wakes up waiters if there are any
1801+
//
1802+
// @param item wakes waiters that are waiting on that item
1803+
// @param handle handle to pass to the waiters
1804+
void wakeUpWaiters(Item& item, WriteHandle handle);
1805+
1806+
// Unmarks item as moving and wakes up any waiters waiting on that item
1807+
//
1808+
// @param item wakes waiters that are waiting on that item
1809+
// @param handle handle to pass to the waiters
1810+
typename RefcountWithFlags::Value unmarkMovingAndWakeUpWaiters(Item &item, WriteHandle handle);
1811+
18001812
// Try to move the item down to the next memory tier
18011813
//
18021814
// @param item the item to evict
@@ -2006,7 +2018,7 @@ class CacheAllocator : public CacheBase {
20062018
// wake up any readers that wait for the move to complete
20072019
// it's safe to do now, as we have the item marked exclusive and
20082020
// no other reader can be added to the waiters list
2009-
wakeUpWaiters(candidate->getKey(), WriteHandle{});
2021+
wakeUpWaiters(*candidate, WriteHandle{});
20102022

20112023
if (token.isValid() && shouldWriteToNvmCacheExclusive(*candidate)) {
20122024
nvmCache_->put(*candidate, std::move(token));
@@ -2018,7 +2030,7 @@ class CacheAllocator : public CacheBase {
20182030
XDCHECK(!candidate->isAccessible());
20192031
XDCHECK(candidate->getKey() == evictedToNext->getKey());
20202032

2021-
wakeUpWaiters(candidate->getKey(), std::move(evictedToNext));
2033+
wakeUpWaiters(*candidate, std::move(evictedToNext));
20222034
}
20232035
XDCHECK(!candidate->isExclusive() && !candidate->isMoving());
20242036

@@ -2072,15 +2084,16 @@ class CacheAllocator : public CacheBase {
20722084
promotions++;
20732085
removeFromMMContainer(*candidate);
20742086
XDCHECK(!candidate->isExclusive() && !candidate->isMoving());
2075-
// it's safe to recycle the item here as there are no more
2076-
// references and the item could not been marked as moving
2077-
// by other thread since it's detached from MMContainer.
2078-
auto res = releaseBackToAllocator(*candidate, RemoveContext::kEviction,
2079-
/* isNascent */ false);
2080-
XDCHECK(res == ReleaseRes::kReleased);
2087+
// it's safe to recycle the item here as there are no more
2088+
// references and the item could not been marked as moving
2089+
// by other thread since it's detached from MMContainer.
2090+
auto res = releaseBackToAllocator(*candidate, RemoveContext::kEviction,
2091+
/* isNascent */ false);
2092+
XDCHECK(res == ReleaseRes::kReleased);
2093+
wakeUpWaiters(*candidate, std::move(promoted));
20812094
} else {
2082-
//we failed to allocate a new item, this item is no longer moving
2083-
auto ref = candidate->unmarkMoving();
2095+
// we failed to allocate a new item, this item is no longer moving
2096+
auto ref = unmarkMovingAndWakeUpWaiters(*candidate, {});
20842097
if (UNLIKELY(ref == 0)) {
20852098
const auto res =
20862099
releaseBackToAllocator(*candidate,
@@ -2259,7 +2272,7 @@ class CacheAllocator : public CacheBase {
22592272

22602273
WriteHandle handleWithWaitContextForMovingItem(Item& item);
22612274

2262-
size_t wakeUpWaiters(folly::StringPiece key, WriteHandle&& handle);
2275+
size_t wakeUpWaitersLocked(folly::StringPiece key, WriteHandle&& handle);
22632276

22642277
class MoveCtx {
22652278
public:

‎cachelib/allocator/Refcount.h‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -367,11 +367,19 @@ class FOLLY_PACK_ATTR RefcountWithFlags {
367367
XDCHECK((curValue & kAccessRefMask) != 0);
368368
return true;
369369
};
370-
auto newValue = [](const Value curValue) {
371-
return (curValue - static_cast<Value>(1)) & ~getAdminRef<kExclusive>();
370+
371+
Value retValue;
372+
auto newValue = [&retValue](const Value curValue) {
373+
retValue = (curValue - static_cast<Value>(1)) & ~getAdminRef<kExclusive>();
374+
return retValue;
372375
};
373-
return markInternal(predicate, newValue);
376+
377+
auto res = markInternal(predicate, newValue);
378+
XDCHECK(res);
379+
380+
return retValue & kRefMask;
374381
}
382+
375383
bool isMoving() const noexcept {
376384
auto raw = getRaw();
377385
return (raw & getAdminRef<kExclusive>()) && ((raw & kAccessRefMask) != 0);

0 commit comments

Comments
 (0)