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

Move rmm::device_buffer instead of copying #403

Merged
merged 1 commit into from
May 21, 2021

Conversation

harrism
Copy link
Member

@harrism harrism commented May 20, 2021

Simple PR that adds a std::move around passing a device_buffer into a column factory. This will be necessary once rapidsai/rmm/#775 is merged.

@harrism harrism added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels May 20, 2021
@harrism harrism self-assigned this May 20, 2021
@harrism harrism requested a review from a team as a code owner May 20, 2021 01:03
@github-actions github-actions bot added the libcuspatial Relates to the cuSpatial C++ library label May 20, 2021
@harrism harrism requested review from cwharris and removed request for thomcom and zhangjianting May 20, 2021 01:03
@harrism
Copy link
Member Author

harrism commented May 20, 2021

It's a little late but this is a trivial fix that will help avoid breakages later.

@harrism
Copy link
Member Author

harrism commented May 20, 2021

Actually it's not late. cuSpatial burn down isn't until next week. :)

@harrism
Copy link
Member Author

harrism commented May 21, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 2e45b3f into rapidsai:branch-21.06 May 21, 2021
rapids-bot bot pushed a commit to rapidsai/rmm that referenced this pull request Jun 2, 2021
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function libcuspatial Relates to the cuSpatial C++ library non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants