Vectorize "sum" function - #10992
Conversation
| void NO_INLINE addMany(const Value * __restrict ptr, size_t count) | ||
| { | ||
| /// Compiler cannot unroll this loop, do it manually. | ||
| /// (at least for floats, most likely due to the lack of -fassociative-math) |
There was a problem hiding this comment.
Why not add -fassociative-math then?
There was a problem hiding this comment.
It will make Kahan summation algorithm to have no effect.
But we can enable
-fno-math-errno
-fno-rounding-math
-fno-signaling-nans
-fno-signed-zeros
-fno-trapping-math
-fassociative-math
-freciprocal-math
for all codebase and disable it (with pragma or function attributes) whenever something like Kahan summation is used.
More care should be taken to figure out these places.
|
|
Next steps (in subsequent PRs):
|
|
|
||
| /// Vectorized version | ||
| template <typename Value> | ||
| void NO_INLINE addMany(const Value * __restrict ptr, size_t count) |
There was a problem hiding this comment.
I think we can make this algorithm more public. Probably, move it to ColumnsCommon.h or somewhere to Common.
There was a problem hiding this comment.
As I've googled, __restrict means that data accessed by ptr can't be changed externally.
Probably, we may add comment about it.
There was a problem hiding this comment.
I think we can make this algorithm more public.
Whenever we will have a second use case.
| { | ||
| auto raw_sum = to_sum + from_sum; | ||
| auto rhs_compensated = raw_sum - to_sum; | ||
| auto compensations = ((from_sum - rhs_compensated) + (to_sum - (raw_sum - rhs_compensated))) + compensation + from_compensation; |
There was a problem hiding this comment.
I've tried to understand what is going on here and gave up.
Can we simplify it, or add a comment with formula and idea how do we merge?
E.g. why it is not just
addImpl(to_sum, to_compensation, from_sum);
addImpl(to_sum, to_compensation, -from_compensation);Or maybe it is reasonable to choose which sum is bigger?
There was a problem hiding this comment.
It's tricky due to non-associative float math. And Kahan summation algorithm depends on it.
We cannot reorder or simplify expressions.
I'll add a comment.
There was a problem hiding this comment.
Yes, I understand that Kahan summation depends on non-associative float math.
I just don't understand what this code does. And why, e.g., it's hot just two calls of addImpl.
Code in addImpl also depends on non-associative math, but I can understand it (at least by reading wikipedia article).
New comments also don't let me understand code better.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Make queries with
sumaggregate function and without GROUP BY keys to run multiple times faster.Detailed description / Documentation draft:
The performance of sum and sumKahan is increased multiple times.
TODO: Do the same for avg, min, max.