| From: | Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com> |
|---|---|
| To: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
| Cc: | Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, chaturvedipalak1911(at)gmail(dot)com |
| Subject: | Re: Better shared data structure management and resizable shared data structures |
| Date: | 2026-04-02 22:10:21 |
| Message-ID: | CAEze2WhMOHVgH2Xeyzx=VEk-Ta_YnQUqT+TdBiv5Lx8ESn2WZA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, 1 Apr 2026 at 20:17, Heikki Linnakangas <hlinnaka(at)iki(dot)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)
| From | Date | Subject | |
|---|---|---|---|
| Next Message | David Rowley | 2026-04-02 22:12:15 | Re: Small and unlikely overflow hazard in bms_next_member() |
| Previous Message | Matheus Alcantara | 2026-04-02 21:41:35 | Re: Add custom EXPLAIN options support to auto_explain |