PGPROC alignment (was Re: pgsql: Separate RecoveryConflictReasons from procsignals)
Heikki Linnakangas <hlinnaka@iki.fi>
From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Andres Freund <andres@anarazel.de>
Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>,
"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2026-02-10T17:14:44Z
Lists: pgsql-hackers
Attachments
- 0001-Align-PGPROC-to-cache-line-boundary.patch (text/x-patch) patch 0001
(moving to pgsql-hackers) On 10/02/2026 18:41, Andres Freund wrote: > On 2026-02-10 17:52:16 +0200, Heikki Linnakangas wrote: >> If there's a performance reason to keep have it be aligned - and maybe there >> is - we should pad it explicitly. > > We should make it a power of two or such. There are some workloads where the > indexing from GetPGProcByNumber() shows up, because it ends up having to be > implemented as a 64 bit multiplication, which has a reasonably high latency > (3-5 cycles). Whereas a shift has a latency of 1 and typically higher > throughput too. Power of two means going to 1024 bytes. That's a lot of padding. Where have you seen that show up? Attached is a patch to align to cache line boundary. That's straightforward if that's what we want to do. > Re false sharing: We should really separate stuff that changes (like > e.g. pendingRecoveryConflicts) and never changing stuff (backendType). You > don't need overlapping structs to have false sharing issues if you mix > different access patterns inside a struct that's accessed across processes... Makes sense, although I don't want to optimize too hard for performance, at the expense of readability. The current order is pretty random anyway, though. It'd probably be good to move the subxids cache to the end of the struct. That'd act as natural padding, as it's not very frequently used, especially the tail end of the cache. Or come to think of it, it might be good to move the subxids cache out of PGPROC altogether. It's mostly frequently accessed in GetSnapshotData(), and for that it'd actually be better if it was in a separate "mirrored" array, similar to the main xid and subxidStates. That would eliminate the pgprocnos[pgxactoff] lookup from GetSnapshotdata() altogether. I'm a little reluctant to mess with this without a concrete benchmark though. Got one in mind? - Heikki
Commits
-
Use C11 alignas in typedef definitions
- 5c2a8d272b9a 19 (unreleased) landed
-
Align PGPROC to cache line boundary
- 412f78c66eed 19 (unreleased) landed
-
Rearrange fields in PGPROC, for clarity
- 2e0853176f8f 19 (unreleased) landed
-
Split PGPROC 'links' field into two, for clarity
- 36bbcd5be3ff 19 (unreleased) landed
-
Remove useless store to local variable
- ab32a9e21d37 19 (unreleased) landed
-
Separate RecoveryConflictReasons from procsignals
- 17f51ea81875 19 (unreleased) cited