Re: Non-decimal integer literals

From: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: John Naylor <john(dot)naylor(at)enterprisedb(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Non-decimal integer literals
Date: 2022-11-24 08:35:08
Message-ID: 068f0344-ccce-c623-3cfe-d09752db158d@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 23.11.22 09:54, David Rowley wrote:
> On Wed, 23 Nov 2022 at 02:37, Peter Eisentraut
> <peter(dot)eisentraut(at)enterprisedb(dot)com> wrote:
>> Here is a new patch.
>
> This looks like quite an inefficient way to convert a hex string into an int64:
>
> while (*ptr && isxdigit((unsigned char) *ptr))
> {
> int8 digit = hexlookup[(unsigned char) *ptr];
>
> if (unlikely(pg_mul_s64_overflow(tmp, 16, &tmp)) ||
> unlikely(pg_sub_s64_overflow(tmp, digit, &tmp)))
> goto out_of_range;
>
> ptr++;
> }
>
> I wonder if you'd be better off with something like:
>
> while (*ptr && isxdigit((unsigned char) *ptr))
> {
> if (unlikely(tmp & UINT64CONST(0xF000000000000000)))
> goto out_of_range;
>
> tmp = (tmp << 4) | hexlookup[(unsigned char) *ptr++];
> }
>
> Going by [1], clang will actually use multiplication by 16 to
> implement the former. gcc is better and shifts left by 4, so likely
> won't improve things for gcc. It seems worth doing it this way for
> anything that does not have HAVE__BUILTIN_OP_OVERFLOW anyway.

My code follows the style used for parsing the decimal integers.
Keeping that consistent is valuable I think. I think the proposed
change makes the code significantly harder to understand. Also, what
you are suggesting here would amount to an attempt to make parsing
hexadecimal integers even faster than parsing decimal integers. Is that
useful?

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Aleksander Alekseev 2022-11-24 08:36:04 Re: Add 64-bit XIDs into PostgreSQL 15
Previous Message mahendrakar s 2022-11-24 08:20:49 Re: [PoC] Federated Authn/z with OAUTHBEARER