Re: In PG12, query with float calculations is slower than PG11 - Mailing list pgsql-hackers

Hi,

On Thu, Feb 6, 2020 at 2:55 PM Andres Freund <andres@anarazel.de> wrote:
> On 2020-02-06 14:25:03 +0900, keisuke kuroda wrote:
> > That's because check_float8_val() (in PG 12) is a function
> > whose arguments must be evaluated before
> > it is called (it is inline, but that's irrelevant),
> > whereas CHECKFLOATVAL() (in PG11) is a macro
> > whose arguments are only substituted into its body.
>
> Hm - it's not that clear to me that it is irrelevant that the function
> gets inlined. The compiler should know that isinf is side-effect free,
> and that it doesn't have to evaluate before necessary.
>
> Normally isinf is implemented by a compiler intrisic within the system
> headers. But not in your profile:
> > ★ 5.41%  postgres  libc-2.17.so       [.] __isinf
>
> I checked, and I don't see any references to isinf from within float.c
> (looking at the disassembly - there's some debug strings containing the
> word, but that's it).
>
> What compiler & compiler version on what kind of architecture is this?

As Kuroda-san mentioned, I also checked the behavior that he reports.
The compiler I used is an ancient one (CentOS 7 default):

$ gcc --version
gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-39)

Compiler dependent behavior of inlining might be relevant here, but
there is one more thing to consider. The if () condition in
check_float8_val (PG 12) and CHECKFLOATVAL (PG 11) is calculated
differently, causing isinf() to be called more times in PG 12:

static inline void
check_float8_val(const float8 val, const bool inf_is_valid,
                 const bool zero_is_valid)
{
    if (!inf_is_valid && unlikely(isinf(val)))
        ereport(ERROR,
                (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
                 errmsg("value out of range: overflow")));

#define CHECKFLOATVAL(val, inf_is_valid, zero_is_valid)         \
do {                                                            \
    if (isinf(val) && !(inf_is_valid))                          \
        ereport(ERROR,                                          \
                (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),   \
          errmsg("value out of range: overflow")));             \

called thusly:

    check_float8_val(result, isinf(val1) || isinf(val2),
                     val1 == 0.0 || val2 == 0.0);

and

    CHECKFLOATVAL(result, isinf(arg1) || isinf(arg2),
                  arg1 == 0 || arg2 == 0);

from float8_mul() and float8mul() in PG 12 and PG 11, respectively.

You may notice that the if () condition is reversed, so while PG 12
calculates isinf(arg1) || isinf(arg2) first and isinf(result) only if
the first is false, which it is in most cases, PG 11 calculates
isinf(result) first, followed by isinf(arg1) || isinf(arg2) if the
former is true.  I don't understand why such reversal was necessary,
but it appears to be the main factor behind this slowdown.  So, even
if PG 12's check_float8_val() is perfectly inlined, this slowdown
couldn't be helped.

Thanks,
Amit