On Wed, Aug 5, 2026 at 8:40 PM Chao Li <[email protected]> wrote: > > > > > On Aug 6, 2026, at 08:19, Masahiko Sawada <[email protected]> wrote: > > > > On Tue, Jun 30, 2026 at 11:03 AM Haibo Yan <[email protected]> wrote: > >> > >> On Tue, Jun 30, 2026 at 10:53 AM Masahiko Sawada <[email protected]> > >> wrote: > >>> > >>> On Mon, Jun 29, 2026 at 5:53 PM Haibo Yan <[email protected]> wrote: > >>>> > >>>> On Mon, Jun 29, 2026 at 2:55 PM Masahiko Sawada <[email protected]> > >>>> wrote: > >>>>> > >>>>> On Sun, Jun 28, 2026 at 7:20 PM Haibo Yan <[email protected]> wrote: > >>>>>> > >>>>>> On Thu, Jun 25, 2026 at 3:16 PM Masahiko Sawada > >>>>>> <[email protected]> wrote: > >>>>>>> > >>>>>>> On Thu, Jun 25, 2026 at 2:31 PM Haibo Yan <[email protected]> > >>>>>>> wrote: > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> On Thu, Jun 25, 2026 at 11:28 AM Masahiko Sawada > >>>>>>>> <[email protected]> wrote: > >>>>>>>>> > >>>>>>>>> Hi all, > >>>>>>>>> > >>>>>>>>> I'd like to propose the $subject. > >>>>>>>>> > >>>>>>>>> Since commit ec8719ccbfcd made hex_decode_safe() SIMD-aware, > >>>>>>>>> decoding > >>>>>>>>> a run of hex digits is now fast. The attached patch reuses > >>>>>>>>> hex_decode_safe() in the UUID input function to speed up parsing. > >>>>>>>>> > >>>>>>>>> We accept several textual forms of a UUID[1]. The fast path handles > >>>>>>>>> the common ones: 32 hex digits, the canonical 8x-4x-4x-4x-12x form > >>>>>>>>> (where "nx" means n hex digits), and either of those wrapped in > >>>>>>>>> braces. Otherwise, it falls back to the ordinary scalar UUID parse. > >>>>>>>>> > >>>>>>>>> I've benchmarked the parse speed using the following query: > >>>>>>>>> > >>>>>>>>> CREATE TEMP TABLE u AS SELECT gen_random_uuid()::text AS t FROM > >>>>>>>>> generate_series(1, 1000000); > >>>>>>>>> EXPLAIN (ANALYZE, TIMING OFF) SELECT t::uuid FROM u; > >>>>>>>>> > >>>>>>>>> I compared the execution time of the second query, which measures > >>>>>>>>> uuid_in() alone, with/without SIMD optimization. Here are results > >>>>>>>>> (the > >>>>>>>>> median of 5 runs): > >>>>>>>>> > >>>>>>>>> HEAD: 208.879 ms > >>>>>>>>> Patched: 40.983 ms > >>>>>>>>> > >>>>>>>>> The improvements look promising to me. But in a realistic pipeline > >>>>>>>>> the > >>>>>>>>> parse is a small fraction of the work, so end-to-end gains could be > >>>>>>>>> much smaller. > >>>>>>>>> > >>>>>>>>> Feedback is very welcome. > >>>>>>>>> > >>>>>>>> I may be missing something, but I wonder whether the fast path is > >>>>>>>> relying on > >>>>>>>> slightly different input semantics from the existing UUID parser. > >>>>>>>> > >>>>>>>> In particular, hex_decode_safe() is not a strict “32 hex characters > >>>>>>>> only” > >>>>>>>> decoder. It skips whitespace, which is fine for its existing > >>>>>>>> callers, but I > >>>>>>>> don’t think UUID input should treat whitespace inside the UUID body > >>>>>>>> as > >>>>>>>> ignorable. > >>>>>>> > >>>>>>> Good catch! hex_decode_safe() skips whitespaces so the patch accepts > >>>>>>> the following UUID value, which is bad: > >>>>>>> > >>>>>>> select '019f00b5-7f8a-722f-b707-59f0ed25cd '::uuid; > >>>>>>> uuid > >>>>>>> -------------------------------------- > >>>>>>> 019f00b5-7f8a-722f-b707-59f0ed25cd00 > >>>>>>> (1 row) > >>>>>>> > >>>>>>>> Also, since hex_decode_safe() returns void, the UUID fast path > >>>>>>>> cannot verify that exactly UUID_LEN bytes were produced. > >>>>>>> > >>>>>>> IIUC hex_decode_safe() does return the output length in bytes. So I > >>>>>>> think we can fallback to the scalar UUID parser if > >>>>>>> esctx.error_occurred is true or if the returned value is not 16. > >>>>>>> > >>>>>> > >>>>>> You’re right, I misread that part. Checking both esctx.error_occurred > >>>>>> and > >>>>>> the returned length sounds good to me. > >>>>>> > >>>>>>>> > >>>>>>>> So I think it would be safer either to pre-validate that the 32 > >>>>>>>> source > >>>>>>>> characters are all hex digits before calling hex_decode_safe(), or > >>>>>>>> to use a > >>>>>>>> UUID-specific strict hex decoder for this path. After that, a > >>>>>>>> comment > >>>>>>>> explaining why hex_decode_safe() is safe here would make the > >>>>>>>> invariant much > >>>>>>>> clearer. > >>>>>>> > >>>>>>> IIUC hex_decode_simd_helper() accepts only hex digits so we could > >>>>>>> re-use it for UUID parsing. Let me check if the above idea of using > >>>>>>> the return value works for us first. > >>>>>>> > >>>>>> > >>>>>> That sounds reasonable. My main concern was to keep the fast path’s > >>>>>> accepted > >>>>>> input set identical to the scalar UUID parser. Falling back when the > >>>>>> decoded > >>>>>> length is not UUID_LEN, together with regression tests for whitespace > >>>>>> cases, > >>>>>> should address that. > >>>>>> > >>>>>>>> > >>>>>>>> Could you also add a few regression tests for invalid inputs that > >>>>>>>> contain > >>>>>>>> whitespace inside otherwise fast-path-looking UUID strings? For > >>>>>>>> example: > >>>>>>>> > >>>>>>>> --------------------------------------------------------------- > >>>>>>>> > >>>>>>>> SELECT 'a0eebc99 9c0b4ef8bb6d6bb9bd380a11'::uuid; > >>>>>>>> SELECT 'a0eebc999c0b4ef8bb6d6bb9bd380a1 '::uuid; > >>>>>>>> SELECT '{a0eebc999c0b4ef8bb6d6bb9bd380a1 }'::uuid; > >>>>>>>> SELECT 'a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a1 '::uuid; > >>>>>>>> --------------------------------------------------------------- > >>>>>>>> > >>>>>>>> These should continue to be rejected in the same way as the scalar > >>>>>>>> parser. > >>>>>>>> Regards, > >>>>>>> > >>>>>>> Agreed. > >>>>>>> > >>>>> > >>>>> I've attached the updated patch. > >>>>> > >>>>> Regards, > >>>>> > >>>>> -- > >>>>> Masahiko Sawada > >>>>> Amazon Web Services: https://aws.amazon.com > >>>> > >>>> I noticed a few typos in the comments: > >>>> > >>>> src/backend/utils/adt/uuid.c > >>>> line 56: “scalar implmentation” -> “scalar implementation” > >>>> line 109: “swalled” -> “swallowed” > >>>> line 110: “kepping” -> “keeping” > >>>> line 118: “grammer” -> “grammar” > >>>> line 119: “whitespaces” -> “whitespace” > >>>> > >>>> Could you fix them ? > >>> > >>> Oops, I fixed them and rechecked other places. > >>> > >>> I've attached the updated patch. > >>> > >>> Regards, > >>> > >>> -- > >>> Masahiko Sawada > >>> Amazon Web Services: https://aws.amazon.com > >> > >> The code looks good to me now. I only noticed one small typo in the > >> commit trailer: Reviwed-by should be Reviewed-by. > >> > >> Otherwise, it looks good. Thank you for fixing these issues. > >> > > > > After spending more time on this patch, I find out two things: > > > > 1. USE_NO_SIMD doesn't work in uuid.c without including port/simd.h. > > But including port/simd.h seems wrong as it doesn't use any SIMD > > support functions. > > > > 2. hex_decode_safe() is faster than the current UUID parse > > (isxdigit()+strtoul() approach) even without SIMD. I've created a > > small benchmark test tool (attached as 0002 patch, not intended to be > > pushed into the core), and measures UUID parsing performance of three > > approaches: 'scalar' is the current string_to_uuid() that uses > > isxdigit()+strtoul()), 'simd' uses hex_decode_safe() with SIMD, and > > 'nosimd' uses hex_decode_safe() without SIMD, with different shapes of > > UUIDs. Here are results: > > > > =# select path, shape, n_inputs, best_ms::numeric(10,3) from > > uuid_parse_bench(100000, 5); > > path | shape | n_inputs | best_ms > > --------+------------------+----------+--------- > > scalar | canonical | 100000 | 22.661 > > simd | canonical | 100000 | 1.400 > > nosimd | canonical | 100000 | 1.652 > > scalar | bare32 | 100000 | 15.932 > > simd | bare32 | 100000 | 0.471 > > nosimd | bare32 | 100000 | 1.110 > > scalar | braced_canonical | 100000 | 17.330 > > simd | braced_canonical | 100000 | 1.088 > > nosimd | braced_canonical | 100000 | 1.314 > > scalar | braced_bare32 | 100000 | 15.942 > > simd | braced_bare32 | 100000 | 0.488 > > nosimd | braced_bare32 | 100000 | 1.141 > > scalar | dashed4 | 100000 | 16.185 > > simd | dashed4 | 100000 | 16.493 > > nosimd | dashed4 | 100000 | 16.403 > > scalar | invalid_hex | 100000 | 0.199 > > simd | invalid_hex | 100000 | 1.150 > > nosimd | invalid_hex | 100000 | 0.385 > > (18 rows) > > > > Each of shape means: > > - 'canonical': 8x-4x-4x-4x-12x, what uuid_out() emits > > - 'bare32': 32 contiguous hex digits > > - 'braced_canonical': {8x-4x-4x-4x-12x} > > - 'braced_bare32': {32 hex digits} > > - 'dashed4': dash after every group of 4 > > - 'invalid_hdx': canonical but with a invalid digit > > > > 'nosimd' is 10x~ faster than 'scalar' in most cases. All paths are > > mostly the same in 'dashed4' and 'invalid_hex' cases because 'simd' > > and 'nosimd' fall back to the 'scalar' case. According to these > > results, my conclusion is that we can use hex_decode_safe() for > > canonical forms and 32 contiguous hex forms anyway, and let > > hex_decode_safe() choose whether to use SIMD. We would win in either > > case. We still use the current scalar approach for uncommon UUID forms > > and error reporting purposes. > > > > Regards, > > > > -- > > Masahiko Sawada > > Amazon Web Services: https://aws.amazon.com > > <v4-0001-Optimize-UUID-parse-using-SIMD.patch><v4-0002-uuid_parse_bench-module.patch> > > A few comments on v4. > > 1 - 0001 > ``` > +static void > +string_to_uuid(const char *source, pg_uuid_t *uuid, Node *escontext) > +{ > + const char *body = source; > + size_t len = strlen(source); > ``` > > I think it would be better to avoid strlen(). The old code processes at most > UUID_LEN (16) byte pairs, so it does not need to scan arbitrarily far on > malformed input. So, maybe we could use something like strnlen(source, 39) > instead.
While strnlen(source, 39) works there, 39 is a magic number and it's tied to the current format check logic. What is the benefit of using strnlen(source, 39) instead? I'm not sure it warrants having the magic number. > > 2 - 0002 - uuid_parse_bench/Makefile > ``` > +REGRESS = uuid_parse_bench > ``` > > When I tried to run the test, I got an error: > ``` > # +++ regress check in src/test/modules/uuid_parse_bench +++ > # initializing database system by copying initdb template > # using temp instance on port 52544 with PID 56970 > /bin/sh: > /Users/chaol/Documents/code/postgresql/src/test/modules/uuid_parse_bench/sql/uuid_parse_bench.sql: > No such file or directory > diff: > /Users/chaol/Documents/code/postgresql/src/test/modules/uuid_parse_bench/expected/uuid_parse_bench.out: > No such file or directory > ``` > > Not sure if you missed to check in uuid_parse_bench.sql and > uuid_parse_bench.out. Oops, I should have removed that line. The extension creates the sole uuid_parse_bench() SQL function, so you can simply execute uuid_parse_bench() to measure the performance. Please note that the extension is not intended to push to the core. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
