Fix a performance problem with the worker MPM. We now create

transaction pools once, and re-use them for each connection.

Submitted by:	Aaron Bannert <aaron@clove.org>

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@90771 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index f33e7a1f234fd315e93778c1799b852b2eb0af15..5f5c03732aaa4f2b049ba47d234f6b0a336cce27 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,5 +1,9 @@
 Changes with Apache 2.0.25-dev
 
+  *) Fix a performance problem with the worker MPM.  We now create
+     transaction pools once, and re-use them for each connection.
+     [Aaron Bannert <aaron@clove.org>]
+
   *) Modfied mod_mime to prevent mod_negotation from serving a multiview
      of a 'handler' or 'filter', so that any filename extension that does 
      not contribute to the negotiated metadata can't be served without

diff --git a/server/mpm/worker/worker.c b/server/mpm/worker/worker.c
index fad12dcf45e05e582a1aeb2e2908def53b7ffb23..268a1d713d82a7bc9c6be69c89b89a72c7fc035b 100644 (file)
--- a/server/mpm/worker/worker.c
+++ b/server/mpm/worker/worker.c
@@ -125,6 +125,7 @@ static int requests_this_child;
 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 {
@@ -203,6 +204,7 @@ static void signal_workers(void)
     /* 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)
@@ -556,6 +558,7 @@ static void *listener_thread(apr_thread_t *thd, void * dummy)
     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;
@@ -641,8 +644,19 @@ static void *listener_thread(apr_thread_t *thd, void * dummy)
         }
     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;
@@ -678,9 +692,7 @@ static void *listener_thread(apr_thread_t *thd, void * dummy)
     ap_scoreboard_image->parent[process_slot].quiescing = 1;
     kill(ap_my_pid, SIGTERM);
 
-/* this is uncommented when we make a pool-pool
     apr_thread_exit(thd, APR_SUCCESS);
-*/
     return NULL;
 }
 
@@ -695,8 +707,6 @@ static void *worker_thread(apr_thread_t *thd, void * dummy)
 
     free(ti);
 
-    /* apr_pool_create(&ptrans, tpool); */
-
     while (!workers_may_exit) {
         rv = ap_queue_pop(worker_queue, &csd, &ptrans);
         /* We get FD_QUEUE_EINTR whenever ap_queue_pop() has been interrupted
@@ -708,11 +718,16 @@ static void *worker_thread(apr_thread_t *thd, void * dummy)
         }
         process_socket(ptrans, csd, process_slot, thread_slot);
         requests_this_child--; /* FIXME: should be synchronized - aaron */
-        apr_pool_destroy(ptrans);
+
+        /* 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);
     }
 
-    ap_update_child_status(process_slot, thread_slot, (dying) ? SERVER_DEAD : SERVER_GRACEFUL,
-        (request_rec *) NULL);
+    ap_update_child_status(process_slot, thread_slot,
+        (dying) ? SERVER_DEAD : SERVER_GRACEFUL, (request_rec *) NULL);
     apr_lock_acquire(worker_thread_count_mutex);
     worker_thread_count--;
     apr_lock_release(worker_thread_count_mutex);
@@ -731,7 +746,7 @@ static int check_signal(int signum)
     return 0;
 }
 
-static void *start_threads(apr_thread_t *thd, void * dummy)
+static void *start_threads(apr_thread_t *thd, void *dummy)
 {
     thread_starter *ts = dummy;
     apr_thread_t **threads = ts->threads;
@@ -743,12 +758,24 @@ static void *start_threads(apr_thread_t *thd, void * dummy)
     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 queue before we start up the listener
+    /* We must create the fd queues before we start up the listener
      * and worker threads. */
-    worker_queue = apr_pcalloc(pchild, sizeof(*worker_queue));
+    worker_queue = apr_pcalloc(pchild, sizeof(fd_queue_t));
     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;
     my_info->tid = i;
@@ -838,7 +865,7 @@ static void child_main(int child_num_arg)
 
     ap_run_child_init(pchild, ap_server_conf);
 
-    /*done with init critical section */
+    /* done with init critical section */
 
     rv = apr_setup_signal_thread();
     if (rv != APR_SUCCESS) {