Add managed option for RMM Pool memory resource to C API
#305
Conversation
[RELEASE] v24.04
[RELEASE] cuvs v24.06
…nagedTensor function.
…nto c-add-memory-pool
|
Hi @cjnolet I think this implementation using |
|
/ok to test |
|
/ok to test |
|
/ok to test |
There was a problem hiding this comment.
I left two nonblocking comments. This PR did not require a packaging/CI review (which was requested). This PR had some complicated merge history and so it may have needed a review from some changes that were ultimately not in the final changeset. In the future you can try to deselect the packaging/CI review groups. If they are required, you won't be able to deselect them. If you can deselect them, the review wasn't really needed.
| @@ -17,6 +17,7 @@ | |||
| #pragma once | |||
|
|
|||
| #include <cuda_runtime.h> | |||
| #include <stdbool.h> | |||
There was a problem hiding this comment.
I'm not a C wizard -- but is this include really needed? It sounds like it's a compatibility shim for super old-school C code. https://stackoverflow.com/questions/47374356/why-use-stdbool-h-instead-of-bool
I'd be happy to learn more -- but this might have just been an include automatically added by an IDE?
| std::make_shared<rmm::mr::managed_memory_resource>()); | ||
| } else { | ||
| mr = std::static_pointer_cast<rmm::mr::device_memory_resource>( | ||
| std::make_shared<rmm::mr::cuda_memory_resource>()); |
There was a problem hiding this comment.
Should we use an async memory resource by default?
ajit283
force-pushed
the
c-add-memory-pool
branch
from
e876dbe to
8ba0268
Compare
|
/ok to test |
|
/ok to test |
|
/merge |
No description provided.