-
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
Faster struct row comparator #10164
Merged
Merged
Faster struct row comparator #10164
Changes from 1 commit
Commits
Show all changes
58 commits
Select commit
Hold shift + click to select a range
933c974
First commit
devavret a1636e5
testing and profiling deep single hierarchy struct
devavret d59f54c
Merge branch 'branch-22.02' into struct-row-comp
devavret 765dd8d
Merge branch 'branch-22.02' into struct-row-comp
devavret 3d21daf
Make the sandboxed test compile again
devavret 9f32e6b
Update my row_comparator with nullate
devavret 53d3c90
Merge branch 'branch-22.02' into struct-row-comp
devavret 022e2a4
Basic verticalization utility and experimental namespace
devavret 7fef643
clean up most of row operators that I didn't change.
devavret 930d8de
Sliced column test
devavret 0ecc4f8
column order and null precendence support
devavret ff36d2d
Manually managed stack
devavret cd0f938
New depth based method to avoid superimpose nulls
devavret 7b8e060
Put sort2 impl in separate TU
devavret 25eb237
Merge branch 'branch-22.04' into struct-row-comp
devavret d2937cf
Merge branch 'branch-22.04' into struct-row-comp
devavret d55c9c7
Move verticalization code to row_comparator.cpp
devavret 3bd749e
Owning row lex operator
devavret 613d664
merge fixes
devavret 2ef3ac7
Move struct logic out of main row loop and into element_relational_co…
devavret 5577431
pushing even more logic into element_relational_comparator
devavret f037bc0
More optimizations.
devavret 8c54a85
review changes
devavret 9d24a87
Checks to ensure tables can be compared
devavret 294b0cf
Another attempt at new API
devavret a4c799a
Remove stack based struct comparator + cleanups
devavret ecb2eb0
thrust::pair -> cuda::std::pair
devavret 34a6564
optional device spans
devavret fa4abb4
Prevent device comparator construction from any table_device_view
devavret b213210
Nullate default and fix for non nested depth
devavret 6f9bedd
Fix an unsurfaced bug about depth passing
devavret be69ffa
Switch over sort impl to new comparator
devavret 76d535a
Copyright changes to satiate ci
devavret 78d10fc
Migrate struct sort benchmark to nvbench
devavret 15920ee
Avoid optional::value in favor of *
devavret d01fc30
throw when trying to sort List
devavret ac2eb0d
Leftover change for struct sort nvbench
devavret 076c4c1
struct without null pushdown test
devavret e8a9202
Remove temporary sort2_test
devavret a4b1167
Remove temporary sort2 files
devavret 62f6914
leftover sort2 in cmake
devavret 8f628ae
cleanup benchmark headers
devavret dc7d125
Docs
devavret fa7d940
Merge branch 'branch-22.04' into struct-row-comp
devavret 76c883f
Apply suggestions from code review
devavret 98b253b
rmm pool in benchmark + style fixes
devavret 3255dc5
Merge branch 'branch-22.04' into struct-row-comp
devavret 52e3a35
Review changes
devavret 9470f06
More review changes
devavret 7c897c3
Review changes req by @vyasr
devavret e0467c7
add a runtime is_relationally_comparable funtion
devavret fc1e993
Review changes
devavret 096593f
Review changes
devavret f539647
Avoid WAR of storing a table_device_view
devavret 01be0bc
Rename struct_linearize to decompose_structs and Improve docs
devavret de95530
review changes req by @ttnghia
devavret 6c45cd4
Namespace changes and making element comparator private
devavret 9bfd08e
Update cpp/include/cudf/table/experimental/row_operators.cuh
devavret File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -167,26 +167,24 @@ void check_lex_compatibility(table_view const& input) | |
|
||
namespace lex { | ||
|
||
preprocessed_table::preprocessed_table(table_view const& t, | ||
host_span<order const> column_order, | ||
host_span<null_order const> null_precedence, | ||
rmm::cuda_stream_view stream) | ||
: d_column_order(0, stream), | ||
d_null_precedence(0, stream), | ||
d_depths(0, stream), | ||
_has_nulls(has_nested_nulls(t)) | ||
std::shared_ptr<preprocessed_table> preprocessed_table::create( | ||
table_view const& t, | ||
host_span<order const> column_order, | ||
host_span<null_order const> null_precedence, | ||
rmm::cuda_stream_view stream) | ||
{ | ||
check_lex_compatibility(t); | ||
|
||
auto [verticalized_lhs, new_column_order, new_null_precedence, verticalized_col_depths] = | ||
struct_linearize(t, column_order, null_precedence); | ||
decompose_structs(t, column_order, null_precedence); | ||
|
||
d_t = | ||
std::make_unique<table_device_view_owner>(table_device_view::create(verticalized_lhs, stream)); | ||
auto d_t = table_device_view::create(verticalized_lhs, stream); | ||
auto d_column_order = detail::make_device_uvector_async(new_column_order, stream); | ||
auto d_null_precedence = detail::make_device_uvector_async(new_null_precedence, stream); | ||
auto d_depths = detail::make_device_uvector_async(verticalized_col_depths, stream); | ||
|
||
d_column_order = detail::make_device_uvector_async(new_column_order, stream); | ||
d_null_precedence = detail::make_device_uvector_async(new_null_precedence, stream); | ||
d_depths = detail::make_device_uvector_async(verticalized_col_depths, stream); | ||
return std::shared_ptr<preprocessed_table>(new preprocessed_table( | ||
std::move(d_t), std::move(d_column_order), std::move(d_null_precedence), std::move(d_depths))); | ||
Comment on lines
+200
to
+201
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can't do it for the same reason |
||
} | ||
|
||
} // namespace lex | ||
|
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.
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 really want to force people to always construct as a
shared_ptr
?I suppose if someone is going through the trouble of constructing a
preprocessed_table
themselves, then that means they are intending to use it in more than one comparator, in which case it will already need to be ashared_ptr
.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.
exactly. Why not return a shared_ptr if everything that can accept it needs a shared_ptr.