You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Marc Slemko <ma...@worldgate.com> on 1998/01/11 01:18:36 UTC
misc profiling comments
4587 requests in -X mode, 13.22 seconds total.
707770 calls to strcasecmp. 376216 from table_get and ~500000 from all
the table_* functions and 110112 from invoke_handler. Taking 0.40
seconds.
13777 calls to stat() taking 1.14 seconds. Would be easy to whack this
out for this benchmark with a stat() cache, too bad reality isn't all one
file.
(yes, I was getting / which needs mod_dir to play...)
The biggest thing that sticks out is the table_* functions. Yea, context
switching is probably the biggest thing but I'm not looking at child
overhead and we know the solution to that...
Re: misc profiling comments
Posted by Dean Gaudet <dg...@arctic.org>.
On Tue, 13 Jan 1998, Martin Kraemer wrote:
> Is it worth thinking of the hsearch(3) or tsearch(3) library functions?
> In the past, these proved to get us good speed improvements in similar
> environments. But I think you had a even better performance in mind...
The problem is that most tables have so few elements that the overhead
would be prohibitive.
> Is it worth thinking of calculating a perfect hash for the most frequently
> used MIME headers (and always forcing them to single case)? Strcasecmp()
> is IMHO much more expensive than strcmp().
Yup it's worth looking at... but I'm not likely to continue in that
direction right now, so if someone else wants to play with it, feel free.
The perfect hash is really only needed when we're reading headers from the
client. Everywhere else could do a lookup using an enumerated type as the
key.
Dean
Re: misc profiling comments
Posted by Martin Kraemer <Ma...@mch.sni.de>.
On Sat, Jan 10, 1998 at 09:16:19PM -0800, Dean Gaudet wrote:
> I've got the beginnings of a hashed version of the table_* stuff. It's
> not a full hash table, that would be too heavy. There's too many tables,
> and the only "busy" ones are r->headers_in and r->headers_out (I wrote
> some code to monitor all the table ops). I think I forgot to post this
> analysis.
Is it worth thinking of the hsearch(3) or tsearch(3) library functions?
In the past, these proved to get us good speed improvements in similar
environments. But I think you had a even better performance in mind...
Is it worth thinking of calculating a perfect hash for the most frequently
used MIME headers (and always forcing them to single case)? Strcasecmp()
is IMHO much more expensive than strcmp().
> Rather than special case those two I'm putting an extra 32-bits (or maybe
> 64, whatever aligns it right) into the table header. If a bit in the
> bitvector is 0, then it's known that no element matching that hash is in
> the table. This gives a short-circuit for all the table functions. I
> think it'll do the trick.
Is that similar to the trick used in (t)csh? They pre-hash every directory
in the $PATH in order to provide speed improvements when trying to
execute commands (the don't even consider directories where the hash
claims the command cannot reside. That's why you have to say "rehash"
after adding commands).
Martin
--
| S I E M E N S | <Ma...@mch.sni.de> | Siemens Nixdorf
| ------------- | Voice: +49-89-636-46021 | Informationssysteme AG
| N I X D O R F | FAX: +49-89-636-44994 | 81730 Munich, Germany
~~~~~~~~~~~~~~~~My opinions only, of course; pgp key available on request
Re: misc profiling comments
Posted by Dean Gaudet <dg...@arctic.org>.
On Sun, 11 Jan 1998, Paul Sutton wrote:
> On Sun, 11 Jan 1998, Dean Gaudet wrote:
> > On Sun, 11 Jan 1998, Dean Gaudet wrote:
> > > Here it is if you want to play with it. This seems to cut the strcasecmp
> > > calls in half.
> >
> > Uh here it is for real.
> >
> > BTW this also includes code cleanup elsewhere in the server because
> > there's various code that assumes that a table * is the same as an
> > array_header *... despite the existance of the table_elts() routine. Even
> > though the current implementation does "typedef array_header table" it's
> > not required to be that way... which is why table_elts() exists in the
> > first place.
>
> Yep, this all seems to work. But it doesn't seem to make much of a
> difference in performance on my system. I testing requesting an index file
> (/) 1000 times. In fact, I couldn't really such much difference at all --
> there was more variation between subsequent runs of the same httpd than
> between the httpd before and after this patch. Maybe my test case doesn't
> exercise table_set enough?
>
> But I'm definitely in favour of fixing up the the use of tables and
> arrays without with table_elts().
Yeah I'm not sure my current approach is worth it... I didn't see much of
a difference either.
One thing I was considering is to have a special structure for
r->headers_in, when we populate r->headers_in we notice when the string
matches one of the headers we're interested in (i.e. "If-Modified-Since",
"If-Unmodified-Since", ...) and slap it into a vector as well as the
table. To do this requires only one hash (using a perfect hash). Then
later in the code we can use an enumerated type to find the values.
But that's a lot of work. I'm not likely to do it.
But yeah I'll extract the parts of the patch which are a table API
cleanup.
Dean
Re: misc profiling comments
Posted by Paul Sutton <pa...@eu.c2.net>.
On Sun, 11 Jan 1998, Dean Gaudet wrote:
> On Sun, 11 Jan 1998, Dean Gaudet wrote:
> > Here it is if you want to play with it. This seems to cut the strcasecmp
> > calls in half.
>
> Uh here it is for real.
>
> BTW this also includes code cleanup elsewhere in the server because
> there's various code that assumes that a table * is the same as an
> array_header *... despite the existance of the table_elts() routine. Even
> though the current implementation does "typedef array_header table" it's
> not required to be that way... which is why table_elts() exists in the
> first place.
Yep, this all seems to work. But it doesn't seem to make much of a
difference in performance on my system. I testing requesting an index file
(/) 1000 times. In fact, I couldn't really such much difference at all --
there was more variation between subsequent runs of the same httpd than
between the httpd before and after this patch. Maybe my test case doesn't
exercise table_set enough?
But I'm definitely in favour of fixing up the the use of tables and
arrays without with table_elts().
//pcs
Re: misc profiling comments
Posted by Dean Gaudet <dg...@arctic.org>.
On Sun, 11 Jan 1998, Dean Gaudet wrote:
> Here it is if you want to play with it. This seems to cut the strcasecmp
> calls in half.
Uh here it is for real.
BTW this also includes code cleanup elsewhere in the server because
there's various code that assumes that a table * is the same as an
array_header *... despite the existance of the table_elts() routine. Even
though the current implementation does "typedef array_header table" it's
not required to be that way... which is why table_elts() exists in the
first place.
Dean
Index: main/alloc.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/alloc.c,v
retrieving revision 1.67
diff -u -r1.67 alloc.c
--- alloc.c 1998/01/07 16:45:59 1.67
+++ alloc.c 1998/01/11 11:37:48
@@ -584,10 +584,8 @@
* The 'array' functions...
*/
-API_EXPORT(array_header *) make_array(pool *p, int nelts, int elt_size)
+static void make_array_core(array_header *res, pool *p, int nelts, int elt_size)
{
- array_header *res = (array_header *) palloc(p, sizeof(array_header));
-
if (nelts < 1)
nelts = 1; /* Assure sanity if someone asks for
* array of zero elts.
@@ -599,7 +597,13 @@
res->elt_size = elt_size;
res->nelts = 0; /* No active elements yet... */
res->nalloc = nelts; /* ...but this many allocated */
+}
+
+API_EXPORT(array_header *) make_array(pool *p, int nelts, int elt_size)
+{
+ array_header *res = (array_header *) palloc(p, sizeof(array_header));
+ make_array_core(res, p, nelts, elt_size);
return res;
}
@@ -658,17 +662,21 @@
* overhead of the full copy only where it is really needed.
*/
-API_EXPORT(array_header *) copy_array_hdr(pool *p, const array_header *arr)
+static ap_inline void copy_array_hdr_core(array_header *res,
+ const array_header *arr)
{
- array_header *res = (array_header *) palloc(p, sizeof(array_header));
-
res->elts = arr->elts;
-
- res->pool = p;
res->elt_size = arr->elt_size;
res->nelts = arr->nelts;
res->nalloc = arr->nelts; /* Force overflow on push */
+}
+API_EXPORT(array_header *) copy_array_hdr(pool *p, const array_header *arr)
+{
+ array_header *res = (array_header *) palloc(p, sizeof(array_header));
+
+ res->pool = p;
+ copy_array_hdr_core(res, arr);
return res;
}
@@ -690,80 +698,146 @@
* The "table" functions.
*/
-API_EXPORT(table *) make_table(pool *p, int nelts)
+typedef unsigned int table_hash_t;
+#define TABLE_HASH_SIZE (8*sizeof(table_hash_t))
+
+/* Hash using only the first 8 characters... this is a complete hack,
+ * but should work for the strings that appear in the common tables.
+ */
+static ap_inline table_hash_t table_hash(const char *s)
{
- return make_array(p, nelts, sizeof(table_entry));
+ unsigned int h;
+ int i;
+
+ h = 0;
+ for( i = 0; s[i] && i < 8; ++i ) {
+ /* XXX: this assumes ASCII, there's an implicit tolower() here */
+ h = ( h << 1 ) ^ (s[i] & 0x1f);
+ }
+ return 1 << (h % TABLE_HASH_SIZE);
}
-API_EXPORT(table *) copy_table(pool *p, const table *t)
+/* XXX: if you tweak this you should look at is_empty_table() and table_elts()
+ * in alloc.h */
+struct table {
+ /* This has to be first to promote backwards compatibility with
+ * older modules which cast a table * to an array_header *...
+ * they should use the table_elts() function for most of the
+ * cases they do this for.
+ */
+ array_header a;
+ /* Bit h is 0 if there is no key with hash value h in the table.
+ * The converse is *not* true -- a bit of 1 indicates that a key
+ * with value h may be in the table. This simplifies table_unset,
+ * which is a comparatively infrequent operation.
+ */
+ table_hash_t hash_bits;
+};
+
+
+API_EXPORT(table *) make_table(pool *p, int nelts)
{
- return copy_array(p, t);
+ table *t = palloc(p, sizeof(table));
+
+ make_array_core(&t->a, p, nelts, sizeof(table_entry));
+ t->hash_bits = 0;
+ return t;
}
-API_EXPORT(void) clear_table(table *t)
+API_EXPORT(table *) copy_table(pool *p, const table *t)
{
- t->nelts = 0;
+ table *new = palloc(p, sizeof(table));
+
+ make_array_core(&new->a, p, t->a.nalloc, sizeof(table_entry));
+ memcpy(new->a.elts, t->a.elts, t->a.nelts * sizeof(table_entry));
+ new->a.nelts = t->a.nelts;
+ new->hash_bits = t->hash_bits;
+ return new;
}
-API_EXPORT(array_header *) table_elts(table *t)
+API_EXPORT(void) clear_table(table *t)
{
- return t;
+ t->a.nelts = 0;
+ t->hash_bits = 0;
}
API_EXPORT(char *) table_get(const table *t, const char *key)
{
- table_entry *elts = (table_entry *) t->elts;
+ table_entry *elts = (table_entry *) t->a.elts;
int i;
+ table_hash_t h;
if (key == NULL)
return NULL;
- for (i = 0; i < t->nelts; ++i)
+ h = table_hash(key);
+ if (!(t->hash_bits & h))
+ return NULL;
+
+ for (i = 0; i < t->a.nelts; ++i)
if (!strcasecmp(elts[i].key, key))
return elts[i].val;
return NULL;
}
+#ifdef END_HACK
+extern char _end;
+#define is_const(s) ((const char *)(s) < &_end)
+#define dup_nonconst(p, s) (is_const(s) ? (char *)(s) : pstrdup((p), (s)))
+#else
+#define dup_nonconst(p, s) (pstrdup((p), (s)))
+#endif
+
API_EXPORT(void) table_set(table *t, const char *key, const char *val)
{
register int i, j, k;
- table_entry *elts = (table_entry *) t->elts;
+ table_entry *elts = (table_entry *) t->a.elts;
int done = 0;
+ table_hash_t h;
- for (i = 0; i < t->nelts; ) {
- if (!strcasecmp(elts[i].key, key)) {
- if (!done) {
- elts[i].val = pstrdup(t->pool, val);
- done = 1;
- ++i;
- }
- else { /* delete an extraneous element */
- for (j = i, k = i + 1; k < t->nelts; ++j, ++k) {
- elts[j].key = elts[k].key;
- elts[j].val = elts[k].val;
+ h = table_hash(key);
+ if (t->hash_bits & h) {
+ for (i = 0; i < t->a.nelts; ) {
+ if (!strcasecmp(elts[i].key, key)) {
+ if (!done) {
+ elts[i].val = dup_nonconst(t->a.pool, val);
+ done = 1;
+ ++i;
+ }
+ else { /* delete an extraneous element */
+ for (j = i, k = i + 1; k < t->a.nelts; ++j, ++k) {
+ elts[j].key = elts[k].key;
+ elts[j].val = elts[k].val;
+ }
+ --t->a.nelts;
}
- --t->nelts;
}
- }
- else {
- ++i;
+ else {
+ ++i;
+ }
}
}
if (!done) {
- elts = (table_entry *) push_array(t);
- elts->key = pstrdup(t->pool, key);
- elts->val = pstrdup(t->pool, val);
+ elts = (table_entry *) push_array(&t->a);
+ elts->key = dup_nonconst(t->a.pool, key);
+ elts->val = dup_nonconst(t->a.pool, val);
+ t->hash_bits |= h;
}
}
API_EXPORT(void) table_unset(table *t, const char *key)
{
register int i, j, k;
- table_entry *elts = (table_entry *) t->elts;
+ table_entry *elts = (table_entry *) t->a.elts;
+ table_hash_t h;
+
+ h = table_hash(key);
+ if (!(t->hash_bits & h))
+ return;
- for (i = 0; i < t->nelts; ) {
+ for (i = 0; i < t->a.nelts;) {
if (!strcasecmp(elts[i].key, key)) {
/* found an element to skip over
@@ -771,11 +845,11 @@
* a contiguous block of memory. I've chosen one that
* doesn't do a memcpy/bcopy/array_delete, *shrug*...
*/
- for (j = i, k = i + 1; k < t->nelts; ++j, ++k) {
+ for (j = i, k = i + 1; k < t->a.nelts; ++j, ++k) {
elts[j].key = elts[k].key;
elts[j].val = elts[k].val;
}
- --t->nelts;
+ --t->a.nelts;
}
else {
++i;
@@ -785,32 +859,50 @@
API_EXPORT(void) table_merge(table *t, const char *key, const char *val)
{
- table_entry *elts = (table_entry *) t->elts;
+ table_entry *elts = (table_entry *) t->a.elts;
int i;
+ table_hash_t h;
- for (i = 0; i < t->nelts; ++i)
- if (!strcasecmp(elts[i].key, key)) {
- elts[i].val = pstrcat(t->pool, elts[i].val, ", ", val, NULL);
- return;
+ h = table_hash(key);
+ if (t->hash_bits & h) {
+ for (i = 0; i < t->a.nelts; ++i) {
+ if (!strcasecmp(elts[i].key, key)) {
+ elts[i].val = pstrcat(t->a.pool, elts[i].val, ", ", val, NULL);
+ return;
+ }
}
+ }
- elts = (table_entry *) push_array(t);
- elts->key = pstrdup(t->pool, key);
- elts->val = pstrdup(t->pool, val);
+ t->hash_bits |= h;
+ elts = (table_entry *) push_array(&t->a);
+ elts->key = dup_nonconst(t->a.pool, key);
+ elts->val = dup_nonconst(t->a.pool, val);
}
API_EXPORT(void) table_add(table *t, const char *key, const char *val)
{
- table_entry *elts = (table_entry *) t->elts;
+ table_entry *elts = (table_entry *) t->a.elts;
+ table_hash_t h;
- elts = (table_entry *) push_array(t);
- elts->key = pstrdup(t->pool, key);
- elts->val = pstrdup(t->pool, val);
+ h = table_hash(key);
+ t->hash_bits |= h;
+ elts = (table_entry *) push_array(&t->a);
+ elts->key = dup_nonconst(t->a.pool, key);
+ elts->val = dup_nonconst(t->a.pool, val);
}
API_EXPORT(table *) overlay_tables(pool *p, const table *overlay, const table *base)
{
- return append_arrays(p, overlay, base);
+ table *res;
+
+ res = palloc(p, sizeof(table));
+ res->hash_bits = overlay->hash_bits | base->hash_bits;
+ /* behave like append_arrays */
+ res->a.pool = p;
+ copy_array_hdr_core(&res->a, &overlay->a);
+ array_cat(&res->a, &base->a);
+
+ return res;
}
/* And now for something completely abstract ...
@@ -840,7 +932,7 @@
{
va_list vp;
char *argp;
- table_entry *elts = (table_entry *) t->elts;
+ table_entry *elts = (table_entry *) t->a.elts;
int rv, i;
va_start(vp, t);
@@ -848,7 +940,7 @@
argp = va_arg(vp, char *);
do {
- for (rv = 1, i = 0; rv && (i < t->nelts); ++i) {
+ for (rv = 1, i = 0; rv && (i < t->a.nelts); ++i) {
if (elts[i].key && (!argp || !strcasecmp(elts[i].key, argp))) {
rv = (*comp) (rec, elts[i].key, elts[i].val);
}
Index: main/alloc.h
===================================================================
RCS file: /export/home/cvs/apachen/src/main/alloc.h,v
retrieving revision 1.38
diff -u -r1.38 alloc.h
--- alloc.h 1998/01/07 16:45:59 1.38
+++ alloc.h 1998/01/11 11:37:49
@@ -107,13 +107,13 @@
* Common enough to want common support code ...
*/
- typedef struct {
- pool *pool;
- int elt_size;
- int nelts;
- int nalloc;
- char *elts;
- } array_header;
+typedef struct {
+ pool *pool;
+ int elt_size;
+ int nelts;
+ int nalloc;
+ char *elts;
+} array_header;
API_EXPORT(array_header *) make_array(pool *p, int nelts, int elt_size);
API_EXPORT(void *) push_array(array_header *);
@@ -139,14 +139,14 @@
* currently being used...
*/
- typedef array_header table;
+typedef struct table table;
- typedef struct {
- char *key; /* maybe NULL in future;
- * check when iterating thru table_elts
- */
- char *val;
- } table_entry;
+typedef struct {
+ char *key; /* maybe NULL in future;
+ * check when iterating thru table_elts
+ */
+ char *val;
+} table_entry;
API_EXPORT(table *) make_table(pool *p, int nelts);
API_EXPORT(table *) copy_table(pool *p, const table *);
@@ -161,9 +161,13 @@
API_EXPORT(table *) overlay_tables(pool *p, const table *overlay, const table *base);
-API_EXPORT(array_header *) table_elts(table *);
-
-#define is_empty_table(t) (((t) == NULL)||((t)->nelts == 0))
+/* XXX: these know about the definition of struct table in alloc.c. That
+ * definition is not here because it is supposed to be private, and by not
+ * placing it here we are able to get compile-time diagnostics from modules
+ * written which assume that a table is the same as an array_header. -djg
+ */
+#define table_elts(t) ((array_header *)(t))
+#define is_empty_table(t) (((t) == NULL)||(((array_header *)(t))->nelts == 0))
/* routines to remember allocation of other sorts of things...
* generic interface first. Note that we want to have two separate
Index: main/http_main.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/http_main.c,v
retrieving revision 1.263
diff -u -r1.263 http_main.c
--- http_main.c 1998/01/07 16:46:06 1.263
+++ http_main.c 1998/01/11 11:37:54
@@ -167,6 +167,13 @@
DEF_Explain
+#ifdef GPROF
+extern void moncontrol(int);
+#define MONCONTROL(x) moncontrol(x)
+#else
+#define MONCONTROL(x)
+#endif
+
#ifndef MULTITHREAD
/* this just need to be anything non-NULL */
void *dummy_mutex = &dummy_mutex;
@@ -3181,6 +3188,7 @@
if (!pid) {
RAISE_SIGSTOP(MAKE_CHILD);
+ MONCONTROL(1);
/* Disable the restart signal handlers and enable the just_die stuff.
* Note that since restart() just notes that a restart has been
* requested there's no race condition here.
@@ -3376,8 +3384,12 @@
is_graceful = 0;
++generation;
- if (!one_process)
+ if (!one_process) {
detach();
+ }
+ else {
+ MONCONTROL(1);
+ }
my_pid = getpid();
@@ -3587,6 +3599,8 @@
int main(int argc, char *argv[])
{
int c;
+
+ MONCONTROL(0);
#ifdef AUX
(void) set42sig();
Index: modules/standard/mod_cgi.c
===================================================================
RCS file: /export/home/cvs/apachen/src/modules/standard/mod_cgi.c,v
retrieving revision 1.65
diff -u -r1.65 mod_cgi.c
--- mod_cgi.c 1998/01/07 16:46:46 1.65
+++ mod_cgi.c 1998/01/11 11:37:59
@@ -190,7 +190,7 @@
static int log_script(request_rec *r, cgi_server_conf * conf, int ret,
char *dbuf, char *sbuf, BUFF *script_in, BUFF *script_err)
{
- table *hdrs_arr = r->headers_in;
+ array_header *hdrs_arr = table_elts(r->headers_in);
table_entry *hdrs = (table_entry *) hdrs_arr->elts;
char argsbuffer[HUGE_STRING_LEN];
FILE *f;
@@ -227,7 +227,7 @@
}
fputs("%response\n", f);
- hdrs_arr = r->err_headers_out;
+ hdrs_arr = table_elts(r->err_headers_out);
hdrs = (table_entry *) hdrs_arr->elts;
for (i = 0; i < hdrs_arr->nelts; ++i) {
Index: modules/standard/mod_env.c
===================================================================
RCS file: /export/home/cvs/apachen/src/modules/standard/mod_env.c,v
retrieving revision 1.18
diff -u -r1.18 mod_env.c
--- mod_env.c 1998/01/07 16:46:48 1.18
+++ mod_env.c 1998/01/11 11:37:59
@@ -124,6 +124,7 @@
table *new_table;
table_entry *elts;
+ array_header *arr;
int i;
const char *uenv, *unset;
@@ -140,9 +141,10 @@
new_table = copy_table(p, base->vars);
- elts = (table_entry *) add->vars->elts;
+ arr = table_elts(add->vars);
+ elts = (table_entry *)arr->elts;
- for (i = 0; i < add->vars->nelts; ++i) {
+ for (i = 0; i < arr->nelts; ++i) {
table_set(new_table, elts[i].key, elts[i].val);
}
Index: modules/standard/mod_include.c
===================================================================
RCS file: /export/home/cvs/apachen/src/modules/standard/mod_include.c,v
retrieving revision 1.62
diff -u -r1.62 mod_include.c
--- mod_include.c 1998/01/08 03:16:48 1.62
+++ mod_include.c 1998/01/11 11:38:00
@@ -2013,14 +2013,15 @@
{
char tag[MAX_STRING_LEN];
char *tag_val;
- table_entry *elts = (table_entry *) r->subprocess_env->elts;
+ array_header *arr = table_elts(r->subprocess_env);
+ table_entry *elts = (table_entry *) arr->elts;
int i;
if (!(tag_val = get_tag(r->pool, in, tag, sizeof(tag), 1))) {
return 1;
}
else if (!strcmp(tag, "done")) {
- for (i = 0; i < r->subprocess_env->nelts; ++i) {
+ for (i = 0; i < arr->nelts; ++i) {
rvputs(r, elts[i].key, "=", elts[i].val, "\n", NULL);
}
return 0;
Index: modules/standard/mod_negotiation.c
===================================================================
RCS file: /export/home/cvs/apachen/src/modules/standard/mod_negotiation.c,v
retrieving revision 1.63
diff -u -r1.63 mod_negotiation.c
--- mod_negotiation.c 1998/01/07 16:46:54 1.63
+++ mod_negotiation.c 1998/01/11 11:38:01
@@ -1811,7 +1811,7 @@
int vary_by_language = 0;
int vary_by_charset = 0;
int vary_by_encoding = 0;
- array_header *hdrs;
+ table *hdrs;
/* Put headers into err_headers_out, new send_http_header()
* outputs both headers_out and err_headers_out */
Index: modules/standard/mod_setenvif.c
===================================================================
RCS file: /export/home/cvs/apachen/src/modules/standard/mod_setenvif.c,v
retrieving revision 1.11
diff -u -r1.11 mod_setenvif.c
--- mod_setenvif.c 1998/01/07 16:46:56 1.11
+++ mod_setenvif.c 1998/01/11 11:38:03
@@ -302,9 +302,10 @@
}
if (!regexec(b->preg, val, 0, NULL, 0)) {
- elts = (table_entry *) b->features->elts;
+ array_header *arr = table_elts(b->features);
+ elts = (table_entry *) arr->elts;
- for (j = 0; j < b->features->nelts; ++j) {
+ for (j = 0; j < arr->nelts; ++j) {
if (!strcmp(elts[j].val, "!")) {
table_unset(r->subprocess_env, elts[j].key);
}
Re: misc profiling comments
Posted by Dean Gaudet <dg...@arctic.org>.
On Sat, 10 Jan 1998, Marc Slemko wrote:
> On Sat, 10 Jan 1998, Dean Gaudet wrote:
>
> > I've got the beginnings of a hashed version of the table_* stuff. It's
> > not a full hash table, that would be too heavy. There's too many tables,
>
> Unless you can merge them.
>
> No, I don't think you can do it because destruction would impose too much
> overhead. Well, you could merge a couple... probably not useful though.
>
> Your idea sounds reasonable.
Here it is if you want to play with it. This seems to cut the strcasecmp
calls in half.
The hash function needs work, here's the current distribution of
r->headers_in hash values:
0 Server
5 Connection
6 If-Match
6 Last-Modified
7 If-None-Match
7 User-Agent
8 Accept
8 Accept-Ranges
9 Date
12 If-Modified-Since
14 Host
14 If-Unmodified-Since
17 Request-Range
21 Content-Length
21 Content-Type
24 Transfer-Encoding
27 Range
29 ETag
More wins will come when the hash function splits up all of these... plus
whatever the common headers are that clients send.
But I'm not seeing a huge improvement with this in. Like a percent or so.
Oh there's two other hacks in this patch. MONCONTROL is used to turn
off/on gprof profiling so that it only happens in the children (and fixes
yet another gprof problem under linux, I can't help but wonder if anyone
else even uses gprof under linux).
END_HACK is a big speed win, but not portable... and I've got a patch from
Dima that I'm working with that's more portable.
Dean
Re: misc profiling comments
Posted by Marc Slemko <ma...@worldgate.com>.
On Sat, 10 Jan 1998, Dean Gaudet wrote:
> I've got the beginnings of a hashed version of the table_* stuff. It's
> not a full hash table, that would be too heavy. There's too many tables,
Unless you can merge them.
No, I don't think you can do it because destruction would impose too much
overhead. Well, you could merge a couple... probably not useful though.
Your idea sounds reasonable.
[...]
>
> BTW -X really skews the results because of the use of alarm(). At least
> it does under Linux. You should either hack your libc the way I've hacked
> linux's or do the other trick I mentioned -- make a module with a
> child_exit that creates a directory for the child to chdir() to before
> calling exit().
Good idea.
-X doesn't really impact the things I was looking at, but it does impact
reality...
Re: misc profiling comments
Posted by Dmitry Khrustalev <di...@bog.msu.su>.
On Sat, 10 Jan 1998, Dean Gaudet wrote:
> invoke_handler, interesting... I bet you we could short-circuit this with
> a config-time pass through the table to build a tree or something. It is
> constant after config-time.
Here is what i use:
--- /usr/local/apache/ap/apachen/src/main/http_config.c Sat Jan 10 18:38:43 1998
+++ http_config.c Sun Jan 11 13:48:59 1998
@@ -413,58 +413,91 @@
return run_method(r, offsets_into_method_ptrs.auth_checker, 0);
}
-int invoke_handler(request_rec *r)
-{
- module *modp;
- handler_rec *handp;
- char *content_type = r->content_type ? r->content_type : default_type(r);
- char *handler, *p;
-
- if ((p = strchr(content_type, ';')) != NULL) { /* MIME type arguments */
- while (p > content_type && p[-1] == ' ')
- --p; /* strip trailing spaces */
- content_type = pstrndup(r->pool, content_type, p - content_type);
- }
- handler = r->handler ? r->handler : content_type;
+static handler_rec *handlers;
+static handler_rec *wildhandlers;
- /* Pass one --- direct matches */
+static void init_handlers(pool *p) {
+ module *modp;
+ int nhandlers = 0;
+ int nwildhandlers = 0;
+ handler_rec *handp, *ph, *pw;
+ char *ct;
for (modp = top_module; modp; modp = modp->next) {
if (!modp->handlers)
continue;
-
for (handp = modp->handlers; handp->content_type; ++handp) {
- if (!strcasecmp(handler, handp->content_type)) {
- int result = (*handp->handler) (r);
-
- if (result != DECLINED)
- return result;
- }
- }
+ if (strchr(handp->content_type, '*')) {
+ nwildhandlers ++;
+ } else {
+ nhandlers ++;
+ }
+ }
}
-
- /* Pass two --- wildcard matches */
-
+ ph = handlers = palloc(p, sizeof(handler_rec)*(nhandlers + 1));
+ pw = wildhandlers = palloc(p, sizeof(handler_rec)*(nwildhandlers + 1));
for (modp = top_module; modp; modp = modp->next) {
if (!modp->handlers)
continue;
-
for (handp = modp->handlers; handp->content_type; ++handp) {
- char *starp = strchr(handp->content_type, '*');
- int len;
+ ct = pstrdup(p, handp->content_type);
+ str_tolower(ct);
+ if (strchr(handp->content_type, '*')) {
+ pw->content_type = ct;
+ pw->handler = handp->handler;
+ pw ++;
+ } else {
+ ph->content_type = ct;
+ ph->handler = handp->handler;
+ ph ++;
+ }
+ }
+ }
+ pw->content_type = NULL;
+ pw->handler = NULL;
+ ph->content_type = NULL;
+ ph->handler = NULL;
+}
- if (!starp)
- continue;
+int invoke_handler(request_rec *r)
+{
+ handler_rec *handp;
+ char *content_type = pstrdup(r->pool, r->content_type ? r->content_type : default_type(r));
+ char *handler, *p;
- len = starp - handp->content_type;
+ if ((p = strchr(content_type, ';')) != NULL) { /* MIME type arguments */
+ while (p > content_type && p[-1] == ' ')
+ --p; /* strip trailing spaces */
+ p[0] = '\0';
+ }
+ handler = r->handler ? pstrdup(r->pool, r->handler) : content_type;
+ str_tolower(handler);
- if (!len || !strncasecmp(handler, handp->content_type, len)) {
- int result = (*handp->handler) (r);
+ /* Pass one --- direct matches */
- if (result != DECLINED)
- return result;
- }
- }
+ for (handp = handlers; handp->content_type; ++handp) {
+ if (!strcmp(handler, handp->content_type)) {
+ int result = (*handp->handler) (r);
+
+ if (result != DECLINED)
+ return result;
+ }
+ }
+
+ /* Pass two --- wildcard matches */
+
+ for (handp = wildhandlers; handp->content_type; ++handp) {
+ char *starp = strchr(handp->content_type, '*');
+ int len;
+
+ len = starp - handp->content_type;
+
+ if (!len || !strncmp(handler, handp->content_type, len)) {
+ int result = (*handp->handler) (r);
+
+ if (result != DECLINED)
+ return result;
+ }
}
return NOT_IMPLEMENTED;
@@ -1212,6 +1245,7 @@
for (m = top_module; m; m = m->next)
if (m->init)
(*m->init) (s, p);
+ init_handlers(p);
}
void child_init_modules(pool *p, server_rec *s)
Re: misc profiling comments
Posted by Dean Gaudet <dg...@arctic.org>.
I've got the beginnings of a hashed version of the table_* stuff. It's
not a full hash table, that would be too heavy. There's too many tables,
and the only "busy" ones are r->headers_in and r->headers_out (I wrote
some code to monitor all the table ops). I think I forgot to post this
analysis.
Rather than special case those two I'm putting an extra 32-bits (or maybe
64, whatever aligns it right) into the table header. If a bit in the
bitvector is 0, then it's known that no element matching that hash is in
the table. This gives a short-circuit for all the table functions. I
think it'll do the trick.
mod_dir and mod_negotiation do a lot of things that are annoying. Petr
Lampa over a year ago was going to work on it but I guess he never got a
chance. It's safe to say that a per-request cache of directory entries
would really help. Try "DirectoryIndex index" sometime and watch the
fireworks.
invoke_handler, interesting... I bet you we could short-circuit this with
a config-time pass through the table to build a tree or something. It is
constant after config-time.
BTW -X really skews the results because of the use of alarm(). At least
it does under Linux. You should either hack your libc the way I've hacked
linux's or do the other trick I mentioned -- make a module with a
child_exit that creates a directory for the child to chdir() to before
calling exit().
Dean
On Sat, 10 Jan 1998, Marc Slemko wrote:
> 4587 requests in -X mode, 13.22 seconds total.
>
> 707770 calls to strcasecmp. 376216 from table_get and ~500000 from all
> the table_* functions and 110112 from invoke_handler. Taking 0.40
> seconds.
>
> 13777 calls to stat() taking 1.14 seconds. Would be easy to whack this
> out for this benchmark with a stat() cache, too bad reality isn't all one
> file.
>
> (yes, I was getting / which needs mod_dir to play...)
>
> The biggest thing that sticks out is the table_* functions. Yea, context
> switching is probably the biggest thing but I'm not looking at child
> overhead and we know the solution to that...
>
>