Skip to content

Commit 8a7b4d7

Browse files
authored
Merge pull request #37 from vinser52/fix_removeCb
Fix eviction flow and removeCb calls
2 parents c53e100 + 55c2066 commit 8a7b4d7

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
@@ -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)