Horton

Thread

Commits

  1. Lift limitation that PGPROC->links must be the first field

  2. Use dlist/dclist instead of PROC_QUEUE / SHM_QUEUE for heavyweight locks

  1. Cleanup: PGProc->links doesn't need to be the first field anymore

    Heikki Linnakangas <hlinnaka@iki.fi> — 2024-07-03T22:54:18Z

    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)

  2. Re: Cleanup: PGProc->links doesn't need to be the first field anymore

    Aleksander Alekseev <aleksander@timescale.com> — 2024-07-04T12:09:15Z

    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

  3. Re: Cleanup: PGProc->links doesn't need to be the first field anymore

    Andres Freund <andres@anarazel.de> — 2024-07-04T20:20:46Z

    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

  4. Re: Cleanup: PGProc->links doesn't need to be the first field anymore

    Heikki Linnakangas <hlinnaka@iki.fi> — 2024-07-05T08:34:06Z

    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)