| From: | Andres Freund <andres(at)anarazel(dot)de> |
|---|---|
| To: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
| Cc: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>, Sami Imseih <samimseih(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Fix pg_stat_get_backend_wait_event() for aux processes |
| Date: | 2026-02-05 17:21:22 |
| Message-ID: | tw53roer2j4quxh7vlyv62drc5fo6c6zdltvl6d2dttqa62uhi@stwlpdwlftpj |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On 2026-02-04 11:36:14 +0200, Heikki Linnakangas wrote:
> On 04/02/2026 10:02, Bertrand Drouvot wrote:
> > On Tue, Feb 03, 2026 at 10:29:27PM +0200, Heikki Linnakangas wrote:
> > > There might be a performance argument too,
> >
> > yeah, not sure but with the patch in place the size of PGPROC goes from
> > 832 bytes to 824 bytes. Is it worth to add extra padding so that it still remain
> > a multiple of 64?
>
> Hmm, I don't think so. We've never given cacheline alignment any thought
> when we've changed the PGPROC fields in the past (or at least I haven't).
> Perhaps we should, but it would warrant a separate investigation.
I've looked into that in the past, and yes, I think we ought to do it. Not
just because avoiding false sharing is good, but also because the current size
makes indexing more expensive, due to needing multiplications to index the
array of procs, instead of using something like x86's lea.
We really should order PGPROC a bit more so that frequently read and
frequently modified data is on separate lines. The spread nature of it right
now means we have unnecessarily many cachelines transitioning states.
> Now that I look at that, the most frequently accessed fields are not at the
> beginning or end of the struct, so I don't think there's much harm in
> sharing cache lines.
Hm? ->xid, ->xmin are at the front and are very frequently changed. Whereas
e.g. ->pid changes rarely but is read by other processes. And
e.g. fpLocalTransactionId is at the tail and also changes quite frequently.
Won't this change possibly lead to more cacheline contention on the backend
status array? At the moment the struct is not cacheline sized and has a
frequently changing field at the start and you're adding an extremely
frequently changing field at the end. Before there was no false sharing of
the wait event field, because it was far enough from either end of PGPROC.
It's not going to make it hugely worse, but it might still be noticeable.
Greetings,
Andres Freund
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Jacob Champion | 2026-02-05 17:26:34 | Re: client_connection_check_interval default value |
| Previous Message | Andrew Dunstan | 2026-02-05 17:14:27 | Re: FileFallocate misbehaving on XFS |