CuteChuanChuan commented on code in PR #25652:
URL: https://github.com/apache/datafusion/pull/25652#discussion_r4124447995
##########
datafusion/physical-plan/src/aggregates/group_values/multi_group_by/dictionary.rs:
##########
@@ -348,59 +318,39 @@ impl<K: ArrowDictionaryKeyType + Send + Sync> GroupColumn
let dict = array.as_dictionary::<K>();
let dict_keys = dict.keys();
let dict_values = dict.values();
- let num_distinct = dict_values.len();
-
- // The fallback is in a separate #[cold] function so its code does not
- // appear inline here and cannot prevent LLVM from pipelining /
unrolling
- // the hot lookup-table loops below.
- if rhs_rows.len() < num_distinct {
- self.equal_to_per_row(
- lhs_rows,
- dict_values,
- dict,
- rhs_rows,
- equal_to_results,
- );
- return;
- }
-
- let mut val_hashes = vec![0u64; dict_values.len()];
- create_hashes(
- std::slice::from_ref(dict_values),
- &self.random_state,
- &mut val_hashes,
- )
- .unwrap();
- let lookup = self.build_lookup_table(dict_values, &val_hashes);
- let group_to_inner = self.group_to_inner.as_slice();
+ self.sync_value_cache(dict_values);
+ let raw_keys = dict_keys.values();
if dict_keys.null_count() == 0 {
// No null keys : skip the get_bit guard: we only ever write false,
// so overwriting an already-false bit is a no-op.
- let raw_keys = dict_keys.values();
for (idx, (&lhs_row, &rhs_row)) in
lhs_rows.iter().zip(rhs_rows.iter()).enumerate()
Review Comment:
Thanks for the numbers. I applied this and then ran the whole
`dictionary_group_values` bench against main.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
