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

Tom Lane <tgl@sss.pgh.pa.us>

From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@lists.postgresql.org, Thomas Munro <thomas.munro@enterprisedb.com>, Andrew Gierth <andrew@tao11.riddles.org.uk>, Alexander Kuzmenkov <a.kuzmenkov@postgrespro.ru>
Date: 2018-10-03T17:31:09Z
Lists: pgsql-hackers
Andres Freund <andres@anarazel.de> writes:
> On 2018-10-03 12:54:52 -0400, Tom Lane wrote:
>> (1) The need to use sprintf for portability means that we need very
>> tight constraints on the precision spec *and* the buffer size *and*
>> the format type (%f pretty much destroys certainty about how long the
>> output string is).  So this isn't going to be general purpose code.
>> I think just writing it into float[48]out is sufficient.

> Well, the numbers suggest it's also useful to do so from snprintf - it's
> not that rare that we output floating point numbers from semi
> performance critical code, even leaving aside float[48]out.  So I'm not
> convinced that we shouldn't do this from within snprintf.c too. Now we
> could open-code it twice, but i'm not sure I see the point.

I do not see the point of messing with snprintf.c here.  I doubt that
strfromd is faster than the existing sprintf call (because the latter
can use ".*" instead of serializing and deserializing the precision).
Even if it is, I do not want to expose an attractive-nuisance API
in a header, and I think this would be exactly that.

> If we just define the API as having to guarantee there's enough space
> for the output format, I think it'll work well enough for now?

No, because that's a recipe for buffer-overflow bugs.  It's *hard*
to be sure the buffer is big enough, and easy to make breakable
assumptions.

> snprintf.c already assumes everything floating point can be output in
> 1024 chars, no?

Indeed, and it's got hacks like a forced limit to precision 350 in order
to make that safe.  I don't want to be repeating the reasoning in
fmtfloat() in a bunch of other places.

			regards, tom lane


Commits

  1. Improve snprintf.c's handling of NaN, Infinity, and minus zero.

  2. Rationalize snprintf.c's handling of "ll" formats.

  3. Provide fast path in snprintf.c for conversion specs that are just "%s".

  4. Make assorted performance improvements in snprintf.c.

  5. Set snprintf.c's maximum number of NL arguments to be 31.

  6. Always use our own versions of *printf().