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.

Sign up for GitHub

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

Jump to bottom

[FEA] Make stream arguments for device_buffer explicit #418

Closed
jrhemstad opened this issue Jun 22, 2020 · 2 comments · Fixed by #775
Closed

[FEA] Make stream arguments for device_buffer explicit #418

jrhemstad opened this issue Jun 22, 2020 · 2 comments · Fixed by #775
Labels
feature request New feature or request inactive-30d inactive-90d

Comments

@jrhemstad
Copy link
Contributor

Is your feature request related to a problem? Please describe.

device_buffer member functions all take streams, but they currently default to the default stream. I think this is problematic as users may forget that the operation is stream ordered.

Describe the solution you'd like

Make the stream arguments explicit to require the user to be aware of the fact that the operations are stream ordered.
@jrhemstad jrhemstad added the feature request New feature or request label Jun 22, 2020
This was referenced Jun 23, 2020
Closed
Closed
@github-actions
Copy link

This issue has been marked rotten due to no recent activity in the past 90d. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed.

@github-actions
Copy link

This issue has been marked stale due to no recent activity in the past 30d. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be marked rotten if there is no activity in the next 60d.

@harrism harrism mentioned this issue Apr 28, 2021
Closed
@harrism harrism mentioned this issue May 19, 2021
Merged
4 tasks
@rapids-bot rapids-bot bot closed this as completed in #775 Jun 2, 2021
rapids-bot bot pushed a commit that referenced this issue Jun 2, 2021
@harrism
Explicit streams in device_buffer (#775)
7cbcd97 
Removes default stream arguments from `rmm::device_buffer`. Now copy construction requires a stream argument (default copy ctor deleted), and copy assignment is disallowed (operator deleted). Move construction and assignment are still supported, and move assignment still use the most recently used stream for deallocating any previous data.

Also improves device_buffer tests (implements TODOs in code).

I don't think this should be merged until RAPIDS dependent libraries are ready for it. I have a libcudf PR in progress for this.

Fixes #418

- [x] cuDF PR: rapidsai/cudf#8280
- [x] cuGraph PR: rapidsai/cugraph#1609
- [x] cuSpatial PR: rapidsai/cuspatial#403
- [x] cuML does not yet use device_buffer

Authors:
  - Mark Harris (https://github.com/harrism)

Approvers:
  - Rong Ou (https://github.com/rongou)
  - Keith Kraus (https://github.com/kkraus14)
  - Conor Hoekstra (https://github.com/codereport)
  - GALI PREM SAGAR (https://github.com/galipremsagar)

URL: #775
@harrism harrism mentioned this issue Jun 2, 2021
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature request New feature or request inactive-30d inactive-90d
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Explicit streams in device_buffer harrism/rmm
1 participant
@jrhemstad