ARROW-13451: [C++] WIP: POC: benchmark using hash aggregate kernels for scalar aggregation #10813
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
Author
|
This is just to see how the hash aggregate kernels perform compared to the dedicated scalar aggregation kernels in the case that there is only one group. I only tested MinMax and Count. I didn't test Sum/Mean since the scalar kernels use pairwise summation and the hash aggregate kernels use naive summation. Unfortunately, it's rather terrible. For count: DetailsAt 2 orders of magnitude slower, the hash aggregate kernel isn't anywhere near the dedicated scalar one. The scalar kernel essentially just calls CountSetBits, while the hash aggregate kernel must use VisitSetBitRuns and index into a length-1 array of counts. Also, a good amount of time (~10% of the runtime according to perf) is spent just allocating and filling an array of group IDs to use at the start. For min_max the story is not so clear. The hash aggregate kernel actually wins for floats, but loses badly (not as badly as with Count) for integers. DetailsFor integers, the scalar kernel has a SIMD variant that gets leveraged. For doubles, it's unclear to me why the scalar kernel is slower; they both boil down to just min/max calls, though the scalar kernel uses fmin/fmax and the hash kernel uses std::min/std::max. |
Member
Author
|
Ah, and for min_max, the scalar kernel becomes much faster if it calls std::max instead of std::fmax: So in all cases the hash aggregate kernel is considerably slower. |
Member
|
|
Member
Author
|
Ah indeed. Testing it with std::fmax in both kernels, the hash aggregate kernel is slower in all cases: |
Member
|
Well that's disappointing. I'll rewrite the JIRA some- we could still consolidate to a single compute::Function and decide between scalar and grouped aggregation in Dispatch* |
Member
|
Thanks for looking into this, @lidavidm |
Member
Author
|
It would have been nice to maintain half as many aggregate kernels. As you say, we could consolidate the implementations into a single logical function (we'd have to consolidate HashAggregateFunction and ScalarAggregateFunction, though?). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.