Re: MAX_BACKENDS size (comment accuracy)

2025-02-24 Thread Nathan Bossart
On Mon, Feb 24, 2025 at 03:38:24PM -0500, Andres Freund wrote: > Makes sense. Committed, thanks. -- nathan

Re: MAX_BACKENDS size (comment accuracy)

2025-02-24 Thread Andres Freund
Hi, On 2025-02-24 13:52:51 -0600, Nathan Bossart wrote: > The recent commits for this drew my attention to the comment for > MAX_BACKENDS. Specifically, it says we check the value in > RegisterBackgroundWorker() (which appears to have been untrue since we > added max_worker_processes) and relevan

Re: MAX_BACKENDS size (comment accuracy)

2025-02-24 Thread Nathan Bossart
The recent commits for this drew my attention to the comment for MAX_BACKENDS. Specifically, it says we check the value in RegisterBackgroundWorker() (which appears to have been untrue since we added max_worker_processes) and relevant GUC check hooks (which I removed last year in commit 0b1fe14).

Re: MAX_BACKENDS size (comment accuracy)

2025-02-24 Thread Andres Freund
Hi, On 2025-02-23 10:39:36 +1300, Thomas Munro wrote: > On Sun, Feb 23, 2025 at 4:16 AM Andres Freund wrote: > > We do count the number of lwlock share lockers and the number of buffer > > refcounts within those bits. And obviously 0 lockers/refcounts has to be > > valid. So I think the limit is

Re: MAX_BACKENDS size (comment accuracy)

2025-02-22 Thread Thomas Munro
On Sun, Feb 23, 2025 at 4:16 AM Andres Freund wrote: > We do count the number of lwlock share lockers and the number of buffer > refcounts within those bits. And obviously 0 lockers/refcounts has to be > valid. So I think the limit is correct? Ah, right. That makes perfect sense. The 18 bits ne

Re: MAX_BACKENDS size (comment accuracy)

2025-02-22 Thread Andres Freund
Hi, On 2025-02-22 18:54:08 +1300, Thomas Munro wrote: > On Sat, Feb 22, 2025 at 7:27 AM Andres Freund wrote: > +#define MAX_BACKENDS_BITS18 > +#define MAX_BACKENDS((1 << MAX_BACKENDS_BITS)-1) > > +1 for working forwards from the bits. But why not call it > PROC_NUMBER_BITS?

Re: MAX_BACKENDS size (comment accuracy)

2025-02-21 Thread Thomas Munro
On Sat, Feb 22, 2025 at 7:27 AM Andres Freund wrote: +#define MAX_BACKENDS_BITS18 +#define MAX_BACKENDS((1 << MAX_BACKENDS_BITS)-1) +1 for working forwards from the bits. But why not call it PROC_NUMBER_BITS? After 024c5211 and 024c5211^, the ID for backends is a ProcNumber,

Re: MAX_BACKENDS size (comment accuracy)

2025-02-21 Thread Andres Freund
Hi, On 2025-01-26 12:55:15 -0800, Jacob Brazeal wrote: > The patch series looks good. Thanks for reviewing! And sorry for not getting back to you about your review comments earlier. > > StaticAssertDecl((MAX_BACKENDS & LW_FLAG_MASK) == 0, > > "MAX_BACKENDS and LW_FLAG_MASK overlap"); > > Sh

Re: MAX_BACKENDS size (comment accuracy)

2025-02-12 Thread Andres Freund
Hi, On 2025-02-09 12:41:58 -0800, Jacob Brazeal wrote: > > Halfing the size of LWLock and laying > > the ground work for making the wait-list lock-free imo would be very well > > worth the reduction in an unrealistic limit... > > BTW, I spent a week or two researching the lock-free queue idea, > s

Re: MAX_BACKENDS size (comment accuracy)

2025-02-09 Thread Jacob Brazeal
> Halfing the size of LWLock and laying > the ground work for making the wait-list lock-free imo would be very well > worth the reduction in an unrealistic limit... BTW, I spent a week or two researching the lock-free queue idea, specifically looking at what it might look like to have a Michael-Sc

Re: MAX_BACKENDS size (comment accuracy)

2025-01-26 Thread Jacob Brazeal
I had a typo earlier: I should have said: > StaticAssertDecl((MAX_BACKENDS & LW_FLAG_MASK) == 0, > "MAX_BACKENDS and LW_FLAG_MASK overlap"); Should this check that LW_LOCK_MASK & LW_FLAG_MASK == 0? To also ensure the LW_VAL_EXCLUSIVE bit does not overlap.

Re: MAX_BACKENDS size (comment accuracy)

2025-01-26 Thread Jacob Brazeal
Looking at v1-0003-WIP-Base-LWLock-limits-directly-on-MAX_BACKENDS.patch, I'm curious about the following assert; > #define LW_VAL_EXCLUSIVE (MAX_BACKENDS + 1) ... > StaticAssertDecl(MAX_BACKENDS < LW_VAL_EXCLUSIVE, "MAX_BACKENDS too big for lwlock.c"); Since LW_VAL_EXCLUSIVE is already defined

Re: MAX_BACKENDS size (comment accuracy)

2025-01-26 Thread Jacob Brazeal
I find I didn't send the previous reply to the mailing list, so I'll copy it here. --- The patch series looks good. It looks like this currently leaves 10 bits of unused space (bits 20 - 29) in the state. > StaticAssertDecl((MAX_BACKENDS & LW_FLAG_MASK) == 0, > "MAX_BACKENDS and LW_FLAG_MASK o

Re: MAX_BACKENDS size (comment accuracy)

2025-01-26 Thread Andres Freund
Hi, On 2025-01-26 13:10:35 -0500, Andres Freund wrote: > On 2025-01-25 16:06:29 -0800, Jacob Brazeal wrote: > > diff --git a/src/backend/storage/lmgr/lwlock.c > > b/src/backend/storage/lmgr/lwlock.c index 2f558ffea1..d3a2619072 100644 --- > > a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/s

Re: MAX_BACKENDS size (comment accuracy)

2025-01-26 Thread Andres Freund
Hi, On 2025-01-25 23:35:51 -0800, Jacob Brazeal wrote: > While we are on the topic of comments from lwlock.c, there is one other one > that confused me, in LWLockWaitListLock: > * /* and then spin without atomic operations until lock is released */ { > SpinDelayStatus delayStatus; init_local_spi

Re: MAX_BACKENDS size (comment accuracy)

2025-01-26 Thread Andres Freund
Hi, On 2025-01-25 16:06:29 -0800, Jacob Brazeal wrote: > > In lwlocks.c, we have the following comment, related to LWLock state: > > > > > > */* Must be greater than MAX_BACKENDS - which is 2^23-1, so we're fine. > > */#define LW_SHARED_MASK ((uint32) ((1 << 24)-1))* > > > > However, MAX_BACKENDS

Re: MAX_BACKENDS size (comment accuracy)

2025-01-25 Thread Jacob Brazeal
While we are on the topic of comments from lwlock.c, there is one other one that confused me, in LWLockWaitListLock: * /* and then spin without atomic operations until lock is released */ { SpinDelayStatus delayStatus; init_local_spin_delay(&delayStatus); while (old_state & LW_FLAG

Re: MAX_BACKENDS size (comment accuracy)

2025-01-25 Thread Jacob Brazeal
Thinking a bit further about this, the purpose of the LW_SHARED_MASK section of the state is to count the number of lock-sharers. Thus, we only care about the actual number of backends (up to 2^18-1) here and not the size of the ProcNumber data type. So I do think the comment should read 2^18-1 and

MAX_BACKENDS size (comment accuracy)

2025-01-25 Thread Jacob Brazeal
Hello all, In lwlocks.c, we have the following comment, related to LWLock state: */* Must be greater than MAX_BACKENDS - which is 2^23-1, so we're fine. */#define LW_SHARED_MASK ((uint32) ((1 << 24)-1))* However, MAX_BACKENDS is set to 2^18-1. Here is the comment in postmaster.h: */*