Re: make LWLockCounter a global variable

From: Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Nathan Bossart <nathandbossart(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: make LWLockCounter a global variable
Date: 2025-08-29 06:52:48
Message-ID: aLFOQOb4slZdIzGb@ip-10-97-1-34.eu-west-3.compute.internal
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On Thu, Aug 28, 2025 at 05:56:07PM -0400, Tom Lane wrote:
> Nathan Bossart <nathandbossart(at)gmail(dot)com> writes:
> > In lwlock.c, uses of LWLockCounter must first calculate its address in
> > shared memory with something like this:
>
> > LWLockCounter = (int *) ((char *) MainLWLockArray - sizeof(int));
>
> > This appears to have been started by commit 82e861f in order to fix
> > EXEC_BACKEND builds, but it could also be fixed by adding it to the
> > BackendParameters struct. I find the current approach somewhat difficult
> > to read and understand, so I'd like to switch to the latter approach. This
> > is admittedly just nitpicking...
>
> No objection here. As a small improvement, perhaps you could swap
> around the code in LWLockShmemSize so that the order in which it
> considers size contributions matches the physical layout, more
> or less like
>
> /* Calculate total number of locks needed in the main array. */
> numLocks += NumLWLocksForNamedTranches();
>
> + /* Space for dynamic allocation counter, plus room for alignment. */
> + size = sizeof(int) + LWLOCK_PADDED_SIZE;
> +
> /* Space for the LWLock array. */
> - size = mul_size(numLocks, sizeof(LWLockPadded));
> + size = add_size(size, mul_size(numLocks, sizeof(LWLockPadded)));
>
> - /* Space for dynamic allocation counter, plus room for alignment. */
> - size = add_size(size, sizeof(int) + LWLOCK_PADDED_SIZE);
> -
> /* space for named tranches. */
> size = add_size(size, mul_size(NamedLWLockTrancheRequests, sizeof(NamedLWLockTranche)));
>
> I find it a little confusing that that code doesn't line up
> exactly with what CreateLWLocks does.

+1.

Another option could be to not change CreateLWLocks() at all, except removing the
local variable:

@@ -423,7 +424,6 @@ CreateLWLocks(void)
if (!IsUnderPostmaster)
{
Size spaceLocks = LWLockShmemSize();
- int *LWLockCounter;

to use the global variable. That way we preserve the current memory layout and
there is no need to change LWLockShmemSize().

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Joel Jacobson 2025-08-29 07:02:43 Assert single row returning SQL-standard functions
Previous Message Amit Kapila 2025-08-29 06:48:50 Re: How can end users know the cause of LR slot sync delays?