Feedback-ID: ia2694551:Fastmail
Date: Tue, 23 Jan 2024 15:42:39 +0100
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: make BuiltinTrancheNames less ugly
Message-ID: <202401231442.aldy4ap76psg@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="2cgsne47fmqrtubo"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <202401231147.clytrnaysp43@alvherre.pgsql>
Archived-At: 
 <https://www.postgresql.org/message-id/202401231442.aldy4ap76psg%40alvherre.pgsql>
Precedence: bulk


--2cgsne47fmqrtubo
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2024-Jan-23, Alvaro Herrera wrote:

> ... oh, actually FreeBSD failed with this strange problem while linking:

I figured this out.  For dtrace support, we need to run dtrace on all
the objects produced by the compilation step, but because I took
lwlock.o out of the objects used to produce the backend into its own
"library", it needs to be included explicitly.  (I think this is not a
problem for things like nodes/copyfuncs.c only because those files don't
have any use of TRACE macros).

So here's v3, which now passes fully in CI.

I'm a total newbie to Meson, so it's likely that there are better ways
to implement this.  I'll leave this here for a little bit in case
anybody wants to comment.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"El sudor es la mejor cura para un pensamiento enfermo" (Bardia)

--2cgsne47fmqrtubo
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
	filename="v3-0001-Use-designated-initializers-for-BuiltinTrancheNam.patch"

From 97e86277ec9a406fc625141b59d81757d4358a93 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Tue, 23 Jan 2024 10:36:14 +0100
Subject: [PATCH v3] Use designated initializers for BuiltinTrancheNames

---
 src/backend/meson.build                       |   5 +-
 src/backend/storage/lmgr/Makefile             |   3 +-
 .../storage/lmgr/generate-lwlocknames.pl      |  10 +-
 src/backend/storage/lmgr/lwlock.c             | 110 ++++++------------
 src/backend/storage/lmgr/meson.build          |  12 +-
 5 files changed, 55 insertions(+), 85 deletions(-)

diff --git a/src/backend/meson.build b/src/backend/meson.build
index 8767aaba67..6ce6292ea1 100644
--- a/src/backend/meson.build
+++ b/src/backend/meson.build
@@ -127,7 +127,10 @@ backend_objs = [postgres_lib.extract_all_objects(recursive: false)]
 if dtrace.found() and host_system != 'darwin'
   backend_input += custom_target(
     'probes.o',
-    input: ['utils/probes.d', postgres_lib.extract_objects(backend_sources, timezone_sources)],
+    input: ['utils/probes.d',
+      postgres_lib.extract_objects(backend_sources, timezone_sources),
+      lwlock_lib.extract_objects(lwlock_source)
+      ],
     output: 'probes.o',
     command: [dtrace, '-C', '-G', '-o', '@OUTPUT@', '-s', '@INPUT@'],
     install: false,
diff --git a/src/backend/storage/lmgr/Makefile b/src/backend/storage/lmgr/Makefile
index 504480e847..81da6ee13a 100644
--- a/src/backend/storage/lmgr/Makefile
+++ b/src/backend/storage/lmgr/Makefile
@@ -12,13 +12,14 @@ subdir = src/backend/storage/lmgr
 top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 
+override CPPFLAGS := -I. $(CPPFLAGS)
+
 OBJS = \
 	condition_variable.o \
 	deadlock.o \
 	lmgr.o \
 	lock.o \
 	lwlock.o \
-	lwlocknames.o \
 	predicate.o \
 	proc.o \
 	s_lock.o \
diff --git a/src/backend/storage/lmgr/generate-lwlocknames.pl b/src/backend/storage/lmgr/generate-lwlocknames.pl
index 7b93ecf6c1..a679a4ff54 100644
--- a/src/backend/storage/lmgr/generate-lwlocknames.pl
+++ b/src/backend/storage/lmgr/generate-lwlocknames.pl
@@ -10,7 +10,6 @@ use Getopt::Long;
 my $output_path = '.';
 
 my $lastlockidx = -1;
-my $continue = "\n";
 
 GetOptions('outdir:s' => \$output_path);
 
@@ -29,8 +28,6 @@ print $h $autogen;
 print $h "/* there is deliberately not an #ifndef LWLOCKNAMES_H here */\n\n";
 print $c $autogen, "\n";
 
-print $c "const char *const IndividualLWLockNames[] = {";
-
 #
 # First, record the predefined LWLocks listed in wait_event_names.txt.  We'll
 # cross-check those with the ones in lwlocknames.txt.
@@ -97,12 +94,10 @@ while (<$lwlocknames>)
 	while ($lastlockidx < $lockidx - 1)
 	{
 		++$lastlockidx;
-		printf $c "%s	\"<unassigned:%d>\"", $continue, $lastlockidx;
-		$continue = ",\n";
+		printf $c "[%s] = \"<unassigned:%d>\",\n", $lastlockidx, $lastlockidx;
 	}
-	printf $c "%s	\"%s\"", $continue, $trimmedlockname;
+	printf $c "[%s] = \"%s\",\n", $lockidx, $trimmedlockname;
 	$lastlockidx = $lockidx;
-	$continue = ",\n";
 
 	print $h "#define $lockname (&MainLWLockArray[$lockidx].lock)\n";
 }
@@ -112,7 +107,6 @@ die
   . "lwlocknames.txt"
   if $i < scalar @wait_event_lwlocks;
 
-printf $c "\n};\n";
 print $h "\n";
 printf $h "#define NUM_INDIVIDUAL_LWLOCKS		%s\n", $lastlockidx + 1;
 
diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index 2f2de5a562..8aad9c6690 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -115,8 +115,8 @@ StaticAssertDecl(LW_VAL_EXCLUSIVE > (uint32) MAX_BACKENDS,
  * There are three sorts of LWLock "tranches":
  *
  * 1. The individually-named locks defined in lwlocknames.h each have their
- * own tranche.  The names of these tranches appear in IndividualLWLockNames[]
- * in lwlocknames.c.
+ * own tranche.  The names of these tranches come from lwlocknames.c into
+ * BuiltinTranchNames[] below.
  *
  * 2. There are some predefined tranches for built-in groups of locks.
  * These are listed in enum BuiltinTrancheIds in lwlock.h, and their names
@@ -129,75 +129,43 @@ StaticAssertDecl(LW_VAL_EXCLUSIVE > (uint32) MAX_BACKENDS,
  * All these names are user-visible as wait event names, so choose with care
  * ... and do not forget to update the documentation's list of wait events.
  */
-extern const char *const IndividualLWLockNames[];	/* in lwlocknames.c */
-
 static const char *const BuiltinTrancheNames[] = {
-	/* LWTRANCHE_XACT_BUFFER: */
-	"XactBuffer",
-	/* LWTRANCHE_COMMITTS_BUFFER: */
-	"CommitTsBuffer",
-	/* LWTRANCHE_SUBTRANS_BUFFER: */
-	"SubtransBuffer",
-	/* LWTRANCHE_MULTIXACTOFFSET_BUFFER: */
-	"MultiXactOffsetBuffer",
-	/* LWTRANCHE_MULTIXACTMEMBER_BUFFER: */
-	"MultiXactMemberBuffer",
-	/* LWTRANCHE_NOTIFY_BUFFER: */
-	"NotifyBuffer",
-	/* LWTRANCHE_SERIAL_BUFFER: */
-	"SerialBuffer",
-	/* LWTRANCHE_WAL_INSERT: */
-	"WALInsert",
-	/* LWTRANCHE_BUFFER_CONTENT: */
-	"BufferContent",
-	/* LWTRANCHE_REPLICATION_ORIGIN_STATE: */
-	"ReplicationOriginState",
-	/* LWTRANCHE_REPLICATION_SLOT_IO: */
-	"ReplicationSlotIO",
-	/* LWTRANCHE_LOCK_FASTPATH: */
-	"LockFastPath",
-	/* LWTRANCHE_BUFFER_MAPPING: */
-	"BufferMapping",
-	/* LWTRANCHE_LOCK_MANAGER: */
-	"LockManager",
-	/* LWTRANCHE_PREDICATE_LOCK_MANAGER: */
-	"PredicateLockManager",
-	/* LWTRANCHE_PARALLEL_HASH_JOIN: */
-	"ParallelHashJoin",
-	/* LWTRANCHE_PARALLEL_QUERY_DSA: */
-	"ParallelQueryDSA",
-	/* LWTRANCHE_PER_SESSION_DSA: */
-	"PerSessionDSA",
-	/* LWTRANCHE_PER_SESSION_RECORD_TYPE: */
-	"PerSessionRecordType",
-	/* LWTRANCHE_PER_SESSION_RECORD_TYPMOD: */
-	"PerSessionRecordTypmod",
-	/* LWTRANCHE_SHARED_TUPLESTORE: */
-	"SharedTupleStore",
-	/* LWTRANCHE_SHARED_TIDBITMAP: */
-	"SharedTidBitmap",
-	/* LWTRANCHE_PARALLEL_APPEND: */
-	"ParallelAppend",
-	/* LWTRANCHE_PER_XACT_PREDICATE_LIST: */
-	"PerXactPredicateList",
-	/* LWTRANCHE_PGSTATS_DSA: */
-	"PgStatsDSA",
-	/* LWTRANCHE_PGSTATS_HASH: */
-	"PgStatsHash",
-	/* LWTRANCHE_PGSTATS_DATA: */
-	"PgStatsData",
-	/* LWTRANCHE_LAUNCHER_DSA: */
-	"LogicalRepLauncherDSA",
-	/* LWTRANCHE_LAUNCHER_HASH: */
-	"LogicalRepLauncherHash",
-	/* LWTRANCHE_DSM_REGISTRY_DSA: */
-	"DSMRegistryDSA",
-	/* LWTRANCHE_DSM_REGISTRY_HASH: */
-	"DSMRegistryHash",
+#include "lwlocknames.c"
+	[LWTRANCHE_XACT_BUFFER] = "XactBuffer",
+	[LWTRANCHE_COMMITTS_BUFFER] = "CommitTsBuffer",
+	[LWTRANCHE_SUBTRANS_BUFFER] = "SubtransBuffer",
+	[LWTRANCHE_MULTIXACTOFFSET_BUFFER] = "MultiXactOffsetBuffer",
+	[LWTRANCHE_MULTIXACTMEMBER_BUFFER] = "MultiXactMemberBuffer",
+	[LWTRANCHE_NOTIFY_BUFFER] = "NotifyBuffer",
+	[LWTRANCHE_SERIAL_BUFFER] = "SerialBuffer",
+	[LWTRANCHE_WAL_INSERT] = "WALInsert",
+	[LWTRANCHE_BUFFER_CONTENT] = "BufferContent",
+	[LWTRANCHE_REPLICATION_ORIGIN_STATE] = "ReplicationOriginState",
+	[LWTRANCHE_REPLICATION_SLOT_IO] = "ReplicationSlotIO",
+	[LWTRANCHE_LOCK_FASTPATH] = "LockFastPath",
+	[LWTRANCHE_BUFFER_MAPPING] = "BufferMapping",
+	[LWTRANCHE_LOCK_MANAGER] = "LockManager",
+	[LWTRANCHE_PREDICATE_LOCK_MANAGER] = "PredicateLockManager",
+	[LWTRANCHE_PARALLEL_HASH_JOIN] = "ParallelHashJoin",
+	[LWTRANCHE_PARALLEL_QUERY_DSA] = "ParallelQueryDSA",
+	[LWTRANCHE_PER_SESSION_DSA] = "PerSessionDSA",
+	[LWTRANCHE_PER_SESSION_RECORD_TYPE] = "PerSessionRecordType",
+	[LWTRANCHE_PER_SESSION_RECORD_TYPMOD] = "PerSessionRecordTypmod",
+	[LWTRANCHE_SHARED_TUPLESTORE] = "SharedTupleStore",
+	[LWTRANCHE_SHARED_TIDBITMAP] = "SharedTidBitmap",
+	[LWTRANCHE_PARALLEL_APPEND] = "ParallelAppend",
+	[LWTRANCHE_PER_XACT_PREDICATE_LIST] = "PerXactPredicateList",
+	[LWTRANCHE_PGSTATS_DSA] = "PgStatsDSA",
+	[LWTRANCHE_PGSTATS_HASH] = "PgStatsHash",
+	[LWTRANCHE_PGSTATS_DATA] = "PgStatsData",
+	[LWTRANCHE_LAUNCHER_DSA] = "LogicalRepLauncherDSA",
+	[LWTRANCHE_LAUNCHER_HASH] = "LogicalRepLauncherHash",
+	[LWTRANCHE_DSM_REGISTRY_DSA] = "DSMRegistryDSA",
+	[LWTRANCHE_DSM_REGISTRY_HASH] = "DSMRegistryHash",
 };
 
 StaticAssertDecl(lengthof(BuiltinTrancheNames) ==
-				 LWTRANCHE_FIRST_USER_DEFINED - NUM_INDIVIDUAL_LWLOCKS,
+				 LWTRANCHE_FIRST_USER_DEFINED,
 				 "missing entries in BuiltinTrancheNames[]");
 
 /*
@@ -769,13 +737,9 @@ LWLockReportWaitEnd(void)
 static const char *
 GetLWTrancheName(uint16 trancheId)
 {
-	/* Individual LWLock? */
-	if (trancheId < NUM_INDIVIDUAL_LWLOCKS)
-		return IndividualLWLockNames[trancheId];
-
-	/* Built-in tranche? */
+	/* Individual LWLock or built-in tranche? */
 	if (trancheId < LWTRANCHE_FIRST_USER_DEFINED)
-		return BuiltinTrancheNames[trancheId - NUM_INDIVIDUAL_LWLOCKS];
+		return BuiltinTrancheNames[trancheId];
 
 	/*
 	 * It's an extension tranche, so look in LWLockTrancheNames[].  However,
diff --git a/src/backend/storage/lmgr/meson.build b/src/backend/storage/lmgr/meson.build
index da32198f78..a12064ae8a 100644
--- a/src/backend/storage/lmgr/meson.build
+++ b/src/backend/storage/lmgr/meson.build
@@ -5,11 +5,19 @@ backend_sources += files(
   'deadlock.c',
   'lmgr.c',
   'lock.c',
-  'lwlock.c',
   'predicate.c',
   'proc.c',
   's_lock.c',
   'spin.c',
 )
 
-generated_backend_sources += lwlocknames[1]
+# this includes a .c file generated. Is there a better way?
+lwlock_source = files('lwlock.c')
+
+lwlock_lib = static_library('lwlock',
+  lwlock_source,
+  dependencies: [backend_code],
+  include_directories: include_directories('../../../include/storage'),
+  kwargs: internal_lib_args,
+  )
+backend_link_with += lwlock_lib
-- 
2.39.2


--2cgsne47fmqrtubo--