Hi,
On 2026-02-10 00:14:23 +0200, Heikki Linnakangas wrote:
> Putting this patch aside for a moment, I don't much like our current
> interface for defining shared memory structs anyway. The
> [SubSystem]ShmemSize() functions feel too detached from the
> ShmemInitStruct() calls.
Very much agreed.
Besides the other weaknesses, it's particularly annoying to use in extensions,
because extensions need to chain the shmem init hook.
I also really dislike that we support unattributed shared memory
allocations. For one they end up interspersing memory from different
subsystems, for another the lack of attributability is bad; it sure is weird
that one can't see the amount of memory used by the buffer mapping table or
the lock table.
> I feel that it'd be good to have a single definition of each shmem struct,
> and derive all the other things from there.
>
> Attached is a proof-of-concept of what I have in mind. Don't look too
> closely at how it's implemented, it's very hacky and EXEC_BACKEND mode is
> slightly broken, for example. The point is to demonstrate what the callers
> would look like. I converted only a few subsystems to use the new API, the
> rest still use ShmemInitStruct() and ShmemInitHash().
>
> With this, initialization of a subsystem that defines a shared memory area
> looks like this:
>
> --------------
>
> /* This struct lives in shared memory */
> typedef struct
> {
> int field;
> } FoobarSharedCtlData;
>
> static void FoobarShmemInit(void *arg);
>
> /* Descriptor for the shared memory area */
> ShmemStructDesc FoobarShmemDesc = {
> .name = "Foobar subsystem",
> .size = sizeof(FoobarSharedCtlData),
> .init_fn = FoobarShmemInit,
> };
I wonder if the size determination should be a callback too. That way
extensions wouldn't need to bother with shmem_request_hook, as introduced in
commit 4f2400cb3f1
Author: Robert Haas <[email protected]>
Date: 2022-05-13 09:31:06 -0400
Add a new shmem_request_hook hook.
Currently, preloaded libraries are expected to request additional
shared memory and LWLocks in _PG_init(). However, it is not unusal
for such requests to depend on MaxBackends, which won't be
initialized at that time. Such requests could also depend on GUCs
that other modules might change. This introduces a new hook where
modules can safely use MaxBackends and GUCs to request additional
shared memory and LWLocks.
...
And it'd allow us to make the ShmemStructDesc variables const.
Eventually I'd really like to not have multiple lists of core subsystems
(e.g. currently in CalculateShmemSize(), BaseInit() & InitPostgres(), each
top-level sigsetjmp() block, ...) but drive it all through one table. This
seems like a nice step towards that.
> The ShmemStructDesc provides room for extending the facility in the future.
> For example, you could specify alignment there, or an additional "attach"
> callback when you need to do more per-backend initialization in EXEC_BACKEND
> mode. And with the resizeable shared memory, a max size.
And perhaps information about how to deal with NUMAn (e.g. interleave the proc
array, but use more complicated behaviour for buffer blocks) and whether to
use huge pages (it's not clear it's a win for e.g. pgproc).
> Thoughts?
There's stuff to quibble with in the details (being a POC), but I think as a
whole this would be quite an improvement.
Greetings,
Andres Freund
