Rachelint commented on code in PR #11943:
URL: https://github.com/apache/datafusion/pull/11943#discussion_r1725009117
##########
datafusion/physical-plan/src/aggregates/group_values/row.rs:
##########
@@ -121,16 +135,31 @@ impl GroupValues for GroupValuesRows {
create_hashes(cols, &self.random_state, batch_hashes)?;
for (row, &target_hash) in batch_hashes.iter().enumerate() {
+ let mode = self.mode;
let entry = self.map.get_mut(target_hash, |(exist_hash,
group_idx)| {
// Somewhat surprisingly, this closure can be called even if
the
// hash doesn't match, so check the hash first with an integer
// comparison first avoid the more expensive comparison with
// group value. https://github.com/apache/datafusion/pull/11718
- target_hash == *exist_hash
- // verify that the group that we are inserting with hash is
- // actually the same key value as the group in
- // existing_idx (aka group_values @ row)
- && group_rows.row(row) == group_values.row(*group_idx)
+ if target_hash != *exist_hash {
+ return false;
+ }
+
+ // verify that the group that we are inserting with hash is
+ // actually the same key value as the group in
+ // existing_idx (aka group_values @ row)
+ match mode {
+ GroupStatesMode::Flat => {
+ group_rows.row(row)
+ == group_values.current().unwrap().row(*group_idx)
+ }
+ GroupStatesMode::Blocked(_) => {
+ let blocked_index = BlockedGroupIndex::new(*group_idx);
+ group_rows.row(row)
+ == group_values[blocked_index.block_id]
Review Comment:
Yes, hashtable actually the biggest bottleneck now according to the
flamegraph...
In high cardinality case and large amount of blocks exist, may be it is
actually a point we can optimization.
It seems the copy and page fault caused by `rehashing` and
`sse2::Group::load` the bottleneck of hashbrown?
#7095 is very cool, but I still not really understand about it...
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
