Skip to content

Commit f513b06

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 9fa830c commit f513b06

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
@@ -1494,10 +1494,17 @@ CacheAllocator<CacheTrait>::findEviction(TierId tid, PoolId pid, ClassId cid) {
14941494
// for chained items, the ownership of the parent can change. We try to
14951495
// evict what we think as parent and see if the eviction of parent
14961496
// recycles the child we intend to.
1497-
auto toReleaseHandle =
1498-
itr->isChainedItem()
1499-
? advanceIteratorAndTryEvictChainedItem(tid, pid, itr)
1500-
: advanceIteratorAndTryEvictRegularItem(tid, pid, mmContainer, itr);
1497+
1498+
ItemHandle toReleaseHandle = tryEvictToNextMemoryTier(tid, pid, itr);
1499+
bool movedToNextTier = false;
1500+
if(toReleaseHandle) {
1501+
movedToNextTier = true;
1502+
} else {
1503+
toReleaseHandle =
1504+
itr->isChainedItem()
1505+
? advanceIteratorAndTryEvictChainedItem(tid, pid, itr)
1506+
: advanceIteratorAndTryEvictRegularItem(tid, pid, mmContainer, itr);
1507+
}
15011508

15021509
if (toReleaseHandle) {
15031510
if (toReleaseHandle->hasChainedItem()) {
@@ -1534,7 +1541,7 @@ CacheAllocator<CacheTrait>::findEviction(TierId tid, PoolId pid, ClassId cid) {
15341541
// recycle the candidate.
15351542
if (ReleaseRes::kRecycled ==
15361543
releaseBackToAllocator(itemToRelease, RemoveContext::kEviction,
1537-
/* isNascent */ false, candidate)) {
1544+
/* isNascent */ movedToNextTier, candidate)) {
15381545
return candidate;
15391546
}
15401547
}
@@ -1601,6 +1608,7 @@ template <typename ItemPtr>
16011608
typename CacheAllocator<CacheTrait>::WriteHandle
16021609
CacheAllocator<CacheTrait>::tryEvictToNextMemoryTier(
16031610
TierId tid, PoolId pid, ItemPtr& item) {
1611+
if(item->isChainedItem()) return {}; // TODO: We do not support ChainedItem yet
16041612
if(item->isExpired()) return acquire(item);
16051613

16061614
TierId nextTier = tid; // TODO - calculate this based on some admission policy
@@ -1634,9 +1642,6 @@ template <typename CacheTrait>
16341642
typename CacheAllocator<CacheTrait>::WriteHandle
16351643
CacheAllocator<CacheTrait>::advanceIteratorAndTryEvictRegularItem(
16361644
TierId tid, PoolId pid, MMContainer& mmContainer, EvictionIterator& itr) {
1637-
auto evictHandle = tryEvictToNextMemoryTier(tid, pid, itr);
1638-
if(evictHandle) return evictHandle;
1639-
16401645
Item& item = *itr;
16411646

16421647
const bool evictToNvmCache = shouldWriteToNvmCache(item);
@@ -1655,7 +1660,7 @@ CacheAllocator<CacheTrait>::advanceIteratorAndTryEvictRegularItem(
16551660
// if we remove the item from both access containers and mm containers
16561661
// below, we will need a handle to ensure proper cleanup in case we end up
16571662
// not evicting this item
1658-
evictHandle = accessContainer_->removeIf(item, &itemEvictionPredicate);
1663+
auto evictHandle = accessContainer_->removeIf(item, &itemEvictionPredicate);
16591664

16601665
if (!evictHandle) {
16611666
++itr;

0 commit comments

Comments
 (0)