Re: Performance improvements for src/port/snprintf.c - Mailing list pgsql-hackers

From Andres Freund
Subject Re: Performance improvements for src/port/snprintf.c
Date
Msg-id 20181003173936.uv3udmpovqizg4x6@alap3.anarazel.de
Whole thread Raw
In response to Re: Performance improvements for src/port/snprintf.c  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Performance improvements for src/port/snprintf.c  (Tom Lane <tgl@sss.pgh.pa.us>)
List 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


pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: Performance improvements for src/port/snprintf.c
Next
From: Tom Lane
Date:
Subject: Re: Performance improvements for src/port/snprintf.c