-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[Improvement](hash) opt for pack_fixeds #59410
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
base: master
Are you sure you want to change the base?
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
56aeb36 to
bd35002
Compare
|
run buildall |
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
update fix
bd35002 to
2311a1f
Compare
|
run buildall |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
|
run buildall |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
HappenLee
left a comment
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
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
|
run buildall |
|
run buildall |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
|
run buildall |
|
run buildall |
|
run buildall |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
|
run buildall |
format update format fix update fix fix
|
run buildall |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
HappenLee
left a comment
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
|
PR approved by at least one committer and no changes requested. |
What problem does this PR solve?
This pull request refactors and optimizes the handling of null maps and key packing in hash join and hash table code, with a focus on improving SIMD (Single Instruction, Multiple Data) usage and simplifying null bitmap logic. The changes replace older byte-searching utilities with new, more efficient SIMD-based functions, update how null bitmaps are packed and processed, and streamline column null data replacement. Additionally, the logic for determining hash key types and handling fixed key serialization is improved for better correctness and performance.
Key improvements and changes:
SIMD utilities and null map handling
contain_oneandcontain_zeroinsimd/bits.h, replacing the oldercontain_byteand related logic for checking the presence of ones or zeros in null maps, resulting in more efficient null detection.contain_oneandcontain_zerofunctions, simplifying and unifying the logic for detecting nulls in columns and filters. [1] [2] [3] [4] [5] [6]Hash key and null bitmap packing
MethodKeysFixed, introducing new templates and helper functions for interleaved null map packing, and replacing the old bitmap size calculation with a simplified approach. This improves both performance and maintainability. [1] [2]Column null data replacement
replace_column_null_datamethods for vector and decimal columns by removing unnecessary null count checks and optimizing the replacement logic. [1] [2]Hash key type logic
hash_key_type.hto handle cases where the number of data types exceeds the bit size, defaulting to serialized keys as needed. [1] [2]Code cleanup and dependency updates
These changes collectively improve performance, maintainability, and correctness in hash join operations, especially in handling nullable columns and SIMD optimizations.
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)