-
Notifications
You must be signed in to change notification settings - Fork 915
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
support RMM aligned resource adapter in JNI [skip ci] #8266
Conversation
java/src/main/native/src/RmmJni.cpp
Outdated
Initialized_resource, allocation_alignment, alignment_threshold); | ||
} | ||
|
||
auto wrapped = make_tracking_adaptor(Initialized_resource.get(), RMM_ALLOC_SIZE_ALIGNMENT); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RMM_ALLOC_SIZE_ALIGNMENT
seems wrong if we know we're using the aligned adapter and a different alignment. I think there needs to be a max(RMM_ALLOC_SIZE_ALIGNMENT, allocation_alignment)
or something similar here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, the actual allocation size for aligned adapter is a bit complicated. Just curious, why are we tracking the total allocation size ourselves and not using the get_mem_info()
method from the device memory resource?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because many device memory resource implementations do not implement get_mem_info()
, the ARENA allocator apparently being one of them:
$ git grep supports_get_mem_info | grep false
benchmarks/utilities/simulated_memory_resource.hpp: bool supports_get_mem_info() const noexcept override { return false; }
include/rmm/mr/device/arena_memory_resource.hpp: bool supports_get_mem_info() const noexcept override { return false; }
include/rmm/mr/device/binning_memory_resource.hpp: bool supports_get_mem_info() const noexcept override { return false; }
include/rmm/mr/device/cuda_async_memory_resource.hpp: bool supports_get_mem_info() const noexcept override { return false; }
include/rmm/mr/device/fixed_size_memory_resource.hpp: bool supports_get_mem_info() const noexcept override { return false; }
include/rmm/mr/device/pool_memory_resource.hpp: bool supports_get_mem_info() const noexcept override { return false; }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I guess that api is really only for cuda. Added the max of two alignment sizes.
@gpucibot merge |
Depends on rapidsai/rmm#768.