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;
