Extend admission policies, statistics, add multi thread eviction and promotion#2
Open
igchor wants to merge 62 commits intointel:developfrom
Open
Extend admission policies, statistics, add multi thread eviction and promotion#2igchor wants to merge 62 commits intointel:developfrom
igchor wants to merge 62 commits intointel:developfrom
Conversation
It's implementation is mostly based on PosixShmSegment. Also, extend ShmManager and ShmSegmentOpts to support this new segment type.
After introducing file segment type, nameToKey_ does not provide enough information to recover/remove segments on restart. This commit fixes that by replacing nameToKey_ with nameToOpts_. Previously, the Key from nameToKey_ map was only used in a single DCHECK().
* New class MemoryTierCacheConfig allows to configure a memory tier. Setting tier size and location of a file for file-backed memory are supported in this initial implementation; * New member, vector of memory tiers, is added to class CacheAllocatorConfig. * New test suite, chelib/allocator/tests/MemoryTiersTest.cpp, demonstrates the usage of and tests extended config API.
to allow using new configureMemoryTiers() API with legacy behavior. Move validation code for memory tiers to validate() method and convert ratios to sizes lazily (on get)..
It wrongly assumed that the only possible segment type is PosixSysV segment.
… handling when NVM cache state is not enabled
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.
Without this fix removeCb called even in case when Item is moved between tiers.
It fails because CentOS is EOL. We might want to consider using CentOS Streams but for now, just remove it. Right now, we rely on build-cachelib-centos workflow anyway.
Disabled test suite allocator-test-AllocatorTypeTest to skip sporadically failing tests.
…m#43) Compensation results in ratios being different than originially specified.
The main purpose of this patch is to better simulate workloads in cachebench. Setting touchValue to true allows to see performance impact of using different mediums for memory cache.
Extend cachbench with touch value
* 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 <victoria.mcgrath@intel.com>
Add memory usage statistics for slabs and allocation classes
Add option to print memory stats in bytes only
Added per tier pool class rolling average latency
Hot queue iterator for 2Q. Will start at Hot queue and move to Warm queue if hot queue is exhausted. Useful for promotion semantics if using 2Q replacement. rebased on to develop and added some tests.
b7300e2 to
a3e29dd
Compare
vinser52
reviewed
Aug 10, 2022
| }; | ||
|
|
||
| template <typename CacheT> | ||
| class BackgroundPromoter : public PeriodicWorker { |
There was a problem hiding this comment.
BackgroundPromoter and BackgroundEvictor classes look the same. Do we really need two classes?
| } | ||
| if (numTiers_ > 1 && !config_.moveCb) { | ||
| XLOG(WARN, "No moveCb set, using memcpy for moving items between tiers."); | ||
| config_.moveCb = [](Item& oldItem, Item& newItem, Item* parentItem){ |
There was a problem hiding this comment.
Some legacy logic depends on whether moveCb is set or not. Are we sure that we are not breaking legacy behavior?
| uint32_t creationTime, | ||
| uint32_t expiryTime) { | ||
| uint32_t expiryTime, | ||
| bool fromEvictorThread) { |
There was a problem hiding this comment.
Does fromEvictorThread also set to true when it is called from promoter thread? If so, let´s rename variable to have more generic name, e.g. fromBgThread?
and add additional parameters to control allocation and eviction of items. Co-authored-by: Daniel Byrne <byrnedj12@gmail.com>
a3e29dd to
a2721d1
Compare
vinser52
pushed a commit
that referenced
this pull request
Sep 12, 2022
Summary: AdRanker ASAN canary flagged a possible UBSan violation. ## Error Failed Run: https://fburl.com/servicelab/apytosry ``` #0 0x562e3adb59bc in facebook::cachelib::objcache2::ObjectCacheSizeController<facebook::cachelib::CacheAllocator<facebook::cachelib::LruCacheTrait> >::work() buck-out/v2/gen/fbcode/47d914adeee3d982/cachelib/experimental/objcache2/__object-cache__/headers/cachelib/experimental/objcache2/ObjectCacheSizeController-inl.h #1 0x562de7610f78 in facebook::cachelib::PeriodicWorker::loop() fbcode/cachelib/common/PeriodicWorker.cpp:55 #2 0x7f7632c524e4 in execute_native_thread_routine /home/engshare/third-party2/libgcc/11.x/src/gcc-11.x/x86_64-facebook-linux/libstdc++-v3/src/c++11/../../../.././libstdc++-v3/src/c++11/thread.cc:82:18 #3 0x7f7632f6ec0e in start_thread /home/engshare/third-party2/glibc/2.34/src/glibc-2.34/nptl/pthread_create.c:434:8 #4 0x7f76330011db in clone3 /home/engshare/third-party2/glibc/2.34/src/glibc-2.34/misc/../sysdeps/unix/sysv/linux/x86_64/clone3.S:81 UndefinedBehaviorSanitizer: integer-divide-by-zero buck-out/v2/gen/fbcode/47d914adeee3d982/cachelib/experimental/objcache2/__object-cache__/headers/cachelib/experimental/objcache2/ObjectCacheSizeController-inl.h:33:40 in ``` Reviewed By: jiayuebao Differential Revision: D39024188 fbshipit-source-id: 64ad644c360565e638fa3ca74616a315038382ab
Merged
f6ad2ae to
09d7bab
Compare
09d7bab to
ebfca17
Compare
9ba4e79 to
c1ff397
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change is