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.

Sign up for GitHub

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

Jump to bottom

[FEA] Performance - search contains using slow approach #3806

Closed
ChuckHastings opened this issue Jan 16, 2020 · 1 comment · Fixed by #11202
Closed

[FEA] Performance - search contains using slow approach #3806

ChuckHastings opened this issue Jan 16, 2020 · 1 comment · Fixed by #11202
Assignees
@ttnghia
Labels
feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. Performance Performance related issue

Comments

@ChuckHastings
Copy link
Contributor

Is your feature request related to a problem? Please describe.
The contains (scalar) implementation uses a known inefficient thrust call (find).

Describe the solution you'd like
Instead of doing a find and comparing the result to end(), we should do a count_if and return true if count_if > 0.

Describe alternatives you've considered

Additional context
@ChuckHastings ChuckHastings added feature request New feature or request Needs Triage Need team to review and classify labels Jan 16, 2020
@kkraus14 kkraus14 added libcudf Affects libcudf (C++/CUDA) code. Performance Performance related issue and removed Needs Triage Need team to review and classify labels Jan 17, 2020
@ttnghia ttnghia self-assigned this Jun 30, 2022
@GregoryKimball
Copy link
Contributor

Thanks @ttnghia for taking a look. For context to this issue here is a very useful discussion in thrust

@ttnghia ttnghia mentioned this issue Jul 6, 2022
Merged
rapids-bot bot pushed a commit that referenced this issue Jul 8, 2022
@ttnghia
Improve performance for cudf::contains when searching for a scalar (#…
4d4632a 
…11202)

The current implementation of `cudf::contains(column_view, scalar)` uses `thrust::find` and `thrust::any_of` (which also calls `thrust::find_if` under the hood). These thrust APIs were known to have performance regression (https://github.com/NVIDIA/thrust/issues/1016).

This PR replaces `thrust::find` and `thrust::any_of` in `cudf::contains` by `thrust::count_if`, which improves performance significantly.
Benchmarks show that the run time can be reduced as much as 80% after modification, or up to 5X speedup.

Closes #3806.

Authors:
  - Nghia Truong (https://github.com/ttnghia)

Approvers:
  - Karthikeyan (https://github.com/karthikeyann)
  - Bradley Dice (https://github.com/bdice)

URL: #11202
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ttnghia ttnghia

Labels
feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. Performance Performance related issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Improve performance for cudf::contains when searching for a scalar ttnghia/cudf
4 participants
@kkraus14 @ttnghia @GregoryKimball @ChuckHastings