After working with my two proposed worker MPM models, I've become more
confident in the simple model. I'll continue benchmarking both designs,
but I wanted to get this one out to fix what's in CVS right now, and
so I can provide some more tweaks I've been working on (turn the LIFO
queue to a FIFO, fix the scoreboard states, setconcurrency, etc...).
Again, I've tested this patch extensively on solaris and linux with
favorable results.
Patch description:
- Don't reuse transaction pools, instead create one for each new
connection request. This seems to incur less overhead than trying
to shuffle the pools around for reuse.
- Get rid of one of the fd_queue condition variables. We don't need
to wait on the queue if it's full, that means the caller didn't
allocate enough entries in the queue. This relieves some overhead
from signal/condition management.
-aaron
Index: server/mpm/worker/worker.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/server/mpm/worker/worker.c,v
retrieving revision 1.21
diff -u -r1.21 worker.c
--- server/mpm/worker/worker.c 2001/08/30 20:50:06 1.21
+++ server/mpm/worker/worker.c 2001/09/17 02:46:01
@@ -123,7 +123,6 @@
static int num_listensocks = 0;
static apr_socket_t **listensocks;
static fd_queue_t *worker_queue;
-static fd_queue_t *pool_queue; /* a resource pool of context pools */
/* The structure used to pass unique initialization info to each thread */
typedef struct {
@@ -204,7 +203,6 @@
/* XXX: This will happen naturally on a graceful, and we don't care otherwise.
ap_queue_signal_all_wakeup(worker_queue); */
ap_queue_interrupt_all(worker_queue);
- ap_queue_interrupt_all(pool_queue);
}
AP_DECLARE(apr_status_t) ap_mpm_query(int query_code, int *result)
@@ -558,7 +556,6 @@
int thread_slot = ti->tid;
apr_pool_t *tpool = apr_thread_pool_get(thd);
apr_socket_t *csd = NULL;
- apr_socket_t *dummycsd = NULL;
apr_pool_t *ptrans; /* Pool for per-transaction stuff */
apr_socket_t *sd = NULL;
int n;
@@ -644,20 +641,9 @@
}
got_fd:
if (!workers_may_exit) {
+ /* create a new transaction pool for each accepted socket */
+ apr_pool_create(&ptrans, tpool);
- /* pull the next available transaction pool from the queue */
- if ((rv = ap_queue_pop(pool_queue, &dummycsd, &ptrans))
- != FD_QUEUE_SUCCESS) {
- if (rv == FD_QUEUE_EINTR) {
- goto got_fd;
- }
- else { /* got some error in the queue */
- csd = NULL;
- ap_log_error(APLOG_MARK, APLOG_ERR, rv, ap_server_conf,
- "ap_queue_pop");
- }
- }
-
if ((rv = apr_accept(&csd, sd, ptrans)) != APR_SUCCESS) {
csd = NULL;
ap_log_error(APLOG_MARK, APLOG_ERR, rv, ap_server_conf,
@@ -692,7 +678,9 @@
ap_scoreboard_image->parent[process_slot].quiescing = 1;
kill(ap_my_pid, SIGTERM);
+/* Unsure if this can be safely uncommented. -aaron
apr_thread_exit(thd, APR_SUCCESS);
+*/
return NULL;
}
@@ -718,12 +706,7 @@
}
process_socket(ptrans, csd, process_slot, thread_slot);
requests_this_child--; /* FIXME: should be synchronized - aaron */
-
- /* get this transaction pool ready for the next request */
- apr_pool_clear(ptrans);
- /* don't bother checking if we were interrupted in ap_queue_push,
- * because we're going to check workers_may_exit right now anyway. */
- ap_queue_push(pool_queue, NULL, ptrans);
+ apr_pool_destroy(ptrans);
}
ap_update_child_status(process_slot, thread_slot,
@@ -758,23 +741,11 @@
int i = 0;
int threads_created = 0;
apr_thread_t *listener;
- apr_pool_t *ptrans;
- apr_socket_t *dummycsd = NULL;
/* We must create the fd queues before we start up the listener
* and worker threads. */
- worker_queue = apr_pcalloc(pchild, sizeof(fd_queue_t));
+ worker_queue = apr_pcalloc(pchild, sizeof(*worker_queue));
ap_queue_init(worker_queue, ap_threads_per_child, pchild);
-
- /* create the resource pool of available transaction pools */
- pool_queue = apr_pcalloc(pchild, sizeof(fd_queue_t));
- ap_queue_init(pool_queue, ap_threads_per_child, pchild);
- /* fill the pool_queue with real pools */
- for (i = 0; i < ap_threads_per_child; i++) {
- ptrans = NULL;
- apr_pool_create(&ptrans, pchild);
- ap_queue_push(pool_queue, dummycsd, ptrans);
- }
my_info = (proc_info *)malloc(sizeof(proc_info));
my_info->pid = my_child_num;
Index: server/mpm/worker/fdqueue.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/server/mpm/worker/fdqueue.c,v
retrieving revision 1.5
diff -u -r1.5 fdqueue.c
--- server/mpm/worker/fdqueue.c 2001/08/27 23:50:12 1.5
+++ server/mpm/worker/fdqueue.c 2001/09/17 02:46:02
@@ -89,7 +89,6 @@
* XXX: We should at least try to signal an error here, it is
* indicative of a programmer error. -aaron */
pthread_cond_destroy(&queue->not_empty);
- pthread_cond_destroy(&queue->not_full);
pthread_mutex_destroy(&queue->one_big_mutex);
return FD_QUEUE_SUCCESS;
@@ -107,8 +106,6 @@
return FD_QUEUE_FAILURE;
if (pthread_cond_init(&queue->not_empty, NULL) != 0)
return FD_QUEUE_FAILURE;
- if (pthread_cond_init(&queue->not_full, NULL) != 0)
- return FD_QUEUE_FAILURE;
bounds = queue_capacity + 1;
queue->head = queue->tail = 0;
@@ -136,9 +133,13 @@
return FD_QUEUE_FAILURE;
}
- /* Keep waiting until we wake up and find that the queue is not full. */
- while (ap_queue_full(queue)) {
- pthread_cond_wait(&queue->not_full, &queue->one_big_mutex);
+ /* If the caller didn't allocate enough slots and tries to push
+ * too many, too bad. */
+ if (ap_queue_full(queue)) {
+ if (pthread_mutex_unlock(&queue->one_big_mutex) != 0) {
+ return FD_QUEUE_FAILURE;
+ }
+ return FD_QUEUE_OVERFLOW;
}
queue->data[queue->tail].sd = sd;
@@ -187,9 +188,6 @@
queue->head = (queue->head + 1) % queue->bounds;
}
queue->blanks++;
-
- /* we just consumed a slot, so we're no longer full */
- pthread_cond_signal(&queue->not_full);
if (pthread_mutex_unlock(&queue->one_big_mutex) != 0) {
return FD_QUEUE_FAILURE;
Index: server/mpm/worker/fdqueue.h
===================================================================
RCS file: /home/cvspublic/httpd-2.0/server/mpm/worker/fdqueue.h,v
retrieving revision 1.5
diff -u -r1.5 fdqueue.h
--- server/mpm/worker/fdqueue.h 2001/08/24 16:49:39 1.5
+++ server/mpm/worker/fdqueue.h 2001/09/17 02:46:02
@@ -72,6 +72,7 @@
#define FD_QUEUE_FAILURE -1 /* Needs to be an invalid file descriptor because
of queue_pop semantics */
#define FD_QUEUE_EINTR APR_EINTR
+#define FD_QUEUE_OVERFLOW -2
struct fd_queue_elem_t {
apr_socket_t *sd;
@@ -87,7 +88,6 @@
int blanks;
pthread_mutex_t one_big_mutex;
pthread_cond_t not_empty;
- pthread_cond_t not_full;
int cancel_state;
};
typedef struct fd_queue_t fd_queue_t;
