NVIDIA / spark-rapids-jni

RAPIDS Accelerator JNI For Apache Spark
Apache License 2.0
36 stars 64 forks source link

Switch `rmm::get_current_device_resource` to use references via `rmm::get_current_device_resource_ref` #2372

Closed hyperbolic2346 closed 1 month ago

hyperbolic2346 commented 1 month ago

This changes the use of rmm::get_current_device_resource to rmm::get_current_device_resource_ref to match up with the RMM incoming deprecation. I don't know when the original methods will be deprecated, but the new methods are available now, so it seemed a good idea to switch over.

closes #2358

hyperbolic2346 commented 1 month ago

build

harrism commented 1 month ago

@hyperbolic2346 @abellina you guys are getting a step ahead of me. Probably would have been nice to wait on https://github.com/rapidsai/cudf/pull/16679 so you can use the cudf wrappers. That way if we need to change anything we can do it in one place in cudf.

hyperbolic2346 commented 1 month ago

@harrism sorry about that. I will include that wrapper once it is merged.

abellina commented 1 month ago

Sorry @hyperbolic2346 and @harrism I didn't realize https://github.com/rapidsai/cudf/pull/16679 was in the works.

harrism commented 1 month ago

BTW, I believe that this PR introduces a bug until rapidsai/rmm/#1661 is merged. That PR makes get_current_device_resource_ref() get the ref of the MR set with set_current_device_resource(). Before that change, resource_refs and resource pointers were kept separate because the ref APIs were not intended to be used across RAPIDS yet.

abellina commented 4 weeks ago

Thanks @harrism we'll revert this for now until the RMM pr is merged.

harrism commented 3 weeks ago

OK, rapidsai/rmm#1661 is now merged.