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:

Previous
From: "Matheus Alcantara"
Date:
Subject: Re: Add custom EXPLAIN options support to auto_explain
Next
From: David Rowley
Date:
Subject: Re: Small and unlikely overflow hazard in bms_next_member()