Skip to content

Commit e17bab7

Browse files
igchorbyrnedj
authored andcommitted
parent 8dde327
author Chorazewicz, Igor <[email protected]> 1632834667 +0200 committer Daniel Byrne <[email protected]> 1671070203 -0800 Initial multi-tier support implementation Extend CompressedPtr to work with multiple tiers Now it's size is 8 bytes intead of 4. Original CompressedPtr stored only some offset with a memory Allocator. For multi-tier implementation, this is not enough. We must also store tierId and when uncompressing, select a proper allocator. An alternative could be to just resign from CompressedPtr but they are leveraged to allow the cache to be mapped to different addresses on shared memory. Changing CompressedPtr impacted CacheItem size - it increased from 32 to 44 bytes. Implemented async Item movement between tiers (with corrected behaivour of tryEvictToNextMemoryTier). Fix ReaperSkippingSlabTraversalWhileSlabReleasing test The issue was caused by incorrect behaviour of the CacheAllocator<CacheTrait>::tryEvictToNextMemoryTier method in case the evicted item is expired. We cannot simply return a handle to it, but we need to remove it from the access container and MM container. Enable workarounds in tests Add basic multi-tier test Set correct size for each memory tier Do not compensate for rounding error when calculating tier sizes (pmem#43) Compensation results in ratios being different than originially specified. Fixed total cache size in CacheMemoryStats (pmem#38) Return a sum of sizes of each tier instead of just 1st tier's size. Issue75 rebased (pmem#88) * pmem#75: Use actual tier sizes (rounded down to slab size and decreased by header size) when creating new memory pools * Added getPoolSize method to calculate combined pool size for all tiers; added pool size validation to tests * Explicitly specified type for totalCacheSize to avoid overflow * Minor test change * Reworked tests * Minor change * Deleted redundant tests * Deleted unused constant * First set of changes to cache configuration API to enable multi-tier caches (facebook#138) Summary: These changes introduce per-tier cache configuration required to implement features discussed here: facebook#102. These specific changes enable single DRAM tier configs only which are compatible with the current version of cachelib. Configuration API will be expanded as multi-tier changes in other parts of the library are introduced. Pull Request resolved: facebook#138 Reviewed By: therealgymmy Differential Revision: D36189766 Pulled By: jiayuebao fbshipit-source-id: 947aa0cd800ea6accffc1b7b6b0c9693aa7fc0a5 Co-authored-by: Victoria McGrath <[email protected]> Fix eviction flow and removeCb calls Without this fix removeCb called even in case when Item is moved between tiers. Fix issue with "Destorying an unresolved handle" The issue happened when ReadHandleImpl ctor needs to destroy waitContext_ because addWaitContextForMovingItem() returns false. So before destroying waitContext_ we are calling discard method to notify ~ItemWaitContext() that Item is ready. Fix slab release code Get tier id of item before calling any function on allocator (which needs the tierID).
1 parent 8dde327 commit e17bab7

25 files changed

+1400
-349
lines changed

cachelib/allocator/CacheAllocator-inl.h

Lines changed: 579 additions & 173 deletions
Large diffs are not rendered by default.

cachelib/allocator/CacheAllocator.h

Lines changed: 213 additions & 31 deletions
Large diffs are not rendered by default.

cachelib/allocator/CacheAllocatorConfig.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
#include <string>
2727

2828
#include "cachelib/allocator/Cache.h"
29-
#include "cachelib/allocator/MemoryTierCacheConfig.h"
3029
#include "cachelib/allocator/MM2Q.h"
3130
#include "cachelib/allocator/MemoryMonitor.h"
3231
#include "cachelib/allocator/MemoryTierCacheConfig.h"
@@ -380,7 +379,6 @@ class CacheAllocatorConfig {
380379
std::map<std::string, std::string> serialize() const;
381380

382381
// The max number of memory cache tiers
383-
// TODO: increase this number when multi-tier configs are enabled
384382
inline static const size_t kMaxCacheMemoryTiers = 2;
385383

386384
// Cache name for users to indentify their own cache.
@@ -884,11 +882,6 @@ CacheAllocatorConfig<T>& CacheAllocatorConfig<T>::configureMemoryTiers(
884882
template <typename T>
885883
const typename CacheAllocatorConfig<T>::MemoryTierConfigs&
886884
CacheAllocatorConfig<T>::getMemoryTierConfigs() const {
887-
for (auto &tier_config: memoryTierConfigs) {
888-
if (auto *v = std::get_if<PosixSysVSegmentOpts>(&tier_config.shmOpts)) {
889-
const_cast<PosixSysVSegmentOpts*>(v)->usePosix = usePosixShm;
890-
}
891-
}
892885
return memoryTierConfigs;
893886
}
894887

cachelib/allocator/CacheItem-inl.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,21 @@ bool CacheItem<CacheTrait>::isNvmEvicted() const noexcept {
266266
return ref_.isNvmEvicted();
267267
}
268268

269+
template <typename CacheTrait>
270+
void CacheItem<CacheTrait>::markIncomplete() noexcept {
271+
ref_.markIncomplete();
272+
}
273+
274+
template <typename CacheTrait>
275+
void CacheItem<CacheTrait>::unmarkIncomplete() noexcept {
276+
ref_.unmarkIncomplete();
277+
}
278+
279+
template <typename CacheTrait>
280+
bool CacheItem<CacheTrait>::isIncomplete() const noexcept {
281+
return ref_.isIncomplete();
282+
}
283+
269284
template <typename CacheTrait>
270285
void CacheItem<CacheTrait>::markIsChainedItem() noexcept {
271286
XDCHECK(!hasChainedItem());

cachelib/allocator/CacheItem.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ class CACHELIB_PACKED_ATTR CacheItem {
141141
* to be mapped to different addresses on shared memory.
142142
*/
143143
using CompressedPtr = facebook::cachelib::CompressedPtr;
144+
using SingleTierPtrCompressor = MemoryAllocator::SingleTierPtrCompressor<Item>;
144145
using PtrCompressor = MemoryAllocator::PtrCompressor<Item>;
145146

146147
// Get the required size for a cache item given the size of memory
@@ -241,6 +242,14 @@ class CACHELIB_PACKED_ATTR CacheItem {
241242
void unmarkNvmEvicted() noexcept;
242243
bool isNvmEvicted() const noexcept;
243244

245+
/**
246+
* Marks that the item is migrating between memory tiers and
247+
* not ready for access now. Accessing thread should wait.
248+
*/
249+
void markIncomplete() noexcept;
250+
void unmarkIncomplete() noexcept;
251+
bool isIncomplete() const noexcept;
252+
244253
/**
245254
* Function to set the timestamp for when to expire an item
246255
*

cachelib/allocator/Handle.h

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,12 @@ struct ReadHandleImpl {
402402
}
403403
}
404404

405+
protected:
406+
friend class ReadHandleImpl;
407+
// Method used only by ReadHandleImpl ctor
408+
void discard() {
409+
it_.store(nullptr, std::memory_order_relaxed);
410+
}
405411
private:
406412
// we are waiting on Item* to be set to a value. One of the valid values is
407413
// nullptr. So choose something that we dont expect to indicate a ptr
@@ -481,7 +487,15 @@ struct ReadHandleImpl {
481487

482488
// Handle which has the item already
483489
FOLLY_ALWAYS_INLINE ReadHandleImpl(Item* it, CacheT& alloc) noexcept
484-
: alloc_(&alloc), it_(it) {}
490+
: alloc_(&alloc), it_(it) {
491+
if (it_ && it_->isIncomplete()) {
492+
waitContext_ = std::make_shared<ItemWaitContext>(alloc);
493+
if (!alloc_->addWaitContextForMovingItem(it->getKey(), waitContext_)) {
494+
waitContext_->discard();
495+
waitContext_.reset();
496+
}
497+
}
498+
}
485499

486500
// handle that has a wait context allocated. Used for async handles
487501
// In this case, the it_ will be filled in asynchronously and mulitple

cachelib/allocator/MemoryTierCacheConfig.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ class MemoryTierCacheConfig {
7171

7272
const ShmTypeOpts& getShmTypeOpts() const noexcept { return shmOpts; }
7373

74+
private:
7475
// Ratio is a number of parts of the total cache size to be allocated for this
7576
// tier. E.g. if X is a total cache size, Yi are ratios specified for memory
7677
// tiers, and Y is the sum of all Yi, then size of the i-th tier
@@ -81,10 +82,6 @@ class MemoryTierCacheConfig {
8182
// Options specific to shm type
8283
ShmTypeOpts shmOpts;
8384

84-
private:
85-
// TODO: introduce a container for tier settings when adding support for
86-
// file-mapped memory
87-
8885
MemoryTierCacheConfig() = default;
8986
};
9087
} // namespace cachelib

cachelib/allocator/PoolOptimizer.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ void PoolOptimizer::optimizeRegularPoolSizes() {
5151

5252
void PoolOptimizer::optimizeCompactCacheSizes() {
5353
try {
54+
// TODO: should optimizer look at each tier individually?
55+
// If yes, then resizePools should be per-tier
5456
auto strategy = cache_.getPoolOptimizeStrategy();
5557
if (!strategy) {
5658
strategy = strategy_;

cachelib/allocator/Refcount.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,10 @@ class FOLLY_PACK_ATTR RefcountWithFlags {
116116
// unevictable in the past.
117117
kUnevictable_NOOP,
118118

119+
// Item is accecible but content is not ready yet. Used by eviction
120+
// when Item is moved between memory tiers.
121+
kIncomplete,
122+
119123
// Unused. This is just to indciate the maximum number of flags
120124
kFlagMax,
121125
};
@@ -336,6 +340,14 @@ class FOLLY_PACK_ATTR RefcountWithFlags {
336340
void unmarkNvmEvicted() noexcept { return unSetFlag<kNvmEvicted>(); }
337341
bool isNvmEvicted() const noexcept { return isFlagSet<kNvmEvicted>(); }
338342

343+
/**
344+
* Marks that the item is migrating between memory tiers and
345+
* not ready for access now. Accessing thread should wait.
346+
*/
347+
void markIncomplete() noexcept { return setFlag<kIncomplete>(); }
348+
void unmarkIncomplete() noexcept { return unSetFlag<kIncomplete>(); }
349+
bool isIncomplete() const noexcept { return isFlagSet<kIncomplete>(); }
350+
339351
// Whether or not an item is completely drained of access
340352
// Refcount is 0 and the item is not linked, accessible, nor exclusive
341353
bool isDrained() const noexcept { return getRefWithAccessAndAdmin() == 0; }

cachelib/allocator/memory/AllocationClass.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ AllocationClass::AllocationClass(ClassId classId,
5050
poolId_(poolId),
5151
allocationSize_(allocSize),
5252
slabAlloc_(s),
53-
freedAllocations_{slabAlloc_.createPtrCompressor<FreeAlloc>()} {
53+
freedAllocations_{slabAlloc_.createSingleTierPtrCompressor<FreeAlloc>()} {
5454
checkState();
5555
}
5656

@@ -102,7 +102,7 @@ AllocationClass::AllocationClass(
102102
currSlab_(s.getSlabForIdx(*object.currSlabIdx())),
103103
slabAlloc_(s),
104104
freedAllocations_(*object.freedAllocationsObject(),
105-
slabAlloc_.createPtrCompressor<FreeAlloc>()),
105+
slabAlloc_.createSingleTierPtrCompressor<FreeAlloc>()),
106106
canAllocate_(*object.canAllocate()) {
107107
if (!slabAlloc_.isRestorable()) {
108108
throw std::logic_error("The allocation class cannot be restored.");
@@ -356,9 +356,9 @@ std::pair<bool, std::vector<void*>> AllocationClass::pruneFreeAllocs(
356356
// allocated slab, release any freed allocations belonging to this slab.
357357
// Set the bit to true if the corresponding allocation is freed, false
358358
// otherwise.
359-
FreeList freeAllocs{slabAlloc_.createPtrCompressor<FreeAlloc>()};
360-
FreeList notInSlab{slabAlloc_.createPtrCompressor<FreeAlloc>()};
361-
FreeList inSlab{slabAlloc_.createPtrCompressor<FreeAlloc>()};
359+
FreeList freeAllocs{slabAlloc_.createSingleTierPtrCompressor<FreeAlloc>()};
360+
FreeList notInSlab{slabAlloc_.createSingleTierPtrCompressor<FreeAlloc>()};
361+
FreeList inSlab{slabAlloc_.createSingleTierPtrCompressor<FreeAlloc>()};
362362

363363
lock_->lock_combine([&]() {
364364
// Take the allocation class free list offline

0 commit comments

Comments
 (0)