pmem / CacheLib

Pluggable in-process caching engine to build and scale high performance services
https://www.cachelib.org
Apache License 2.0
5 stars 13 forks source link

Async eviction #16

Closed vinser52 closed 2 years ago

vinser52 commented 2 years ago

This change is Reviewable

victoria-mcgrath commented 2 years ago

cachelib/allocator/CacheAllocator-inl.h, line 1106 at r7 (raw file):

/* Next two methods are used to asynchronously move Item between memory tiers.
 *
 * The thread, which moves Item allocate, new Item in the tier we are moving to

Should comma go before "allocate"?

victoria-mcgrath commented 2 years ago

cachelib/allocator/CacheAllocator-inl.h, line 1109 at r7 (raw file):

 * and calls moveRegularItemOnEviction() method. This method does the following:
 *  1. Create MoveCtx and put it to the movesMap.
 *  2. Updates the access container with the new item from the tier we are

"Update"? For consistency with the rest of the bullets.

victoria-mcgrath commented 2 years ago

cachelib/allocator/CacheAllocator-inl.h, line 1113 at r7 (raw file):

 *  3. Copy data from the old Item to the new one.
 *  4. Unset thekNotReady flag and Notify MoveCtx
 * 

Extra spaces.

victoria-mcgrath commented 2 years ago

cachelib/allocator/CacheAllocator-inl.h, line 1106 at r7 (raw file):

Previously, victoria-mcgrath wrote…
Should comma go before "allocate"?

"allocates"

victoria-mcgrath commented 2 years ago

cachelib/allocator/CacheAllocator.h, line 1803 at r3 (raw file):

Previously, vinser52 (Sergei Vinogradov) wrote…
It was my initial idea. But folly::ConcurrentHashMap does not provide an interface that allows to modify mapped value in place. In our use case, we need to add waiters to the MoveCtx. But find method returns only a const iterator. There is `assign` method that allows to replace existing mapped value with the new one - it is not what we need as well.

Is extending an existing class an option?

igchor commented 2 years ago

Changes already merged into develop.