Re: postmaster uses more CPU in 18 beta1 with io_method=io_uring

> On Jun 30, 2025, at 12:27 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> Hi,
>
> On 2025-06-05 14:32:10 -0400, Andres Freund wrote:
>> On 2025-06-05 12:47:52 -0400, Tom Lane wrote:
>>> Andres Freund <andres(at)anarazel(dot)de> writes:
>>>> I think this is a big enough pitfall that it's, obviously assuming the patch
>>>> has a sensible complexity, worth fixing this in 18. RMT, anyone, what do you
>>>> think?
>>>
>>> Let's see the patch ... but yeah, I'd rather not ship 18 like this.
>>
>> I've attached a first draft.
>>
>> I can't make heads or tails of the ordering in configure.ac, so the function
>> test is probably in the wrong place.
>
> Any comments on that patch? I'd hoped for some review comments... Unless I'll
> hear otherwise, I'll just do a bit more polish and push..

Thanks for doing this work!

I just read through the v1 patch and it looks good. I have just a few small nit-picky questions:

+ #if defined(HAVE_LIBURING_QUEUE_INIT_MEM) && defined(IORING_SETUP_NO_MMAP) && 1

The '1' looks like cruft, or am I missing something?

+ /* FIXME: This should probably not stay at DEBUG1? */

Worth fixing before pushing?

Also, this returns 'Size' but in the function uses 'size_t' I assume that's intentional?

+ static Size
+ pgaio_uring_ring_shmem_size(void)

The next, similar, function below this one returns 'size_t'.

Finally, and this may be me missing something everyone else knows is convention.

+ * XXX: We allocate memory for all PgAioUringContext instances and, if

Is there any reason to keep the 'XXX'? You ask yourself a question in that comment, do you know the answer or was that a request to reviewers for feedback? :)

I hope that is helpful.

-greg

>
> Greetings,
>
> Andres