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>
Subject: Re: Performance improvements for src/port/snprintf.c
In-reply-to: <20181003170043.thdyezozi343dkuc@alap3.anarazel.de>
References: <6FEC34CD-B8C0-44E2-8D1F-2320568BDC78@anarazel.de>
 <30551.1538517271@sss.pgh.pa.us>
 <20181003060936.zul7mfkcu5w7rotg@alap3.anarazel.de>
 <25583.1538569214@sss.pgh.pa.us>
 <20181003155207.b3lqmovuv2c5c4id@alap3.anarazel.de>
 <26510.1538582852@sss.pgh.pa.us>
 <20181003161454.ozeioucsimjg5g6m@alap3.anarazel.de>
 <27041.1538583733@sss.pgh.pa.us>
 <20181003163717.oycshpp2hnctvtfj@alap3.anarazel.de>
 <28321.1538585692@sss.pgh.pa.us>
 <20181003170043.thdyezozi343dkuc@alap3.anarazel.de>
Comments: In-reply-to Andres Freund <andres@anarazel.de>
	message dated "Wed, 03 Oct 2018 10:00:43 -0700"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2131.1538587869.1@sss.pgh.pa.us>
Date: Wed, 03 Oct 2018 13:31:09 -0400
Message-ID: <2132.1538587869@sss.pgh.pa.us>
Precedence: bulk

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