It just occurred to me that the Windows latch implementation goes
through a lot of trouble to dynamically assign the shared Windows event
handles to the latches in OwnLatch, but there's really no reason why
they can't be statically assigned in InitSharedLatch instead. We have to
allocate the same amount of event handles anyway.
That makes the implementation a lot simpler, eliminating the shared
memory block dedicated to latches altogether, and all the related
bookkeeping. We no longer need NumSharedLatches() function anymore
either, each InitSharedLatch call can allocate a new event handle directly.
It was the separation of InitSharedLatch and OwnLatch steps that made
this possible, which is probably why this didn't occur to me earlier.
Separating those steps was definitely a good move.
Unless someone sees a problem with this, I'll commit the attached patch
to do the simplification.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
diff --git a/src/backend/port/unix_latch.c b/src/backend/port/unix_latch.c
index 9b2a1f6..fa7f272 100644
--- a/src/backend/port/unix_latch.c
+++ b/src/backend/port/unix_latch.c
@@ -129,8 +129,10 @@ InitLatch(volatile Latch *latch)
* is initially owned by no-one, use OwnLatch to associate it with the
* current process.
*
- * NB: When you introduce a new shared latch, you must increase the shared
- * latch count in NumSharedLatches in win32_latch.c!
+ * InitSharedLatch needs to be called in postmaster before forking child
+ * processes, usually right after allocating the shared memory block
+ * containing the latch with ShmemInitStruct. The Unix implementation
+ * doesn't actually require that, but the Windows one does.
*/
void
InitSharedLatch(volatile Latch *latch)
@@ -323,29 +325,6 @@ ResetLatch(volatile Latch *latch)
}
/*
- * LatchShmemSize
- * Compute space needed for latch's shared memory
- *
- * Not needed for Unix implementation.
- */
-Size
-LatchShmemSize(void)
-{
- return 0;
-}
-
-/*
- * LatchShmemInit
- * Allocate and initialize shared memory needed for latches
- *
- * Not needed for Unix implementation.
- */
-void
-LatchShmemInit(void)
-{
-}
-
-/*
* SetLatch uses SIGUSR1 to wake up the process waiting on the latch. Wake
* up WaitLatch.
*/
diff --git a/src/backend/port/win32_latch.c b/src/backend/port/win32_latch.c
index da06202..2b41fa3 100644
--- a/src/backend/port/win32_latch.c
+++ b/src/backend/port/win32_latch.c
@@ -3,8 +3,10 @@
* win32_latch.c
* Windows implementation of latches.
*
- * The Windows implementation uses Windows events. See unix_latch.c for
- * information on usage.
+ * See unix_latch.c for information on usage.
+ *
+ * The Windows implementation uses Windows events that are inherited by
+ * all postmaster child processes.
*
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
@@ -24,83 +26,60 @@
#include "replication/walsender.h"
#include "storage/latch.h"
#include "storage/shmem.h"
-#include "storage/spin.h"
-
-/*
- * Shared latches are implemented with Windows events that are shared by
- * all postmaster child processes. At postmaster startup we create enough
- * Event objects, and mark them as inheritable so that they are accessible
- * in child processes. The handles are stored in sharedHandles.
- */
-typedef struct
-{
- slock_t mutex; /* protects all the other fields */
- int maxhandles; /* number of shared handles created */
- int nfreehandles; /* number of free handles in array */
- HANDLE handles[1]; /* free handles, variable length */
-} SharedEventHandles;
-
-static SharedEventHandles *sharedHandles;
void
InitLatch(volatile Latch *latch)
{
- latch->event = CreateEvent(NULL, TRUE, FALSE, NULL);
- latch->is_shared = false;
latch->is_set = false;
+ latch->owner_pid = MyProcPid;
+ latch->is_shared = false;
+
+ latch->event = CreateEvent(NULL, TRUE, FALSE, NULL);
+ if (latch->event == NULL)
+ elog(ERROR, "CreateEvent failed: error code %d", (int) GetLastError());
}
void
InitSharedLatch(volatile Latch *latch)
{
- latch->is_shared = true;
+ SECURITY_ATTRIBUTES sa;
+
latch->is_set = false;
- latch->event = NULL;
+ latch->owner_pid = 0;
+ latch->is_shared = true;
+
+ /*
+ * Set up security attributes to specify that the events are
+ * inherited.
+ */
+ ZeroMemory(&sa, sizeof(sa));
+ sa.nLength = sizeof(sa);
+ sa.bInheritHandle = TRUE;
+
+ latch->event = CreateEvent(&sa, TRUE, FALSE, NULL);
+ if (latch->event == NULL)
+ elog(ERROR, "CreateEvent failed: error code %d", (int) GetLastError());
}
void
OwnLatch(volatile Latch *latch)
{
- HANDLE event;
-
/* Sanity checks */
Assert(latch->is_shared);
- if (latch->event != 0)
+ if (latch->owner_pid != 0)
elog(ERROR, "latch already owned");
- /* Reserve an event handle from the shared handles array */
- SpinLockAcquire(&sharedHandles->mutex);
- if (sharedHandles->nfreehandles <= 0)
- {
- SpinLockRelease(&sharedHandles->mutex);
- elog(ERROR, "out of shared event objects");
- }
- sharedHandles->nfreehandles--;
- event = sharedHandles->handles[sharedHandles->nfreehandles];
- SpinLockRelease(&sharedHandles->mutex);
-
- latch->event = event;
+ latch->owner_pid = MyProcPid;
}
void
DisownLatch(volatile Latch *latch)
{
Assert(latch->is_shared);
- Assert(latch->event != NULL);
-
- /* Put the event handle back to the pool */
- SpinLockAcquire(&sharedHandles->mutex);
- if (sharedHandles->nfreehandles >= sharedHandles->maxhandles)
- {
- SpinLockRelease(&sharedHandles->mutex);
- elog(PANIC, "too many free event handles");
- }
- sharedHandles->handles[sharedHandles->nfreehandles] = latch->event;
- sharedHandles->nfreehandles++;
- SpinLockRelease(&sharedHandles->mutex);
+ Assert(latch->owner_pid == MyProcPid);
- latch->event = NULL;
+ latch->owner_pid = 0;
}
bool
@@ -217,68 +196,3 @@ ResetLatch(volatile Latch *latch)
{
latch->is_set = false;
}
-
-/*
- * Number of shared latches, used to allocate the right number of shared
- * Event handles at postmaster startup. You must update this if you
- * introduce a new shared latch!
- */
-static int
-NumSharedLatches(void)
-{
- int numLatches = 0;
-
- /* Each walsender needs one latch */
- numLatches += max_wal_senders;
-
- return numLatches;
-}
-
-/*
- * LatchShmemSize
- * Compute space needed for latch's shared memory
- */
-Size
-LatchShmemSize(void)
-{
- return offsetof(SharedEventHandles, handles) +
- NumSharedLatches() * sizeof(HANDLE);
-}
-
-/*
- * LatchShmemInit
- * Allocate and initialize shared memory needed for latches
- */
-void
-LatchShmemInit(void)
-{
- Size size = LatchShmemSize();
- bool found;
-
- sharedHandles = ShmemInitStruct("SharedEventHandles", size, &found);
-
- /* If we're first, initialize the struct and allocate handles */
- if (!found)
- {
- int i;
- SECURITY_ATTRIBUTES sa;
-
- /*
- * Set up security attributes to specify that the events are
- * inherited.
- */
- ZeroMemory(&sa, sizeof(sa));
- sa.nLength = sizeof(sa);
- sa.bInheritHandle = TRUE;
-
- SpinLockInit(&sharedHandles->mutex);
- sharedHandles->maxhandles = NumSharedLatches();
- sharedHandles->nfreehandles = sharedHandles->maxhandles;
- for (i = 0; i < sharedHandles->maxhandles; i++)
- {
- sharedHandles->handles[i] = CreateEvent(&sa, TRUE, FALSE, NULL);
- if (sharedHandles->handles[i] == NULL)
- elog(ERROR, "CreateEvent failed: error code %d", (int) GetLastError());
- }
- }
-}
diff --git a/src/backend/storage/ipc/ipci.c b/src/backend/storage/ipc/ipci.c
index 0083513..492ac9a 100644
--- a/src/backend/storage/ipc/ipci.c
+++ b/src/backend/storage/ipc/ipci.c
@@ -30,7 +30,6 @@
#include "replication/walsender.h"
#include "storage/bufmgr.h"
#include "storage/ipc.h"
-#include "storage/latch.h"
#include "storage/pg_shmem.h"
#include "storage/pmsignal.h"
#include "storage/procarray.h"
@@ -118,7 +117,6 @@ CreateSharedMemoryAndSemaphores(bool makePrivate, int port)
size = add_size(size, SInvalShmemSize());
size = add_size(size, PMSignalShmemSize());
size = add_size(size, ProcSignalShmemSize());
- size = add_size(size, LatchShmemSize());
size = add_size(size, BgWriterShmemSize());
size = add_size(size, AutoVacuumShmemSize());
size = add_size(size, WalSndShmemSize());
@@ -219,7 +217,6 @@ CreateSharedMemoryAndSemaphores(bool makePrivate, int port)
*/
PMSignalShmemInit();
ProcSignalShmemInit();
- LatchShmemInit();
BgWriterShmemInit();
AutoVacuumShmemInit();
WalSndShmemInit();
diff --git a/src/include/storage/latch.h b/src/include/storage/latch.h
index 221cc93..fac79f8 100644
--- a/src/include/storage/latch.h
+++ b/src/include/storage/latch.h
@@ -25,9 +25,8 @@ typedef struct
{
sig_atomic_t is_set;
bool is_shared;
-#ifndef WIN32
int owner_pid;
-#else
+#ifdef WIN32
HANDLE event;
#endif
} Latch;
@@ -46,9 +45,6 @@ extern void SetLatch(volatile Latch *latch);
extern void ResetLatch(volatile Latch *latch);
#define TestLatch(latch) (((volatile Latch *) latch)->is_set)
-extern Size LatchShmemSize(void);
-extern void LatchShmemInit(void);
-
/*
* Unix implementation uses SIGUSR1 for inter-process signaling, Win32 doesn't
* need this.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
