Re: Better shared data structure management and resizable shared data structures

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)

In response to

Responses

Browse pgsql-hackers by date

  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