You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
cuML should make more of an effort to use containers (especially RMM device containers) and eradicate all instances of raw allocation.
I have recently been refactoring all RAPIDS repos to use rmm::device_async_resource_ref rather than device_memory_resource*. While these are partially compatible, the former has a different interface for allocate() and deallocate(), defined by cuda::mr::memory_resource concepts.
More disciplined libraries like libcudf and libcuspatial have NO raw allocation. Instead they use storage containers based on rmm::device_buffer (including rmm::device_uvector and rmm::device_scalar). In addition to having strong stream-ordered semantics, these containers take resource_refs and abstract the calls to allocate and deallocate. Therefore, refactoring libcudf and libcuspatial has been nearly trivial, since the interface changes are all covered by RMM classes. The calling library only needs to pass around the resource_refs.
In contrast cuML has multiple places that directly call the allocate()/deallocate() functions of the memory resource (at least they aren't calling cudaMalloc directly!). For example:
Here we have a struct that has raw pointer members (first No-no). This struct can be either own the memory or be a view of the memory (2nd no-no). This is achieved by providing allocate() and deallocate() member functions that use raw calls to allocate/deallocate (3rd no-no). I tried changing the struct so that rather than raw pointers it uses device_uvector, but those can't be used as non-owning views, so this doesn't work.
Other locations where cuML uses raw allocation are in svm/results.cuh, svm/svc_impl.cuh.
Also, cpp/test/sg/svc_test.cu has a BIG no-no use where it ONLY calls the current device resource's deallocate() function even though the allocation happens elsewhere (somewhere I can't find). This is really bad because it's not guaranteed that the same resource was used to allocate the memory.
The text was updated successfully, but these errors were encountered:
…rce_ref (#5853)
Closes#5839
Replaces all occurrences of rmm::mr::device_memory_resource* in parameters with rmm::device_async_resource_ref. Also updates calls to raw `resource->allocate()` to use `resource_ref.allocate_async()` with default alignment argument. Same for `deallocate()`.
Ideally would replace these with `device_uvector` / `device_buffer` but that is outside scope of this PR. See #5852
Authors:
- Mark Harris (https://github.com/harrism)
Approvers:
- Bradley Dice (https://github.com/bdice)
- Dante Gama Dessavre (https://github.com/dantegd)
- Divye Gala (https://github.com/divyegala)
URL: #5853
Describe the bug
cuML should make more of an effort to use containers (especially RMM device containers) and eradicate all instances of raw allocation.
I have recently been refactoring all RAPIDS repos to use
rmm::device_async_resource_ref
rather thandevice_memory_resource*
. While these are partially compatible, the former has a different interface forallocate()
anddeallocate()
, defined bycuda::mr::memory_resource
concepts.More disciplined libraries like libcudf and libcuspatial have NO raw allocation. Instead they use storage containers based on
rmm::device_buffer
(includingrmm::device_uvector
andrmm::device_scalar
). In addition to having strong stream-ordered semantics, these containers take resource_refs and abstract the calls to allocate and deallocate. Therefore, refactoring libcudf and libcuspatial has been nearly trivial, since the interface changes are all covered by RMM classes. The calling library only needs to pass around the resource_refs.In contrast cuML has multiple places that directly call the
allocate()
/deallocate()
functions of the memory resource (at least they aren't calling cudaMalloc directly!). For example:cuml/cpp/include/cuml/tsa/arima_common.h
Lines 62 to 113 in 654d95a
Here we have a struct that has raw pointer members (first No-no). This struct can be either own the memory or be a view of the memory (2nd no-no). This is achieved by providing
allocate()
anddeallocate()
member functions that use raw calls to allocate/deallocate (3rd no-no). I tried changing the struct so that rather than raw pointers it usesdevice_uvector
, but those can't be used as non-owning views, so this doesn't work.Other locations where cuML uses raw allocation are in
svm/results.cuh
,svm/svc_impl.cuh
.Also,
cpp/test/sg/svc_test.cu
has a BIG no-no use where it ONLY calls the current device resource'sdeallocate()
function even though the allocation happens elsewhere (somewhere I can't find). This is really bad because it's not guaranteed that the same resource was used to allocate the memory.The text was updated successfully, but these errors were encountered: