Re: GetRelationPath() vs critical sections

Hi,

On 2025-02-21 18:20:39 +1300, Thomas Munro wrote:
> On Fri, Feb 21, 2025 at 9:28 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
> > The patch curently uses a hardcoded 6 for the length of MAX_BACKENDS. Does
> > anybody have a good idea for how to either
> >
> > a) derive 6 from MAX_BACKENDS in a way that can be used in a C array size
>
> Do we even check the binary digits? BTW I see a place in lwlock.c
> that still talks about 2^23-1, looks like it is out of date.

Yea. I actually posted a patch addressing that recently-ish:
https://postgr.es/m/7bye3hvkdk7ybt4ofigimr4ahshsj657aqatfc5trd5pbp5qd4%40dir2wsx2lgo6

Patch 0001 in that series would be nice to have here, because it moves
MAX_BACKENDS out of postmaster.h. I kind had hoped somebody would comment on
whether pg_config_manual.h is a good place for the define.

I guess we could also move it to storage/procnumber.h.

> Hmmm, I wonder if it would be better to start by declaring how many bits we
> want to use, given that is our real constraint. And then:
>
> #define PROCNUMBER_BITS 18
> #define MAX_BACKENDS ((1 << PROCNUMBER_BITS) - 1)
> #define PROCNUMBER_CHARS DECIMAL_DIGITS_FOR_BITS(PROCNUMBER_BITS)
>
> ... with a little helper ported to preprocessor hell from Hacker's
> Delight magic[1] for that last bit. See attached. But if that's a
> bit too nuts...

This seems a bit too complicated to be worth it. I am inclined to think the
approach taken in the patch of just having a regression test verifying that
the numbers match is good enough.

The one arguments I can see for something like DECIMAL_DIGITS_TABLE is that we
could define OIDCHARS the same way. With a quick grep I couldn't immediately
find other candidates though.

> > b) Use a static assert to check that it fits?
>
> Right, easy stuff like sizeof(CppString2(MAX_BACKENDS)) - 1 can only
> work if the token is a decimal number. I suppose you could just use
> constants:
>
> #define MAX_BACKENDS 0x3ffff
> #define PROCNUMBER_BITS 18
> #define PROCNUMBER_CHARS 6
>
> ... and then use the previous magic to statically assert their
> relationship inside one translation unit, to keep it out of a widely
> included header.

I think we could also just define MAX_BACKENDS as a decimal number. I don't
find 0x3ffff that meaningfully better than 262143

Greetings,

Andres Freund