I'm posting this for comments before committing...
Here are some more changes the worker MPM code to reduce
the time spent in mutex-protected regions.
This patch takes advantage of a new property of the worker
design that resulted from Aaron's queue_info changes: the
number of connections in the queue can no longer exceed the
number of idle worker threads.
There are two changes in this patch:
* Eliminate the use of the queue->not_full condition
variable (because the idle thread count in queue_info
keeps the listener from being able to overflow the queue)
* Change the queue back to a stack. The reason for this
is that it reduces the cost of the push and pop operations
to a near-minimal level. I think it's now safe to use a
LIFO structure again, because we can depend on there being
enough idle threads to handle all the connections in the
fdqueue.
--Brian
Index: server/mpm/worker/fdqueue.h
===================================================================
RCS file: /home/cvs/httpd-2.0/server/mpm/worker/fdqueue.h,v
retrieving revision 1.18
diff -u -r1.18 fdqueue.h
--- server/mpm/worker/fdqueue.h 28 Apr 2002 05:28:18 -0000 1.18
+++ server/mpm/worker/fdqueue.h 28 Apr 2002 06:44:15 -0000
@@ -88,14 +88,11 @@
typedef struct fd_queue_elem_t fd_queue_elem_t;
struct fd_queue_t {
- int head;
- int tail;
fd_queue_elem_t *data;
int nelts;
int bounds;
apr_thread_mutex_t *one_big_mutex;
apr_thread_cond_t *not_empty;
- apr_thread_cond_t *not_full;
int terminated;
};
typedef struct fd_queue_t fd_queue_t;
Index: server/mpm/worker/fdqueue.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/mpm/worker/fdqueue.c,v
retrieving revision 1.20
diff -u -r1.20 fdqueue.c
--- server/mpm/worker/fdqueue.c 28 Apr 2002 05:28:18 -0000 1.20
+++ server/mpm/worker/fdqueue.c 28 Apr 2002 06:44:15 -0000
@@ -214,7 +214,6 @@
* XXX: We should at least try to signal an error here, it is
* indicative of a programmer error. -aaron */
apr_thread_cond_destroy(queue->not_empty);
- apr_thread_cond_destroy(queue->not_full);
apr_thread_mutex_destroy(queue->one_big_mutex);
return APR_SUCCESS;
@@ -235,11 +234,7 @@
if ((rv = apr_thread_cond_create(&queue->not_empty, a)) != APR_SUCCESS) {
return rv;
}
- if ((rv = apr_thread_cond_create(&queue->not_full, a)) != APR_SUCCESS) {
- return rv;
- }
- queue->head = queue->tail = 0;
queue->data = apr_palloc(a, queue_capacity * sizeof(fd_queue_elem_t));
queue->bounds = queue_capacity;
queue->nelts = 0;
@@ -268,13 +263,9 @@
}
AP_DEBUG_ASSERT(!queue->terminated);
-
- while (ap_queue_full(queue)) {
- apr_thread_cond_wait(queue->not_full, queue->one_big_mutex);
- }
+ AP_DEBUG_ASSERT(!ap_queue_full(queue));
- elem = &queue->data[queue->tail];
- queue->tail = (queue->tail + 1) % queue->bounds;
+ elem = &queue->data[queue->nelts];
elem->sd = sd;
elem->p = p;
queue->nelts++;
@@ -323,18 +314,11 @@
}
}
- elem = &queue->data[queue->head];
- queue->head = (queue->head + 1) % queue->bounds;
+ elem = &queue->data[--queue->nelts];
*sd = elem->sd;
*p = elem->p;
elem->sd = NULL;
elem->p = NULL;
- queue->nelts--;
-
- /* signal not_full if we were full before this pop */
- if (queue->nelts == queue->bounds - 1) {
- apr_thread_cond_signal(queue->not_full);
- }
rv = apr_thread_mutex_unlock(queue->one_big_mutex);
return rv;
@@ -348,9 +332,6 @@
return rv;
}
apr_thread_cond_broadcast(queue->not_empty);
- /* We shouldn't have multiple threads sitting in not_full, but
- * broadcast just in case. */
- apr_thread_cond_broadcast(queue->not_full);
if ((rv = apr_thread_mutex_unlock(queue->one_big_mutex)) != APR_SUCCESS) {
return rv;
}
