public inbox for [email protected]
help / color / mirror / Atom feedFrom: Matthias van de Meent <[email protected]>
To: Heikki Linnakangas <[email protected]>
Cc: Ashutosh Bapat <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: [email protected]
Subject: Re: Better shared data structure management and resizable shared data structures
Date: Sat, 4 Apr 2026 14:00:11 +0200
Message-ID: <CAEze2WjQZff3znd6CtG-OBzYZMMqy5TyQSoAo=QTFT38tDndeQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAExHW5vM1bneLYfg0wGeAa=52UiJ3z4vKd3AJ72X8Fw6k3KKrg@mail.gmail.com>
<[email protected]>
<CA+Tgmob8ot7qam7a650nx_4jyEoFLs8s_kPMc1+EB7-dr56=WA@mail.gmail.com>
<[email protected]>
<CA+TgmoauqYXm8iA3FGRAVKxYShUxWBiS_MSLmQpTrmO7wNHamw@mail.gmail.com>
<[email protected]>
<CAExHW5uTNWOSxJDWQAUnS0tZawob2_J3dRAtc67NHNZ98X4_xA@mail.gmail.com>
<CAExHW5t439y61YD9bc7d5wZWHp6J=M43Qu3eEZOBPguZML7o2A@mail.gmail.com>
<CAExHW5v5FVZbsO9sLzztMZ11C3hgGStE=HkkV2bQkCyncess4w@mail.gmail.com>
<[email protected]>
<CAExHW5tCC0T1ky=Jnq-AvMxa67Adaw7aQ4iQAO=BSdHcbSNBVg@mail.gmail.com>
<[email protected]>
<CAExHW5tS7GncN90oJWOSzW_3F1EHL9xwe59L7Req3nUVgmObUw@mail.gmail.com>
<[email protected]>
<CAEze2WhMOHVgH2Xeyzx=VEk-Ta_YnQUqT+TdBiv5Lx8ESn2WZA@mail.gmail.com>
<CAExHW5s6h=c_q2m72Nvyj1ghMEhPkOBkeN5Htn7YR=1BrNN-Sw@mail.gmail.com>
<[email protected]>
On Sat, 4 Apr 2026 at 02:45, Heikki Linnakangas <[email protected]> wrote:
>
> On 03/04/2026 16:12, Ashutosh Bapat wrote:
> > On Fri, Apr 3, 2026 at 3:40 AM Matthias van de Meent
> > <[email protected]> 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
view thread (82+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Better shared data structure management and resizable shared data structures
In-Reply-To: <CAEze2WjQZff3znd6CtG-OBzYZMMqy5TyQSoAo=QTFT38tDndeQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox