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] device_buffer constructor with existing gpu memory and without data copying #874

Closed
ahmet-uyar opened this issue Sep 17, 2021 · 4 comments
Labels
? - Needs Triage Need team to review and classify feature request New feature or request

Comments

@ahmet-uyar
Copy link

Is your feature request related to a problem? Please describe.
We are developing a distributed dataframe processing engine using CuDF (https://github.com/cylondata/cylon).
We would like to gather multiple tables from multiple workers each running on a separate gpu.
Each worker is working on the same table template with different data.
We collect the tables by using MPI_Gather.
First we gather the first columns of the tables from all workers, then the second columns, and so on.

On the receiving worker, a device_buffer is created to receive buffers from all workers. This buffer holds received data consecutively in the gpu memory.
So, when receiving column 0 data buffer from all workers, receiving device_buffer has the data in the following format:

<worker0 data><worker1 data>...<worker(n-1) data>

On the receiving worker, we would like to create a CuDF column for each table separately that requires a rmm::device_buffer.
Now, we are using the constructor:

 
device_buffer (void const *source_data, 
                std::size_t size, 
                cuda_stream_view stream, 
                mr::device_memory_resource *mr=mr::get_current_device_resource())

However, this copies the data. This copying is not necessary in our case. Since the data is already on the gpu and nobody else will use that.

Describe the solution you'd like
We would like a device_buffer constructor that takes a gpu memory address and the data size. It will assume that the data is already there and it will not perform a copy. Basically the signature of the above constructor is fine as long as it does not copy the source data.

Describe alternatives you've considered
We are using the following constructor currently but that performs a memory copy of the data.

 
device_buffer (void const *source_data, 
                std::size_t size, 
                cuda_stream_view stream, 
                mr::device_memory_resource *mr=mr::get_current_device_resource())

Additional context
Add any other context, code examples, or references to existing implementations about the feature request here.

@ahmet-uyar ahmet-uyar added ? - Needs Triage Need team to review and classify feature request New feature or request labels Sep 17, 2021
@jrhemstad
Copy link
Contributor

On the receiving worker, we would like to create a CuDF column for each table separately that requires a rmm::device_buffer.

What you want to use instead of column is a column_view.

column_view is a "non-owning" type and was designed specifically for these kinds of situations where you have some existing data that wasn't created as a cudf::column but you want to operate on in cudf. You can see https://github.com/rapidsai/cudf/blob/branch-21.10/cpp/docs/DEVELOPER_GUIDE.md#views-and-ownership for more information.

@ahmet-uyar
Copy link
Author

@jrhemstad thanks for the reply.

So we can construct a large table after MPI gather having the data of all transmitted tables. Then, we can create table_views for each received table from each worker. That should actually work.

@harrism
Copy link
Member

harrism commented Sep 21, 2021

Regarding the request for device_buffer on existing memory, I don't think we would add this as device_buffer is an owning container, not a view. For general use, what we would use here is likely some form of std::span, which we would like to add to either libcu++ or Thrust in the future.

@harrism
Copy link
Member

harrism commented Sep 21, 2021

I'm going to close this, but please feel free to re-open if you don't feel this is resolved. Thanks for the well-written feature request, @ahmet-uyar!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
? - Needs Triage Need team to review and classify feature request New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants