On 03/04/2026 16:12, Ashutosh Bapat wrote:
On Fri, Apr 3, 2026 at 3:40 AM Matthias van de Meent
<[email protected]> wrote:
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?
None of the current code currently uses it, that's correct. I felt it
might become very handy in the future or in extensions, if you wanted to
reuse the same function for initializing different shmem areas, for
example. It's a pretty common pattern to have an opaque pointer like
that in any callbacks.
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?
My resizable shared memory structure patches use it as a handle to the
structure to be resized.
Right. And hash tables and SLRUs use a desc-like object already, so for
symmetry it feels natural to have it for plain structs too.
I wonder if we should make it optional though, for the common case that
you have no intention of doing anything more with the shmem region that
you'd need a desc for. I'm thinking you could just pass NULL for the
desc pointer:
ShmemRequestStruct(NULL,
.name = "pg_stat_statements",
.size = sizeof(pgssSharedState),
.ptr = (void **) &pgss,
};
- Heikki