On 03/04/2026 16:12, Ashutosh Bapat wrote:
> On Fri, Apr 3, 2026 at 3:40 AM Matthias van de Meent
> <boekewurm+postgres@gmail.com> 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
