Replace raw streams with rmm::cuda_stream_view (part 1) #6646
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
harrism
added
3 - Ready for Review
|
Please update the changelog in order to start CI tests. View the gpuCI docs here. |
Codecov Report
@@ Coverage Diff @@
## branch-0.17 #6646 +/- ##
===============================================
- Coverage 82.27% 81.79% -0.48%
===============================================
Files 94 95 +1
Lines 15533 16320 +787
===============================================
+ Hits 12779 13349 +570
- Misses 2754 2971 +217
Continue to review full report at Codecov.
|
devavret
suggested changes
Nov 3, 2020
harrism
commented
Nov 3, 2020
davidwendt
approved these changes
Nov 3, 2020
devavret
approved these changes
Nov 5, 2020
|
I moved the CMake changes out to #6732 |
|
This PR doesn't actually need java-codeowners or cmake-codeowners reviews, those changes have been merged separately. |
This seems to not be a problem. Everything builds OK. This is because of the implicit conversion from |
harrism
added
5 - Ready to Merge
kkraus14
approved these changes
Nov 11, 2020
|
rerun tests |
|
I can't repro the dask_cudf test failures locally. What's up with those? |
|
rerun tests |
1 similar comment
|
rerun tests |
This was referenced Nov 13, 2020
harrism
added a commit
that referenced
this pull request
Nov 20, 2020
Converting libcudf to use rmm::cuda_stream_view will require a LOT of changes, so I'm splitting it into multiple PRs to ease reviewing. This is the second PR in the series. This series of PRs will
Replace usage of cudaStream_t with rmm::cuda_stream_view
Replace usage of 0 or nullptr as a stream identifier with rmm::cuda_stream_default
Ensure all APIs always order the stream parameter before the memory resource parameter. #5119
Contributes to #6645 and #5119
Depends on #6646 so this PR will look much bigger until that one is merged.
Also fixes #6706 (to_arrow and to_dlpack are not properly synchronized).
This second PR converts:
table.hpp (and source / dependencies)
column_factories.hpp (and source / dependencies)
headers in include/detail/aggregation (and source / dependencies)
include/detail/groupby/sort_helper.hpp (and source / dependencies)
include/detail/utilities/cuda.cuh (and dependencies)
binary ops
concatenate
copy_if
copy_range
fill
gather
get_value
hash groupby
quantiles
reductions
repeat
replace
reshape
round
scatter
search
sequence
sorting
stream compaction
harrism
added a commit
to rapidsai/cuspatial
that referenced
this pull request
Nov 24, 2020
This PR converts all usage of cudaStream_t in cuSpatial to rmm::cuda_stream_view, following on from rapidsai/cudf#6646 and rapidsai/cudf#6648 Also reorders stream parameters to occur before MR parameters in all functions.
kkraus14
pushed a commit
that referenced
this pull request
Nov 24, 2020
Converting libcudf to use `rmm::cuda_stream_view` will require a LOT of changes, so I'm splitting it into multiple PRs to ease reviewing. This is the third PR in the series. This series of PRs will - Replace usage of `cudaStream_t` with `rmm::cuda_stream_view` - Replace usage of `0` or `nullptr` as a stream identifier with `rmm::cuda_stream_default` - Ensure all APIs always order the stream parameter before the memory resource parameter. #5119 Contributes to #6645 and #5119 Depends on #6646 and #6648 so this PR will look much bigger until they are merged. This third PR converts: - remaining dictionary functionality - cuio - lists - scalar - strings - groupby - join - contiguous_split - get_element - datetime_ops - extract - merge - partitioning - minmax reduction - scan - byte_cast - clamp - interleave_columns - is_sorted - groupby - rank - tests - concurrent map classes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Converting libcudf to use
rmm::cuda_stream_viewwill require a LOT of changes, so I'm splitting it into multiple PRs to ease reviewing. This is the first PR in the series. This series of PRs willcudaStream_twithrmm::cuda_stream_view0ornullptras a stream identifier withrmm::cuda_stream_defaultThis first PR converts:
namespace detailand adds streamless public versions.namespace detailand adds streamless public versions.Contributes to #6645 and #5119
Depends on #6732.