Hi,
On 2025-03-03 21:31:42 +0100, Tomas Vondra wrote:
> On 3/3/25 19:10, Andres Freund wrote:
> > On 2024-09-21 20:33:49 +0200, Tomas Vondra wrote:
> >> I've finally pushed this, after many rounds of careful testing to ensure
> >> no regressions, and polishing.
> >
> > One minor nit: I don't like that FP_LOCK_SLOTS_PER_BACKEND is now non-constant
> > while looking like a constant:
> >
> > #define FP_LOCK_SLOTS_PER_BACKEND (FP_LOCK_SLOTS_PER_GROUP * FastPathLockGroupsPerBackend)
> >
> > I don't think it's a good idea to have non-function-like #defines that
> > reference variables that can change from run to run.
> >
>
> Fair point, although it can't change "run to run" - not without a
> restart.
That's what I meant with "run to run".
> It's not a proper constant, of course, but it seemed close
> enough. Yes, it might confuse people into thinking it's a constant, or
> is there some additional impact?
That seems plenty. I just looked at the shem sizing function and was confused
because I didn't see where the max_locks_per_transaction affects the
allocation size.
> The one fix I can think of is making it look more like a function,
> possibly just like this:
>
> #define FastPathLockSlotsPerBackend() \
> (FP_LOCK_SLOTS_PER_GROUP * FastPathLockGroupsPerBackend)
>
> Or do you have another suggestion?
That'd work for me.
Greetings,
Andres Freund
