From: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
---|---|
To: | John Naylor <john(dot)naylor(at)enterprisedb(dot)com> |
Cc: | Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Jelte Fennema <Jelte(dot)Fennema(at)microsoft(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Merlin Moncure <mmoncure(at)gmail(dot)com>, Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net> |
Subject: | Re: [PATCH] Optimize json_lex_string by batching character copying |
Date: | 2022-08-21 05:47:33 |
Message-ID: | 20220821054733.GA780295@nathanxps13 |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Aug 19, 2022 at 01:42:15PM -0700, Nathan Bossart wrote:
> On Fri, Aug 19, 2022 at 03:11:36PM +0700, John Naylor wrote:
>> This is done. Also:
>> - a complete overhaul of the pg_lfind8* tests
>> - using a typedef for the vector type
>> - some refactoring, name changes and other cleanups (a few of these
>> could also be applied to the 32-byte element path, but that is left
>> for future work)
>>
>> TODO: json-specific tests of the new path
>
> This looks pretty good to me. Should we rename vector_broadcast() and
> vector_has_zero() to indicate that they are working with bytes (e.g.,
> vector_broadcast_byte())? We might be able to use vector_broadcast_int()
> in the 32-bit functions, and your other vector functions already have a
> _byte suffix.
>
> In general, the approach you've taken seems like a decent readability
> improvement. I'd be happy to try my hand at adjusting the 32-bit path and
> adding ARM versions of all this stuff.
I spent some more time looking at this one, and I had a few ideas that I
thought I'd share. 0001 is your v6 patch with a few additional changes,
including simplying the assertions for readability, splitting out the
Vector type into Vector8 and Vector32 (needed for ARM), and adjusting
pg_lfind32() to use the new tools in simd.h. 0002 adds ARM versions of
everything, which obsoletes the other thread I started [0]. This is still
a little rough around the edges (e.g., this should probably be more than 2
patches), but I think it helps demonstrate a more comprehensive design than
what I've proposed in the pg_lfind32-for-ARM thread [0].
Apologies if I'm stepping on your toes a bit here.
[0] https://postgr.es/m/20220819200829.GA395728%40nathanxps13
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachment | Content-Type | Size |
---|---|---|
0001-json_lex_string-SIMD.patch | text/x-diff | 17.7 KB |
0002-ARM-SIMD-support.patch | text/x-diff | 4.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Pavel Stehule | 2022-08-21 05:51:07 | Re: Schema variables - new implementation for Postgres 15 |
Previous Message | Julien Rouhaud | 2022-08-21 04:36:21 | Re: Schema variables - new implementation for Postgres 15 |