agora inbox for pgsql-committers@postgresql.orghelp / color / mirror / Atom feed
pgsql: Convert all remaining subsystems to use the new shmem allocation 3+ messages / 3 participants [nested] [flat]
* pgsql: Convert all remaining subsystems to use the new shmem allocation @ 2026-04-05 23:27 Heikki Linnakangas <heikki.linnakangas@iki.fi> 0 siblings, 1 reply; 3+ messages in thread From: Heikki Linnakangas @ 2026-04-05 23:27 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Convert all remaining subsystems to use the new shmem allocation API This removes all remaining uses of ShmemInitStruct() and ShmemInitHash() from built-in code. Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Reviewed-by: Matthias van de Meent <boekewurm+postgres@gmail.com> Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://www.postgresql.org/message-id/CAExHW5vM1bneLYfg0wGeAa=52UiJ3z4vKd3AJ72X8Fw6k3KKrg@mail.gmail... Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/9b5acad3f40fa6015f367fbf887ae5c1a93a3698 Modified Files -------------- src/backend/access/common/syncscan.c | 76 +++++---- src/backend/access/nbtree/nbtutils.c | 54 ++++--- src/backend/access/transam/twophase.c | 75 ++++----- src/backend/access/transam/xlog.c | 82 +++++----- src/backend/access/transam/xlogprefetcher.c | 51 +++--- src/backend/access/transam/xlogrecovery.c | 35 +++-- src/backend/access/transam/xlogwait.c | 50 +++--- src/backend/postmaster/autovacuum.c | 79 +++++----- src/backend/postmaster/bgworker.c | 105 ++++++------- src/backend/postmaster/checkpointer.c | 56 +++---- src/backend/postmaster/datachecksum_state.c | 41 ++--- src/backend/postmaster/pgarch.c | 43 +++-- src/backend/postmaster/walsummarizer.c | 60 +++---- src/backend/replication/logical/launcher.c | 56 +++---- src/backend/replication/logical/logicalctl.c | 29 ++-- src/backend/replication/logical/origin.c | 59 ++++--- src/backend/replication/logical/slotsync.c | 41 ++--- src/backend/replication/slot.c | 64 ++++---- src/backend/replication/walreceiverfuncs.c | 51 +++--- src/backend/replication/walsender.c | 59 ++++--- src/backend/storage/ipc/ipci.c | 124 +-------------- src/backend/storage/lmgr/lock.c | 109 +++++-------- src/backend/utils/activity/backend_status.c | 173 ++++++++------------- src/backend/utils/activity/pgstat_shmem.c | 158 ++++++++++--------- src/backend/utils/activity/wait_event.c | 83 +++++----- src/backend/utils/misc/injection_point.c | 57 ++++--- src/include/access/nbtree.h | 2 - src/include/access/syncscan.h | 2 - src/include/access/twophase.h | 3 - src/include/access/xlog.h | 2 - src/include/access/xlogprefetcher.h | 3 - src/include/access/xlogrecovery.h | 3 - src/include/access/xlogwait.h | 2 - src/include/pgstat.h | 4 - src/include/postmaster/autovacuum.h | 4 - src/include/postmaster/bgworker_internals.h | 2 - src/include/postmaster/bgwriter.h | 3 - src/include/postmaster/datachecksum_state.h | 4 - src/include/postmaster/pgarch.h | 2 - src/include/postmaster/walsummarizer.h | 2 - src/include/replication/logicalctl.h | 2 - src/include/replication/logicallauncher.h | 3 - src/include/replication/origin.h | 4 - src/include/replication/slot.h | 4 - src/include/replication/slotsync.h | 2 - src/include/replication/walreceiver.h | 2 - src/include/replication/walsender.h | 2 - src/include/storage/lock.h | 2 - src/include/storage/subsystemlist.h | 27 ++++ src/include/utils/backend_status.h | 8 - src/include/utils/injection_point.h | 3 - src/include/utils/wait_event.h | 2 - .../modules/injection_points/injection_points.c | 59 +++---- src/test/modules/test_aio/test_aio.c | 107 ++++++------- 54 files changed, 927 insertions(+), 1208 deletions(-) ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: pgsql: Convert all remaining subsystems to use the new shmem allocation @ 2026-04-06 05:41 Michael Paquier <michael@paquier.xyz> parent: Heikki Linnakangas <heikki.linnakangas@iki.fi> 0 siblings, 1 reply; 3+ messages in thread From: Michael Paquier @ 2026-04-06 05:41 UTC (permalink / raw) To: Heikki Linnakangas <heikki.linnakangas@iki.fi>; +Cc: pgsql-committers@lists.postgresql.org On Sun, Apr 05, 2026 at 11:27:44PM +0000, Heikki Linnakangas wrote: > Convert all remaining subsystems to use the new shmem allocation API > > This removes all remaining uses of ShmemInitStruct() and > ShmemInitHash() from built-in code. > > src/backend/utils/misc/injection_point.c | 57 ++++--- drongo, that compiles without USE_INJECTION_POINTS, is complaining about this bit around line 240: const ShmemCallbacks InjectionPointShmemCallbacks = { #ifdef USE_INJECTION_POINTS .request_fn = InjectionPointShmemRequest, .init_fn = InjectionPointShmemInit, #endif }; Link: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=drongo&dt=2026-04-06%2004%3A09%3A20 And the error: ../pgsql/src/backend/utils/misc/injection_point.c(240): error C2059: syntax error: '}' Why not putting the whole InjectionPointShmemCallbacks inside a USE_INJECTION_POINTS block? We should not care about shmem allocations when --enable-injection-points is not used. subsystemlist.h expects the callbacks to always be defined, so your intention is to have no ifdefs there. Still, it seems a bit pointless to me to define callbacks we are not going to use depending on the build options evoked? Attached is one idea, which I doubt you'll like. :) -- Michael diff --git a/src/include/storage/subsystemlist.h b/src/include/storage/subsystemlist.h index 5e092552c725..9ad619080be2 100644 --- a/src/include/storage/subsystemlist.h +++ b/src/include/storage/subsystemlist.h @@ -79,7 +79,9 @@ PG_SHMEM_SUBSYSTEM(SyncScanShmemCallbacks) PG_SHMEM_SUBSYSTEM(AsyncShmemCallbacks) PG_SHMEM_SUBSYSTEM(StatsShmemCallbacks) PG_SHMEM_SUBSYSTEM(WaitEventCustomShmemCallbacks) +#ifdef USE_INJECTION_POINTS PG_SHMEM_SUBSYSTEM(InjectionPointShmemCallbacks) +#endif PG_SHMEM_SUBSYSTEM(WaitLSNShmemCallbacks) PG_SHMEM_SUBSYSTEM(LogicalDecodingCtlShmemCallbacks) PG_SHMEM_SUBSYSTEM(DataChecksumsShmemCallbacks) diff --git a/src/backend/utils/misc/injection_point.c b/src/backend/utils/misc/injection_point.c index a7c99e097ea4..aa455c62bcc0 100644 --- a/src/backend/utils/misc/injection_point.c +++ b/src/backend/utils/misc/injection_point.c @@ -230,19 +230,15 @@ injection_point_cache_get(const char *name) return NULL; } -#endif /* USE_INJECTION_POINTS */ const ShmemCallbacks InjectionPointShmemCallbacks = { -#ifdef USE_INJECTION_POINTS .request_fn = InjectionPointShmemRequest, .init_fn = InjectionPointShmemInit, -#endif }; /* * Reserve space for the dynamic shared hash table */ -#ifdef USE_INJECTION_POINTS static void InjectionPointShmemRequest(void *arg) { @@ -259,7 +255,7 @@ InjectionPointShmemInit(void *arg) for (int i = 0; i < MAX_INJECTION_POINTS; i++) pg_atomic_init_u64(&ActiveInjectionPoints->entries[i].generation, 0); } -#endif +#endif /* USE_INJECTION_POINTS */ /* * Attach a new injection point. Attachments: [text/plain] inj-shmem-subsystem.patch (1.6K, ../../adNHcBVJO5gIOp1l@paquier.xyz/2-inj-shmem-subsystem.patch) download | inline diff: diff --git a/src/include/storage/subsystemlist.h b/src/include/storage/subsystemlist.h index 5e092552c725..9ad619080be2 100644 --- a/src/include/storage/subsystemlist.h +++ b/src/include/storage/subsystemlist.h @@ -79,7 +79,9 @@ PG_SHMEM_SUBSYSTEM(SyncScanShmemCallbacks) PG_SHMEM_SUBSYSTEM(AsyncShmemCallbacks) PG_SHMEM_SUBSYSTEM(StatsShmemCallbacks) PG_SHMEM_SUBSYSTEM(WaitEventCustomShmemCallbacks) +#ifdef USE_INJECTION_POINTS PG_SHMEM_SUBSYSTEM(InjectionPointShmemCallbacks) +#endif PG_SHMEM_SUBSYSTEM(WaitLSNShmemCallbacks) PG_SHMEM_SUBSYSTEM(LogicalDecodingCtlShmemCallbacks) PG_SHMEM_SUBSYSTEM(DataChecksumsShmemCallbacks) diff --git a/src/backend/utils/misc/injection_point.c b/src/backend/utils/misc/injection_point.c index a7c99e097ea4..aa455c62bcc0 100644 --- a/src/backend/utils/misc/injection_point.c +++ b/src/backend/utils/misc/injection_point.c @@ -230,19 +230,15 @@ injection_point_cache_get(const char *name) return NULL; } -#endif /* USE_INJECTION_POINTS */ const ShmemCallbacks InjectionPointShmemCallbacks = { -#ifdef USE_INJECTION_POINTS .request_fn = InjectionPointShmemRequest, .init_fn = InjectionPointShmemInit, -#endif }; /* * Reserve space for the dynamic shared hash table */ -#ifdef USE_INJECTION_POINTS static void InjectionPointShmemRequest(void *arg) { @@ -259,7 +255,7 @@ InjectionPointShmemInit(void *arg) for (int i = 0; i < MAX_INJECTION_POINTS; i++) pg_atomic_init_u64(&ActiveInjectionPoints->entries[i].generation, 0); } -#endif +#endif /* USE_INJECTION_POINTS */ /* * Attach a new injection point. [application/pgp-signature] signature.asc (832B, ../../adNHcBVJO5gIOp1l@paquier.xyz/3-signature.asc) download ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: pgsql: Convert all remaining subsystems to use the new shmem allocation @ 2026-04-06 12:31 Heikki Linnakangas <hlinnaka@iki.fi> parent: Michael Paquier <michael@paquier.xyz> 0 siblings, 0 replies; 3+ messages in thread From: Heikki Linnakangas @ 2026-04-06 12:31 UTC (permalink / raw) To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-committers@lists.postgresql.org On 06/04/2026 08:41, Michael Paquier wrote: > On Sun, Apr 05, 2026 at 11:27:44PM +0000, Heikki Linnakangas wrote: >> Convert all remaining subsystems to use the new shmem allocation API >> >> This removes all remaining uses of ShmemInitStruct() and >> ShmemInitHash() from built-in code. >> >> src/backend/utils/misc/injection_point.c | 57 ++++--- > > drongo, that compiles without USE_INJECTION_POINTS, is complaining > about this bit around line 240: > const ShmemCallbacks InjectionPointShmemCallbacks = { > #ifdef USE_INJECTION_POINTS > .request_fn = InjectionPointShmemRequest, > .init_fn = InjectionPointShmemInit, > #endif > }; > > Link: > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=drongo&dt=2026-04-06%2004%3A09%3A20 > And the error: > ../pgsql/src/backend/utils/misc/injection_point.c(240): error C2059: syntax error: '}' Oh, I didn't realize that that an empty initializer isn't allowed. > Why not putting the whole InjectionPointShmemCallbacks inside a > USE_INJECTION_POINTS block? We should not care about shmem > allocations when --enable-injection-points is not used. > > subsystemlist.h expects the callbacks to always be defined, so your > intention is to have no ifdefs there. Still, it seems a bit pointless > to me to define callbacks we are not going to use depending on the > build options evoked? Attached is one idea, which I doubt you'll > like. :) Looks perfectly fine to me, I'll push that. Thanks! - Heikki ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-04-06 12:31 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-04-05 23:27 pgsql: Convert all remaining subsystems to use the new shmem allocation Heikki Linnakangas <heikki.linnakangas@iki.fi> 2026-04-06 05:41 ` Michael Paquier <michael@paquier.xyz> 2026-04-06 12:31 ` Heikki Linnakangas <hlinnaka@iki.fi>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox