On Thu, Aug 6, 2026 at 8:07 PM Chao Li <[email protected]> wrote: > > > > > On Aug 7, 2026, at 00:09, Masahiko Sawada <[email protected]> wrote: > > > > 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. > > It doesn't have to be exactly 39; 1024 (long enough) would also work, or > perhaps something based on UUID_LEN, such as UUID_LEN * 3. I think the main > point is to avoid unbounded scanning on malformed input. > > The old code did not have this issue because it only examined as much input > as needed based on UUID_LEN. The new fast path starts to use strlen(), so > this would be a new risk introduced by the optimization.
I don't think the scan can be really unbounded. string_to_uuid() receives a cstring, so by the time it is called the caller has already walked or copied the whole string to produce it. So unless the unbounded scan can be reached in some path I have overlooked, I'd prefer to keep strlen() here. Happy to change it if you still think it is worth it. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
