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

Provide stream-aware containers #823

Open
upsj opened this issue Oct 27, 2022 · 3 comments
Open

Provide stream-aware containers #823

upsj opened this issue Oct 27, 2022 · 3 comments
Labels
thrust For all items related to Thrust.

Comments

@upsj
Copy link

upsj commented Oct 27, 2022

device_ptr and device_reference use the stream provided by the CUDA system (probably the default stream, though I didn't check). That means that there is no (to me obvious) way to use them safely with the par_nosync execution policy or in other non-default stream contexts. They are a really convenient tool for copying single values between host and device, so it would be really helpful if we could find a way to make them stream-aware, or provide a stream-aware alternative.

This issue came up in rapidsai/cudf#11919 (comment)

@jrhemstad
Copy link
Collaborator

it would be really helpful if we could find a way to make them stream-aware, or provide a stream-aware alternative.

I don't think it will be possible/wise to make them stream aware in a safe and easy way. These entities try and emulate a normal pointer and reference where there is no opportunity to inject a stream. For example device_vector::operator[] can't take a stream. Or device_ptr's deference operator can't take a stream.

While there is no arguing that they are convenient, their convenience can be a tremendous footgun for both stream ordering and hiding host/device copies. This is why RMM's device_uvector doesn't attempt to provide the device_ptr/device_reference functionality and instead opts for explicit element-wise functions that accept a stream.

There is certainly room for improvement with stream-ordered data structures in Thrust and beyond. This is something that @griwes will be thinking about and working on in the medium-term future.

@lilohuang
Copy link

No need to make device_vector::operator[] to take a stream. The device_vector should provide a constructor with stream-aware (or execution policy) like device_uvector, and then device_ptr or its base class should hold the associated stream to make data access to be stream-aware.

@jrhemstad jrhemstad added the thrust For all items related to Thrust. label Feb 22, 2023
@jrhemstad jrhemstad changed the title device_ptr and device_reference are not stream-aware Provide stream-aware containers Mar 7, 2023
@jrhemstad
Copy link
Collaborator

I'll keep this issue open for now, but it's likely that we'll solve this problem in libcu++ and not Thrust.

@jarmak-nv jarmak-nv transferred this issue from NVIDIA/thrust Nov 8, 2023
@github-project-automation github-project-automation bot moved this to Todo in CCCL Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
thrust For all items related to Thrust.
Projects
Status: Todo
Development

No branches or pull requests

3 participants