Skip to content

Commit 267f7d7

Browse files
committed
updated to work with slab release process
1 parent d4fe4a8 commit 267f7d7

File tree

1 file changed

+66
-51
lines changed

1 file changed

+66
-51
lines changed

cachelib/allocator/CacheAllocator-inl.h

Lines changed: 66 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1533,25 +1533,12 @@ CacheAllocator<CacheTrait>::findEviction(TierId tid, PoolId pid, ClassId cid) {
15331533
chainedItem_
15341534
? &toRecycle_->asChainedItem().getParentItem(compressor_)
15351535
: nullptr;
1536-
auto l_ = chainedItem_
1537-
? chainedItemLocks_.tryLockExclusive(toRecycleParent_->getKey())
1538-
: chainedItemLocks_.tryLockExclusive(toRecycle_->getKey());
1539-
if (chainedItem_ && !l_) {
1540-
++itr;
1541-
continue;
1542-
}
15431536
Item* candidate_;
15441537
Item* syncItem_;
15451538
//sync on the parent item for chained items to move to next tier
15461539
if (!lastTier && chainedItem_) {
1547-
if (&toRecycle_->asChainedItem().getParentItem(compressor_) !=
1548-
toRecycleParent_) {
1549-
++itr;
1550-
continue;
1551-
}
15521540
syncItem_ = toRecycleParent_;
15531541
candidate_ = toRecycle_;
1554-
XDCHECK(l_);
15551542
} else if (lastTier && chainedItem_) {
15561543
candidate_ = toRecycleParent_;
15571544
syncItem_ = toRecycleParent_;
@@ -1582,9 +1569,25 @@ CacheAllocator<CacheTrait>::findEviction(TierId tid, PoolId pid, ClassId cid) {
15821569
// it. We could abort right here, but we need to cleanup in case
15831570
// unmarkForEviction() returns 0 - so just go through normal path.
15841571
if (!chainedItem ||
1585-
&toRecycle_->asChainedItem().getParentItem(compressor_) ==
1586-
toRecycleParent_) {
1572+
&toRecycle_->asChainedItem().getParentItem(compressor_) ==
1573+
toRecycleParent_) {
15871574
mmContainer.remove(itr);
1575+
} else if (chainedItem &&
1576+
&toRecycle->asChainedItem().getParentItem(compressor_) !=
1577+
toRecycleParent_) {
1578+
auto ref = syncItem_->unmarkMoving();
1579+
if (UNLIKELY(ref == 0)) {
1580+
wakeUpWaiters(*syncItem_,{});
1581+
const auto res =
1582+
releaseBackToAllocator(*syncItem_, RemoveContext::kNormal, false);
1583+
XDCHECK(res == ReleaseRes::kReleased);
1584+
}
1585+
auto parentHandle = acquire(syncItem_);
1586+
if (parentHandle) {
1587+
wakeUpWaiters(*syncItem_,std::move(parentHandle));
1588+
} //in case where parent handle is null that means some other thread
1589+
++itr;
1590+
continue;
15881591
}
15891592
return;
15901593
}
@@ -3052,16 +3055,12 @@ void CacheAllocator<CacheTrait>::releaseSlabImpl(TierId tid,
30523055
auto startTimeSec = util::getCurrentTimeSec();
30533056
Item& item = *static_cast<Item*>(alloc);
30543057

3055-
if (!item.isChainedItem() && !item.hasChainedItem()) {
3056-
// TODO: no support for chained items
3057-
3058-
// Need to mark an item for release before proceeding
3059-
// If we can't mark as moving, it means the item is already freed
3060-
const bool isAlreadyFreed =
3061-
!markMovingForSlabRelease(releaseContext, alloc, throttler);
3062-
if (isAlreadyFreed) {
3063-
continue;
3064-
}
3058+
// Need to mark an item for release before proceeding
3059+
// If we can't mark as moving, it means the item is already freed
3060+
const bool isAlreadyFreed =
3061+
!markMovingForSlabRelease(releaseContext, alloc, throttler);
3062+
if (isAlreadyFreed) {
3063+
continue;
30653064
}
30663065

30673066
// Try to move this item and make sure we can free the memory
@@ -3326,36 +3325,33 @@ void CacheAllocator<CacheTrait>::evictForSlabRelease(
33263325
auto& expectedParent = item.asChainedItem().getParentItem(compressor_);
33273326

33283327
const std::string parentKey = expectedParent.getKey().str();
3328+
//TODO: this might not be needed since we have already marked parent
3329+
// as moving
33293330
auto l = chainedItemLocks_.tryLockExclusive(parentKey);
33303331

33313332
if (!l) {
33323333
continue;
3333-
} else if (expectedParent.getKey() != parentKey) {
3334-
continue;
3335-
} else if (&expectedParent != &item.asChainedItem().getParentItem(compressor_)) {
3336-
continue;
3337-
} else if (!expectedParent.isAccessible()) {
3338-
continue;
3339-
} else if (!expectedParent.hasChainedItem()) {
3340-
continue;
33413334
}
3335+
XDCHECK_EQ(expectedParent.getKey(),parentKey);
3336+
XDCHECK_EQ(&expectedParent,&item.asChainedItem().getParentItem(compressor_));
3337+
XDCHECK(expectedParent.isAccessible());
3338+
XDCHECK(expectedParent.hasChainedItem());
33423339

33433340
evicted = &expectedParent;
3341+
XDCHECK(evicted->isMoving());
33443342
token = createPutToken(*evicted);
3345-
if (evicted->markForEviction()) {
3346-
XDCHECK_EQ(&expectedParent,&item.asChainedItem().getParentItem(compressor_));
3347-
// unmark the child so it will be freed
3348-
// TODO entire chain just gets evicted since moveForSlabRelease
3349-
// returns false
3350-
XDCHECK(!item.isMoving());
3351-
unlinkItemForEviction(*evicted);
3352-
// wake up any readers that wait for the move to complete
3353-
// it's safe to do now, as we have the item marked exclusive and
3354-
// no other reader can be added to the waiters list
3355-
wakeUpWaiters(*evicted, {});
3356-
} else {
3357-
continue;
3358-
}
3343+
auto ret = evicted->markForEvictionWhenMoving();
3344+
XDCHECK(ret);
3345+
XDCHECK_EQ(&expectedParent,&item.asChainedItem().getParentItem(compressor_));
3346+
// unmark the child so it will be freed
3347+
// TODO entire chain just gets evicted since moveForSlabRelease
3348+
// returns false
3349+
XDCHECK(!item.isMoving());
3350+
unlinkItemForEviction(*evicted);
3351+
// wake up any readers that wait for the move to complete
3352+
// it's safe to do now, as we have the item marked exclusive and
3353+
// no other reader can be added to the waiters list
3354+
wakeUpWaiters(*evicted, {});
33593355
} else {
33603356
evicted = &item;
33613357

@@ -3448,13 +3444,32 @@ bool CacheAllocator<CacheTrait>::markMovingForSlabRelease(
34483444
bool itemFreed = true;
34493445
bool markedMoving = false;
34503446
TierId tid = getTierId(alloc);
3451-
const auto fn = [&markedMoving, &itemFreed](void* memory) {
3447+
const auto fn = [this, &markedMoving, &itemFreed](void* memory) {
34523448
// Since this callback is executed, the item is not yet freed
34533449
itemFreed = false;
34543450
Item* item = static_cast<Item*>(memory);
3455-
XDCHECK(!item->isChainedItem()); // TODO: chained items not supported
3456-
if (item->markMoving()) {
3457-
markedMoving = true;
3451+
bool chainedItem_ = item->isChainedItem();
3452+
Item* syncItem_ = chainedItem_
3453+
? &item->asChainedItem().getParentItem(compressor_)
3454+
: item;
3455+
if (syncItem_->markMoving()) {
3456+
if (chainedItem_ &&
3457+
syncItem_ != &item->asChainedItem().getParentItem(compressor_)) {
3458+
//case where parent changed
3459+
auto ref = syncItem_->unmarkMoving();
3460+
if (UNLIKELY(ref == 0)) {
3461+
wakeUpWaiters(*syncItem_,{});
3462+
const auto res =
3463+
releaseBackToAllocator(*syncItem_, RemoveContext::kNormal, false);
3464+
XDCHECK(res == ReleaseRes::kReleased);
3465+
}
3466+
auto parentHandle = acquire(syncItem_);
3467+
if (parentHandle) {
3468+
wakeUpWaiters(*syncItem_,std::move(parentHandle));
3469+
} //in case where parent handle is null that means some other thread
3470+
} else {
3471+
markedMoving = true;
3472+
}
34583473
}
34593474
};
34603475

0 commit comments

Comments
 (0)