Skip to content

Commit c0e5e1e

Browse files
committed
added some debug checks around chained item check
1 parent 3d6f06b commit c0e5e1e

File tree

2 files changed

+19
-14
lines changed

2 files changed

+19
-14
lines changed

cachelib/allocator/CacheAllocator-inl.h

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1700,13 +1700,14 @@ CacheAllocator<CacheTrait>::findEviction(TierId tid, PoolId pid, ClassId cid) {
17001700
const auto res =
17011701
releaseBackToAllocator(*toRecycleParent, RemoveContext::kNormal, false);
17021702
XDCHECK(res == ReleaseRes::kReleased);
1703+
} else {
1704+
auto parentHandle = acquire(toRecycleParent);
1705+
if (parentHandle) {
1706+
wakeUpWaiters(*toRecycleParent,std::move(parentHandle));
1707+
} //in case where parent handle is null that means some other thread
1708+
// would have called wakeUpWaiters with null handle and released
1709+
// parent back to allocator
17031710
}
1704-
auto parentHandle = acquire(toRecycleParent);
1705-
if (parentHandle) {
1706-
wakeUpWaiters(*toRecycleParent,std::move(parentHandle));
1707-
} //in case where parent handle is null that means some other thread
1708-
// would have called wakeUpWaiters with null handle and released
1709-
// parent back to allocator
17101711
return toRecycle;
17111712
} else {
17121713
wakeUpWaiters(*candidate, std::move(evictedToNext));
@@ -1730,6 +1731,7 @@ CacheAllocator<CacheTrait>::findEviction(TierId tid, PoolId pid, ClassId cid) {
17301731
candidate->getConfiguredTTL().count());
17311732
}
17321733

1734+
XDCHECK(!candidate->isChainedItem());
17331735
// check if by releasing the item we intend to, we actually
17341736
// recycle the candidate.
17351737
auto ret = releaseBackToAllocator(*candidate, RemoveContext::kEviction,
@@ -1806,14 +1808,12 @@ CacheAllocator<CacheTrait>::tryEvictToNextMemoryTier(
18061808
if(item.isChainedItem()) {
18071809
chainedItem = true;
18081810
parentItem = &item.asChainedItem().getParentItem(compressor_);
1809-
if (!parentItem->isMoving()) {
1810-
XDCHECK(item.isInMMContainer()); //parent changed
1811-
return WriteHandle{};
1812-
}
1811+
XDCHECK(parentItem->isMoving());
18131812
XDCHECK(item.isChainedItem() && item.getRefCount() == 1);
18141813
XDCHECK_EQ(0, parentItem->getRefCount());
1815-
newItemHdl =
1816-
allocateChainedItemInternalTier(*parentItem, item.getSize(), nextTier);
1814+
newItemHdl = allocateChainedItemInternalTier(*parentItem,
1815+
item.getSize(),
1816+
nextTier);
18171817
} else {
18181818
// this assert can fail if parent changed
18191819
XDCHECK(item.isMoving());
@@ -3491,7 +3491,8 @@ bool CacheAllocator<CacheTrait>::markMovingForSlabRelease(
34913491
auto pid = allocInfo.poolId;
34923492
auto cid = allocInfo.classId;
34933493
auto& mmContainer = getMMContainer(tid, pid, cid);
3494-
mmContainer.withContainerLock([this, &syncItem, &item, &markedMoving]() {
3494+
mmContainer.withContainerLock([this, &mmContainer,
3495+
&syncItem, &item, &markedMoving]() {
34953496
//we rely on the mmContainer lock to safely check that the item is
34963497
//currently in the mmContainer (no other threads are currently allocating
34973498
//this item). This is needed to sync on the case where a chained item
@@ -3502,6 +3503,8 @@ bool CacheAllocator<CacheTrait>::markMovingForSlabRelease(
35023503
return;
35033504
}
35043505
bool chainedItem_ = item->isChainedItem();
3506+
XDCHECK_EQ(&getMMContainer(*item),&mmContainer);
3507+
XDCHECK_EQ(item->isChainedItem(),chainedItem_);
35053508
Item* syncItem_ = chainedItem_
35063509
? &item->asChainedItem().getParentItem(compressor_)
35073510
: item;
@@ -3511,6 +3514,8 @@ bool CacheAllocator<CacheTrait>::markMovingForSlabRelease(
35113514
auto l_ = chainedItem_
35123515
? chainedItemLocks_.tryLockExclusive(syncItem_->getKey())
35133516
: decltype(chainedItemLocks_.tryLockExclusive(syncItem_->getKey()))();
3517+
3518+
XDCHECK_EQ(item->isChainedItem(),chainedItem_);
35143519
if (chainedItem_ &&
35153520
( !l_ || &item->asChainedItem().getParentItem(compressor_) != syncItem_) ) {
35163521
markedMoving = false;

cachelib/cachebench/test_configs/small_moving_bg.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
"test_config" :
2222
{
2323
"preallocateCache" : true,
24-
"numOps" : 40000000,
24+
"numOps" : 20000000,
2525
"numThreads" : 32,
2626
"numKeys" : 250000,
2727
"generator": "online",

0 commit comments

Comments
 (0)