Skip to content
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

[FEA] Option to 4KiB align large buffers #762

Closed
rongou opened this issue Apr 22, 2021 · 4 comments · Fixed by #768
Closed

[FEA] Option to 4KiB align large buffers #762

rongou opened this issue Apr 22, 2021 · 4 comments · Fixed by #768
Labels
feature request New feature or request proposal Change current process or code

Comments

@rongou
Copy link
Contributor

rongou commented Apr 22, 2021

Is your feature request related to a problem? Please describe.
For spark-rapids we can spill device buffers to disk using GPUDirect Storage (GDS). With GDS it's more efficient to read/write 4KiB-aligned buffers. Unaligned buffers incur an additional copy to a bounce buffer.

Both the pool_memory_resource and arena_memory_resource are currently hard coded to align at 256 bytes. It'd be helpful to align at 4KiB for larger buffers.

Describe the solution you'd like
Add an optional constructor parameter to both pool_memory_resource and arena_memory_resource to allow aligning large buffers differently, e.g. large_buffer_allocation_alignment so that the user can set it to 4096 if desired. For any allocation with size below that threshold, it can still be aligned at 256 bytes; for requests with size larger than that, use the specified parameter for alignment.

Describe alternatives you've considered
I can add this only to arena_memory_resource, but then its behavior would diverge from pool_memory_resource. We currently do support selecting either MR in the spark-rapids plugin.

Additional context
GDS doc that talks about alignment: https://docs.nvidia.com/gpudirect-storage/best-practices-guide/index.html.

@harrism @jrhemstad @jlowe @abellina

@rongou rongou added feature request New feature or request ? - Needs Triage Need team to review and classify labels Apr 22, 2021
@kkraus14 kkraus14 added proposal Change current process or code and removed ? - Needs Triage Need team to review and classify labels Apr 22, 2021
@kkraus14
Copy link
Contributor

cc @vuule as well. I imagine this has uses in cuIO

@jrhemstad
Copy link
Contributor

This will be resolved by the new cuda::memory_resource interface that includes alignment in the allocate/deallocate parameters.

@rongou
Copy link
Contributor Author

rongou commented Apr 22, 2021

It'll require each caller to set the alignment explicitly on every call though, right? Is there an option to change the default alignment for the memory resource? Also is there an ETA?

@jrhemstad
Copy link
Contributor

jrhemstad commented Apr 22, 2021

It'll require each caller to set the alignment explicitly on every call though, right?

Nothing that can't be handled with an adaptor or in the resource implementation.

Is there an option to change the default alignment for the memory resource?

You can do whatever you want inside a resource implementation.

Also is there an ETA?

It will be a release or two before we're incorporating this into RMM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request proposal Change current process or code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants