On Tue, Jul 28, 2015 at 3:28 PM, Heikki Linnakangas <hlinn...@iki.fi> wrote: > * The patch requires that the LWLOCK_INDIVIDUAL_NAMES array is kept in sync > with the list of individual locks in lwlock.h. Sooner or later someone will > add an LWLock and forget to update the names-array. That needs to be made > less error-prone, so that the names are maintained in the same place as the > #defines. Perhaps something like rmgrlist.h.
This is a good idea, but it's not easy to do in the style of rmgrlist.h, because I don't believe there's any way to define a macro that expands to a preprocessor directive. Attached is a patch that instead generates the list of macros from a text file, and also generates an array inside lwlock.c with the lock names that gets used by the Trace_lwlocks stuff where applicable. Any objections to this solution to the problem? If not, I'd like to go ahead and push this much. I can't test the Windows changes locally, though, so it would be helpful if someone could check that out. > * Instead of having LWLOCK_INDIVIDUAL_NAMES to name "individual" locks, how > about just giving each one of them a separate tranche? I don't think it's good to split things up to that degree; standardizing on one name per fixed lwlock and one per tranche otherwise seems like a good compromise to me. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
diff --git a/src/backend/Makefile b/src/backend/Makefile index 98b978f..d16ab10 100644 --- a/src/backend/Makefile +++ b/src/backend/Makefile @@ -106,7 +106,7 @@ endif endif # aix # Update the commonly used headers before building the subdirectories -$(SUBDIRS:%=%-recursive): $(top_builddir)/src/include/parser/gram.h $(top_builddir)/src/include/catalog/schemapg.h $(top_builddir)/src/include/utils/fmgroids.h $(top_builddir)/src/include/utils/errcodes.h $(top_builddir)/src/include/utils/probes.h +$(SUBDIRS:%=%-recursive): $(top_builddir)/src/include/parser/gram.h $(top_builddir)/src/include/catalog/schemapg.h $(top_builddir)/src/include/storage/lwlocknames.h $(top_builddir)/src/include/utils/fmgroids.h $(top_builddir)/src/include/utils/errcodes.h $(top_builddir)/src/include/utils/probes.h # run this unconditionally to avoid needing to know its dependencies here: submake-schemapg: @@ -135,6 +135,9 @@ postgres.o: $(OBJS) parser/gram.h: parser/gram.y $(MAKE) -C parser gram.h +storage/lmgr/lwlocknames.h: storage/lmgr/generate-lwlocknames.pl storage/lmgr/lwlocknames.txt + $(MAKE) -C storage/lmgr lwlocknames.h + utils/fmgroids.h: utils/Gen_fmgrtab.pl catalog/Catalog.pm $(top_srcdir)/src/include/catalog/pg_proc.h $(MAKE) -C utils fmgroids.h @@ -165,6 +168,11 @@ $(top_builddir)/src/include/catalog/schemapg.h: catalog/schemapg.h cd '$(dir $@)' && rm -f $(notdir $@) && \ $(LN_S) "$$prereqdir/$(notdir $<)" . +$(top_builddir)/src/include/storage/lwlocknames.h: storage/lmgr/lwlocknames.h + prereqdir=`cd '$(dir $<)' >/dev/null && pwd` && \ + cd '$(dir $@)' && rm -f $(notdir $@) && \ + $(LN_S) "$$prereqdir/$(notdir $<)" . + $(top_builddir)/src/include/utils/errcodes.h: utils/errcodes.h prereqdir=`cd '$(dir $<)' >/dev/null && pwd` && \ cd '$(dir $@)' && rm -f $(notdir $@) && \ @@ -192,6 +200,7 @@ distprep: $(MAKE) -C bootstrap bootparse.c bootscanner.c $(MAKE) -C catalog schemapg.h postgres.bki postgres.description postgres.shdescription $(MAKE) -C replication repl_gram.c repl_scanner.c + $(MAKE) -C storage lwlocknames.h $(MAKE) -C utils fmgrtab.c fmgroids.h errcodes.h $(MAKE) -C utils/misc guc-file.c $(MAKE) -C utils/sort qsort_tuple.c @@ -307,6 +316,7 @@ maintainer-clean: distclean catalog/postgres.shdescription \ replication/repl_gram.c \ replication/repl_scanner.c \ + storage/lmgr/lwlocknames.h \ utils/fmgroids.h \ utils/fmgrtab.c \ utils/errcodes.h \ diff --git a/src/backend/storage/lmgr/.gitignore b/src/backend/storage/lmgr/.gitignore new file mode 100644 index 0000000..9355cae --- /dev/null +++ b/src/backend/storage/lmgr/.gitignore @@ -0,0 +1,2 @@ +/lwlocknames.c +/lwlocknames.h diff --git a/src/backend/storage/lmgr/Makefile b/src/backend/storage/lmgr/Makefile index e12a854..e941f2d 100644 --- a/src/backend/storage/lmgr/Makefile +++ b/src/backend/storage/lmgr/Makefile @@ -24,8 +24,17 @@ s_lock_test: s_lock.c $(top_builddir)/src/port/libpgport.a $(CC) $(CPPFLAGS) $(CFLAGS) -DS_LOCK_TEST=1 $(srcdir)/s_lock.c \ $(TASPATH) -L $(top_builddir)/src/port -lpgport -o s_lock_test +# see explanation in ../../parser/Makefile +lwlocknames.c: lwlocknames.h ; + +lwlocknames.h: $(top_srcdir)/src/backend/storage/lmgr/lwlocknames.txt generate-lwlocknames.pl + $(PERL) $(srcdir)/generate-lwlocknames.pl $< + check: s_lock_test ./s_lock_test -clean distclean maintainer-clean: +clean distclean: rm -f s_lock_test + +maintainer-clean: clean + rm -f lwlocknames.h diff --git a/src/backend/storage/lmgr/generate-lwlocknames.pl b/src/backend/storage/lmgr/generate-lwlocknames.pl new file mode 100644 index 0000000..7ce4e2a --- /dev/null +++ b/src/backend/storage/lmgr/generate-lwlocknames.pl @@ -0,0 +1,62 @@ +#!/usr/bin/perl +# +# Generate lwlocknames.h and lwlocknames.c from lwlocknames.txt +# Copyright (c) 2000-2015, PostgreSQL Global Development Group + +use warnings; +use strict; + +print + "/* autogenerated from src/backend/storage/lmgr/lwlocknames.txt, do not edit */\n"; +print "/* there is deliberately not an #ifndef LWLOCKNAMES_H here */\n"; + +my $lastlockidx = -1; +my $continue = "\n"; + +open my $lwlocknames, $ARGV[0] or die; + +# Include PID in suffix in case parallel make runs this multiple times. +my $htmp = "lwlocknames.h.tmp$$"; +my $ctmp = "lwlocknames.c.tmp$$"; +open H, '>', $htmp or die "Could not open $htmp: $!"; +open C, '>', $ctmp or die "Could not open $ctmp: $!"; + +print C "static char *MainLWLockNames[] = {"; + +while (<$lwlocknames>) +{ + chomp; + + # Skip comments + next if /^#/; + next if /^\s*$/; + + die "unable to parse lwlocknames.txt" + unless /^(\w+)\s+(\d+)$/; + + (my $lockname, my $lockidx) = ($1, $2); + + die "lwlocknames.txt not in order" if $lockidx < $lastlockidx; + die "lwlocknames.txt has duplicates" if $lockidx == $lastlockidx; + + while ($lastlockidx < $lockidx - 1) + { + ++$lastlockidx; + printf C "%s \"<unassigned:%d>\"", $continue, $lastlockidx; + $continue = ",\n"; + } + printf C "%s \"%s\"", $continue, $lockname; + $lastlockidx = $lockidx; + $continue = ",\n"; + + print H "#define $lockname (&MainLWLockArray[$lockidx].lock)\n"; +} + +printf C "\n};\n"; +print H "\n"; +printf H "#define NUM_INDIVIDUAL_LWLOCKS %s\n", $lastlockidx + 1; + +rename($htmp, 'lwlocknames.h') || die "rename: $htmp: $!"; +rename($ctmp, 'lwlocknames.c') || die "rename: $ctmp: $!"; + +close $lwlocknames; diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index 687ed63..db10a96 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -95,6 +95,9 @@ #include "utils/hsearch.h" #endif +/* Constants for lwlock names */ +#include "lwlocknames.c" + /* We use the ShmemLock spinlock to protect LWLockAssign */ extern slock_t *ShmemLock; @@ -183,18 +186,32 @@ PRINT_LWDEBUG(const char *where, LWLock *lock, LWLockMode mode) if (Trace_lwlocks) { uint32 state = pg_atomic_read_u32(&lock->state); - - ereport(LOG, - (errhidestmt(true), - errhidecontext(true), - errmsg("%d: %s(%s %d): excl %u shared %u haswaiters %u waiters %u rOK %d", - MyProcPid, - where, T_NAME(lock), T_ID(lock), - !!(state & LW_VAL_EXCLUSIVE), - state & LW_SHARED_MASK, - !!(state & LW_FLAG_HAS_WAITERS), - pg_atomic_read_u32(&lock->nwaiters), - !!(state & LW_FLAG_RELEASE_OK)))); + int id = T_ID(lock); + + if (lock->tranche == 0 && id < NUM_INDIVIDUAL_LWLOCKS) + ereport(LOG, + (errhidestmt(true), + errhidecontext(true), + errmsg("%d: %s(%s): excl %u shared %u haswaiters %u waiters %u rOK %d", + MyProcPid, + where, MainLWLockNames[id], + !!(state & LW_VAL_EXCLUSIVE), + state & LW_SHARED_MASK, + !!(state & LW_FLAG_HAS_WAITERS), + pg_atomic_read_u32(&lock->nwaiters), + !!(state & LW_FLAG_RELEASE_OK)))); + else + ereport(LOG, + (errhidestmt(true), + errhidecontext(true), + errmsg("%d: %s(%s %d): excl %u shared %u haswaiters %u waiters %u rOK %d", + MyProcPid, + where, T_NAME(lock), id, + !!(state & LW_VAL_EXCLUSIVE), + state & LW_SHARED_MASK, + !!(state & LW_FLAG_HAS_WAITERS), + pg_atomic_read_u32(&lock->nwaiters), + !!(state & LW_FLAG_RELEASE_OK)))); } } @@ -204,11 +221,20 @@ LOG_LWDEBUG(const char *where, LWLock *lock, const char *msg) /* hide statement & context here, otherwise the log is just too verbose */ if (Trace_lwlocks) { - ereport(LOG, - (errhidestmt(true), - errhidecontext(true), - errmsg("%s(%s %d): %s", where, - T_NAME(lock), T_ID(lock), msg))); + int id = T_ID(lock); + + if (lock->tranche == 0 && id < NUM_INDIVIDUAL_LWLOCKS) + ereport(LOG, + (errhidestmt(true), + errhidecontext(true), + errmsg("%s(%s): %s", where, + MainLWLockNames[id], msg))); + else + ereport(LOG, + (errhidestmt(true), + errhidecontext(true), + errmsg("%s(%s %d): %s", where, + T_NAME(lock), id, msg))); } } diff --git a/src/backend/storage/lmgr/lwlocknames.txt b/src/backend/storage/lmgr/lwlocknames.txt new file mode 100644 index 0000000..96bbfe8 --- /dev/null +++ b/src/backend/storage/lmgr/lwlocknames.txt @@ -0,0 +1,47 @@ +# 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. + +# 0 is available; was formerly BufFreelistLock +ShmemIndexLock 1 +OidGenLock 2 +XidGenLock 3 +ProcArrayLock 4 +SInvalReadLock 5 +SInvalWriteLock 6 +WALBufMappingLock 7 +WALWriteLock 8 +ControlFileLock 9 +CheckpointLock 10 +CLogControlLock 11 +SubtransControlLock 12 +MultiXactGenLock 13 +MultiXactOffsetControlLock 14 +MultiXactMemberControlLock 15 +RelCacheInitLock 16 +CheckpointerCommLock 17 +TwoPhaseStateLock 18 +TablespaceCreateLock 19 +BtreeVacuumLock 20 +AddinShmemInitLock 21 +AutovacuumLock 22 +AutovacuumScheduleLock 23 +SyncScanLock 24 +RelationMappingLock 25 +AsyncCtlLock 26 +AsyncQueueLock 27 +SerializableXactHashLock 28 +SerializableFinishedListLock 29 +SerializablePredicateLockListLock 30 +OldSerXidLock 31 +SyncRepLock 32 +BackgroundWorkerLock 33 +DynamicSharedMemoryControlLock 34 +AutoFileLock 35 +ReplicationSlotAllocationLock 36 +ReplicationSlotControlLock 37 +CommitTsControlLock 38 +CommitTsLock 39 +ReplicationOriginLock 40 diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h index e278fa0..0c9b545 100644 --- a/src/include/pg_config_manual.h +++ b/src/include/pg_config_manual.h @@ -281,7 +281,7 @@ /* * Enable debugging print statements for lock-related operations. */ -/* #define LOCK_DEBUG */ +#define LOCK_DEBUG 1 /* * Enable debugging print statements for WAL-related operations; see diff --git a/src/include/storage/.gitignore b/src/include/storage/.gitignore new file mode 100644 index 0000000..209c8be --- /dev/null +++ b/src/include/storage/.gitignore @@ -0,0 +1 @@ +/lwlocknames.h diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h index cbd6318..ec3f350 100644 --- a/src/include/storage/lwlock.h +++ b/src/include/storage/lwlock.h @@ -87,56 +87,8 @@ typedef union LWLockPadded } LWLockPadded; extern PGDLLIMPORT LWLockPadded *MainLWLockArray; -/* - * Some commonly-used locks have predefined positions within MainLWLockArray; - * defining macros here makes it much easier to keep track of these. 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. - */ -/* 0 is available; was formerly BufFreelistLock */ -#define ShmemIndexLock (&MainLWLockArray[1].lock) -#define OidGenLock (&MainLWLockArray[2].lock) -#define XidGenLock (&MainLWLockArray[3].lock) -#define ProcArrayLock (&MainLWLockArray[4].lock) -#define SInvalReadLock (&MainLWLockArray[5].lock) -#define SInvalWriteLock (&MainLWLockArray[6].lock) -#define WALBufMappingLock (&MainLWLockArray[7].lock) -#define WALWriteLock (&MainLWLockArray[8].lock) -#define ControlFileLock (&MainLWLockArray[9].lock) -#define CheckpointLock (&MainLWLockArray[10].lock) -#define CLogControlLock (&MainLWLockArray[11].lock) -#define SubtransControlLock (&MainLWLockArray[12].lock) -#define MultiXactGenLock (&MainLWLockArray[13].lock) -#define MultiXactOffsetControlLock (&MainLWLockArray[14].lock) -#define MultiXactMemberControlLock (&MainLWLockArray[15].lock) -#define RelCacheInitLock (&MainLWLockArray[16].lock) -#define CheckpointerCommLock (&MainLWLockArray[17].lock) -#define TwoPhaseStateLock (&MainLWLockArray[18].lock) -#define TablespaceCreateLock (&MainLWLockArray[19].lock) -#define BtreeVacuumLock (&MainLWLockArray[20].lock) -#define AddinShmemInitLock (&MainLWLockArray[21].lock) -#define AutovacuumLock (&MainLWLockArray[22].lock) -#define AutovacuumScheduleLock (&MainLWLockArray[23].lock) -#define SyncScanLock (&MainLWLockArray[24].lock) -#define RelationMappingLock (&MainLWLockArray[25].lock) -#define AsyncCtlLock (&MainLWLockArray[26].lock) -#define AsyncQueueLock (&MainLWLockArray[27].lock) -#define SerializableXactHashLock (&MainLWLockArray[28].lock) -#define SerializableFinishedListLock (&MainLWLockArray[29].lock) -#define SerializablePredicateLockListLock (&MainLWLockArray[30].lock) -#define OldSerXidLock (&MainLWLockArray[31].lock) -#define SyncRepLock (&MainLWLockArray[32].lock) -#define BackgroundWorkerLock (&MainLWLockArray[33].lock) -#define DynamicSharedMemoryControlLock (&MainLWLockArray[34].lock) -#define AutoFileLock (&MainLWLockArray[35].lock) -#define ReplicationSlotAllocationLock (&MainLWLockArray[36].lock) -#define ReplicationSlotControlLock (&MainLWLockArray[37].lock) -#define CommitTsControlLock (&MainLWLockArray[38].lock) -#define CommitTsLock (&MainLWLockArray[39].lock) -#define ReplicationOriginLock (&MainLWLockArray[40].lock) - -#define NUM_INDIVIDUAL_LWLOCKS 41 +/* Names for fixed lwlocks */ +#include "lwlocknames.h" /* * It's a bit odd to declare NUM_BUFFER_PARTITIONS and NUM_LOCK_PARTITIONS diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm index 6b16e69..fe1f08d 100644 --- a/src/tools/msvc/Solution.pm +++ b/src/tools/msvc/Solution.pm @@ -285,6 +285,22 @@ s{PG_VERSION_STR "[^"]+"}{__STRINGIFY(x) #x\n#define __STRINGIFY2(z) __STRINGIFY 'src/include/utils/fmgroids.h'); } + if (IsNewer( + 'src/backend/storage/lmgr/lwlocknames.txt', 'src/include/storage/lwlocknames.h')) + { + print "Generating lwlocknames.c and fmgroids.h...\n"; + chdir('src/backend/storage/lmgr'); + system('perl generate-lwlocknames.pl lwlocknames.txt'); + chdir('../../../..'); + } + if (IsNewer( + 'src/include/storage/lwlocknames.h', + 'src/backend/storage/lmgr/fmgroids.h')) + { + copyFile('src/backend/storage/lmgr/lwlocknames.h', + 'src/include/storage/lwlocknames.h'); + } + if (IsNewer('src/include/utils/probes.h', 'src/backend/utils/probes.d')) { print "Generating probes.h...\n";
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers