-
Notifications
You must be signed in to change notification settings - Fork 904
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
Fast path for experimental::row::equality
#12676
Fast path for experimental::row::equality
#12676
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## branch-23.04 #12676 +/- ##
===============================================
Coverage ? 85.85%
===============================================
Files ? 158
Lines ? 25204
Branches ? 0
===============================================
Hits ? 21638
Misses ? 3566
Partials ? 0 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
group_nunique benchmarks
|
group_rank_scan benchmarks
|
rank_scan benchmarks
|
distinct benchmarks
|
unique benchmarks
|
contains_scalar benchmarks
|
Some of these look like big wins! |
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.
Do we know the performance impact on compilation for this change?
In general I think this is a nice run time performance and it might justify the compilation penalty, but I'd like to know what the penalty would be here.
Nits on a few places with _
on incoming variables and not on member variables.
@hyperbolic2346 I added the compilation impact to the description of this PR |
Co-authored-by: Nghia Truong <[email protected]>
Why do we have a V shape? Whyyyyyyy? |
It's good! The V is because this commit is faster than the baseline runs before and after. |
Oh I see. So the benchmarks in the graph are independent, aren't they? I thought that the latter commits merged into the former ones. |
@ttnghia in a way, the latter commits are merged into the former ones because the baseline is |
/merge |
This PR adds a fast path for primitive types similar to
experimental::row::lexicographic
. The compilation impact for building on bare-metal from source with command./build.sh libcudf tests benchmarks
for baseline16m43.607s
vs this branch17m13.987s
.This PR is a part of #12593.
Algorithms and benchmarks (those that were available are linked) affected by this change:
experimental::row::equality::self_comparator
group_nunique
group_rank_scan
rank_scan
contains_table
distinct
unique
rank
experimental::row::equality::two_table_comparator
struct_binary_ops
lists/contains
contains_scalar
(This algorithm does not need a primitive type optimization because the enclosing struct already type-dispatches based on nested vs non-nested types)contains_table
one_hot_encode
Checklist