Perf: Short circuit expensive comparissions in GroupColumn #23344
Perf: Short circuit expensive comparissions in GroupColumn #23344Rich-T-kid wants to merge 3 commits into
Conversation
|
cc @zhuqi-lucas as this is related to #22715 |
| /// Used to order columns in [`GroupValuesColumn::compare_order`] so cheap comparisons | ||
| /// eliminate rows before expensive ones are evaluated. | ||
| /// see <https://github.com/apache/datafusion/issues/23342> | ||
| fn compare_tier(data_type: &DataType) -> u8 { |
There was a problem hiding this comment.
Shouldn't this belong to a common place as this can be used across other operators like HashJoins
There was a problem hiding this comment.
I suppose 🤔 , is there a specific case where you think this would be useful for hashJoins?
There was a problem hiding this comment.
When JoinKeyComparator finds groups matching groups per row similarly, correct me if I am wrong. Here the compare method can also use this optimized ordering right ?
There was a problem hiding this comment.
Im honestly not too familiar with the join operator logic. but presumably the same concepts should apply there. Ideally i'd like to move forward with #23342. but I can make a follow up ticket to move this into a higher modules for code re-use
Which issue does this PR close?
Rationale for this change
see #23342
Note : the exact ranking here is a bit arbitrary & i'm happy to change it. The core idea is that its much cheaper to do boolean checks and fixed size byte comparisons as opposed to chasing pointers to comparing strings.
an alternative approach may be to split this into two tiers:
What changes are included in this PR?
compare_tier()to rank each supported DataType by comparison cost (1 = cheap fixed-width integers → 5 = expensive variable-length bytes)compare_order: Vec to GroupValuesColumn, built once at construction viabuild_group_compare_order(), sovectorized_equal_toevaluates columns cheapest-first and maximizes early-exit savings before reaching expensive string comparisonsAre these changes tested?
This PR only change the order in which comparisons occurs. functionally nothing changes.
Are there any user-facing changes?
no