Return-Path: Delivered-To: apmail-httpd-dev-archive@httpd.apache.org Received: (qmail 34876 invoked by uid 500); 20 Feb 2002 08:39:54 -0000 Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm Precedence: bulk Reply-To: dev@httpd.apache.org list-help: list-unsubscribe: list-post: Delivered-To: mailing list dev@httpd.apache.org Received: (qmail 34850 invoked from network); 20 Feb 2002 08:39:54 -0000 Date: Wed, 20 Feb 2002 00:40:06 -0800 From: Justin Erenkrantz To: dev@httpd.apache.org Subject: Optimize mod_mime was Re: [PATCH] Add content-type specific filters Message-ID: <20020220084005.GA26092@ebuilt.com> Mail-Followup-To: Justin Erenkrantz , dev@httpd.apache.org References: <20020218065507.GI26092@ebuilt.com> <3C70B867.5080902@pacbell.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3C70B867.5080902@pacbell.net> User-Agent: Mutt/1.5.0i X-AntiVirus: scanned for viruses by AMaViS 0.2.1-pre3 (http://amavis.org/) X-Spam-Rating: daedalus.apache.org 1.6.2 0/1000/N On Mon, Feb 18, 2002 at 12:16:39AM -0800, Brian Pane wrote: > This reminds me: mod_mime needs some performance work. > > I probably won't have time to address this between now > and 2.0 GA, and the performance problem isn't big > enough to be a GA showstopper, but I'll summarize the > issues here in case anyone else is interested in tackling > the problem. Hehe. I just tried to tackle it and Remove*Filter has got me beaten. I give up. The actual splitting out of the input/output filters isn't too bad for merging and addition. However, the semantic for remove is that it must be propogated downward - you can't just remove it at that level - you must propogate removals downward and do removals at each step. The mod_mime code just can't do this cleanly. I think the real thing to do is to have another hash for remove_mappings. Also, I'll ask again, is there any function where you can pass in a filter name and receive the handle? (Since you only have the name, you can't register it.) I'm thinking we have to traverse the trie, right? But, there's no function now. Oh, joy. Another function for me to write. =) Anyway, this patch doesn't even compile, but you get the idea if you want to pick up the pieces of my shattered dream. -- justin Index: modules/http/mod_mime.c =================================================================== RCS file: /home/cvs/httpd-2.0/modules/http/mod_mime.c,v retrieving revision 1.76 diff -u -r1.76 mod_mime.c --- modules/http/mod_mime.c 8 Dec 2001 02:04:51 -0000 1.76 +++ modules/http/mod_mime.c 20 Feb 2002 08:13:05 -0000 @@ -106,8 +106,8 @@ char *language_type; /* Added with AddLanguage... */ char *handler; /* Added with AddHandler... */ char *charset_type; /* Added with AddCharset... */ - char *input_filters; /* Added with AddInputFilter... */ - char *output_filters; /* Added with AddOutputFilter... */ + ap_filter_rec_t *input_filters; /* Added with AddInputFilter... */ + ap_filter_rec_t *output_filters; /* Added with AddOutputFilter... */ } extension_info; #define MULTIMATCH_UNSET 0 @@ -121,6 +121,8 @@ * extension_info structure */ apr_array_header_t *remove_mappings; /* A simple list, walked once */ + ap_filter_rec_t *remove_input_filters; /* Filters that need to removed. */ + ap_filter_rec_t *remove_output_filters; char *default_language; /* Language if no AddLanguage ext found */ @@ -156,6 +158,8 @@ new->extension_mappings = NULL; new->remove_mappings = NULL; + new->remove_input_filters = NULL; + new->remove_output_filters = NULL; new->default_language = NULL; @@ -163,6 +167,53 @@ return new; } + +/* Note: does not preserve ordering. */ +static ap_filter_rec_t* copy_filters(ap_filter_rec_t *f, apr_pool_t *p) +{ + ap_filter_rec_t *g, *h; + + h = NULL; + + while (f) { + g = apr_pcalloc(p, sizeof(ap_filter_rec_t)); + g->name = apr_pstrdup(p, f->name); + g->next = h; + h = g; + f = f->next; + } + + return h; +} + +static ap_filter_rec_t* remove_filters(ap_filter_rec_t *remove, + ap_filter_rec_t *list) +{ + ap_filter_rec_t *ret = list; + + while (remove) { + ap_filter_rec_t *trailer = NULL; + list = ret; + while (list) { + if (!strcasecmp(remove->name, list->name)) { + if (!trailer) { + ret = ret->next; + } + else { + trailer->next = list->next; + } + } + else { + trailer = list; + } + list = list->next; + } + remove = remove->next; + } + + return ret; +} + /* * Overlay one hash table of extension_mappings onto another */ @@ -193,11 +244,19 @@ if (overlay_info->charset_type) { new_info->charset_type = overlay_info->charset_type; } + /* Should we attempt to merge these filters? */ if (overlay_info->input_filters) { - new_info->input_filters = overlay_info->input_filters; + new_info->input_filters = copy_filters(overlay_info->input_filters, p); + } + else if (base_info->input_filters) { + new_info->input_filters = copy_filters(base_info->input_filters, p); } if (overlay_info->output_filters) { - new_info->output_filters = overlay_info->output_filters; + new_info->output_filters = copy_filters(overlay_info->output_filters, + p); + } + else if (base_info->output_filters) { + new_info->output_filters = copy_filters(base_info->output_filters, p); } return new_info; @@ -258,8 +317,22 @@ if (new->extension_mappings) { if (add->remove_mappings) remove_items(p, add->remove_mappings, new->extension_mappings); + if (add->remove_input_filters && + new->extension_mappings->input_filters) { + new->extension_mappings->input_filters = + remove_filters(add->remove_input_filters, + new->extension_mappings->input_filters); + } + if (add->remove_output_filters && + new->extension_mappings->output_filters) { + new->extension_mappings->output_filters = + remove_filters(add->remove_output_filters, + new->extension_mappings->output_filters); + } } new->remove_mappings = NULL; + new->remove_output_filters = NULL; + new->remove_input_filters = NULL; new->default_language = add->default_language ? add->default_language : base->default_language; @@ -324,6 +397,69 @@ suffix->offset = (int) (long) cmd->info; return NULL; } + +static const char *add_filter_info(cmd_parms *cmd, void *m_, + const char *arg, const char *arg2) +{ + mime_dir_config *m = (mime_dir_config *) m_; + extension_info *exinfo; + const char *filter, *filters; + ap_filter_rec_t *old, *new; + + if (!m->extension_mappings) { + m->extension_mappings = apr_hash_make(cmd->pool); + exinfo = NULL; + } + else { + exinfo = (extension_info*)apr_hash_get(m->extension_mappings, arg2, + APR_HASH_KEY_STRING); + } + if (!exinfo) { + exinfo = apr_pcalloc(cmd->pool, sizeof(extension_info)); + apr_hash_set(m->extension_mappings, arg2, + APR_HASH_KEY_STRING, exinfo); + } + + filters = arg; + while (*filters && (filter = ap_getword(cmd->pool, &filters, ';'))) { + new = apr_pcalloc(cmd->pool, sizeof(ap_filter_rec_t)); + new->name = apr_pstrdup(cmd->pool, filter); + /* cmd->info == 0 -> input_filters + * cmd->info == 1 -> output_filters + */ + if (!cmd->info) { + new->next = exinfo->input_filters; + exinfo->input_filters = new; + } + else { + new->next = exinfo->output_filters; + exinfo->output_filters = new; + } + } + return NULL; +} + +static const char *remove_filter_info(cmd_parms *cmd, void *m_, + const char *arg, const char *arg2) +{ + mime_dir_config *m = (mime_dir_config *) m_; + ap_filter_rec_t *new; + + new = apr_pcalloc(cmd->pool, sizeof(ap_filter_rec_t)); + new->name = apr_pstrdup(cmd->pool, arg); + /* cmd->info == 0 -> input_filters + * cmd->info == 1 -> output_filters + */ + if (!cmd->info) { + new->next = m->remove_input_filters; + m->remove_input_filters = new; + } + else { + new->next = m->remove_output_filters; + m->remove_output_filters = new; + } + return NULL; +} /* The sole bit of server configuration that the MIME module has is * the name of its config file, so... @@ -383,14 +519,12 @@ AP_INIT_ITERATE2("AddHandler", add_extension_info, (void *)APR_XtOffsetOf(extension_info, handler), OR_FILEINFO, "a handler name followed by one or more file extensions"), -AP_INIT_ITERATE2("AddInputFilter", add_extension_info, - (void *)APR_XtOffsetOf(extension_info, input_filters), OR_FILEINFO, +AP_INIT_ITERATE2("AddInputFilter", add_filter_info, 0, OR_FILEINFO, "input filter name (or ; delimited names) followed by one or more file extensions"), AP_INIT_ITERATE2("AddLanguage", add_extension_info, (void *)APR_XtOffsetOf(extension_info, language_type), OR_FILEINFO, "a language (e.g., fr), followed by one or more file extensions"), -AP_INIT_ITERATE2("AddOutputFilter", add_extension_info, - (void *)APR_XtOffsetOf(extension_info, output_filters), OR_FILEINFO, +AP_INIT_ITERATE2("AddOutputFilter", add_filter_info, 1, OR_FILEINFO, "output filter name (or ; delimited names) followed by one or more file extensions"), AP_INIT_ITERATE2("AddType", add_extension_info, (void *)APR_XtOffsetOf(extension_info, forced_type), OR_FILEINFO, @@ -409,14 +543,12 @@ AP_INIT_ITERATE("RemoveHandler", remove_extension_info, (void *)APR_XtOffsetOf(extension_info, handler), OR_FILEINFO, "one or more file extensions"), -AP_INIT_ITERATE("RemoveInputFilter", remove_extension_info, - (void *)APR_XtOffsetOf(extension_info, input_filters), OR_FILEINFO, +AP_INIT_ITERATE("RemoveInputFilter", remove_filter_info, 0, OR_FILEINFO, "one or more file extensions"), AP_INIT_ITERATE("RemoveLanguage", remove_extension_info, (void *)APR_XtOffsetOf(extension_info, language_type), OR_FILEINFO, "one or more file extensions"), -AP_INIT_ITERATE("RemoveOutputFilter", remove_extension_info, - (void *)APR_XtOffsetOf(extension_info, output_filters), OR_FILEINFO, +AP_INIT_ITERATE("RemoveOutputFilter", remove_filter_info, 1, OR_FILEINFO, "one or more file extensions"), AP_INIT_ITERATE("RemoveType", remove_extension_info, (void *)APR_XtOffsetOf(extension_info, forced_type), OR_FILEINFO, @@ -837,19 +969,19 @@ * hook, which may be too early (dunno.) */ if (exinfo->input_filters && r->proxyreq == PROXYREQ_NONE) { - const char *filter, *filters = exinfo->input_filters; - while (*filters - && (filter = ap_getword(r->pool, &filters, ';'))) { - ap_add_input_filter(filter, NULL, r, r->connection); + ap_filter_rec_t *f = exinfo->input_filters; + while (f) { + ap_add_input_filter(f->name, NULL, r, r->connection); + f = f->next; } if (conf->multimatch & MULTIMATCH_FILTERS) found = 1; } if (exinfo->output_filters && r->proxyreq == PROXYREQ_NONE) { - const char *filter, *filters = exinfo->output_filters; - while (*filters - && (filter = ap_getword(r->pool, &filters, ';'))) { - ap_add_output_filter(filter, NULL, r, r->connection); + ap_filter_rec_t *f = exinfo->output_filters; + while (f) { + ap_add_output_filter(f->name, NULL, r, r->connection); + f = f->next; } if (conf->multimatch & MULTIMATCH_FILTERS) found = 1;