Skip to content

Commit 55c2066

Browse files
author
Sergei Vinogradov
committed
Fix eviction flow and removeCb calls
Without this fix removeCb called even in case when Item is moved between tiers.
1 parent b06846a commit 55c2066

1 file changed

Lines changed: 14 additions & 9 deletions

File tree

cachelib/allocator/CacheAllocator-inl.h

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1438,10 +1438,17 @@ CacheAllocator<CacheTrait>::findEviction(TierId tid, PoolId pid, ClassId cid) {
14381438
// for chained items, the ownership of the parent can change. We try to
14391439
// evict what we think as parent and see if the eviction of parent
14401440
// recycles the child we intend to.
1441-
auto toReleaseHandle =
1442-
itr->isChainedItem()
1443-
? advanceIteratorAndTryEvictChainedItem(tid, pid, itr)
1444-
: advanceIteratorAndTryEvictRegularItem(tid, pid, mmContainer, itr);
1441+
1442+
ItemHandle toReleaseHandle = tryEvictToNextMemoryTier(tid, pid, itr);
1443+
bool movedToNextTier = false;
1444+
if(toReleaseHandle) {
1445+
movedToNextTier = true;
1446+
} else {
1447+
toReleaseHandle =
1448+
itr->isChainedItem()
1449+
? advanceIteratorAndTryEvictChainedItem(tid, pid, itr)
1450+
: advanceIteratorAndTryEvictRegularItem(tid, pid, mmContainer, itr);
1451+
}
14451452

14461453
if (toReleaseHandle) {
14471454
if (toReleaseHandle->hasChainedItem()) {
@@ -1472,7 +1479,7 @@ CacheAllocator<CacheTrait>::findEviction(TierId tid, PoolId pid, ClassId cid) {
14721479
// recycle the candidate.
14731480
if (ReleaseRes::kRecycled ==
14741481
releaseBackToAllocator(itemToRelease, RemoveContext::kEviction,
1475-
/* isNascent */ false, candidate)) {
1482+
/* isNascent */ movedToNextTier, candidate)) {
14761483
return candidate;
14771484
}
14781485
}
@@ -1539,6 +1546,7 @@ template <typename ItemPtr>
15391546
typename CacheAllocator<CacheTrait>::ItemHandle
15401547
CacheAllocator<CacheTrait>::tryEvictToNextMemoryTier(
15411548
TierId tid, PoolId pid, ItemPtr& item) {
1549+
if(item->isChainedItem()) return {}; // TODO: We do not support ChainedItem yet
15421550
if(item->isExpired()) return acquire(item);
15431551

15441552
TierId nextTier = tid; // TODO - calculate this based on some admission policy
@@ -1572,9 +1580,6 @@ template <typename CacheTrait>
15721580
typename CacheAllocator<CacheTrait>::ItemHandle
15731581
CacheAllocator<CacheTrait>::advanceIteratorAndTryEvictRegularItem(
15741582
TierId tid, PoolId pid, MMContainer& mmContainer, EvictionIterator& itr) {
1575-
auto evictHandle = tryEvictToNextMemoryTier(tid, pid, itr);
1576-
if(evictHandle) return evictHandle;
1577-
15781583
Item& item = *itr;
15791584

15801585
const bool evictToNvmCache = shouldWriteToNvmCache(item);
@@ -1593,7 +1598,7 @@ CacheAllocator<CacheTrait>::advanceIteratorAndTryEvictRegularItem(
15931598
// if we remove the item from both access containers and mm containers
15941599
// below, we will need a handle to ensure proper cleanup in case we end up
15951600
// not evicting this item
1596-
evictHandle = accessContainer_->removeIf(item, &itemEvictionPredicate);
1601+
auto evictHandle = accessContainer_->removeIf(item, &itemEvictionPredicate);
15971602

15981603
if (!evictHandle) {
15991604
++itr;

0 commit comments

Comments
 (0)