This change may be controversial, so I'm soliciting
comments before committing...

The prep_walk_cache() function in server/request.c
is one of the slower parts of the core server.  About
60% of its run time is due to the apr_pool_userdata_*
functions.

The attached patch eliminates the use of the apr_pool_userdata_*
functions by adding hooks for the various walk caches to the
core_request_config structure (stored in r->request_config).

--Brian


Index: include/http_core.h
===================================================================
RCS file: /home/cvs/httpd-2.0/include/http_core.h,v
retrieving revision 1.55
diff -u -r1.55 http_core.h
--- include/http_core.h 2001/12/13 14:50:36     1.55
+++ include/http_core.h 2001/12/26 06:58:26
@@ -330,10 +330,21 @@
 
 /* Per-request configuration */
 
+typedef enum {
+    AP_WALK_DIRECTORY,
+    AP_WALK_LOCATION,
+    AP_WALK_FILE,
+    AP_NUM_WALK_CACHES
+} ap_walk_cache_type;
+
 typedef struct {
     /* bucket brigade used by getline for look-ahead and 
      * ap_get_client_block for holding left-over request body */
     struct apr_bucket_brigade *bb;
+
+    /* a place to hold per-request working data for
+     * ap_directory_walk, ap_location_walk, and ap_file_walk */
+    void *walk_cache[AP_NUM_WALK_CACHES];
 } core_request_config;
 
 /* Per-directory configuration */
Index: server/core.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/core.c,v
retrieving revision 1.121
diff -u -r1.121 core.c
--- server/core.c       2001/12/18 13:48:52     1.121
+++ server/core.c       2001/12/26 06:58:28
@@ -3399,17 +3399,18 @@
 
 static int core_create_req(request_rec *r)
 {
+    core_request_config *req_cfg;
+    req_cfg = apr_palloc(r->pool, sizeof(core_request_config));
+    memset(req_cfg, 0, sizeof(*req_cfg));
     if (r->main) {
-        ap_set_module_config(r->request_config, &core_module,
-              ap_get_module_config(r->main->request_config, &core_module));
+        core_request_config *main_req_cfg = (core_request_config *)
+            ap_get_module_config(r->main->request_config, &core_module);
+        req_cfg->bb = main_req_cfg->bb;
     }
     else {
-        core_request_config *req_cfg;
-
-        req_cfg = apr_pcalloc(r->pool, sizeof(core_request_config));
         req_cfg->bb = apr_brigade_create(r->pool);
-        ap_set_module_config(r->request_config, &core_module, req_cfg);
     }
+    ap_set_module_config(r->request_config, &core_module, req_cfg);
 
     ap_add_input_filter("NET_TIME", NULL, r, r->connection);
     
Index: server/request.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/request.c,v
retrieving revision 1.86
diff -u -r1.86 request.c
--- server/request.c    2001/12/14 03:30:23     1.86
+++ server/request.c    2001/12/26 06:58:29
@@ -296,9 +296,10 @@
     apr_array_header_t *walked;         /* The list of walk_walked_t results */
 } walk_cache_t;
 
-static walk_cache_t *prep_walk_cache(const char *cache_name, request_rec *r)
+static walk_cache_t *prep_walk_cache(ap_walk_cache_type t, request_rec *r)
 {
     walk_cache_t *cache;
+    core_request_config *my_req_cfg;
 
     /* Find the most relevant, recent entry to work from.  That would be
      * this request (on the second call), or the parent request of a
@@ -306,25 +307,32 @@
      * this _walk()er with a copy it is allowed to munge.  If there is no
      * parent or prior cached request, then create a new walk cache.
      */
-    if ((apr_pool_userdata_get((void **)&cache, cache_name, r->pool)
-                != APR_SUCCESS)
-        || !cache) {
-        if ((r->main && (apr_pool_userdata_get((void **)&cache, 
-                                               cache_name,
-                                               r->main->pool)
-                         == APR_SUCCESS) && cache)
-         || (r->prev && (apr_pool_userdata_get((void **)&cache, 
-                                               cache_name,
-                                               r->prev->pool)
-                         == APR_SUCCESS) && cache)) {
-            cache = apr_pmemdup(r->pool, cache, sizeof(*cache));
+    my_req_cfg = (core_request_config *)
+        ap_get_module_config(r->request_config, &core_module);
+
+    if (!my_req_cfg || !(cache = my_req_cfg->walk_cache[t])) {
+        core_request_config *req_cfg;
+        if ((r->main &&
+             (req_cfg = (core_request_config *)
+              ap_get_module_config(r->main->request_config, &core_module)) &&
+             req_cfg->walk_cache[t]) ||
+            (r->prev &&
+             (req_cfg = (core_request_config *)
+              ap_get_module_config(r->prev->request_config, &core_module)) &&
+             req_cfg->walk_cache[t])) {
+            cache = apr_pmemdup(r->pool, req_cfg->walk_cache[t],
+                                sizeof(*cache));
             cache->walked = apr_array_copy(r->pool, cache->walked);
         }
         else {
             cache = apr_pcalloc(r->pool, sizeof(*cache));
             cache->walked = apr_array_make(r->pool, 4, sizeof(walk_walked_t));
+        }
+        if (!my_req_cfg) {
+            my_req_cfg = apr_pcalloc(r->pool, sizeof(core_request_config));
+            ap_set_module_config(r->request_config, &core_module, my_req_cfg);
         }
-        apr_pool_userdata_setn(cache, cache_name, NULL, r->pool);
+        my_req_cfg->walk_cache[t] = cache;
     }
     return cache;
 }
@@ -483,7 +491,7 @@
      */
     r->filename = entry_dir;
 
-    cache = prep_walk_cache("ap_directory_walk::cache", r);
+    cache = prep_walk_cache(AP_WALK_DIRECTORY, r);
 
     /* If this is not a dirent subrequest with a preconstructed 
      * r->finfo value, then we can simply stat the filename to
@@ -1057,7 +1065,7 @@
     walk_cache_t *cache;
     const char *entry_uri;
 
-    cache = prep_walk_cache("ap_location_walk::cache", r);
+    cache = prep_walk_cache(AP_WALK_LOCATION, r);
     
     /* No tricks here, there are no <Locations > to parse in this vhost.
      * We won't destroy the cache, just in case _this_ redirect is later
@@ -1213,7 +1221,7 @@
         return OK;
     }
 
-    cache = prep_walk_cache("ap_file_walk::cache", r);
+    cache = prep_walk_cache(AP_WALK_FILE, r);
 
     /* No tricks here, there are just no <Files > to parse in this context.
      * We won't destroy the cache, just in case _this_ redirect is later

Reply via email to