Collect built-in LWLock tranche names statically, not dynamically.
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 14 May 2020 15:10:31 +0000 (11:10 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 14 May 2020 15:10:31 +0000 (11:10 -0400)
There is little point in using the LWLockRegisterTranche mechanism for
built-in tranche names.  It wastes cycles, it creates opportunities for
bugs (since failing to register a tranche name is a very hard-to-detect
problem), and the lack of any centralized list of names encourages
sloppy nonconformity in name choices.  Moreover, since we have a
centralized list of the tranches anyway in enum BuiltinTrancheIds, we're
certainly not buying any flexibility in return for these disadvantages.

Hence, nuke all the backend-internal LWLockRegisterTranche calls,
and instead provide a const array of the builtin tranche names.

(I have in mind to change a bunch of these names shortly, but this
patch is just about getting them into one place.)

Discussion: https://postgr.es/m/9056.1589419765@sss.pgh.pa.us

src/backend/access/transam/slru.c
src/backend/access/transam/xlog.c
src/backend/replication/logical/origin.c
src/backend/replication/slot.c
src/backend/storage/buffer/buf_init.c
src/backend/storage/ipc/procarray.c
src/backend/storage/lmgr/lwlock.c

index 3ba9fc9c4931813a7132e122e5b451232da99a81..3572b01091ef8e75e743c6353635366c6a88c0b6 100644 (file)
@@ -235,9 +235,6 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
        else
                Assert(found);
 
-       /* Register SLRU tranche in the main tranches array */
-       LWLockRegisterTranche(tranche_id, name);
-
        /*
         * Initialize the unshared control struct, including directory path. We
         * assume caller set PagePrecedes.
index a53e6d963346d0e471d6203ae20e8bffd0b7c32e..4284659099bf450cb6e4f8d310efc98fd16eb42b 100644 (file)
@@ -5116,10 +5116,8 @@ XLOGShmemInit(void)
                /* both should be present or neither */
                Assert(foundCFile && foundXLog);
 
-               /* Initialize local copy of WALInsertLocks and register the tranche */
+               /* Initialize local copy of WALInsertLocks */
                WALInsertLocks = XLogCtl->Insert.WALInsertLocks;
-               LWLockRegisterTranche(LWTRANCHE_WAL_INSERT,
-                                                         "wal_insert");
 
                if (localControlFile)
                        pfree(localControlFile);
@@ -5155,7 +5153,6 @@ XLOGShmemInit(void)
                (WALInsertLockPadded *) allocptr;
        allocptr += sizeof(WALInsertLockPadded) * NUM_XLOGINSERT_LOCKS;
 
-       LWLockRegisterTranche(LWTRANCHE_WAL_INSERT, "wal_insert");
        for (i = 0; i < NUM_XLOGINSERT_LOCKS; i++)
        {
                LWLockInitialize(&WALInsertLocks[i].l.lock, LWTRANCHE_WAL_INSERT);
index d0d2b46a654b855b28e76342a0519faaeb3e75f2..923ea3ffd720fc93629f5a9afa53207448b46f3b 100644 (file)
@@ -517,9 +517,6 @@ ReplicationOriginShmemInit(void)
                        ConditionVariableInit(&replication_states[i].origin_cv);
                }
        }
-
-       LWLockRegisterTranche(replication_states_ctl->tranche_id,
-                                                 "replication_origin");
 }
 
 /* ---------------------------------------------------------------------------
index abae74c9a59bf2041bf8c722f52524a77fbd29ae..d3d1033beba3836296e198ee648da95b6e7a5753 100644 (file)
@@ -140,9 +140,6 @@ ReplicationSlotsShmemInit(void)
                ShmemInitStruct("ReplicationSlot Ctl", ReplicationSlotsShmemSize(),
                                                &found);
 
-       LWLockRegisterTranche(LWTRANCHE_REPLICATION_SLOT_IO_IN_PROGRESS,
-                                                 "replication_slot_io");
-
        if (!found)
        {
                int                     i;
index af62d48c323d34020844ace99f4756259dde3541..895485698a20071a6b45d887f211139e501c28dc 100644 (file)
@@ -87,9 +87,6 @@ InitBufferPool(void)
                                                NBuffers * (Size) sizeof(LWLockMinimallyPadded),
                                                &foundIOLocks);
 
-       LWLockRegisterTranche(LWTRANCHE_BUFFER_IO_IN_PROGRESS, "buffer_io");
-       LWLockRegisterTranche(LWTRANCHE_BUFFER_CONTENT, "buffer_content");
-
        /*
         * The array used to sort to-be-checkpointed buffer ids is located in
         * shared memory, to avoid having to allocate significant amounts of
index 363000670b2f919f334dee2e7be4b56aedfaab9a..6a94448565bb0d5385ff2acdb6c89df90b850b57 100644 (file)
@@ -267,8 +267,6 @@ CreateSharedProcArray(void)
                                                        mul_size(sizeof(bool), TOTAL_MAX_CACHED_SUBXIDS),
                                                        &found);
        }
-
-       LWLockRegisterTranche(LWTRANCHE_PROC, "proc");
 }
 
 /*
index 4c14e51c67f2454b1386690335a748b07bbe329b..61bec10b79eefa10e101aeef850d8b52ea5da8be 100644 (file)
@@ -108,14 +108,85 @@ extern slock_t *ShmemLock;
 #define LW_SHARED_MASK                         ((uint32) ((1 << 24)-1))
 
 /*
- * This is indexed by tranche ID and stores the names of all tranches known
- * to the current backend.
+ * 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 MainLWLockNames[]
+ * in lwlocknames.c.
+ *
+ * 2. There are some predefined tranches for built-in groups of locks.
+ * These are listed in enum BuiltinTrancheIds in lwlock.h, and their names
+ * appear in BuiltinTrancheNames[] below.
+ *
+ * 3. Extensions can create new tranches, via either RequestNamedLWLockTranche
+ * or LWLockRegisterTranche.  The names of these that are known in the current
+ * process appear in LWLockTrancheNames[].
  */
-static const char **LWLockTrancheArray = NULL;
-static int     LWLockTranchesAllocated = 0;
 
-#define T_NAME(lock) \
-       (LWLockTrancheArray[(lock)->tranche])
+static const char *const BuiltinTrancheNames[] = {
+       /* LWTRANCHE_CLOG_BUFFERS: */
+       "clog",
+       /* LWTRANCHE_COMMITTS_BUFFERS: */
+       "commit_timestamp",
+       /* LWTRANCHE_SUBTRANS_BUFFERS: */
+       "subtrans",
+       /* LWTRANCHE_MXACTOFFSET_BUFFERS: */
+       "multixact_offset",
+       /* LWTRANCHE_MXACTMEMBER_BUFFERS: */
+       "multixact_member",
+       /* LWTRANCHE_ASYNC_BUFFERS: */
+       "async",
+       /* LWTRANCHE_OLDSERXID_BUFFERS: */
+       "oldserxid",
+       /* LWTRANCHE_WAL_INSERT: */
+       "wal_insert",
+       /* LWTRANCHE_BUFFER_CONTENT: */
+       "buffer_content",
+       /* LWTRANCHE_BUFFER_IO_IN_PROGRESS: */
+       "buffer_io",
+       /* LWTRANCHE_REPLICATION_ORIGIN: */
+       "replication_origin",
+       /* LWTRANCHE_REPLICATION_SLOT_IO_IN_PROGRESS: */
+       "replication_slot_io",
+       /* LWTRANCHE_PROC: */
+       "proc",
+       /* LWTRANCHE_BUFFER_MAPPING: */
+       "buffer_mapping",
+       /* LWTRANCHE_LOCK_MANAGER: */
+       "lock_manager",
+       /* LWTRANCHE_PREDICATE_LOCK_MANAGER: */
+       "predicate_lock_manager",
+       /* LWTRANCHE_PARALLEL_HASH_JOIN: */
+       "parallel_hash_join",
+       /* LWTRANCHE_PARALLEL_QUERY_DSA: */
+       "parallel_query_dsa",
+       /* LWTRANCHE_SESSION_DSA: */
+       "session_dsa",
+       /* LWTRANCHE_SESSION_RECORD_TABLE: */
+       "session_record_table",
+       /* LWTRANCHE_SESSION_TYPMOD_TABLE: */
+       "session_typmod_table",
+       /* LWTRANCHE_SHARED_TUPLESTORE: */
+       "shared_tuplestore",
+       /* LWTRANCHE_TBM: */
+       "tbm",
+       /* LWTRANCHE_PARALLEL_APPEND: */
+       "parallel_append",
+       /* LWTRANCHE_SXACT: */
+       "serializable_xact"
+};
+
+StaticAssertDecl(lengthof(BuiltinTrancheNames) ==
+                                LWTRANCHE_FIRST_USER_DEFINED - NUM_INDIVIDUAL_LWLOCKS,
+                                "missing entries in BuiltinTrancheNames[]");
+
+/*
+ * This is indexed by tranche ID minus LWTRANCHE_FIRST_USER_DEFINED, and
+ * stores the names of all dynamically-created tranches known to the current
+ * process.  Any unused entries in the array will contain NULL.
+ */
+static const char **LWLockTrancheNames = NULL;
+static int     LWLockTrancheNamesAllocated = 0;
 
 /*
  * This points to the main array of LWLocks in shared memory.  Backends inherit
@@ -149,19 +220,29 @@ typedef struct NamedLWLockTrancheRequest
        int                     num_lwlocks;
 } NamedLWLockTrancheRequest;
 
-NamedLWLockTrancheRequest *NamedLWLockTrancheRequestArray = NULL;
+static NamedLWLockTrancheRequest *NamedLWLockTrancheRequestArray = NULL;
 static int     NamedLWLockTrancheRequestsAllocated = 0;
+
+/*
+ * NamedLWLockTrancheRequests is both the valid length of the request array,
+ * and the length of the shared-memory NamedLWLockTrancheArray later on.
+ * This variable and NamedLWLockTrancheArray are non-static so that
+ * postmaster.c can copy them to child processes in EXEC_BACKEND builds.
+ */
 int                    NamedLWLockTrancheRequests = 0;
 
+/* points to data in shared memory: */
 NamedLWLockTranche *NamedLWLockTrancheArray = NULL;
 
 static bool lock_named_request_allowed = true;
 
 static void InitializeLWLocks(void);
-static void RegisterLWLockTranches(void);
-
 static inline void LWLockReportWaitStart(LWLock *lock);
 static inline void LWLockReportWaitEnd(void);
+static const char *GetLWTrancheName(uint16 trancheId);
+
+#define T_NAME(lock) \
+       GetLWTrancheName((lock)->tranche)
 
 #ifdef LWLOCK_STATS
 typedef struct lwlock_stats_key
@@ -285,7 +366,7 @@ print_lwlock_stats(int code, Datum arg)
        {
                fprintf(stderr,
                                "PID %d lwlock %s %p: shacq %u exacq %u blk %u spindelay %u dequeue self %u\n",
-                               MyProcPid, LWLockTrancheArray[lwstats->key.tranche],
+                               MyProcPid, GetLWTrancheName(lwstats->key.tranche),
                                lwstats->key.instance, lwstats->sh_acquire_count,
                                lwstats->ex_acquire_count, lwstats->block_count,
                                lwstats->spin_delay_count, lwstats->dequeue_self_count);
@@ -332,7 +413,7 @@ get_lwlock_stats_entry(LWLock *lock)
  * allocated in the main array.
  */
 static int
-NumLWLocksByNamedTranches(void)
+NumLWLocksForNamedTranches(void)
 {
        int                     numLocks = 0;
        int                     i;
@@ -353,7 +434,8 @@ LWLockShmemSize(void)
        int                     i;
        int                     numLocks = NUM_FIXED_LWLOCKS;
 
-       numLocks += NumLWLocksByNamedTranches();
+       /* Calculate total number of locks needed in the main array. */
+       numLocks += NumLWLocksForNamedTranches();
 
        /* Space for the LWLock array. */
        size = mul_size(numLocks, sizeof(LWLockPadded));
@@ -368,7 +450,7 @@ LWLockShmemSize(void)
        for (i = 0; i < NamedLWLockTrancheRequests; i++)
                size = add_size(size, strlen(NamedLWLockTrancheRequestArray[i].tranche_name) + 1);
 
-       /* Disallow named LWLocks' requests after startup */
+       /* Disallow adding any more named tranches. */
        lock_named_request_allowed = false;
 
        return size;
@@ -376,7 +458,7 @@ LWLockShmemSize(void)
 
 /*
  * Allocate shmem space for the main LWLock array and all tranches and
- * initialize it.  We also register all the LWLock tranches here.
+ * initialize it.  We also register extension LWLock tranches here.
  */
 void
 CreateLWLocks(void)
@@ -416,8 +498,10 @@ CreateLWLocks(void)
                InitializeLWLocks();
        }
 
-       /* Register all LWLock tranches */
-       RegisterLWLockTranches();
+       /* Register named extension LWLock tranches in the current process. */
+       for (int i = 0; i < NamedLWLockTrancheRequests; i++)
+               LWLockRegisterTranche(NamedLWLockTrancheArray[i].trancheId,
+                                                         NamedLWLockTrancheArray[i].trancheName);
 }
 
 /*
@@ -426,7 +510,7 @@ CreateLWLocks(void)
 static void
 InitializeLWLocks(void)
 {
-       int                     numNamedLocks = NumLWLocksByNamedTranches();
+       int                     numNamedLocks = NumLWLocksForNamedTranches();
        int                     id;
        int                     i;
        int                     j;
@@ -452,7 +536,10 @@ InitializeLWLocks(void)
        for (id = 0; id < NUM_PREDICATELOCK_PARTITIONS; id++, lock++)
                LWLockInitialize(&lock->lock, LWTRANCHE_PREDICATE_LOCK_MANAGER);
 
-       /* Initialize named tranches. */
+       /*
+        * Copy the info about any named tranches into shared memory (so that
+        * other processes can see it), and initialize the requested LWLocks.
+        */
        if (NamedLWLockTrancheRequests > 0)
        {
                char       *trancheNames;
@@ -485,51 +572,6 @@ InitializeLWLocks(void)
        }
 }
 
-/*
- * Register named tranches and tranches for fixed LWLocks.
- */
-static void
-RegisterLWLockTranches(void)
-{
-       int                     i;
-
-       if (LWLockTrancheArray == NULL)
-       {
-               LWLockTranchesAllocated = 128;
-               LWLockTrancheArray = (const char **)
-                       MemoryContextAllocZero(TopMemoryContext,
-                                                                  LWLockTranchesAllocated * sizeof(char *));
-               Assert(LWLockTranchesAllocated >= LWTRANCHE_FIRST_USER_DEFINED);
-       }
-
-       for (i = 0; i < NUM_INDIVIDUAL_LWLOCKS; ++i)
-               LWLockRegisterTranche(i, MainLWLockNames[i]);
-
-       LWLockRegisterTranche(LWTRANCHE_BUFFER_MAPPING, "buffer_mapping");
-       LWLockRegisterTranche(LWTRANCHE_LOCK_MANAGER, "lock_manager");
-       LWLockRegisterTranche(LWTRANCHE_PREDICATE_LOCK_MANAGER,
-                                                 "predicate_lock_manager");
-       LWLockRegisterTranche(LWTRANCHE_PARALLEL_QUERY_DSA,
-                                                 "parallel_query_dsa");
-       LWLockRegisterTranche(LWTRANCHE_SESSION_DSA,
-                                                 "session_dsa");
-       LWLockRegisterTranche(LWTRANCHE_SESSION_RECORD_TABLE,
-                                                 "session_record_table");
-       LWLockRegisterTranche(LWTRANCHE_SESSION_TYPMOD_TABLE,
-                                                 "session_typmod_table");
-       LWLockRegisterTranche(LWTRANCHE_SHARED_TUPLESTORE,
-                                                 "shared_tuplestore");
-       LWLockRegisterTranche(LWTRANCHE_TBM, "tbm");
-       LWLockRegisterTranche(LWTRANCHE_PARALLEL_APPEND, "parallel_append");
-       LWLockRegisterTranche(LWTRANCHE_PARALLEL_HASH_JOIN, "parallel_hash_join");
-       LWLockRegisterTranche(LWTRANCHE_SXACT, "serializable_xact");
-
-       /* Register named tranches. */
-       for (i = 0; i < NamedLWLockTrancheRequests; i++)
-               LWLockRegisterTranche(NamedLWLockTrancheArray[i].trancheId,
-                                                         NamedLWLockTrancheArray[i].trancheName);
-}
-
 /*
  * InitLWLockAccess - initialize backend-local state needed to hold LWLocks
  */
@@ -570,8 +612,7 @@ GetNamedLWLockTranche(const char *tranche_name)
                lock_pos += NamedLWLockTrancheRequestArray[i].num_lwlocks;
        }
 
-       if (i >= NamedLWLockTrancheRequests)
-               elog(ERROR, "requested tranche is not registered");
+       elog(ERROR, "requested tranche is not registered");
 
        /* just to keep compiler quiet */
        return NULL;
@@ -595,32 +636,47 @@ LWLockNewTrancheId(void)
 }
 
 /*
- * Register a tranche ID in the lookup table for the current process.  This
- * routine will save a pointer to the tranche name passed as an argument,
+ * Register a dynamic tranche name in the lookup table of the current process.
+ *
+ * This routine will save a pointer to the tranche name passed as an argument,
  * so the name should be allocated in a backend-lifetime context
- * (TopMemoryContext, static variable, or similar).
+ * (TopMemoryContext, static constant, or similar).
  */
 void
 LWLockRegisterTranche(int tranche_id, const char *tranche_name)
 {
-       Assert(LWLockTrancheArray != NULL);
+       /* This should only be called for user-defined tranches. */
+       if (tranche_id < LWTRANCHE_FIRST_USER_DEFINED)
+               return;
+
+       /* Convert to array index. */
+       tranche_id -= LWTRANCHE_FIRST_USER_DEFINED;
 
-       if (tranche_id >= LWLockTranchesAllocated)
+       /* If necessary, create or enlarge array. */
+       if (tranche_id >= LWLockTrancheNamesAllocated)
        {
-               int                     i = LWLockTranchesAllocated;
-               int                     j = LWLockTranchesAllocated;
+               int                     newalloc;
 
-               while (i <= tranche_id)
-                       i *= 2;
+               newalloc = Max(LWLockTrancheNamesAllocated, 8);
+               while (newalloc <= tranche_id)
+                       newalloc *= 2;
 
-               LWLockTrancheArray = (const char **)
-                       repalloc(LWLockTrancheArray, i * sizeof(char *));
-               LWLockTranchesAllocated = i;
-               while (j < LWLockTranchesAllocated)
-                       LWLockTrancheArray[j++] = NULL;
+               if (LWLockTrancheNames == NULL)
+                       LWLockTrancheNames = (const char **)
+                               MemoryContextAllocZero(TopMemoryContext,
+                                                                          newalloc * sizeof(char *));
+               else
+               {
+                       LWLockTrancheNames = (const char **)
+                               repalloc(LWLockTrancheNames, newalloc * sizeof(char *));
+                       memset(LWLockTrancheNames + LWLockTrancheNamesAllocated,
+                                  0,
+                                  (newalloc - LWLockTrancheNamesAllocated) * sizeof(char *));
+               }
+               LWLockTrancheNamesAllocated = newalloc;
        }
 
-       LWLockTrancheArray[tranche_id] = tranche_name;
+       LWLockTrancheNames[tranche_id] = tranche_name;
 }
 
 /*
@@ -666,8 +722,8 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks)
        }
 
        request = &NamedLWLockTrancheRequestArray[NamedLWLockTrancheRequests];
-       Assert(strlen(tranche_name) + 1 < NAMEDATALEN);
-       StrNCpy(request->tranche_name, tranche_name, NAMEDATALEN);
+       Assert(strlen(tranche_name) + 1 <= NAMEDATALEN);
+       strlcpy(request->tranche_name, tranche_name, NAMEDATALEN);
        request->num_lwlocks = num_lwlocks;
        NamedLWLockTrancheRequests++;
 }
@@ -709,23 +765,42 @@ LWLockReportWaitEnd(void)
 }
 
 /*
- * Return an identifier for an LWLock based on the wait class and event.
+ * Return the name of an LWLock tranche.
  */
-const char *
-GetLWLockIdentifier(uint32 classId, uint16 eventId)
+static const char *
+GetLWTrancheName(uint16 trancheId)
 {
-       Assert(classId == PG_WAIT_LWLOCK);
+       /* Individual LWLock? */
+       if (trancheId < NUM_INDIVIDUAL_LWLOCKS)
+               return MainLWLockNames[trancheId];
+
+       /* Built-in tranche? */
+       if (trancheId < LWTRANCHE_FIRST_USER_DEFINED)
+               return BuiltinTrancheNames[trancheId - NUM_INDIVIDUAL_LWLOCKS];
 
        /*
-        * It is quite possible that user has registered tranche in one of the
-        * backends (e.g. by allocating lwlocks in dynamic shared memory) but not
-        * all of them, so we can't assume the tranche is registered here.
+        * It's an extension tranche, so look in LWLockTrancheNames[].  However,
+        * it's possible that the tranche has never been registered in the current
+        * process, in which case give up and return "extension".
         */
-       if (eventId >= LWLockTranchesAllocated ||
-               LWLockTrancheArray[eventId] == NULL)
+       trancheId -= LWTRANCHE_FIRST_USER_DEFINED;
+
+       if (trancheId >= LWLockTrancheNamesAllocated ||
+               LWLockTrancheNames[trancheId] == NULL)
                return "extension";
 
-       return LWLockTrancheArray[eventId];
+       return LWLockTrancheNames[trancheId];
+}
+
+/*
+ * Return an identifier for an LWLock based on the wait class and event.
+ */
+const char *
+GetLWLockIdentifier(uint32 classId, uint16 eventId)
+{
+       Assert(classId == PG_WAIT_LWLOCK);
+       /* The event IDs are just tranche numbers. */
+       return GetLWTrancheName(eventId);
 }
 
 /*