-
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
Add string scalar support in AST #13061
Add string scalar support in AST #13061
Conversation
All benchmarks results are almost same. ./_deps/nvbench-src/scripts/nvbench_compare.py JOIN_NV.before.json ../../fea-string_scalar_ast_compare/release/JOIN_NV.after.json
['JOIN_NV.before.json', '../../fea-string_scalar_ast_compare/release/JOIN_NV.after.json']
# inner_join_32bit
[0] Quadro GV100
inner_join_64bit[0] Quadro GV100
inner_join_32bit_nulls[0] Quadro GV100
inner_join_64bit_nulls[0] Quadro GV100
left_join_32bit[0] Quadro GV100
left_join_64bit[0] Quadro GV100
left_join_32bit_nulls[0] Quadro GV100
left_join_64bit_nulls[0] Quadro GV100
full_join_32bit[0] Quadro GV100
full_join_64bit[0] Quadro GV100
full_join_32bit_nulls[0] Quadro GV100
full_join_64bit_nulls[0] Quadro GV100
mixed_inner_join_32bit[0] Quadro GV100
mixed_inner_join_64bit[0] Quadro GV100
mixed_inner_join_32bit_nulls[0] Quadro GV100
mixed_inner_join_64bit_nulls[0] Quadro GV100
mixed_left_join_32bit[0] Quadro GV100
mixed_left_join_64bit[0] Quadro GV100
mixed_left_join_32bit_nulls[0] Quadro GV100
mixed_left_join_64bit_nulls[0] Quadro GV100
mixed_full_join_32bit[0] Quadro GV100
mixed_full_join_64bit[0] Quadro GV100
mixed_full_join_32bit_nulls[0] Quadro GV100
mixed_full_join_64bit_nulls[0] Quadro GV100
mixed_left_semi_join_32bit[0] Quadro GV100
mixed_left_semi_join_64bit[0] Quadro GV100
mixed_left_semi_join_32bit_nulls[0] Quadro GV100
mixed_left_semi_join_64bit_nulls[0] Quadro GV100
mixed_left_anti_join_32bit[0] Quadro GV100
mixed_left_anti_join_64bit[0] Quadro GV100
mixed_left_anti_join_32bit_nulls[0] Quadro GV100
mixed_left_anti_join_64bit_nulls[0] Quadro GV100
Summary
./_deps/gbench-src/tools/compare.py benchmarks AST.before.json ../../fea-string_scalar_ast_compare/release/AST.after.json Comparing AST.before.json to ../../fea-string_scalar_ast_compare/release/AST.after.json
./_deps/gbench-src/tools/compare.py benchmarks JOIN.before.json ../../fea-string_scalar_ast_compare/release/JOIN.after.json Comparing JOIN.before.json to ../../fea-string_scalar_ast_compare/release/JOIN.after.json
|
Compile time differences (From .ninja_log)
|
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.
The code looks fine to me. Do we have a plan for how this generic scalar can be pulled out of AST code, though? I believe there is some refactoring needed of the scalar device views for that to be possible, could we open an issue documenting that before we merge this?
Created #13160 to track this. |
…nn/cudf into fea-string_scalar_ast_compare
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.
LGTM, thanks!
/merge |
Depends on #13061 Add Java bindings for string scalar support in AST Add unit test for string comparison - column vs column, column vs literal. Authors: - Karthikeyan (https://github.com/karthikeyann) Approvers: - Jason Lowe (https://github.com/jlowe) - MithunR (https://github.com/mythrocks) URL: #13072
Depends on #13061 Add Python bindings for string scalar support in AST Add unit test for string comparison - column vs column, column vs literal. Authors: - Karthikeyan (https://github.com/karthikeyann) - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Vyas Ramasubramani (https://github.com/vyasr) - Ashwin Srinath (https://github.com/shwina) URL: #13073
Depends on rapidsai#13061 Add Python bindings for string scalar support in AST Add unit test for string comparison - column vs column, column vs literal. Authors: - Karthikeyan (https://github.com/karthikeyann) - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Vyas Ramasubramani (https://github.com/vyasr) - Ashwin Srinath (https://github.com/shwina) URL: rapidsai#13073
Description
Adding string scalar support in AST.
A new generic scalar device view class is added in AST to support numeric, timestamp, duration and string scalars.
Register count did not change, and benchmark results are almost same.
Compile time - There is major increase in join.cu by 15%. Other files are in range of -2% to 7%
Addressed part of #8858
Checklist