On 03/04/2026 01:10, Matthias van de Meent 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 kind of started from that thought too, but the design has since
evolved to what it is. Robert in particular was skeptical of that
approach, and I think I've come around on most of his feedback.
A per-struct size callback isn't very ergonomic in places like
LockManagerShmemRequest(), which derives the size of two different
things from the same calculated value. You'd need to repeat the
calculation for each one, or pass it through global variables or
something. PredicateLockShmemInit() is another extreme example. It
already has that problem to some extent, and already uses global
variables (I'm all ears if you have suggestions to improve it!) but I
feel that with a size callback this kind of stuff would get even harder.
Another reason is that it's good to have only one way of doing the
initialization, instead of one simple way and a different way for more
complex scenarios. If the simple way was vastly simpler, it might be
worth it, but I don't think there's that much difference here.
BTW, I also considered another way of initializing structs for simple
cases: instead of providing an init callback function, you could provide
the struct contents directly in the ShmemRequestStruct() call. To pick a
random example, slotsync.c currently looks like this:
static void
SlotSyncShmemRequest(void *arg)
{
ShmemRequestStruct(.name = "Slot Sync Data",
.size = sizeof(SlotSyncCtxStruct),
.ptr = (void **) &SlotSyncCtx,
);
}
static void
SlotSyncShmemInit(void *arg)
{
memset(SlotSyncCtx, 0, sizeof(SlotSyncCtxStruct));
SlotSyncCtx->pid = InvalidPid;
SpinLockInit(&SlotSyncCtx->mutex);
}
But it could look like this instead:
static void
SlotSyncShmemRequest(void *arg)
{
SlotSyncCtxStruct init_content;
memset(SlotSyncCtx, 0, sizeof(SlotSyncCtxStruct));
init_content.pid
SpinLockInit(&SlotSyncCtx->mutex);
ShmemRequestStruct(.name = "Slot Sync Data",
.size = sizeof(SlotSyncCtxStruct),
.ptr = (void **) &SlotSyncCtx,
.init_content = &init_content,
);
}
That'd only work for small, simple structs, though. Since the initial
contents would be copied in this model, it won't work for anything with
pointers to itself, for example. And it's not much less code after all.
- Heikki
