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

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

>>>>> "Tom" == Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

Tom> Thanks. Just scanning through the code quickly, I note that it
Tom> assumes IEEE float format, which is probably okay but I suppose we
Tom> might want a configure switch to disable it (and revert to
Tom> platform sprintf).

Yeah, but even s390 these days supports IEEE floats in hardware so I'm
not sure there are any platforms left that don't (that we care about).

Tom> I couldn't immediately figure out if it's got endianness
Tom> assumptions; but even if it does, that'd likely only affect the
Tom> initial disassembly of the IEEE format, so probably not a huge
Tom> deal.

Upstream docs say it's fine with big-endian as long as the endianness of
ints and floats is the same.

Tom> I wonder which variant of the code you were testing (e.g.
Tom> HAS_UINT128 or not).

I was using clang 3.9.1 on FreeBSD amd64, and HAS_UINT128 ends up
enabled by this test:

#if defined(__SIZEOF_INT128__) && !defined(_MSC_VER) && !defined(RYU_ONLY_64_BIT_OPS)
#define HAS_UINT128
...

>> The regression tests for float8 fail of course since Ryu's output
>> format differs (it always includes an exponent, but the code for
>> that part can be tweaked without touching the main algorithm).

Tom> Yeah, one would hope. But I wonder whether it always produces the
Tom> same low-order digits, and if not, whether people will complain.

It won't produce the same low-order digits in general, since it has a
different objective: rather than outputting a decimal value which is the
true float value rounded to a fixed size by decimal rounding rules, it
produces the shortest decimal value which falls within the binary float
rounding interval of the true float value. i.e. the objective is to be
able to round-trip back to float and get the identical result.

One option would be to stick with snprintf if extra_float_digits is less
than 0 (or less than or equal to 0 and make the default 1) and use ryu
otherwise, so that the option to get rounded floats is still there.
(Apparently some people do use negative values of extra_float_digits.)
Unlike other format-changing GUCs, this one already exists and is
already used by people who want more or less precision, including by
pg_dump where rount-trip conversion is the requirement.

Here are some examples of differences in digits, comparing ryu output
with extra_float_digits=3:

Pi: ryu 3.141592653589793E0
sprintf 3.14159265358979312
e: ryu 2.7182818284590455E0
sprintf 2.71828182845904553
1/10: ryu 1E-1
sprintf 0.100000000000000006
1/3: ryu 3.333333333333333E-1
sprintf 0.333333333333333315
2/3: ryu 6.666666666666666E-1
sprintf 0.66666666666666663

--
Andrew.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Edmund Horner 2018-10-06 05:20:37 Re: Calculate total_table_pages after set_base_rel_sizes()
Previous Message Tom Lane 2018-10-06 03:59:15 Re: Performance improvements for src/port/snprintf.c