diff --git a/cachelib/allocator/CacheAllocator-inl.h b/cachelib/allocator/CacheAllocator-inl.h index 1b494d15bb..00e8f45d3c 100644 --- a/cachelib/allocator/CacheAllocator-inl.h +++ b/cachelib/allocator/CacheAllocator-inl.h @@ -1641,7 +1641,13 @@ typename CacheAllocator::WriteHandle CacheAllocator::tryEvictToNextMemoryTier( TierId tid, PoolId pid, Item& item) { if(item.isChainedItem()) return {}; // TODO: We do not support ChainedItem yet - if(item.isExpired()) return acquire(&item); + if(item.isExpired()) { + auto handle = removeIf(item, [](const Item& it) { + return it.getRefCount() == 0; + }); + + if (handle) { return handle; } + } TierId nextTier = tid; // TODO - calculate this based on some admission policy while (++nextTier < getNumTiers()) { // try to evict down to the next memory tiers @@ -3067,16 +3073,12 @@ CacheAllocator::evictNormalItem(Item& item, // We remove the item from both access and mm containers. It doesn't matter // if someone else calls remove on the item at this moment, the item cannot // be freed as long as we have the moving bit set. - auto handle = accessContainer_->removeIf(item, std::move(predicate)); - + auto handle = removeIf(item, std::move(predicate)); if (!handle) { return handle; } - XDCHECK_EQ(reinterpret_cast(handle.get()), - reinterpret_cast(&item)); XDCHECK_EQ(1u, handle->getRefCount()); - removeFromMMContainer(item); // now that we are the only handle and we actually removed something from // the RAM cache, we enqueue it to nvmcache. @@ -3188,6 +3190,21 @@ CacheAllocator::evictChainedItemForSlabRelease(ChainedItem& child) { return parentHandle; } +template +template +typename CacheAllocator::WriteHandle +CacheAllocator::removeIf(Item& item, Fn&& predicate) { + auto handle = accessContainer_->removeIf(item, std::forward(predicate)); + + if (handle) { + XDCHECK_EQ(reinterpret_cast(handle.get()), + reinterpret_cast(&item)); + removeFromMMContainer(item); + } + + return handle; +} + template bool CacheAllocator::removeIfExpired(const ReadHandle& handle) { if (!handle) { @@ -3196,14 +3213,7 @@ bool CacheAllocator::removeIfExpired(const ReadHandle& handle) { // We remove the item from both access and mm containers. // We want to make sure the caller is the only one holding the handle. - auto removedHandle = - accessContainer_->removeIf(*(handle.getInternal()), itemExpiryPredicate); - if (removedHandle) { - removeFromMMContainer(*(handle.getInternal())); - return true; - } - - return false; + return (bool)removeIf(*(handle.getInternal()), itemExpiryPredicate); } template diff --git a/cachelib/allocator/CacheAllocator.h b/cachelib/allocator/CacheAllocator.h index 02557dfe24..ca2c686cbd 100644 --- a/cachelib/allocator/CacheAllocator.h +++ b/cachelib/allocator/CacheAllocator.h @@ -1806,6 +1806,12 @@ class CacheAllocator : public CacheBase { // handle on failure. caller can retry. WriteHandle evictChainedItemForSlabRelease(ChainedItem& item); + // Helper function to remove a item if predicates is true. + // + // @return last handle to the item on success. empty handle on failure. + template + WriteHandle removeIf(Item& item, Fn&& predicate); + // Helper function to remove a item if expired. // // @return true if it item expire and removed successfully. diff --git a/run_tests.sh b/run_tests.sh index ad098c85e0..f7814f5edc 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -1,8 +1,7 @@ #!/bin/bash # Newline separated list of tests to ignore -BLACKLIST="allocator-test-AllocatorTypeTest -allocator-test-NavySetupTest +BLACKLIST="allocator-test-NavySetupTest shm-test-test_page_size" if [ "$1" == "long" ]; then