Message-ID: <4e9a124d-29cb-4fd5-99fb-a5016e508f20@vondra.me>
Date: Wed, 2 Apr 2025 17:49:07 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Improve monitoring of shared memory allocations
To: Rahila Syed <rahilasyed90@gmail.com>
Cc: Nazir Bilal Yavuz <byavuz81@gmail.com>, Andres Freund
 <andres@anarazel.de>, PostgreSQL-development <pgsql-hackers@postgresql.org>
References: <CAH2L28vHzRankszhqz7deXURxKncxfirnuW68zD7+hVAqaS5GQ@mail.gmail.com>
 <k6f6ynjvb7lvebhygaiqsfrdohq672uughk3q4ve4q5jqljywz@7jbrz724epeq>
 <CAH2L28uL-8EQTRSeyTpW1DqAXsDXRkXCkT1dus2u6p4HYDrxAg@mail.gmail.com>
 <CAH2L28uGLhkXBKDWFKm5XZtp_0nNqpYQ3Hc35vG++mM7wuOhgg@mail.gmail.com>
 <CAN55FZ3cJxy0VkeXpuO3K4BpjzJo3S6oU+iMyc00P6gEjqPztw@mail.gmail.com>
 <CAH2L28uwxJREzB62UjRDBumE87hHWUJJvRwxqqbO+7qFmoZfTg@mail.gmail.com>
 <3e40eeec-d8bf-4496-854e-485dd901f6a2@vondra.me>
 <CAH2L28uG_g1Ljo8aL-g1MupJXO4Y7-a-bUCriE7w2213+KSGdA@mail.gmail.com>
 <6bf7194e-4c34-4e6d-8215-f6acf8903974@vondra.me>
 <6705dbd2-060b-4f3c-9fcb-1c7f10880b26@vondra.me>
 <CAH2L28tzCFEk2bxQ+oYv6zda=LFLfd_9cmq7HzsT4nj9KN1Yvg@mail.gmail.com>
 <66283922-0448-44e7-bbdd-38aaf2be34b9@vondra.me>
 <CAH2L28soW40DafNmLEEWMrNDTMWYZ9STWSS5axP910WQUT=JcQ@mail.gmail.com>
 <7987a0b5-9933-457a-b5ba-d28e9cce883b@vondra.me>
 <CAH2L28vaktm0UXw19y22w+DThcqV80-UKtVfMxPeVw8ro2yTDg@mail.gmail.com>
Content-Language: en-US
From: Tomas Vondra <tomas@vondra.me>
In-Reply-To: <CAH2L28vaktm0UXw19y22w+DThcqV80-UKtVfMxPeVw8ro2yTDg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/4e9a124d-29cb-4fd5-99fb-a5016e508f20%40vondra.me>
Precedence: bulk

Thanks!

I've pushed the first two parts, improving the shmem accounting.

I've done a couple additional adjustments before commit, namely:

1) I've renamed hash_get_init_size() to just hash_get_size(). We've not
indicated that it's "initial" allocation before, I don't think we need
to indicate to do that now. So I've just removed the "shared" part, as
that's no longer relevant.

2) I've removed one of the compute_buckets_and_segs() in hash_create,
because AFAICS we don't need to recalculate the nsegs - we already have
the correct value in the hctl, so I just used that.

The fact that we call compute_buckets_and_segs() in multiple places,
relying on getting the exact same result seemed a bit fragile to me.
With this we now have just two callers (instead of 3), and I don't think
we can do better - the first call is in hash_get_size(), and I don't see
a convenient way to pass the info init_hash() where we do the 2nd call.

3) In 0002, I've reworked how ProcGlobalShmemSize() calculates the size
of the required memory - the earlier patch added PGProcShmemSize(), but
the ProcGlobalShmemSize() redid the calculation all over. I changed that
to call the new function, and also added a new FastPathLockShmemSize()
to do the same for the fast-path lock arrays (which I realized is now
allocated as a separate area, so it'll be shown separately).

4) I realized PGProcShmemSize() did the calculation slightly differently
from ProcGlobalShmemSize() - by directly multiplying the values, not
using mul_size(). I don't recall why we use mul_size(), but it seems
like the "usual" way. So I did it that way.

I notice we still do the plain multiplication later when setting the
ProcGlobal fields. Seems a bit weird, but we always did that - the patch
does not really change that.

I'll now mark this as committed. I haven't done about the alignment. My
conclusion from the discussion was we don't quite need to do that, but
if we do I think it's a matter for a separate patch - perhaps something
like the 0003.


Thanks for the patch, reviews, etc.

-- 
Tomas Vondra