Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NUMA bindigs support for private memory #195

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vinser52
Copy link
Contributor

@vinser52 vinser52 commented Feb 20, 2023

This PR adds NUMA binding support for private memory. I tried to align the implementation with shared memory approach and introduced PrivateMemoryManager. By current design, SlabAllocator allocates and owns private memory but shared memory is supplied as external memory to SlabAllocator. With this PR the memory is always supplied as external to SlabAllocator and probably we have to clean up SlabAllocator implementation because it is not required to own memory anymore.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 20, 2023
@vinser52
Copy link
Contributor Author

vinser52 commented Apr 4, 2023

Hi @therealgymmy, could you please take a look at this PR? It was opened more than a month ago.

@therealgymmy
Copy link
Contributor

@vinser52: apologies for the late review.

Instead of supporting this, I think we should deprecate private memory altogether. Today when a cachelib user has configued memory-monitor (where we may madvise away pages to make more memory available for the rest of the system), we will always use SHM.

This means we will just support Shm for multi-tier memory, and internally we can start moving everyone off private memory mode.

https://github.com/facebook/CacheLib/blob/main/cachelib/allocator/CacheAllocator-inl.h#L53-L54

CacheAllocator<CacheTrait>::CacheAllocator(
    typename CacheAllocator<CacheTrait>::InitMemType type, Config config)
    : isOnShm_{type != InitMemType::kNone ? true
                                          : config.memMonitoringEnabled()},

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants