Re: pgsql: Speed up conversion of signed integers to C strings.

Speed up conversion of signed integers to C strings.

A hand-coded implementation turns out to be much faster than calling
printf(). In passing, add a few more regresion tests.

Andres Freund, with assorted, mostly cosmetic changes.

Branch
------
master

Details
-------
http://git.postgresql.org/gitweb?p=postgresql.git;a=commitdiff;h=4fc115b2e981f8c63165ca86a23215380a3fda66

Modified Files
--------------
src/backend/utils/adt/int8.c | 8 +--
src/backend/utils/adt/numutils.c | 115 ++++++++++++++++++++++++++++++++----
src/include/utils/builtins.h | 1 +
src/test/regress/expected/int2.out | 13 ++++
src/test/regress/expected/int4.out | 13 ++++
src/test/regress/expected/int8.out | 13 ++++
src/test/regress/sql/int2.sql | 4 +
src/test/regress/sql/int4.sql | 4 +
src/test/regress/sql/int8.sql | 4 +
9 files changed, 157 insertions(+), 18 deletions(-)

Robert Haas <rhaas(at)postgresql(dot)org> writes:
> Speed up conversion of signed integers to C strings.

This patch breaks the build here:

gcc -O1 -Wall -Wmissing-prototypes -Wpointer-arith -fno-strict-aliasing -g -I../../../../src/include -D_XOPEN_SOURCE_EXTENDED -c -o numutils.o numutils.c
numutils.c: In function `pg_ltoa':
numutils.c:139: `INT32_MIN' undeclared (first use in this function)
numutils.c:139: (Each undeclared identifier is reported only once
numutils.c:139: for each function it appears in.)
numutils.c: In function `pg_lltoa':
numutils.c:190: `INT64_MIN' undeclared (first use in this function)
make: *** [numutils.o] Error 1
$
regards, tom lane

I wrote:
> Robert Haas <rhaas(at)postgresql(dot)org> writes:
>> Speed up conversion of signed integers to C strings.

> This patch breaks the build here:

... and while I'm looking at it, the added int8 regression test cases
definitely merit a WTF. They aren't testing what I would expect.
What they are testing is platform-specific behavior of excessive
shifting, which is why some of the buildfarm members are pink.

regards, tom lane

On Fri, Nov 19, 2010 at 11:47 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I wrote:
>> Robert Haas <rhaas(at)postgresql(dot)org> writes:
>>> Speed up conversion of signed integers to C strings.
>
>> This patch breaks the build here:
>
> ... and while I'm looking at it, the added int8 regression test cases
> definitely merit a WTF.  They aren't testing what I would expect.
> What they are testing is platform-specific behavior of excessive
> shifting, which is why some of the buildfarm members are pink.

Ugh. I made an attempt at a fix to both of these issues, but I'm not
totally sure I got it right, and I'm too tired to stay up any later to
see what happens. I'll check the BF in the morning.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 11/20/2010 01:10 AM, Robert Haas wrote:
> On Fri, Nov 19, 2010 at 11:47 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I wrote:
>>> Robert Haas<rhaas(at)postgresql(dot)org> writes:
>>>> Speed up conversion of signed integers to C strings.
>>> This patch breaks the build here:
>> ... and while I'm looking at it, the added int8 regression test cases
>> definitely merit a WTF. They aren't testing what I would expect.
>> What they are testing is platform-specific behavior of excessive
>> shifting, which is why some of the buildfarm members are pink.
> Ugh. I made an attempt at a fix to both of these issues, but I'm not
> totally sure I got it right, and I'm too tired to stay up any later to
> see what happens. I'll check the BF in the morning.

If you change a test that has alternative result files, you need to
change all the result files. See src/test/regress/resultmap.

In this case you missed out changing int8-exp-three-digits.out - see
<http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=dawn_bat&dt=2010-11-20%2005%3A30%3A07>

cheers

andrew

On Sat, Nov 20, 2010 at 5:23 AM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>> Ugh.  I made an attempt at a fix to both of these issues, but I'm not
>> totally sure I got it right, and I'm too tired to stay up any later to
>> see what happens.  I'll check the BF in the morning.
>
> If you change a test that has alternative result files, you need to change
> all the result files. See src/test/regress/resultmap.
>
> In this case you missed out changing int8-exp-three-digits.out - see
> <http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=dawn_bat&dt=2010-11-20%2005%3A30%3A07>

Sorry, I always forget about the alternate expected output files,
because they never seem to be the ones that my platform needs.

Ah, the joy of debugging by buildfarm. Nothing better than having the
whole world watch your mistakes in painstaking detail!

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 11/20/2010 07:10 AM, Robert Haas wrote:
> Ah, the joy of debugging by buildfarm. Nothing better than having the
> whole world watch your mistakes in painstaking detail!

At least this way you get to do it while your memory is fresh. In the
old days we only found out some things were broken weeks or months later.

cheers

andrew

Robert Haas wrote:
> On Sat, Nov 20, 2010 at 5:23 AM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
> >> Ugh. ?I made an attempt at a fix to both of these issues, but I'm not
> >> totally sure I got it right, and I'm too tired to stay up any later to
> >> see what happens. ?I'll check the BF in the morning.
> >
> > If you change a test that has alternative result files, you need to change
> > all the result files. See src/test/regress/resultmap.
> >
> > In this case you missed out changing int8-exp-three-digits.out - see
> > <http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=dawn_bat&dt=2010-11-20%2005%3A30%3A07>
>
> Sorry, I always forget about the alternate expected output files,
> because they never seem to be the ones that my platform needs.
>
> Ah, the joy of debugging by buildfarm. Nothing better than having the
> whole world watch your mistakes in painstaking detail!

Yes, in a way when you commit a patch you become a slave of the
buildfarm. I can say that with experience based on the recent patches I
have written.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Andrew Dunstan wrote:
>
>
> On 11/20/2010 07:10 AM, Robert Haas wrote:
> > Ah, the joy of debugging by buildfarm. Nothing better than having the
> > whole world watch your mistakes in painstaking detail!
>
> At least this way you get to do it while your memory is fresh. In the
> old days we only found out some things were broken weeks or months later.

That is true. I used to pull CVS using binary search to find the commit
that broke things.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +