[FEA] Consider creating a pinned host memory resource that can leverage all of the allocation strategies

[felipeblazing]

trafficstars

Is your feature request related to a problem? Please describe. I wish I could allocate a pool of pinned host memory for allocations that will often times be short lived. An example of when we would use this is when sending information between two nodes using the tcp protocol. We have to copy the memory to host in order to send it over the wire. Having a pinned memory buffer would make this more efficient.

Describe the solution you'd like Allow the memory providers to either specify specific low level allocators or some way of being able to do this for just the specific case of pinned memory.

Describe alternatives you've considered We currently have a fixed size pinned memory pool that we use for this situation.

[harrism]

Since pinned host memory is accessible as device memory, I think we should create a pinned_memory_resource that is a device_memory_resource. Then it can be used as upstream of any other resource. The only sticking point I can think of is naming, since we already have a pinned_memory_resource that is a host_memory_resource.

[github-actions[bot]]

This issue has been marked rotten due to no recent activity in the past 90d. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed.

[github-actions[bot]]

This issue has been marked stale due to no recent activity in the past 30d. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be marked rotten if there is no activity in the next 60d.

[harrism]

This is still relevant, but now depends on https://github.com/NVIDIA/libcudacxx/pull/105

[github-actions[bot]]

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

[leofang]

Maybe relevant to cupy/cupy#4892?

[github-actions[bot]]

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

[github-actions[bot]]

This issue has been labeled inactive-90d due to no recent activity in the past 90 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed.

[rongou]

I'll take a crack at this since we also need it for Spark.

[harrism]

If you proceed ahead of cuda::memory_resource this will just create more for us to refactor.

[rongou]

Is there an ETA on cuda::memory_resource?

[harrism]

I thought we were close before Christmas. But we keep running into more design issues.

[jakirkham]

Admittedly there may not be a clear answer to this, but are there ways for others interested in this to help pull cuda::memory_resource across the finish line?

[github-actions[bot]]

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

[rongou]

Chatted with @jrhemstad offline, one option is to move the current pinned_memory_resource from host to device, then we can wrap it with the existing pools.

@abellina

[jrhemstad]

Chatted with @jrhemstad offline, one option is to move the current pinned_memory_resource from host to device, then we can wrap it with the existing pools.

No need to move it, just make a new one that inherits from device_memory_resource instead of host_memory_resource. I think we could even make the existing inherit from both since the functions have different signatures in host vs device.

[github-actions[bot]]

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

[github-actions[bot]]

This issue has been labeled inactive-90d due to no recent activity in the past 90 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed.

[abellina]

Chatted with @jrhemstad offline, one option is to move the current pinned_memory_resource from host to device, then we can wrap it with the existing pools.

No need to move it, just make a new one that inherits from device_memory_resource instead of host_memory_resource. I think we could even make the existing inherit from both since the functions have different signatures in host vs device.

I have been playing with this for a test https://github.com/rapidsai/cudf/issues/14314 in cuDF. I think what @jrhemstad said here around the APIs being different between host and device allocations is important. Actually, why would we want allocate(size, stream) and deallocate(size, stream) and why would we want to maintain the stream ordered behavior of pool_memory_resource for the host?

Just for reference, what I did was I hacked out the streams and had a single free_list for the whole pool, then just had an allocate(size) and it did the same pool logic: allocate from blocks in the list, split, merge, etc just with the single pool.

I do think we want to invest time on this given the results I am seeing in https://github.com/rapidsai/cudf/issues/14314, so I'd be curious to know if anyone has cycles to take a look at this.

[harrism]

Actually, why would we want allocate(size, stream) and deallocate(size, stream) and why would we want to maintain the stream ordered behavior of pool_memory_resource for the host?

Because pinned host memory is accessed by device copies and kernels, which are stream ordered. Any non-stream-ordered allocation would have to synchronize, losing a lot of the benefits of stream-ordered allocation. I think it would be a mistake not to provide a stream-ordered pinned host memory pool.

so I'd be curious to know if anyone has cycles to take a look at this.

I am planning to work on this because I am also finding it is needed to improve spilling performance for Python. Current plan is to do it once we adopt cuda::memory_resource, because it turns the kind of memory allocated by the MR into a (template) parameter, rather than having separate host_mr and device_mr types. This enables us to use any machinery built for allocating one kind of memory (e.g. device memory) for any other memory kind. cuda::memory_resource also separates synchronous and stream-ordered allocation APIs.

This said, we should also look into timing and cost (including opportunity cost) and

1. whether a temporary stop-gap like described above is worth committing to RMM
2. whether we should just wait for cuda::memory_resource to be merged and then build it "the right way"
3. whether we should provide a stop-gap in a non-release branch of RMM and then do (2).