Re: Better shared data structure management and resizable shared data structures - Mailing list pgsql-hackers
| From | Matthias van de Meent |
|---|---|
| Subject | Re: Better shared data structure management and resizable shared data structures |
| Date | |
| Msg-id | CAEze2WjQZff3znd6CtG-OBzYZMMqy5TyQSoAo=QTFT38tDndeQ@mail.gmail.com Whole thread |
| In response to | Re: Better shared data structure management and resizable shared data structures (Heikki Linnakangas <hlinnaka@iki.fi>) |
| Responses |
Re: Better shared data structure management and resizable shared data structures
|
| List | pgsql-hackers |
On Sat, 4 Apr 2026 at 02:45, Heikki Linnakangas <hlinnaka@iki.fi> wrote: > > 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. That's cool, but if that common initialization path is common enough to need special coding, then how come that this patch make PG use it? I can think of many systems that "just" initialize a hash table or "just" allocate a shmem area. > It's a pretty common pattern to have an opaque pointer like > that in any callbacks. I agree that it's a rather common pattern, but from an OOP perspective, shouldn't the argument be the ShmemCallbacks*? Users can embed the struct to extend the data carried if they need it to. > >> 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, > }; That would help, though I'd still wonder why we'd have separate Opts and Desc structs. IIUC, they generally carry (exactly) the same data. Maybe moving it into a `.handle` or `.desc` field in Shmem*Opts could make that part of the code a bit cleaner; as it'd further clarify that it's very much an optional field. I'll check out your latest version in a bit. Kind regards, Matthias van de Meent
pgsql-hackers by date: