Cleanup: PGProc->links doesn't need to be the first field anymore
pgproc.h has this:
struct PGPROC
{
/* proc->links MUST BE FIRST IN STRUCT (see ProcSleep,ProcWakeup,etc) */
dlist_node links; /* list link if process is in a list */
dlist_head *procgloballist; /* procglobal list that owns this PGPROC */
...
I don't see any particular reason for 'links' to be the first field. We
used to do things like "proc = (PGPROC *) waitQueue->links.next", but
since commit 5764f611e1, this has been a "dlist", and dlist_container()
can handle the list link being anywhere in the struct.
I tried moving it and ran the regression tests. That revealed one place
where we still don't use dlist_container:
if (!dlist_is_empty(procgloballist))
{
MyProc = (PGPROC *) dlist_pop_head_node(procgloballist);
...
I believe that was just an oversight. Trivial patch attached.
--
Heikki Linnakangas
Neon (https://neon.tech)
Attachments:
0001-Lift-limitation-that-PGPROC-links-must-be-the-first-.patchtext/x-patch; charset=UTF-8; name=0001-Lift-limitation-that-PGPROC-links-must-be-the-first-.patchDownload+1-3
Hi Heikki,
I tried moving it and ran the regression tests. That revealed one place
where we still don't use dlist_container:if (!dlist_is_empty(procgloballist))
{
MyProc = (PGPROC *) dlist_pop_head_node(procgloballist);
...I believe that was just an oversight. Trivial patch attached.
I tested your patch. LGTM.
PGPROC is exposed to third-party code, but since we don't change the
structure this time, the extensions will not be affected.
--
Best regards,
Aleksander Alekseev
Hi,
On 2024-07-04 01:54:18 +0300, Heikki Linnakangas wrote:
pgproc.h has this:
struct PGPROC
{
/* proc->links MUST BE FIRST IN STRUCT (see ProcSleep,ProcWakeup,etc) */
dlist_node links; /* list link if process is in a list */
dlist_head *procgloballist; /* procglobal list that owns this PGPROC */
...I don't see any particular reason for 'links' to be the first field. We used
to do things like "proc = (PGPROC *) waitQueue->links.next", but since
commit 5764f611e1, this has been a "dlist", and dlist_container() can handle
the list link being anywhere in the struct.
Indeed.
I tried moving it and ran the regression tests. That revealed one place
where we still don't use dlist_container:if (!dlist_is_empty(procgloballist))
{
MyProc = (PGPROC *) dlist_pop_head_node(procgloballist);
...I believe that was just an oversight. Trivial patch attached.
Oops. Yes, I clearly should have used dlist_container() here.
+1
Greetings,
Andres Freund
On 04/07/2024 23:20, Andres Freund wrote:
On 2024-07-04 01:54:18 +0300, Heikki Linnakangas wrote:
I believe that was just an oversight. Trivial patch attached.
Oops. Yes, I clearly should have used dlist_container() here.
Committed, thanks.
--
Heikki Linnakangas
Neon (https://neon.tech)