Here's my proposed patch that replaces the tables with a hash table.
--Brian
Roy T. Fielding wrote:
>On Thu, Jun 21, 2001 at 02:21:11PM -0700, Brian Pane wrote:
>
>>I spent some time recently profiling the latest 2.0 httpd source with gprof.
>>One surprising bottleneck that showed up in the results was the find_ct
>>function in mod_mime, which spends a lot of time in apr_table_get
>>calls. Using the default httpd.conf, the tables for languages and
>>charsets are somewhat large, so the time spent scanning them on each
>>request is significant.
>>
>>It looks straightforward to fix this by replacing the tables with hash
>>tables.
>>Is anybody working on this already? If not, I'm willing to contribute a
>>patch if there's interest.
>>
>
>Sounds like a great idea to me. I was thinking of doing the same with
>methods and header fields, but have no time to do it myself.
>
>....Roy
>
Index: mod_mime.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/http/mod_mime.c,v
retrieving revision 1.42
diff -u -r1.42 mod_mime.c
--- mod_mime.c 2001/06/18 05:36:31 1.42
+++ mod_mime.c 2001/06/22 03:21:33
@@ -66,6 +66,7 @@
#include "apr.h"
#include "apr_strings.h"
#include "apr_lib.h"
+#include "apr_hash.h"
#define APR_WANT_STRFUNC
#include "apr_want.h"
@@ -96,12 +97,19 @@
char *name;
} attrib_info;
+/* Information to which an extension can be mapped
+ */
+typedef struct extension_info {
+ char *forced_type; /* Additional AddTyped stuff */
+ char *encoding_type; /* Added with AddEncoding... */
+ char *language_type; /* Added with AddLanguage... */
+ char *handler; /* Added with AddHandler... */
+ char *charset_type; /* Added with AddCharset... */
+} extension_info;
+
typedef struct {
- apr_table_t *forced_types; /* Additional AddTyped stuff */
- apr_table_t *encoding_types; /* Added with AddEncoding... */
- apr_table_t *language_types; /* Added with AddLanguage... */
- apr_table_t *handlers; /* Added with AddHandler... */
- apr_table_t *charset_types; /* Added with AddCharset... */
+ apr_hash_t *extension_mappings; /* Map from extension name to
+ * extension_info structure */
apr_array_header_t *handlers_remove; /* List of handlers to remove */
apr_array_header_t *types_remove; /* List of MIME types to
remove */
apr_array_header_t *encodings_remove; /* List of encodings to remove */
@@ -138,11 +146,7 @@
mime_dir_config *new =
(mime_dir_config *) apr_palloc(p, sizeof(mime_dir_config));
- new->forced_types = apr_table_make(p, 4);
- new->encoding_types = apr_table_make(p, 4);
- new->charset_types = apr_table_make(p, 4);
- new->language_types = apr_table_make(p, 4);
- new->handlers = apr_table_make(p, 4);
+ new->extension_mappings = apr_hash_make(p);
new->handlers_remove = apr_array_make(p, 4, sizeof(attrib_info));
new->types_remove = apr_array_make(p, 4, sizeof(attrib_info));
new->encodings_remove = apr_array_make(p, 4, sizeof(attrib_info));
@@ -154,6 +158,47 @@
return new;
}
+/*
+ * Overlay one hash table of extension_mappings onto another
+ */
+static void overlay_extension_mappings(apr_pool_t *p,
+ apr_hash_t *overlay, apr_hash_t *base)
+{
+ apr_hash_index_t *index;
+ for (index = apr_hash_first(overlay); index;
+ index = apr_hash_next(index)) {
+ char *key;
+ apr_ssize_t klen;
+ extension_info *overlay_info, *base_info;
+
+ apr_hash_this(index, (const void**)&key, &klen, (void**)&overlay_info);
+ base_info = (extension_info*)apr_hash_get(base, key, klen);
+ if (base_info) {
+ if (overlay_info->forced_type) {
+ base_info->forced_type = overlay_info->forced_type;
+ }
+ if (overlay_info->encoding_type) {
+ base_info->encoding_type = overlay_info->encoding_type;
+ }
+ if (overlay_info->language_type) {
+ base_info->language_type = overlay_info->language_type;
+ }
+ if (overlay_info->handler) {
+ base_info->handler = overlay_info->handler;
+ }
+ if (overlay_info->charset_type) {
+ base_info->charset_type = overlay_info->charset_type;
+ }
+ }
+ else {
+ base_info = (extension_info*)apr_palloc(p, sizeof(extension_info));
+ memcpy(base_info, overlay_info, sizeof(extension_info));
+ apr_hash_set(base, key, klen, base_info);
+ }
+ }
+
+}
+
static void *merge_mime_dir_configs(apr_pool_t *p, void *basev, void *addv)
{
mime_dir_config *base = (mime_dir_config *) basev;
@@ -162,34 +207,44 @@
(mime_dir_config *) apr_palloc(p, sizeof(mime_dir_config));
int i;
attrib_info *suffix;
+
+ new->extension_mappings = apr_hash_make(p);
+ overlay_extension_mappings(p, new->extension_mappings,
+ base->extension_mappings);
+ overlay_extension_mappings(p, new->extension_mappings,
+ add->extension_mappings);
- new->forced_types = apr_table_overlay(p, add->forced_types,
- base->forced_types);
- new->encoding_types = apr_table_overlay(p, add->encoding_types,
- base->encoding_types);
- new->charset_types = apr_table_overlay(p, add->charset_types,
- base->charset_types);
- new->language_types = apr_table_overlay(p, add->language_types,
- base->language_types);
- new->handlers = apr_table_overlay(p, add->handlers,
- base->handlers);
suffix = (attrib_info *) add->handlers_remove->elts;
for (i = 0; i < add->handlers_remove->nelts; i++) {
- apr_table_unset(new->handlers, suffix[i].name);
+ extension_info *exinfo =
+ (extension_info*)apr_hash_get(new->extension_mappings,
+ suffix[i].name, APR_HASH_KEY_STRING);
+ if (exinfo) {
+ exinfo->handler = NULL;
+ }
}
suffix = (attrib_info *) add->types_remove->elts;
for (i = 0; i < add->types_remove->nelts; i++) {
- apr_table_unset(new->forced_types, suffix[i].name);
+ extension_info *exinfo =
+ (extension_info*)apr_hash_get(new->extension_mappings,
+ suffix[i].name, APR_HASH_KEY_STRING);
+ if (exinfo) {
+ exinfo->forced_type = NULL;
+ }
}
suffix = (attrib_info *) add->encodings_remove->elts;
for (i = 0; i < add->encodings_remove->nelts; i++) {
- apr_table_unset(new->encoding_types, suffix[i].name);
+ extension_info *exinfo =
+ (extension_info*)apr_hash_get(new->extension_mappings,
+ suffix[i].name, APR_HASH_KEY_STRING);
+ if (exinfo) {
+ exinfo->encoding_type = NULL;
+ }
}
-
new->type = add->type ? add->type : base->type;
new->handler = add->handler ? add->handler : base->handler;
new->default_language = add->default_language ?
@@ -198,17 +253,32 @@
return new;
}
+static extension_info *get_extension_info(apr_pool_t *p,
mime_dir_config *m,
+ const char* key)
+{
+ extension_info *exinfo;
+
+ exinfo = (extension_info*)apr_hash_get(m->extension_mappings, key,
+ APR_HASH_KEY_STRING);
+ if (!exinfo) {
+ exinfo = apr_pcalloc(p, sizeof(extension_info));
+ apr_hash_set(m->extension_mappings, key, APR_HASH_KEY_STRING, exinfo);
+ }
+ return exinfo;
+}
+
static const char *add_type(cmd_parms *cmd, void *m_, const char *ct_,
const char *ext)
{
mime_dir_config *m=m_;
char *ct=apr_pstrdup(cmd->pool,ct_);
+ extension_info* exinfo;
if (*ext == '.')
++ext;
-
+ exinfo = get_extension_info(cmd->pool, m, ext);
ap_str_tolower(ct);
- apr_table_setn(m->forced_types, ext, ct);
+ exinfo->forced_type = ct;
return NULL;
}
@@ -217,11 +287,13 @@
{
mime_dir_config *m=m_;
char *enc=apr_pstrdup(cmd->pool,enc_);
+ extension_info* exinfo;
if (*ext == '.')
++ext;
+ exinfo = get_extension_info(cmd->pool, m, ext);
ap_str_tolower(enc);
- apr_table_setn(m->encoding_types, ext, enc);
+ exinfo->encoding_type = enc;
return NULL;
}
@@ -230,12 +302,14 @@
{
mime_dir_config *m=m_;
char *charset=apr_pstrdup(cmd->pool,charset_);
+ extension_info* exinfo;
if (*ext == '.') {
++ext;
}
+ exinfo = get_extension_info(cmd->pool, m, ext);
ap_str_tolower(charset);
- apr_table_setn(m->charset_types, ext, charset);
+ exinfo->charset_type = charset;
return NULL;
}
@@ -244,12 +318,14 @@
{
mime_dir_config *m=m_;
char *lang=apr_pstrdup(cmd->pool,lang_);
+ extension_info* exinfo;
if (*ext == '.') {
++ext;
}
+ exinfo = get_extension_info(cmd->pool, m, ext);
ap_str_tolower(lang);
- apr_table_setn(m->language_types, ext, lang);
+ exinfo->language_type = lang;
return NULL;
}
@@ -258,11 +334,14 @@
{
mime_dir_config *m=m_;
char *hdlr=apr_pstrdup(cmd->pool,hdlr_);
+ extension_info* exinfo;
- if (*ext == '.')
+ if (*ext == '.') {
++ext;
+ }
+ exinfo = get_extension_info(cmd->pool, m, ext);
ap_str_tolower(hdlr);
- apr_table_setn(m->handlers, ext, hdlr);
+ exinfo->handler = hdlr;
return NULL;
}
@@ -362,15 +441,8 @@
{NULL}
};
-/* Hash apr_table_t --- only one of these per daemon; virtual hosts can
- * get private versions through AddType...
- */
-
-#define MIME_HASHSIZE (32)
-#define hash(i) (apr_tolower(i) % MIME_HASHSIZE)
+static apr_hash_t *mime_type_extensions;
-static apr_table_t *hash_buckets[MIME_HASHSIZE];
-
static void mime_post_config(apr_pool_t *p, apr_pool_t *plog,
apr_pool_t *ptemp, server_rec *s)
{
ap_configfile_t *f;
@@ -390,8 +462,7 @@
exit(1);
}
- for (x = 0; x < MIME_HASHSIZE; x++)
- hash_buckets[x] = apr_table_make(p, 10);
+ mime_type_extensions = apr_hash_make(p);
while (!(ap_cfg_getline(l, MAX_STRING_LEN, f))) {
const char *ll = l, *ct;
@@ -403,7 +474,7 @@
while (ll[0]) {
char *ext = ap_getword_conf(p, &ll);
ap_str_tolower(ext); /* ??? */
- apr_table_setn(hash_buckets[hash(ext[0])], ext, ct);
+ apr_hash_set(mime_type_extensions, ext, APR_HASH_KEY_STRING, ct);
}
}
ap_cfg_closefile(f);
@@ -682,6 +753,7 @@
/* Parse filename extensions, which can be in any order */
while ((ext = ap_getword(r->pool, &fn, '.')) && *ext) {
int found = 0;
+ extension_info *exinfo;
#ifdef CASE_BLIND_FILESYSTEM
/* We have a basic problem that folks on case-crippled systems
@@ -690,21 +762,25 @@
ap_str_tolower(ext);
#endif
+ exinfo = (extension_info*) apr_hash_get(conf->extension_mappings,
+ ext, APR_HASH_KEY_STRING);
+
/* Check for Content-Type */
- if ((type = apr_table_get(conf->forced_types, ext))
- || (type = apr_table_get(hash_buckets[hash(*ext)], ext))) {
+ if ((exinfo && ((type = exinfo->forced_type)))
+ || (type = apr_hash_get(mime_type_extensions, ext,
+ APR_HASH_KEY_STRING))) {
r->content_type = type;
found = 1;
}
/* Add charset to Content-Type */
- if ((type = apr_table_get(conf->charset_types, ext))) {
+ if (exinfo && ((type = exinfo->charset_type))) {
charset = type;
found = 1;
}
/* Check for Content-Language */
- if ((type = apr_table_get(conf->language_types, ext))) {
+ if (exinfo && ((type = exinfo->language_type))) {
const char **new;
r->content_language = type; /* back compat. only */
@@ -716,7 +792,7 @@
}
/* Check for Content-Encoding */
- if ((type = apr_table_get(conf->encoding_types, ext))) {
+ if (exinfo && ((type = exinfo->encoding_type))) {
if (!r->content_encoding)
r->content_encoding = type;
else
@@ -726,9 +802,8 @@
}
/* Check for a special handler, but not for proxy request */
- if ((type = apr_table_get(conf->handlers, ext))
- && (PROXYREQ_NONE == r->proxyreq)
- ) {
+ if ((exinfo && ((type = exinfo->handler)))
+ && (PROXYREQ_NONE == r->proxyreq)) {
r->handler = type;
found = 1;
}
