You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/08/08 23:41:47 UTC

[GitHub] [arrow-datafusion] sundy-li opened a new issue #839: Refactor the hash_aggregate

sundy-li opened a new issue #839:
URL: https://github.com/apache/arrow-datafusion/issues/839


   `hash_aggregate` is using `compute::take` to split the columns into lots of small columns.  This is the main reason why `hash_aggregate` works slow. 
   
   https://github.com/apache/arrow-datafusion/blob/4ddd2f5e7582ffe662aea27bbb74c58cd0715152/datafusion/src/physical_plan/hash_aggregate.rs#L422-L438
   
   
   


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow-datafusion] Dandandan commented on issue #839: Refactor the hash_aggregate

Posted by GitBox <gi...@apache.org>.
Dandandan commented on issue #839:
URL: https://github.com/apache/arrow-datafusion/issues/839#issuecomment-895017194


   > It's done to the entire record batches. Considering the batches are 1024 rows with 3 columns, if the key number is 256, then there will be 256 small batches after the take, each batch is about 4 rows. `Simd` will make no sense in this situation.
   > 
   > >
   
   Yes, but not in the code you linked. There the `take` is done once for the arrays in `aggr_input_values`, regardless of how many distinct keys we have in the batch. The take only rearranges the data to be in the same order as the keys but does it for only once for each input array for the aggregate expression values (not even the entire batch).
   
   In other places, yes, operations like `Array::slice` are done for each key that occurred which starts to become inefficient when having a lot of small groups in the batch and also show up in profiles.


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow-datafusion] alamb commented on issue #839: Refactor the hash_aggregate

Posted by GitBox <gi...@apache.org>.
alamb commented on issue #839:
URL: https://github.com/apache/arrow-datafusion/issues/839#issuecomment-895160880


   FWIW I believe @jhorstmann  also had some thoughts in this area (aka on the use of `take` vs some other ways to speed up the distinct calculation) in https://github.com/apache/arrow-datafusion/issues/790#issuecomment-888591358 which may be related to this conversation


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow-datafusion] sundy-li commented on issue #839: Refactor the hash_aggregate

Posted by GitBox <gi...@apache.org>.
sundy-li commented on issue #839:
URL: https://github.com/apache/arrow-datafusion/issues/839#issuecomment-894941589


   >  the take is only done once for each input array.
   
   It's done to the entire record batches. Considering the batches are 1024 rows with 3 columns, if the key number is 256,  then there will be 256 small batches after the take. `Simd` will make no sense in this situation.
   
   > materializing the end keys/states to an array.
   
   Yes, this may be a good point to go.
   
   
   


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow-datafusion] sundy-li edited a comment on issue #839: Refactor the hash_aggregate

Posted by GitBox <gi...@apache.org>.
sundy-li edited a comment on issue #839:
URL: https://github.com/apache/arrow-datafusion/issues/839#issuecomment-894941589


   >  the take is only done once for each input array.
   
   It's done to the entire record batches. Considering the batches are 1024 rows with 3 columns, if the key number is 256,  then there will be 256 small batches after the take, each batch is about 4 rows. `Simd` will make no sense in this situation.
   
   > materializing the end keys/states to an array.
   
   Yes, this may be a good point to go.
   
   
   


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow-datafusion] Dandandan commented on issue #839: Refactor the hash_aggregate

Posted by GitBox <gi...@apache.org>.
Dandandan commented on issue #839:
URL: https://github.com/apache/arrow-datafusion/issues/839#issuecomment-894918837


   You are right that the current hash aggregate is quite a bit slower in this case than it should be.
   
   There is some work already by @alamb to make the hash aggregate faster for smaller keys and already gives a ~2x speedup on a tougher query.
   https://github.com/apache/arrow-datafusion/issues/790
   
   I don't think the slow code is in the code you quoted, the `take` is only done once for each input array. The slower part just below though works on each new input key + input array and does e.g. `slice` on it which has a high overhead because of that.
   There are some ideas linked in the issue to deal with that.
   
   There are currently also some other parts in the code that are even contributing more to the runtime, such as materializing the end keys/states to an array.


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org