On 2014-02-03 11:22:45 -0500, Tom Lane wrote:
> Andres Freund <and...@2ndquadrant.com> writes:
> > On larger, multi-socket, machines, startup takes a fair bit of time. As
> > I was profiling anyway I looked into it and noticed that just about all
> > of it is spent in LWLockAssign() called by InitBufferPool(). Starting
> > with shared_buffers=48GB on the server Nate Boley provided, takes about
> > 12 seconds. Nearly all of it spent taking the ShmemLock spinlock.
> > Simply modifying LWLockAssign() to not take the spinlock when
> > !IsUnderPostmaster speeds it up to 2 seconds. While certainly not making
> > LWLockAssign() prettier it seems enough of a speedup to be worthwile
> > nonetheless.
> 
> Hm.  This patch only works if the postmaster itself never assigns any
> LWLocks except during startup.  That's *probably* all right, but it
> seems a bit scary.  Is there any cheap way to make the logic actually
> be what your comment claims, namely "Interlocking is not necessary during
> postmaster startup"?  I guess we could invent a ShmemInitInProgress global
> flag ...

So, here's a flag implementing things with that flag. I kept your name,
as it's more in line with ipci.c's naming, but it looks kinda odd
besides proc_exit_inprogress.

Greetings,

Andres Freund

-- 
 Andres Freund                     http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/src/backend/storage/ipc/ipci.c b/src/backend/storage/ipc/ipci.c
index c392d4f..ece9674 100644
--- a/src/backend/storage/ipc/ipci.c
+++ b/src/backend/storage/ipc/ipci.c
@@ -48,6 +48,11 @@ shmem_startup_hook_type shmem_startup_hook = NULL;
 static Size total_addin_request = 0;
 static bool addin_request_allowed = true;
 
+/*
+ * Signals whether shared memory is currently being initialized, while the
+ * postmaster hasn't forked any children yet.
+ */
+bool ShmemInitInProgress = false;
 
 /*
  * RequestAddinShmemSpace
@@ -97,6 +102,12 @@ CreateSharedMemoryAndSemaphores(bool makePrivate, int port)
 		int			numSemas;
 
 		/*
+		 * We're now initializing shared memory, and we won't have forked
+		 * children yet.
+		 */
+		ShmemInitInProgress = true;
+
+		/*
 		 * Size of the Postgres shared-memory block is estimated via
 		 * moderately-accurate estimates for the big hogs, plus 100K for the
 		 * stuff that's too small to bother with estimating.
@@ -261,4 +272,7 @@ CreateSharedMemoryAndSemaphores(bool makePrivate, int port)
 	 */
 	if (shmem_startup_hook)
 		shmem_startup_hook();
+
+	if(!IsUnderPostmaster)
+		ShmemInitInProgress = false;
 }
diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index 82ef440..0365f9b 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -355,9 +355,11 @@ CreateLWLocks(void)
  * LWLockAssign - assign a dynamically-allocated LWLock number
  *
  * We interlock this using the same spinlock that is used to protect
- * ShmemAlloc().  Interlocking is not really necessary during postmaster
- * startup, but it is needed if any user-defined code tries to allocate
- * LWLocks after startup.
+ * ShmemAlloc().  Interlocking is not necessary while we're initializing
+ * shared memory, but it is needed for any code allocating LWLocks after
+ * startup.  Since we allocate large amounts of LWLocks for the buffer pool
+ * during startup, we avoid taking the spinlock when not needed, as it has
+ * shown to slowdown startup considerably.
  */
 LWLock *
 LWLockAssign(void)
@@ -368,14 +370,17 @@ LWLockAssign(void)
 	volatile int *LWLockCounter;
 
 	LWLockCounter = (int *) ((char *) MainLWLockArray - 3 * sizeof(int));
-	SpinLockAcquire(ShmemLock);
+	if (!ShmemInitInProgress)
+		SpinLockAcquire(ShmemLock);
 	if (LWLockCounter[0] >= LWLockCounter[1])
 	{
-		SpinLockRelease(ShmemLock);
+		if (!ShmemInitInProgress)
+			SpinLockRelease(ShmemLock);
 		elog(ERROR, "no more LWLocks available");
 	}
 	result = &MainLWLockArray[LWLockCounter[0]++].lock;
-	SpinLockRelease(ShmemLock);
+	if (!ShmemInitInProgress)
+		SpinLockRelease(ShmemLock);
 	return result;
 }
 
diff --git a/src/include/storage/ipc.h b/src/include/storage/ipc.h
index 37eca7a..0f1e00a 100644
--- a/src/include/storage/ipc.h
+++ b/src/include/storage/ipc.h
@@ -63,6 +63,7 @@ typedef void (*shmem_startup_hook_type) (void);
 
 /* ipc.c */
 extern bool proc_exit_inprogress;
+extern bool ShmemInitInProgress;
 
 extern void proc_exit(int code) __attribute__((noreturn));
 extern void shmem_exit(int code);
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to