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

From: Andres Freund <andres(at)anarazel(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Alexander Kuzmenkov <a(dot)kuzmenkov(at)postgrespro(dot)ru>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Performance improvements for src/port/snprintf.c
Date: 2018-10-03 17:39:36
Message-ID: 20181003173936.uv3udmpovqizg4x6@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2018-10-03 13:18:35 -0400, Tom Lane wrote:
> I wrote:
> > Andres Freund <andres(at)anarazel(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2018-10-03 17:40:03 Re: Performance improvements for src/port/snprintf.c
Previous Message Andres Freund 2018-10-03 17:36:39 Re: Performance improvements for src/port/snprintf.c