Skip to content

Commit d817018

Browse files
committed
Fix eviction flow and removeCb calls
Without this fix removeCb called even in case when Item is moved between tiers.
1 parent 7b36c51 commit d817018

File tree

1 file changed

+14
-9
lines changed

1 file changed

+14
-9
lines changed

cachelib/allocator/CacheAllocator-inl.h

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1463,10 +1463,17 @@ CacheAllocator<CacheTrait>::findEviction(TierId tid, PoolId pid, ClassId cid) {
14631463
// for chained items, the ownership of the parent can change. We try to
14641464
// evict what we think as parent and see if the eviction of parent
14651465
// recycles the child we intend to.
1466-
auto toReleaseHandle =
1467-
itr->isChainedItem()
1468-
? advanceIteratorAndTryEvictChainedItem(tid, pid, itr)
1469-
: advanceIteratorAndTryEvictRegularItem(tid, pid, mmContainer, itr);
1466+
1467+
ItemHandle toReleaseHandle = tryEvictToNextMemoryTier(tid, pid, itr);
1468+
bool movedToNextTier = false;
1469+
if(toReleaseHandle) {
1470+
movedToNextTier = true;
1471+
} else {
1472+
toReleaseHandle =
1473+
itr->isChainedItem()
1474+
? advanceIteratorAndTryEvictChainedItem(tid, pid, itr)
1475+
: advanceIteratorAndTryEvictRegularItem(tid, pid, mmContainer, itr);
1476+
}
14701477

14711478
if (toReleaseHandle) {
14721479
if (toReleaseHandle->hasChainedItem()) {
@@ -1497,7 +1504,7 @@ CacheAllocator<CacheTrait>::findEviction(TierId tid, PoolId pid, ClassId cid) {
14971504
// recycle the candidate.
14981505
if (ReleaseRes::kRecycled ==
14991506
releaseBackToAllocator(itemToRelease, RemoveContext::kEviction,
1500-
/* isNascent */ false, candidate)) {
1507+
/* isNascent */ movedToNextTier, candidate)) {
15011508
return candidate;
15021509
}
15031510
}
@@ -1564,6 +1571,7 @@ template <typename ItemPtr>
15641571
typename CacheAllocator<CacheTrait>::ItemHandle
15651572
CacheAllocator<CacheTrait>::tryEvictToNextMemoryTier(
15661573
TierId tid, PoolId pid, ItemPtr& item) {
1574+
if(item->isChainedItem()) return {}; // TODO: We do not support ChainedItem yet
15671575
if(item->isExpired()) return acquire(item);
15681576

15691577
TierId nextTier = tid; // TODO - calculate this based on some admission policy
@@ -1597,9 +1605,6 @@ template <typename CacheTrait>
15971605
typename CacheAllocator<CacheTrait>::ItemHandle
15981606
CacheAllocator<CacheTrait>::advanceIteratorAndTryEvictRegularItem(
15991607
TierId tid, PoolId pid, MMContainer& mmContainer, EvictionIterator& itr) {
1600-
auto evictHandle = tryEvictToNextMemoryTier(tid, pid, itr);
1601-
if(evictHandle) return evictHandle;
1602-
16031608
Item& item = *itr;
16041609

16051610
const bool evictToNvmCache = shouldWriteToNvmCache(item);
@@ -1618,7 +1623,7 @@ CacheAllocator<CacheTrait>::advanceIteratorAndTryEvictRegularItem(
16181623
// if we remove the item from both access containers and mm containers
16191624
// below, we will need a handle to ensure proper cleanup in case we end up
16201625
// not evicting this item
1621-
evictHandle = accessContainer_->removeIf(item, &itemEvictionPredicate);
1626+
auto evictHandle = accessContainer_->removeIf(item, &itemEvictionPredicate);
16221627

16231628
if (!evictHandle) {
16241629
++itr;

0 commit comments

Comments
 (0)