On Wed Jan 24, 2024 at 8:09 AM CST, Alvaro Herrera wrote:
From 3d24b89855888a6650ec1aafb3579d810bfec5ac Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Tue, 23 Jan 2024 10:36:14 +0100
Subject: [PATCH] Remove IndividualLWLockNames

We can just merge the lwlock names into the BuiltinTrancheNames array.
This requires that Meson compiles the file with -I. in CPPFLAGS, which
in turn requires some additional contortions for DTrace support in
FreeBSD.
---
 src/backend/meson.build                          |  4 +++-
 src/backend/storage/lmgr/Makefile                |  3 ++-
 src/backend/storage/lmgr/generate-lwlocknames.pl | 10 ++--------
 src/backend/storage/lmgr/lwlock.c                | 13 ++++---------
 src/backend/storage/lmgr/meson.build             | 12 ++++++++++--
 5 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/src/backend/meson.build b/src/backend/meson.build
index 8767aaba67..57a52c37e0 100644
--- a/src/backend/meson.build
+++ b/src/backend/meson.build
@@ -127,7 +127,9 @@ 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 98fa6035cc..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,9 +129,8 @@ 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[] = {
+#include "lwlocknames.c"
        [LWTRANCHE_XACT_BUFFER] = "XactBuffer",
        [LWTRANCHE_COMMITTS_BUFFER] = "CommitTsBuffer",
        [LWTRANCHE_SUBTRANS_BUFFER] = "SubtransBuffer",
@@ -738,11 +737,7 @@ 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];
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')

I don't understand this comment. Could you explain it a bit more?

+
+lwlock_lib = static_library('lwlock',
+  lwlock_source,
+  dependencies: [backend_code],
+  include_directories: include_directories('../../../include/storage'),
+  kwargs: internal_lib_args,
+  )

Move the paren to the beginning of the line.

+backend_link_with += lwlock_lib

Earlier in the thread you had said this:

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.

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


Reply via email to