From a7c5533b381db87d6eb74c6cf8140471b776a773 Mon Sep 17 00:00:00 2001 From: Igor Chorazewicz Date: Tue, 25 Apr 2023 13:35:24 -0700 Subject: [PATCH 1/2] Fix issue with token creation --- cachelib/allocator/CacheAllocator-inl.h | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/cachelib/allocator/CacheAllocator-inl.h b/cachelib/allocator/CacheAllocator-inl.h index 954d533daa..53ef5aba22 100644 --- a/cachelib/allocator/CacheAllocator-inl.h +++ b/cachelib/allocator/CacheAllocator-inl.h @@ -1603,13 +1603,15 @@ CacheAllocator::findEviction(TierId tid, PoolId pid, ClassId cid) { ? &toRecycle_->asChainedItem().getParentItem(compressor_) : toRecycle_; - if (lastTier) { - // if it's last tier, the item will be evicted - // need to create put token before marking it exclusive - token = createPutToken(*candidate_); - } + // if it's last tier, the item will be evicted + // need to create put token before marking it exclusive + const bool evictToNvmCache = lastTier && shouldWriteToNvmCache(*candidate_); + + auto token_ = evictToNvmCache + ? nvmCache_->createPutToken(candidate_->getKey()) + : typename NvmCacheT::PutToken{}; - if (lastTier && shouldWriteToNvmCache(*candidate_) && !token.isValid()) { + if (evictToNvmCache && !token_.isValid()) { stats_.evictFailConcurrentFill.inc(); } else if ( (lastTier && candidate_->markForEviction()) || (!lastTier && candidate_->markMoving(true)) ) { @@ -1619,6 +1621,7 @@ CacheAllocator::findEviction(TierId tid, PoolId pid, ClassId cid) { // since we won't be moving the item to the next tier toRecycle = toRecycle_; candidate = candidate_; + token = std::move(token_); // Check if parent changed for chained items - if yes, we cannot // remove the child from the mmContainer as we will not be evicting @@ -1626,8 +1629,9 @@ CacheAllocator::findEviction(TierId tid, PoolId pid, ClassId cid) { // unmarkForEviction() returns 0 - so just go through normal path. if (!toRecycle_->isChainedItem() || &toRecycle->asChainedItem().getParentItem(compressor_) == - candidate) + candidate) { mmContainer.remove(itr); + } return; } @@ -1643,8 +1647,9 @@ CacheAllocator::findEviction(TierId tid, PoolId pid, ClassId cid) { } }); - if (!toRecycle) + if (!toRecycle) { continue; + } XDCHECK(toRecycle); XDCHECK(candidate); From 7d844e72f9e72753118a80a5a398216f3e5cbde2 Mon Sep 17 00:00:00 2001 From: Igor Chorazewicz Date: Wed, 26 Apr 2023 08:05:09 -0700 Subject: [PATCH 2/2] Do not increment evictFail* stats if evictFailConcurrentFill were incremented --- cachelib/allocator/CacheAllocator-inl.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cachelib/allocator/CacheAllocator-inl.h b/cachelib/allocator/CacheAllocator-inl.h index 53ef5aba22..48fbafb356 100644 --- a/cachelib/allocator/CacheAllocator-inl.h +++ b/cachelib/allocator/CacheAllocator-inl.h @@ -1633,12 +1633,12 @@ CacheAllocator::findEviction(TierId tid, PoolId pid, ClassId cid) { mmContainer.remove(itr); } return; - } - - if (candidate_->hasChainedItem()) { - stats_.evictFailParentAC.inc(); } else { - stats_.evictFailAC.inc(); + if (candidate_->hasChainedItem()) { + stats_.evictFailParentAC.inc(); + } else { + stats_.evictFailAC.inc(); + } } ++itr;