Replace raw streams with rmm::cuda_stream_view (part 2) #6648
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
2 - In Progress
|
Please update the changelog in order to start CI tests. View the gpuCI docs here. |
Codecov Report
@@ Coverage Diff @@
## branch-0.17 #6648 +/- ##
===============================================
+ Coverage 81.76% 81.82% +0.05%
===============================================
Files 96 96
Lines 16016 16016
===============================================
+ Hits 13096 13105 +9
+ Misses 2920 2911 -9
Continue to review full report at Codecov.
|
kkraus14
approved these changes
Nov 13, 2020
So sorry I'm so used to clang-format now I didn't even notice that... Maybe we should turn on clang-format for the java directory in @codereport 's clang-format upgrade PR? |
devavret
reviewed
Nov 13, 2020
cwharris
approved these changes
Nov 18, 2020
devavret
approved these changes
Nov 18, 2020
harrism
added
5 - Ready to Merge
jlowe
added a commit
that referenced
this pull request
Nov 20, 2020
This fixes the JNI build after #6648. The JNI data_sink was updated to use RMM stream views before the data_sink API was updated.
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_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
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: