pepijnve commented on PR #19994:
URL: https://github.com/apache/datafusion/pull/19994#issuecomment-3829789029
In the microbenchmark, I think it might be preferable to use `!= 0` rather
than `> 0`. With `> 0`, even for the '0% zeroes' benchmark, ~50% of the values
are negative and do not match the condition. The benchmark name suggests we're
taking the branch 0% of the time, while in fact it's 50% which is a bit
misleading. Due to an implementation issue (see comment on
`safe_divide_arrays`) we're also not comparing the same thing entirely.
I had a closer look at what was going on in what I thought was the
`expr_or_expr` implementation, but it turns out that wasn't actually being
used. Instead the general `no_expr` implementation was being used. This was due
to the else branch being literal `null`. I tinkered a bit with the existing
code to make sure `expr_or_expr` is actually used, and that brings the results
much closer, to the point where I'm wondering if the divide-by-zero special
case is still worth it.
The changes I made for `expr_or_expr` are general improvements, so I'll make
a separate PR with those.
```
divide_by_zero_protection/8192 rows, 0% zeros: DivideByZeroProtection
time: [8.0014 µs 8.0112 µs 8.0208 µs]
divide_by_zero_protection/8192 rows, 0% zeros: ExpressionOrExpression
time: [6.6199 µs 6.6250 µs 6.6300 µs]
divide_by_zero_protection/8192 rows, 10% zeros: DivideByZeroProtection
time: [17.586 µs 17.641 µs 17.699 µs]
divide_by_zero_protection/8192 rows, 10% zeros: ExpressionOrExpression
time: [32.909 µs 32.965 µs 33.030 µs]
divide_by_zero_protection/8192 rows, 50% zeros: DivideByZeroProtection
time: [53.277 µs 53.440 µs 53.601 µs]
divide_by_zero_protection/8192 rows, 50% zeros: ExpressionOrExpression
time: [49.745 µs 49.997 µs 50.309 µs]
divide_by_zero_protection/8192 rows, 90% zeros: DivideByZeroProtection
time: [54.223 µs 54.287 µs 54.359 µs]
divide_by_zero_protection/8192 rows, 90% zeros: ExpressionOrExpression
time: [16.432 µs 16.500 µs 16.570 µs]
```
--
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]
