Re: Performance improvements for src/port/snprintf.c
Andres Freund <andres@anarazel.de>
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Alexander Kuzmenkov <a.kuzmenkov@postgrespro.ru>, pgsql-hackers@lists.postgresql.org
Date: 2018-10-03T17:39:36Z
Lists: pgsql-hackers
Hi, On 2018-10-03 13:18:35 -0400, Tom Lane wrote: > I wrote: > > Andres Freund <andres@anarazel.de> writes: > >> - I know it's not new, but is it actually correct to use va_arg(args, int64) > >> for ATYPE_LONGLONG? > > > Well, the problem with just doing s/int64/long long/g is that the > > code would then fail on compilers without a "long long" type. > > We could ifdef our way around that, but I don't think the code would > > end up prettier. > > I spent a bit more time thinking about that point. My complaint about > lack of long long should be moot given that we're now requiring C99. True, I didn't think of that. > So the two cases we need to worry about are (1) long long exists and > is 64 bits, and (2) long long exists and is wider than 64 bits. In > case (1) there's nothing actively wrong with the code as it stands. > In case (2), if we were to fix the problem by s/int64/long long/g, > the result would be that we'd be doing the arithmetic for all > integer-to-text conversions in 128 bits, which seems likely to be > pretty expensive. Yea, that seems quite undesirable. > So a "real" fix would probably require having separate versions of > fmtint for long and long long. I'm not terribly excited about > going there. I can see it happening some day when/if we need to > use 128-bit math more extensively than today, but I do not think > that day is close. Right, that seems a bit off. > (Are there *any* platforms where "long long" is 128 bits today?) Not that I'm aware off. > Having said that, maybe there's a case for changing the type spec > in only the va_arg() call, and leaving snprintf's related local > variables as int64. (Is that what you actually meant?) Then, > if long long really is different from int64, at least we have > predictable truncation behavior after fetching the value, rather > than undefined behavior while fetching it. Hm. I guess that'd be a bit better, but I'm not sure it's worth it. How about we simply add a static assert that long long isn't bigger than int64? Greetings, Andres Freund
Commits
-
Improve snprintf.c's handling of NaN, Infinity, and minus zero.
- 6eb3eb577d76 12.0 landed
-
Rationalize snprintf.c's handling of "ll" formats.
- 595a0eab7f42 12.0 landed
-
Provide fast path in snprintf.c for conversion specs that are just "%s".
- 6d842be6c118 12.0 landed
-
Make assorted performance improvements in snprintf.c.
- abd9ca377d66 12.0 landed
-
Set snprintf.c's maximum number of NL arguments to be 31.
- 625b38ea0e98 12.0 cited
-
Always use our own versions of *printf().
- 96bf88d52711 12.0 cited