Use cuda::mr::memory_resource instead of raw device_memory_resource

[miscco]

This introduces cuda::mr::{async_}resource_ref as a type erased safe resource wrapper that is meant to replace uses of rmm::mr::{host, device}_memory_resource pointers.

We provide both async and classic allocate functions that delegate back to the original resource used to construct the cuda::mr::{async_}resource_ref

In comparison to {host, device}_memory_resource the new feature provides additional compile time checks that will help users avoid common pitfalls with heterogeneous memory allocations.

As a first step we provide the properties cuda::mr::host_accessible and cuda::mr::device_accessible. These properties can be added to an internal or even external type through a free function get_property

// For a user defined resource
struct my_resource {
  friend void get_property(my_resource const&, cuda::mr::device_accessible) noexcept {}
};

// For an external resource
void get_property(some_external_resource const&, cuda::mr::device_accessible) noexcept {}

The advantage is that we can constrain interfaces based on these properties

void do_some_computation_on_device(cuda::mr::async_resource_ref<cuda::mr::device_accessible> mr, ...) { ... }

This function will fail to compile if it is passed any resource that does not support async allocations or is not tagged as providing device accessible memory. In the same way the following function will only compile if the provided resource provides the classic allocate / deallocate interface and is tagged to provide host accessible memory

void do_some_computation_on_host(cuda::mr::resource_ref<cuda::mr::host_accessible> mr, ...) { ... }

The property system is highly flexible and can easily be user provided to add their own properties as needed. That gives it both the flexibility of an inheritance based implementation and the security of a strictly type checked interface

[GPUtester]

Can one of the admins verify this patch?

Admins can comment ok to test to allow this one PR to run or add to allowlist to allow all future PRs from the same author to run.

[jrhemstad]

@miscco so what's your current thinking about the steps we'd take to transition rapids libraries?

Ultimately we want to get rid of the device_memory_resource base class all together. Likewise transition all rapids apis from taking a device_memory_resource* to any_resource_ref<access::device>. But we'll have to work up to that point. The transition from passing a pointer to a reference will be annoying because it means everything will need to change all at the same time.

[miscco]

@miscco so what's your current thinking about the steps we'd take to transition rapids libraries?

Ultimately we want to get rid of the device_memory_resource base class all together. Likewise transition all rapids apis from taking a device_memory_resource* to any_resource_ref<access::device>. But we'll have to work up to that point. The transition from passing a pointer to a reference will be annoying because it means everything will need to change all at the same time.

I guess the advantage that we have is that the new interface itself is non intrusive. So my proposal would be to expand test coverage and merge this PR right away into rmm. We can then think about which parts of rapids to move over to the new shiny.

We might also want to think about adding a new interface first and then remove the old one as a second step afterwards

[vyasr]

Does this PR replace #883 and #840?

[miscco]

Does this PR replace #883 and #840?

Yes, that is the goal

[harrism]

ok to test

[harrism]

@miscco can you target this at branch-22.12? We don't target main with PRs, and 22.10 is entering code freeze later this week. (RAPIDS has an interesting branching model)

[harrism]

You will also need to run clang-format to pass the style checks.

[miscco]

Yeah, I am not fully ready with the device side memory resources. I hope to finish something this week

[bdice]

I discussed with @harrism -- he wants to finish testing downstream RAPIDS repos with commit 41c1bea. In the meantime, I created a branch with temporary changes from my PR review. https://github.com/bdice/rmm/tree/memory_resource-bdice

Feel free to merge those three commits into this PR when ready.

[bdice]

Approving as-is. Let's merge tonight (in the next 5-6 hours, ideally) and let the nightlies rebuild all RAPIDS packages. We'll iron out any problems (like the potential issue with cuGraph) once we see if the nightlies pass.

[harrism]

/merge

[miscco]

Thanks a lot for helping me getting this in 🎉