From: Brian Pane Date: Sun, 28 Apr 2002 05:28:18 +0000 (+0000) Subject: Moved the recycled pool list from the queue to the queue_info structure. X-Git-Tag: 2.0.36~55 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6625520bacdc1a956c5a29f02456060599e861f3;p=apache Moved the recycled pool list from the queue to the queue_info structure. The advantage of doing this is that it enables us to guarantee that the number of ptrans pools in existence at once is no greater than the number of worker threads, and that we'll never have to delete ptrans pools. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@94830 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index 0d5c1bee30..159ba3b0a6 100644 --- a/CHANGES +++ b/CHANGES @@ -1,5 +1,7 @@ Changes with Apache 2.0.37 + *) More efficient pool recycling logic for the worker MPM [Brian Pane] + *) Modify the worker MPM to not accept() new connections until there is an available worker thread. This prevents queued connections from starving for processing time while long-running diff --git a/server/mpm/worker/fdqueue.c b/server/mpm/worker/fdqueue.c index a7f9bc2857..362583986e 100644 --- a/server/mpm/worker/fdqueue.c +++ b/server/mpm/worker/fdqueue.c @@ -63,18 +63,25 @@ struct fd_queue_info_t { apr_thread_mutex_t *idlers_mutex; apr_thread_cond_t *wait_for_idler; int terminated; + int max_idlers; + apr_pool_t **recycled_pools; + int num_recycled; }; static apr_status_t queue_info_cleanup(void *data_) { fd_queue_info_t *qi = data_; + int i; apr_thread_cond_destroy(qi->wait_for_idler); apr_thread_mutex_destroy(qi->idlers_mutex); + for (i = 0; i < qi->num_recycled; i++) { + apr_pool_destroy(qi->recycled_pools[i]); + } return APR_SUCCESS; } apr_status_t ap_queue_info_create(fd_queue_info_t **queue_info, - apr_pool_t *pool) + apr_pool_t *pool, int max_idlers) { apr_status_t rv; fd_queue_info_t *qi; @@ -91,6 +98,10 @@ apr_status_t ap_queue_info_create(fd_queue_info_t **queue_info, if (rv != APR_SUCCESS) { return rv; } + qi->recycled_pools = (apr_pool_t **)apr_palloc(pool, max_idlers * + sizeof(apr_pool_t *)); + qi->num_recycled = 0; + qi->max_idlers = max_idlers; apr_pool_cleanup_register(pool, qi, queue_info_cleanup, apr_pool_cleanup_null); @@ -99,7 +110,8 @@ apr_status_t ap_queue_info_create(fd_queue_info_t **queue_info, return APR_SUCCESS; } -apr_status_t ap_queue_info_set_idle(fd_queue_info_t *queue_info) +apr_status_t ap_queue_info_set_idle(fd_queue_info_t *queue_info, + apr_pool_t *pool_to_recycle) { apr_status_t rv; rv = apr_thread_mutex_lock(queue_info->idlers_mutex); @@ -107,6 +119,11 @@ apr_status_t ap_queue_info_set_idle(fd_queue_info_t *queue_info) return rv; } AP_DEBUG_ASSERT(queue_info->idlers >= 0); + AP_DEBUG_ASSERT(queue_info->num_recycled < queue_info->max_idlers); + if (pool_to_recycle) { + queue_info->recycled_pools[queue_info->num_recycled++] = + pool_to_recycle; + } if (queue_info->idlers++ == 0) { /* Only signal if we had no idlers before. */ apr_thread_cond_signal(queue_info->wait_for_idler); @@ -118,9 +135,11 @@ apr_status_t ap_queue_info_set_idle(fd_queue_info_t *queue_info) return APR_SUCCESS; } -apr_status_t ap_queue_info_wait_for_idler(fd_queue_info_t *queue_info) +apr_status_t ap_queue_info_wait_for_idler(fd_queue_info_t *queue_info, + apr_pool_t **recycled_pool) { apr_status_t rv; + *recycled_pool = NULL; rv = apr_thread_mutex_lock(queue_info->idlers_mutex); if (rv != APR_SUCCESS) { return rv; @@ -139,6 +158,10 @@ apr_status_t ap_queue_info_wait_for_idler(fd_queue_info_t *queue_info) } } queue_info->idlers--; /* Oh, and idler? Let's take 'em! */ + if (queue_info->num_recycled) { + *recycled_pool = + queue_info->recycled_pools[--queue_info->num_recycled]; + } rv = apr_thread_mutex_unlock(queue_info->idlers_mutex); if (rv != APR_SUCCESS) { return rv; @@ -225,10 +248,6 @@ apr_status_t ap_queue_init(fd_queue_t *queue, int queue_capacity, apr_pool_t *a) for (i = 0; i < queue_capacity; ++i) queue->data[i].sd = NULL; - queue->recycled_pools = apr_palloc(a, - queue_capacity * sizeof(apr_pool_t *)); - queue->num_recycled = 0; - apr_pool_cleanup_register(a, queue, ap_queue_destroy, apr_pool_cleanup_null); return APR_SUCCESS; @@ -239,13 +258,11 @@ apr_status_t ap_queue_init(fd_queue_t *queue, int queue_capacity, apr_pool_t *a) * the push operation has completed, it signals other threads waiting * in apr_queue_pop() that they may continue consuming sockets. */ -apr_status_t ap_queue_push(fd_queue_t *queue, apr_socket_t *sd, apr_pool_t *p, - apr_pool_t **recycled_pool) +apr_status_t ap_queue_push(fd_queue_t *queue, apr_socket_t *sd, apr_pool_t *p) { fd_queue_elem_t *elem; apr_status_t rv; - *recycled_pool = NULL; if ((rv = apr_thread_mutex_lock(queue->one_big_mutex)) != APR_SUCCESS) { return rv; } @@ -262,10 +279,6 @@ apr_status_t ap_queue_push(fd_queue_t *queue, apr_socket_t *sd, apr_pool_t *p, elem->p = p; queue->nelts++; - if (queue->num_recycled != 0) { - *recycled_pool = queue->recycled_pools[--queue->num_recycled]; - } - apr_thread_cond_signal(queue->not_empty); if ((rv = apr_thread_mutex_unlock(queue->one_big_mutex)) != APR_SUCCESS) { @@ -281,29 +294,15 @@ apr_status_t ap_queue_push(fd_queue_t *queue, apr_socket_t *sd, apr_pool_t *p, * Once retrieved, the socket is placed into the address specified by * 'sd'. */ -apr_status_t ap_queue_pop(fd_queue_t *queue, apr_socket_t **sd, apr_pool_t **p, - apr_pool_t *recycled_pool) +apr_status_t ap_queue_pop(fd_queue_t *queue, apr_socket_t **sd, apr_pool_t **p) { fd_queue_elem_t *elem; apr_status_t rv; - int delete_pool = 0; if ((rv = apr_thread_mutex_lock(queue->one_big_mutex)) != APR_SUCCESS) { - if (recycled_pool) { - apr_pool_destroy(recycled_pool); - } return rv; } - if (recycled_pool) { - if (queue->num_recycled < queue->bounds) { - queue->recycled_pools[queue->num_recycled++] = recycled_pool; - } - else { - delete_pool = 1; - } - } - /* Keep waiting until we wake up and find that the queue is not empty. */ if (ap_queue_empty(queue)) { if (!queue->terminated) { @@ -312,9 +311,6 @@ apr_status_t ap_queue_pop(fd_queue_t *queue, apr_socket_t **sd, apr_pool_t **p, /* If we wake up and it's still empty, then we were interrupted */ if (ap_queue_empty(queue)) { rv = apr_thread_mutex_unlock(queue->one_big_mutex); - if (delete_pool) { - apr_pool_destroy(recycled_pool); - } if (rv != APR_SUCCESS) { return rv; } @@ -341,9 +337,6 @@ apr_status_t ap_queue_pop(fd_queue_t *queue, apr_socket_t **sd, apr_pool_t **p, } rv = apr_thread_mutex_unlock(queue->one_big_mutex); - if (delete_pool) { - apr_pool_destroy(recycled_pool); - } return rv; } diff --git a/server/mpm/worker/fdqueue.h b/server/mpm/worker/fdqueue.h index c7d5cab1c7..4e72358188 100644 --- a/server/mpm/worker/fdqueue.h +++ b/server/mpm/worker/fdqueue.h @@ -74,9 +74,11 @@ typedef struct fd_queue_info_t fd_queue_info_t; apr_status_t ap_queue_info_create(fd_queue_info_t **queue_info, - apr_pool_t *pool); -apr_status_t ap_queue_info_set_idle(fd_queue_info_t *queue_info); -apr_status_t ap_queue_info_wait_for_idler(fd_queue_info_t *queue_info); + apr_pool_t *pool, int max_idlers); +apr_status_t ap_queue_info_set_idle(fd_queue_info_t *queue_info, + apr_pool_t *pool_to_recycle); +apr_status_t ap_queue_info_wait_for_idler(fd_queue_info_t *queue_info, + apr_pool_t **recycled_pool); apr_status_t ap_queue_info_term(fd_queue_info_t *queue_info); struct fd_queue_elem_t { @@ -94,17 +96,13 @@ struct fd_queue_t { apr_thread_mutex_t *one_big_mutex; apr_thread_cond_t *not_empty; apr_thread_cond_t *not_full; - apr_pool_t **recycled_pools; - int num_recycled; int terminated; }; typedef struct fd_queue_t fd_queue_t; apr_status_t ap_queue_init(fd_queue_t *queue, int queue_capacity, apr_pool_t *a); -apr_status_t ap_queue_push(fd_queue_t *queue, apr_socket_t *sd, apr_pool_t *p, - apr_pool_t **recycled_pool); -apr_status_t ap_queue_pop(fd_queue_t *queue, apr_socket_t **sd, apr_pool_t **p, - apr_pool_t *recycled_pool); +apr_status_t ap_queue_push(fd_queue_t *queue, apr_socket_t *sd, apr_pool_t *p); +apr_status_t ap_queue_pop(fd_queue_t *queue, apr_socket_t **sd, apr_pool_t **p); apr_status_t ap_queue_interrupt_all(fd_queue_t *queue); apr_status_t ap_queue_term(fd_queue_t *queue); diff --git a/server/mpm/worker/worker.c b/server/mpm/worker/worker.c index 378d8a56da..c6111616db 100644 --- a/server/mpm/worker/worker.c +++ b/server/mpm/worker/worker.c @@ -695,7 +695,8 @@ static void *listener_thread(apr_thread_t *thd, void * dummy) } if (listener_may_exit) break; - rv = ap_queue_info_wait_for_idler(worker_queue_info); + rv = ap_queue_info_wait_for_idler(worker_queue_info, + &recycled_pool); if (APR_STATUS_IS_EOF(rv)) { break; /* we've been signaled to die now */ } @@ -807,8 +808,7 @@ static void *listener_thread(apr_thread_t *thd, void * dummy) signal_threads(ST_GRACEFUL); } if (csd != NULL) { - rv = ap_queue_push(worker_queue, csd, ptrans, - &recycled_pool); + rv = ap_queue_push(worker_queue, csd, ptrans); if (rv) { /* trash the connection; we couldn't queue the connected * socket to a worker @@ -867,7 +867,8 @@ static void * APR_THREAD_FUNC worker_thread(apr_thread_t *thd, void * dummy) bucket_alloc = apr_bucket_alloc_create(apr_thread_pool_get(thd)); while (!workers_may_exit) { - rv = ap_queue_info_set_idle(worker_queue_info); + rv = ap_queue_info_set_idle(worker_queue_info, last_ptrans); + last_ptrans = NULL; if (rv != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_EMERG, rv, ap_server_conf, "ap_queue_info_set_idle failed. Attempting to " @@ -877,8 +878,7 @@ static void * APR_THREAD_FUNC worker_thread(apr_thread_t *thd, void * dummy) } ap_update_child_status_from_indexes(process_slot, thread_slot, SERVER_READY, NULL); - rv = ap_queue_pop(worker_queue, &csd, &ptrans, last_ptrans); - last_ptrans = NULL; + rv = ap_queue_pop(worker_queue, &csd, &ptrans); if (rv != APR_SUCCESS) { /* We get APR_EOF during a graceful shutdown once all the connections @@ -986,7 +986,8 @@ static void * APR_THREAD_FUNC start_threads(apr_thread_t *thd, void *dummy) clean_child_exit(APEXIT_CHILDFATAL); } - rv = ap_queue_info_create(&worker_queue_info, pchild); + rv = ap_queue_info_create(&worker_queue_info, pchild, + ap_threads_per_child); if (rv != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_ALERT, rv, ap_server_conf, "ap_queue_info_create() failed");