You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by br...@apache.org on 2001/12/31 09:18:32 UTC

cvs commit: httpd-2.0/server core.c request.c

brianp      01/12/31 00:18:32

  Modified:    include  http_core.h
               server   core.c request.c
  Log:
  Performance fix for prep_walk_cache():
  
  Moved the directory/location/file-walk caches from the
  request's pool userdata hash table to the core_request_config
  struct.
  
  This change removes about 60% of the processing time from
  prep_walk_cache().
  
  Revision  Changes    Path
  1.56      +11 -0     httpd-2.0/include/http_core.h
  
  Index: http_core.h
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/include/http_core.h,v
  retrieving revision 1.55
  retrieving revision 1.56
  diff -u -r1.55 -r1.56
  --- http_core.h	13 Dec 2001 14:50:36 -0000	1.55
  +++ http_core.h	31 Dec 2001 08:18:32 -0000	1.56
  @@ -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 */
  
  
  
  1.123     +7 -6      httpd-2.0/server/core.c
  
  Index: core.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/server/core.c,v
  retrieving revision 1.122
  retrieving revision 1.123
  diff -u -r1.122 -r1.123
  --- core.c	27 Dec 2001 13:28:42 -0000	1.122
  +++ core.c	31 Dec 2001 08:18:32 -0000	1.123
  @@ -3385,17 +3385,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);
       
  
  
  
  1.87      +23 -17    httpd-2.0/server/request.c
  
  Index: request.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/server/request.c,v
  retrieving revision 1.86
  retrieving revision 1.87
  diff -u -r1.86 -r1.87
  --- request.c	14 Dec 2001 03:30:23 -0000	1.86
  +++ request.c	31 Dec 2001 08:18:32 -0000	1.87
  @@ -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,30 @@
        * 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));
           }
  -        apr_pool_userdata_setn(cache, cache_name, NULL, r->pool);
  +        if (my_req_cfg) {
  +            my_req_cfg->walk_cache[t] = cache;
  +        }
       }
       return cache;
   }
  @@ -483,7 +489,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 +1063,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 +1219,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
  
  
  

Re: cvs commit: httpd-2.0/server core.c request.c

Posted by Brian Pane <bp...@pacbell.net>.
William A. Rowe, Jr. wrote:
...

>However, can we find a way to make AP_NUM_WALK_CACHES a bit more flexible?
>
>See, for example, mod_proxy.  We are doing the same bit there.  Would you
>mind extending your patch to allow registration [at startup] of other
>cache members?  This could be used for walk caches, or for other sorts of
>per-request cached data that currently resides in notes (and should reside
>in userdata, until you pointed out how slow that solution really is.)
>

I like the registration idea.  I'll work out an API for
it sometime this week.

--Brian




Re: cvs commit: httpd-2.0/server core.c request.c

Posted by "William A. Rowe, Jr." <wr...@covalent.net>.
> brianp      01/12/31 00:18:32
> 
>   Modified:    include  http_core.h
>                server   core.c request.c
>   Log:
>   Performance fix for prep_walk_cache():
>   
>   Moved the directory/location/file-walk caches from the
>   request's pool userdata hash table to the core_request_config
>   struct.
>   
>   This change removes about 60% of the processing time from
>   prep_walk_cache().

Brian... this is a very cool patch...

>   +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;

However, can we find a way to make AP_NUM_WALK_CACHES a bit more flexible?

See, for example, mod_proxy.  We are doing the same bit there.  Would you
mind extending your patch to allow registration [at startup] of other
cache members?  This could be used for walk caches, or for other sorts of
per-request cached data that currently resides in notes (and should reside
in userdata, until you pointed out how slow that solution really is.)

In any case, nice work :)

Bill



Re: cvs commit: httpd-2.0/server core.c request.c

Posted by "William A. Rowe, Jr." <wr...@covalent.net>.
> brianp      01/12/31 00:18:32
> 
>   Modified:    include  http_core.h
>                server   core.c request.c
>   Log:
>   Performance fix for prep_walk_cache():
>   
>   Moved the directory/location/file-walk caches from the
>   request's pool userdata hash table to the core_request_config
>   struct.
>   
>   This change removes about 60% of the processing time from
>   prep_walk_cache().

Brian... this is a very cool patch...

>   +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;

However, can we find a way to make AP_NUM_WALK_CACHES a bit more flexible?

See, for example, mod_proxy.  We are doing the same bit there.  Would you
mind extending your patch to allow registration [at startup] of other
cache members?  This could be used for walk caches, or for other sorts of
per-request cached data that currently resides in notes (and should reside
in userdata, until you pointed out how slow that solution really is.)

In any case, nice work :)

Bill