-
Notifications
You must be signed in to change notification settings - Fork 902
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
Remove unneeded temporary device vector for strings scatter specialization #7409
Remove unneeded temporary device vector for strings scatter specialization #7409
Conversation
Codecov Report
@@ Coverage Diff @@
## branch-0.19 #7409 +/- ##
==============================================
Coverage ? 82.23%
==============================================
Files ? 101
Lines ? 17060
Branches ? 0
==============================================
Hits ? 14029
Misses ? 3031
Partials ? 0 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cmake lgtm
@gpucibot merge |
The specialization logic for scatter on strings column includes building a temporary
device_vector
ofstring_view
objects for the source column. The builtin iterators for an inputcolumn_view
will work forstring_view
and so this extradevice_vector
is not required.The utilities for creating a
device_vector
forstring_view
s is also changed to usedevice_uvector
instead. This also removed an unnecessary parameter as well as other minor changes.I also added a gbenchmark for scatter that includes strings. Removing the extra
device_vector
showed a small 10-15% performance improvement.