-
Notifications
You must be signed in to change notification settings - Fork 915
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
Improve performance for replace-multi for long strings #12858
Conversation
@@ -704,92 +704,6 @@ std::unique_ptr<column> replace_slice(strings_column_view const& strings, | |||
cudf::detail::copy_bitmask(strings.parent(), stream, mr)); | |||
} | |||
|
|||
namespace { |
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.
This code moved to the new multi.cu
source file.
Values are in |
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.
Mostly nits here. I like this change and think it will be a great addition!
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.
Looks great with one minor non-blocking nit.
Thanks!
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.
Approving CMake changes.
/merge |
Description
Adds more efficient algorithm for multi-string version of
cudf::strings::replace
for longer strings (greater than 256 bytes on average in each row).Checklist