Subject: Re: make BuiltinTrancheNames less ugly

From: Heikki Linnakangas <[email-masked]>
Date: Fri, 23 Feb 2024 11:31:12 +0200

On 12/02/2024 19:01, Tristan Partin wrote:
> On Wed Jan 24, 2024 at 8:09 AM CST, Alvaro Herrera wrote:
>> IMO it would be less ugly to have the origin file lwlocknames.txt be
>> not a text file but a .h with a macro that can be defined by
>> interested parties so that they can extract what they want from the
>> file, like PG_CMDTAG or PG_KEYWORD.  Using Perl for this seems dirty...
> 
> I really like this idea, and would definitely be more inclined to see
> a solution like this.

+1 to that idea from me too. Seems pretty straightforward.

-- 
Heikki Linnakangas
Neon (https://neon.tech)

---

From: Alvaro Herrera <[email-masked]>
Date: Fri, 1 Mar 2024 15:00:49 +0100

> +1 to that idea from me too. Seems pretty straightforward.

OK, here's a patch that does it.  I have not touched Meson yet.

I'm pretty disappointed of not being able to remove
generate-lwlocknames.pl (it now generates the .h, no longer the .c
file), but I can't find a way to do the equivalent #defines in CPP ...
it'd require invoking the C preprocessor twice.  Maybe an intermediate
.h file would solve the problem, but I have no idea how would that work
with Meson.  I guess I'll do it in Make and let somebody suggest a Meson
way.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"El hombre nunca sabe de lo que es capaz hasta que lo intenta" (C. Dickens)
>From c70ae4eff6ec8e8965207d6e0d6d8fcdcd91cf16 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <[email-masked]>
Date: Fri, 1 Mar 2024 13:03:10 +0100
Subject: [PATCH] Rework lwlocknames.txt to become lwlocklist.h

This saves some code and some space in BuiltinTrancheNames, as foreseen
in commit 74a730631065.
---
 src/backend/Makefile                          |  4 +-
 src/backend/storage/lmgr/.gitignore           |  1 -
 src/backend/storage/lmgr/Makefile             |  9 +-
 .../storage/lmgr/generate-lwlocknames.pl      | 52 ++++++------
 src/backend/storage/lmgr/lwlock.c             | 15 ++--
 src/backend/storage/lmgr/lwlocknames.txt      | 60 --------------
 .../utils/activity/wait_event_names.txt       |  8 +-
 src/include/storage/lwlocklist.h              | 82 +++++++++++++++++++
 8 files changed, 124 insertions(+), 107 deletions(-)
 delete mode 100644 src/backend/storage/lmgr/lwlocknames.txt
 create mode 100644 src/include/storage/lwlocklist.h

diff --git a/src/backend/Makefile b/src/backend/Makefile
index d66e2a4b9f..34bb6393c4 100644
--- a/src/backend/Makefile
+++ b/src/backend/Makefile
@@ -110,8 +110,8 @@ $(top_builddir)/src/port/libpgport_srv.a: | submake-libpgport
 parser/gram.h: parser/gram.y
 	$(MAKE) -C parser gram.h
 
-storage/lmgr/lwlocknames.h: storage/lmgr/generate-lwlocknames.pl storage/lmgr/lwlocknames.txt utils/activity/wait_event_names.txt
-	$(MAKE) -C storage/lmgr lwlocknames.h lwlocknames.c
+storage/lmgr/lwlocknames.h: storage/lmgr/generate-lwlocknames.pl $(top_srcdir)/src/include/storage/lwlocklist.h utils/activity/wait_event_names.txt
+	$(MAKE) -C storage/lmgr lwlocknames.h
 
 utils/activity/wait_event_types.h: utils/activity/generate-wait_event_types.pl utils/activity/wait_event_names.txt
 	$(MAKE) -C utils/activity wait_event_types.h pgstat_wait_event.c wait_event_funcs_data.c
diff --git a/src/backend/storage/lmgr/.gitignore b/src/backend/storage/lmgr/.gitignore
index dab4c3f580..8e5b734f15 100644
--- a/src/backend/storage/lmgr/.gitignore
+++ b/src/backend/storage/lmgr/.gitignore
@@ -1,3 +1,2 @@
-/lwlocknames.c
 /lwlocknames.h
 /s_lock_test
diff --git a/src/backend/storage/lmgr/Makefile b/src/backend/storage/lmgr/Makefile
index 1aef423384..9d7dbe5abd 100644
--- a/src/backend/storage/lmgr/Makefile
+++ b/src/backend/storage/lmgr/Makefile
@@ -18,7 +18,6 @@ OBJS = \
 	lmgr.o \
 	lock.o \
 	lwlock.o \
-	lwlocknames.o \
 	predicate.o \
 	proc.o \
 	s_lock.o \
@@ -35,11 +34,7 @@ s_lock_test: s_lock.c $(top_builddir)/src/common/libpgcommon.a $(top_builddir)/s
 		$(TASPATH) -L $(top_builddir)/src/common -lpgcommon \
 		-L $(top_builddir)/src/port -lpgport -lm -o s_lock_test
 
-# see notes in src/backend/parser/Makefile
-lwlocknames.c: lwlocknames.h
-	touch $@
-
-lwlocknames.h: $(top_srcdir)/src/backend/storage/lmgr/lwlocknames.txt $(top_srcdir)/src/backend/utils/activity/wait_event_names.txt generate-lwlocknames.pl
+lwlocknames.h: $(top_srcdir)/src/include/storage/lwlocklist.h $(top_srcdir)/src/backend/utils/activity/wait_event_names.txt generate-lwlocknames.pl
 	$(PERL) $(srcdir)/generate-lwlocknames.pl $^
 
 check: s_lock_test
@@ -47,4 +42,4 @@ check: s_lock_test
 
 clean:
 	rm -f s_lock_test
-	rm -f lwlocknames.h lwlocknames.c
+	rm -f lwlocknames.h
diff --git a/src/backend/storage/lmgr/generate-lwlocknames.pl b/src/backend/storage/lmgr/generate-lwlocknames.pl
index 7b93ecf6c1..f46634a180 100644
--- a/src/backend/storage/lmgr/generate-lwlocknames.pl
+++ b/src/backend/storage/lmgr/generate-lwlocknames.pl
@@ -1,6 +1,6 @@
 #!/usr/bin/perl
 #
-# Generate lwlocknames.h and lwlocknames.c from lwlocknames.txt
+# Generate lwlocknames.h from lwlocklist.h
 # Copyright (c) 2000-2024, PostgreSQL Global Development Group
 
 use strict;
@@ -14,26 +14,22 @@ my $continue = "\n";
 
 GetOptions('outdir:s' => \$output_path);
 
-open my $lwlocknames, '<', $ARGV[0] or die;
+open my $lwlocklist, '<', $ARGV[0] or die;
 open my $wait_event_names, '<', $ARGV[1] or die;
 
 # Include PID in suffix in case parallel make runs this multiple times.
 my $htmp = "$output_path/lwlocknames.h.tmp$$";
-my $ctmp = "$output_path/lwlocknames.c.tmp$$";
 open my $h, '>', $htmp or die "Could not open $htmp: $!";
-open my $c, '>', $ctmp or die "Could not open $ctmp: $!";
 
 my $autogen =
-  "/* autogenerated from src/backend/storage/lmgr/lwlocknames.txt, do not edit */\n";
+  "/* autogenerated from src/backend/storage/lmgr/lwlocklist.h, do not edit */\n";
 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.
+# cross-check those with the ones in lwlocklist.h.
 #
 my @wait_event_lwlocks;
 my $record_lwlocks = 0;
@@ -64,17 +60,30 @@ while (<$wait_event_names>)
 	push(@wait_event_lwlocks, $waiteventname . "Lock");
 }
 
+my $in_comment = 0;
 my $i = 0;
-while (<$lwlocknames>)
+while (<$lwlocklist>)
 {
 	chomp;
 
-	# Skip comments
-	next if /^#/;
+	# Skip uniline C comments and empty lines
+	next if m{^\s*/\*.*\*/$};
 	next if /^\s*$/;
 
-	die "unable to parse lwlocknames.txt"
-	  unless /^(\w+)\s+(\d+)$/;
+	# skip multiline C comments
+	if ($in_comment == 1)
+	{
+		$in_comment = 0 if m{\*/};
+		next;
+	}
+	elsif (m{^\s*/\*})
+	{
+		$in_comment = 1;
+		next;
+	}
+
+	die "unable to parse lwlocklist.h line \"$_\""
+	  unless /^PG_LWLOCK\((\w+),\s+(\d+)\)$/;
 
 	(my $lockname, my $lockidx) = ($1, $2);
 
@@ -82,25 +91,23 @@ while (<$lwlocknames>)
 	$trimmedlockname =~ s/Lock$//;
 	die "lock names must end with 'Lock'" if $trimmedlockname eq $lockname;
 
-	die "lwlocknames.txt not in order" if $lockidx < $lastlockidx;
-	die "lwlocknames.txt has duplicates" if $lockidx == $lastlockidx;
+	die "lwlocklist.h not in order" if $lockidx < $lastlockidx;
+	die "lwlocklist.h has duplicates" if $lockidx == $lastlockidx;
 
-	die "$lockname defined in lwlocknames.txt but missing from "
+	die "$lockname defined in lwlocklist.h but missing from "
 	  . "wait_event_names.txt"
 	  if $i >= scalar @wait_event_lwlocks;
 	die "lists of predefined LWLocks do not match (first mismatch at "
 	  . "$wait_event_lwlocks[$i] in wait_event_names.txt and $lockname in "
-	  . "lwlocknames.txt)"
+	  . "lwlocklist.h)"
 	  if $wait_event_lwlocks[$i] ne $lockname;
 	$i++;
 
 	while ($lastlockidx < $lockidx - 1)
 	{
 		++$lastlockidx;
-		printf $c "%s	\"<unassigned:%d>\"", $continue, $lastlockidx;
 		$continue = ",\n";
 	}
-	printf $c "%s	\"%s\"", $continue, $trimmedlockname;
 	$lastlockidx = $lockidx;
 	$continue = ",\n";
 
@@ -109,18 +116,15 @@ while (<$lwlocknames>)
 
 die
   "$wait_event_lwlocks[$i] defined in wait_event_names.txt but missing from "
-  . "lwlocknames.txt"
+  . "lwlocklist.h"
   if $i < scalar @wait_event_lwlocks;
 
-printf $c "\n};\n";
 print $h "\n";
 printf $h "#define NUM_INDIVIDUAL_LWLOCKS		%s\n", $lastlockidx + 1;
 
 close $h;
-close $c;
 
 rename($htmp, "$output_path/lwlocknames.h")
   || die "rename: $htmp to $output_path/lwlocknames.h: $!";
-rename($ctmp, "$output_path/lwlocknames.c") || die "rename: $ctmp: $!";
 
-close $lwlocknames;
+close $lwlocklist;
diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index d405c61b21..1112389af5 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.  We absorb the names of these tranches from there into
+ * BuiltinTrancheNames here.
  *
  * 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,9 +129,10 @@ 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[] = {
+#define PG_LWLOCK(lockname, id) [id] = CppAsString(lockname),
+#include "storage/lwlocknames.h"
+#undef PG_LWLOCK
 	[LWTRANCHE_XACT_BUFFER] = "XactBuffer",
 	[LWTRANCHE_COMMITTS_BUFFER] = "CommitTsBuffer",
 	[LWTRANCHE_SUBTRANS_BUFFER] = "SubtransBuffer",
@@ -745,11 +746,7 @@ LWLockReportWaitEnd(void)
 static const char *
 GetLWTrancheName(uint16 trancheId)
 {
-	/* Individual LWLock? */
-	if (trancheId < NUM_INDIVIDUAL_LWLOCKS)
-		return IndividualLWLockNames[trancheId];
-
-	/* Built-in tranche? */
+	/* Built-in tranche or individual LWLock? */
 	if (trancheId < LWTRANCHE_FIRST_USER_DEFINED)
 		return BuiltinTrancheNames[trancheId];
 
diff --git a/src/backend/storage/lmgr/lwlocknames.txt b/src/backend/storage/lmgr/lwlocknames.txt
deleted file mode 100644
index 284d168f77..0000000000
--- a/src/backend/storage/lmgr/lwlocknames.txt
+++ /dev/null
@@ -1,60 +0,0 @@
-# Some commonly-used locks have predefined positions within MainLWLockArray;
-# these are defined here.  If you add a lock, add it to the end to avoid
-# renumbering the existing locks; if you remove a lock, consider leaving a gap
-# in the numbering sequence for the benefit of DTrace and other external
-# debugging scripts.  Also, do not forget to update the section
-# WaitEventLWLock of src/backend/utils/activity/wait_event_names.txt.
-
-# 0 is available; was formerly BufFreelistLock
-ShmemIndexLock						1
-OidGenLock							2
-XidGenLock							3
-ProcArrayLock						4
-SInvalReadLock						5
-SInvalWriteLock						6
-WALBufMappingLock					7
-WALWriteLock						8
-ControlFileLock						9
-# 10 was CheckpointLock
-# 11 was XactSLRULock
-# 12 was SubtransSLRULock
-MultiXactGenLock					13
-# 14 was MultiXactOffsetSLRULock
-# 15 was MultiXactMemberSLRULock
-RelCacheInitLock					16
-CheckpointerCommLock				17
-TwoPhaseStateLock					18
-TablespaceCreateLock				19
-BtreeVacuumLock						20
-AddinShmemInitLock					21
-AutovacuumLock						22
-AutovacuumScheduleLock				23
-SyncScanLock						24
-RelationMappingLock					25
-#26 was NotifySLRULock
-NotifyQueueLock						27
-SerializableXactHashLock			28
-SerializableFinishedListLock		29
-SerializablePredicateListLock		30
-# 31 was SerialSLRULock
-SyncRepLock							32
-BackgroundWorkerLock				33
-DynamicSharedMemoryControlLock		34
-AutoFileLock						35
-ReplicationSlotAllocationLock		36
-ReplicationSlotControlLock			37
-#38 was CommitTsSLRULock
-CommitTsLock						39
-ReplicationOriginLock				40
-MultiXactTruncationLock				41
-# 42 was OldSnapshotTimeMapLock
-LogicalRepWorkerLock				43
-XactTruncationLock					44
-# 45 was XactTruncationLock until removal of BackendRandomLock
-WrapLimitsVacuumLock				46
-NotifyQueueTailLock					47
-WaitEventExtensionLock				48
-WALSummarizerLock					49
-DSMRegistryLock						50
-InjectionPointLock				51
-SerialControlLock					52
diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt
index ec2f31f82a..105ca977a5 100644
--- a/src/backend/utils/activity/wait_event_names.txt
+++ b/src/backend/utils/activity/wait_event_names.txt
@@ -279,9 +279,9 @@ Extension	"Waiting in an extension."
 # This class of wait events has its own set of C structure, so these are
 # only used for the documentation.
 #
-# NB: Predefined LWLocks (i.e., those declared in lwlocknames.txt) must be
+# NB: Predefined LWLocks (i.e., those declared in lwlocknames.h) must be
 # listed in the top section of locks and must be listed in the same order as in
-# lwlocknames.txt.
+# lwlocknames.h.
 #
 
 Section: ClassName - WaitEventLWLock
@@ -332,9 +332,9 @@ SerialControl	"Waiting to read or update shared <filename>pg_serial</filename> s
 #
 # END OF PREDEFINED LWLOCKS (DO NOT CHANGE THIS LINE)
 #
-# Predefined LWLocks (i.e., those declared in lwlocknames.txt) must be listed
+# Predefined LWLocks (i.e., those declared in lwlocknames.h) must be listed
 # in the section above and must be listed in the same order as in
-# lwlocknames.txt.  Other LWLocks must be listed in the section below.
+# lwlocknames.h.  Other LWLocks must be listed in the section below.
 #
 
 XactBuffer	"Waiting for I/O on a transaction status SLRU buffer."
diff --git a/src/include/storage/lwlocklist.h b/src/include/storage/lwlocklist.h
new file mode 100644
index 0000000000..3396eed58e
--- /dev/null
+++ b/src/include/storage/lwlocklist.h
@@ -0,0 +1,82 @@
+/*-------------------------------------------------------------------------
+ *
+ * lwlocklist.h
+ *
+ * The predefined LWLock list is kept in its own source file for use by
+ * automatic tools.  The exact representation of a keyword is determined by
+ * the PG_LWLOCK macro, which is not defined in this file; it can be
+ * defined by the caller for special purposes.
+ *
+ * Also, generate-lwlock-names.pl processes this file.
+ *
+ * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * IDENTIFICATION
+ *    src/include/storage/lwlocklist.h
+ *
+ *-------------------------------------------------------------------------
+ */
+
+/*
+ * Some commonly-used locks have predefined positions within MainLWLockArray;
+ * these are defined here.  If you add a lock, add it to the end to avoid
+ * renumbering the existing locks; if you remove a lock, consider leaving a gap
+ * in the numbering sequence for the benefit of DTrace and other external
+ * debugging scripts.  Also, do not forget to update the section
+ * WaitEventLWLock of src/backend/utils/activity/wait_event_names.txt.
+ */
+
+/* 0 is available; was formerly BufFreelistLock */
+PG_LWLOCK(ShmemIndexLock, 1)
+PG_LWLOCK(OidGenLock, 2)
+PG_LWLOCK(XidGenLock, 3)
+PG_LWLOCK(ProcArrayLock, 4)
+PG_LWLOCK(SInvalReadLock, 5)
+PG_LWLOCK(SInvalWriteLock, 6)
+PG_LWLOCK(WALBufMappingLock, 7)
+PG_LWLOCK(WALWriteLock, 8)
+PG_LWLOCK(ControlFileLock, 9)
+/* 10 was CheckpointLock */
+/* 11 was XactSLRULock */
+/* 12 was SubtransSLRULock */
+PG_LWLOCK(MultiXactGenLock, 13)
+/* 14 was MultiXactOffsetSLRULock */
+/* 15 was MultiXactMemberSLRULock */
+PG_LWLOCK(RelCacheInitLock, 16)
+PG_LWLOCK(CheckpointerCommLock, 17)
+PG_LWLOCK(TwoPhaseStateLock, 18)
+PG_LWLOCK(TablespaceCreateLock, 19)
+PG_LWLOCK(BtreeVacuumLock, 20)
+PG_LWLOCK(AddinShmemInitLock, 21)
+PG_LWLOCK(AutovacuumLock, 22)
+PG_LWLOCK(AutovacuumScheduleLock, 23)
+PG_LWLOCK(SyncScanLock, 24)
+PG_LWLOCK(RelationMappingLock, 25)
+/* 26 was NotifySLRULock */
+PG_LWLOCK(NotifyQueueLock, 27)
+PG_LWLOCK(SerializableXactHashLock, 28)
+PG_LWLOCK(SerializableFinishedListLock, 29)
+PG_LWLOCK(SerializablePredicateListLock, 30)
+/* 31 was SerialSLRULock */
+PG_LWLOCK(SyncRepLock, 32)
+PG_LWLOCK(BackgroundWorkerLock, 33)
+PG_LWLOCK(DynamicSharedMemoryControlLock, 34)
+PG_LWLOCK(AutoFileLock, 35)
+PG_LWLOCK(ReplicationSlotAllocationLock, 36)
+PG_LWLOCK(ReplicationSlotControlLock, 37)
+/* 38 was CommitTsSLRULock */
+PG_LWLOCK(CommitTsLock, 39)
+PG_LWLOCK(ReplicationOriginLock, 40)
+PG_LWLOCK(MultiXactTruncationLock, 41)
+/* 42 was OldSnapshotTimeMapLock */
+PG_LWLOCK(LogicalRepWorkerLock, 43)
+PG_LWLOCK(XactTruncationLock, 44)
+/* 45 was XactTruncationLock until removal of BackendRandomLock */
+PG_LWLOCK(WrapLimitsVacuumLock, 46)
+PG_LWLOCK(NotifyQueueTailLock, 47)
+PG_LWLOCK(WaitEventExtensionLock, 48)
+PG_LWLOCK(WALSummarizerLock, 49)
+PG_LWLOCK(DSMRegistryLock, 50)
+PG_LWLOCK(InjectionPointLock, 51)
+PG_LWLOCK(SerialControlLock, 52)
-- 
2.39.2

---

From: "Tristan Partin" <[email-masked]>
Date: Fri, 01 Mar 2024 10:42:19 -0600

> with Meson.  I guess I'll do it in Make and let somebody suggest a Meson

I can help you with Meson if you get the Make implementation done.

-- 
Tristan Partin
Neon (https://neon.tech)