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 | CAEze2WhMOHVgH2Xeyzx=VEk-Ta_YnQUqT+TdBiv5Lx8ESn2WZA@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
Re: Better shared data structure management and resizable shared data structures |
| List | pgsql-hackers |
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)
pgsql-hackers by date: