MIME-Version: 1.0
References: 
 <CAExHW5vM1bneLYfg0wGeAa=52UiJ3z4vKd3AJ72X8Fw6k3KKrg@mail.gmail.com>
 <CA+Tgmob7E3CGDoEBB3mwfk+v1CgcOG8yL0VHje2gy9Ts=e1Zxw@mail.gmail.com>
 <f8fc6f77-292d-4225-9f54-382c39da5ebc@iki.fi>
 <CA+TgmobqwCHp1w6xkTJmZiq4C8Pr0O3tNUvSFZcAgYLkvF45bg@mail.gmail.com>
 <8a6799be-bd42-49fb-8914-856c97bb1977@iki.fi>
 <CA+Tgmob8ot7qam7a650nx_4jyEoFLs8s_kPMc1+EB7-dr56=WA@mail.gmail.com>
 <cc3ff976-c303-45cd-9a8d-a90483170e37@iki.fi>
 <CA+TgmoauqYXm8iA3FGRAVKxYShUxWBiS_MSLmQpTrmO7wNHamw@mail.gmail.com>
 <113724ab-0028-493f-9605-6e8570f0939f@iki.fi>
 <CAExHW5uTNWOSxJDWQAUnS0tZawob2_J3dRAtc67NHNZ98X4_xA@mail.gmail.com>
 <CAExHW5t439y61YD9bc7d5wZWHp6J=M43Qu3eEZOBPguZML7o2A@mail.gmail.com>
 <CAExHW5v5FVZbsO9sLzztMZ11C3hgGStE=HkkV2bQkCyncess4w@mail.gmail.com>
 <b2451225-0a10-4ded-abef-4e9efbde7a8e@iki.fi>
 <CAExHW5tCC0T1ky=Jnq-AvMxa67Adaw7aQ4iQAO=BSdHcbSNBVg@mail.gmail.com>
 <791c3f18-f4de-4d84-ac6b-c7ccc074dd38@iki.fi>
 <CAExHW5tS7GncN90oJWOSzW_3F1EHL9xwe59L7Req3nUVgmObUw@mail.gmail.com>
 <9d919bd9-94dd-4bda-8ccf-ebced4178c53@iki.fi>
In-Reply-To: <9d919bd9-94dd-4bda-8ccf-ebced4178c53@iki.fi>
From: Matthias van de Meent <boekewurm+postgres@gmail.com>
Date: Fri, 3 Apr 2026 00:10:21 +0200
Message-ID: 
 <CAEze2WhMOHVgH2Xeyzx=VEk-Ta_YnQUqT+TdBiv5Lx8ESn2WZA@mail.gmail.com>
Subject: Re: Better shared data structure management and resizable shared data
 structures
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>,
 Robert Haas <robertmhaas@gmail.com>,
	Andres Freund <andres@anarazel.de>,
 pgsql-hackers <pgsql-hackers@postgresql.org>,
	chaturvedipalak1911@gmail.com
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CAEze2WhMOHVgH2Xeyzx%3DVEk-Ta_YnQUqT%2BTdBiv5Lx8ESn2WZA%40mail.gmail.com>
Precedence: bulk

On Wed, 1 Apr 2026 at 20:17, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>
> Yet another version attached (also available at:
> https://github.com/hlinnaka/postgres/tree/shmem-init-refactor-9). The
> main change is the shape of the ShmemRequest*() calls:

I didn't read the whole thread, as it's quite long, but did look at
the patchset for a while to figure out where it's going.

0005:
A few assorted comments:

While I do think it's an improvement over the current APIs, the
improvement seems to be mostly concentrated in the RequestStruct/Hash
department, with only marginal improvements in RegisterShmemCallbacks.
I feel like it's missing the important part: I'd like
direct-from-_PG_init() ShmemRequestStruct/Hash calls. If
ShmemRequestStruct/Hash had a size callback as alternative to the size
field (which would then be called after preload_libraries finishes)
then that would be sufficient for most shmem allocations, and it'd
simplify shmem management for most subsystems.
We'd still need the shmem lifecycle hooks/RegisterShmemCallbacks to
allow conditionally allocated shmem areas (e.g. those used in aio),
but I think that, in general, we shouldn't need a separate callback
function just to get started registering shmem structures.

I also noticed that ShmemCallbacks.%_arg are generally undocumented,
and I couldn't find any users in core (at the end of the patchset)
that actually use the argument. Could it be I missed something?

I don't understand the use of ShmemStructDesc. They generally/always
are private to request_fn(), and their fields are used exclusively
inside the shmem mechanisms, with no reads of its fields that can't
already be deduced from context. Why do we need that struct
everywhere?

> +++ b/src/backend/storage/ipc/shmem.c
[...]
> +    /* Check that it's not already registered in this process */
> +    foreach_ptr(ShmemStructDesc, existing, pending_shmem_requests)
> +    {
> +        if (strcmp(existing->name, options->name) == 0)
> +            ereport(ERROR,
> +                    (errmsg("shared memory struct \"%s\" is already registered",
> +                            options->name)));
> +    }
> +
> +    request = palloc(sizeof(ShmemRequest));
> +    request->options = options;
> +    request->desc = desc;
> +    request->kind = kind;
> +    pending_shmem_requests = lappend(pending_shmem_requests, request);

Apparently, pending_shmem_requests is a list of ShmemRequest, but the
iteration just above on the same list assumes ShmemStructDesc, which
seems wrong to me.

00017:
I like this idea, but I think it missed its chance to make good on an
opportunity to reduce waste in alignments: We know which structs we're
going to allocate at which alignments, so we could save space by
packing the structs. I don't expect it to save much, but it could be a
few 100 of kbs with a few BLCKSZ-aligned allocations.


Kind regards,

Matthias van de Meent
Databricks (https://www.databricks.com)