Re: Performance improvements for src/port/snprintf.c

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>, Alexander Kuzmenkov <a(dot)kuzmenkov(at)postgrespro(dot)ru>
Subject: Re: Performance improvements for src/port/snprintf.c
Date: 2018-10-06 02:58:37
Message-ID: 13178.1538794717@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I stepped back a bit from the raw performance question and thought about
what we actually want functionally in snprintf's float handling. There
are a couple of points worth making:

* The fact that float[48]out explicitly handle NaN and Inf cases is a
leftover from when they had to cope with varying behavior of
platform-specific snprintf implementations. Now that we've standardized
on snprintf.c, it makes a lot more sense to enforce standardized printing
of these values inside snprintf.c. That not only avoids repeated tests
for these cases at different code levels, but ensures that the uniform
behavior exists for all our invocations of *printf, not just float[48]out.

* snprintf.c doesn't really work right for IEEE minus zero, as I recently
noted in another thread (<23662(dot)1538067926(at)sss(dot)pgh(dot)pa(dot)us>). While this
is not of significance for float[48]out, it might be a problem for other
callers. Now that we've enforced usage of snprintf.c across-the-board,
I think it's more important to worry about these corner cases. It's not
that expensive to fix either; we can test for minus zero with something
like this:
static const double dzero = 0.0;
if (value == 0.0 &&
memcmp(&value, &dzero, sizeof(double)) != 0)
(ie, "it's equal to zero but not bitwise equal to zero"). While that
looks like it might be expensive, I find that recent versions of both
gcc and clang can optimize the memcmp call down to something like
cmpq $0, 8(%rsp)
so I think it's well worth the cost to get this right.

The attached proposed patch addresses both of those points.

Also, in service of the performance angle, I went ahead and made a
roughly strfromd-like entry point in snprintf.c, but using an API
that doesn't force textual conversion of the precision spec.

As best I can tell, this patch puts the performance of float8out
on par with what it was in v11, measuring using a tight loop like

while (count-- > 0)
{
char *str = float8out_internal(val);
pfree(str);
CHECK_FOR_INTERRUPTS();
}

For me, this is within a percent or two either way on a couple of
different machines; that's within the noise level.

regards, tom lane

Attachment Content-Type Size
different-float-printf-fix-1.patch text/x-diff 7.8 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Gierth 2018-10-06 03:10:00 Re: Performance improvements for src/port/snprintf.c
Previous Message Andrew Gierth 2018-10-06 02:48:02 Re: Early WIP/PoC for inlining CTEs