coar 98/06/12 08:53:26
Modified: src CHANGES
src/modules/standard mod_autoindex.c
Log:
I've been meaning to clean up these integer-as-string comparisons
for a while, and encountered the misplacement of the parent directory
entry during testing. Needing the parent-directory test in a
second location prompted streamlining simplification of the method.
Revision Changes Path
1.911 +9 -0 apache-1.3/src/CHANGES
Index: CHANGES
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/CHANGES,v
retrieving revision 1.910
retrieving revision 1.911
diff -u -r1.910 -r1.911
--- CHANGES 1998/06/10 21:13:26 1.910
+++ CHANGES 1998/06/12 15:53:21 1.911
@@ -1,5 +1,14 @@
Changes with Apache 1.3.1
+ *) Improve performance of directory listings (mod_autoindex) by comparing
+ integer keys (last-modified and size) as integers rather than converting
+ them to strings first. Also use a set of explicit byte tests rather
+ than strcmp() to check for parent directory-ness of an entry. Oh, and
+ make sure the parent directory (if displayed) is *always* listed first
+ regardless of the sort key. Overall performance winnage should be good
+ in CPU time, instruction cache, and memory usage, particularly for large
+ directories. [Ken Coar]
+
*) Add httpd -t (test) option for running configuration syntax tests only.
If something is broken it complains and exits with a return code
non-equal to 0. This can be used manually by the user to check the
Apache
1.79 +119 -60 apache-1.3/src/modules/standard/mod_autoindex.c
Index: mod_autoindex.c
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/modules/standard/mod_autoindex.c,v
retrieving revision 1.78
retrieving revision 1.79
diff -u -r1.78 -r1.79
--- mod_autoindex.c 1998/06/12 11:20:56 1.78
+++ mod_autoindex.c 1998/06/12 15:53:23 1.79
@@ -136,6 +136,37 @@
#define BY_PATH &c_by_path
/*
+ * Return true if the specified string refers to the parent directory (i.e.,
+ * matches ".." or "../"). Hopefully this one call is significantly less
+ * expensive than multiple strcmp() calls.
+ */
+static int is_parent(const char *name)
+{
+ /*
+ * If it's no name at all, it isn't our parent.
+ */
+ if (name == NULL) {
+ return 0;
+ }
+ /*
+ * Nor is it if the name isn't long enough.
+ */
+ if ((name[0] == '\0') || (name[1] == '\0')) {
+ return 0;
+ }
+ /*
+ * Now, IFF the first two bytes are dots, and the third byte is either
+ * EOS (\0) or a slash followed by EOS, we have a match.
+ */
+ if (((name[0] == '.') && (name[1] == '.'))
+ && ((name[2] == '\0')
+ || ((name[2] == '/') && (name[3] == '\0')))) {
+ return 1;
+ }
+ return 0;
+}
+
+/*
* This routine puts the standard HTML header at the top of the index page.
* We include the DOCTYPE because we may be using features therefrom (i.e.,
* HEIGHT and WIDTH attributes on the icons if we're FancyIndexing).
@@ -417,9 +448,7 @@
char *alt;
char *desc;
size_t size;
- char *size_cmp;
time_t lm;
- char *lm_cmp;
struct ent *next;
int ascending;
char key;
@@ -750,20 +779,15 @@
}
ap_destroy_sub_req(rr);
- }
- if (keyid == K_SIZE) {
- p->size_cmp = ap_palloc(r->pool, 14);
- ap_snprintf(p->size_cmp, 14, "%013d", p->size);
}
+ /*
+ * We don't need to take any special action for the file size key. If
+ * we did, it would go here.
+ */
if (keyid == K_LAST_MOD) {
- struct tm *ts = localtime(&p->lm);
-
- if(ts) {
- p->lm_cmp = ap_palloc(r->pool, 15);
- strftime(p->lm_cmp, 15, "%Y%m%d%H%M%S", ts);
+ if (p->lm < 0) {
+ p->lm = 0;
}
- else
- p->lm_cmp=NULL;
}
return (p);
}
@@ -892,7 +916,7 @@
ap_clear_pool(scratch);
- if ((!strcmp(ar[x]->name, "../")) || (!strcmp(ar[x]->name, ".."))) {
+ if (is_parent(ar[x]->name)) {
t = ap_make_full_path(scratch, name, "../");
ap_getparents(t);
if (t[0] == '\0') {
@@ -986,73 +1010,108 @@
}
}
+/*
+ * Compare two file entries according to the sort criteria. The return
+ * is essentially a signum function value.
+ */
static int dsortf(struct ent **e1, struct ent **e2)
{
char *s1;
char *s2;
- char *s3;
+ struct ent *c1;
+ struct ent *c2;
int result;
+ int compare_by_string = 1;
/*
+ * First, see if either of the entries is for the parent directory.
+ * If so, that *always* sorts lower than anything else.
+ */
+ if (is_parent((*e1)->name)) {
+ return -1;
+ }
+ if (is_parent((*e2)->name)) {
+ return 1;
+ }
+ /*
+ * All of our comparisons will be of the c1 entry against the c2 one,
+ * so assign them appropriately to take care of the ordering.
+ */
+ if ((*e1)->ascending) {
+ c1 = *e1;
+ c2 = *e2;
+ }
+ else {
+ c1 = *e2;
+ c2 = *e1;
+ }
+ /*
* Choose the right values for the sort keys.
*/
- switch ((*e1)->key) {
+ switch (c1->key) {
case K_LAST_MOD:
- s1 = (*e1)->lm_cmp;
- s2 = (*e2)->lm_cmp;
+ /*
+ * Since the last-modified time is a monotonically increasing integer,
+ * we can short-circuit this process with a simple comparison.
+ */
+ result = c1->lm - c2->lm;
+ if (result != 0) {
+ result = (result > 0) ? 1 : -1;
+ }
+ compare_by_string = 0;
break;
case K_SIZE:
- s1 = (*e1)->size_cmp;
- s2 = (*e2)->size_cmp;
+ /*
+ * We can pull the same trick with the size as with the mtime.
+ */
+ result = c1->size - c2->size;
+ if (result != 0) {
+ result = (result > 0) ? 1 : -1;
+ }
+ compare_by_string = 0;
break;
case K_DESC:
- s1 = (*e1)->desc;
- s2 = (*e2)->desc;
+ s1 = c1->desc;
+ s2 = c2->desc;
break;
case K_NAME:
default:
- s1 = (*e1)->name;
- s2 = (*e2)->name;
+ s1 = c1->name;
+ s2 = c2->name;
break;
}
- /*
- * If we're supposed to sort in DEscending order, reverse the arguments.
- */
- if (!(*e1)->ascending) {
- s3 = s1;
- s1 = s2;
- s2 = s3;
- }
- /*
- * Take some care, here, in case one string or the other (or both) is
- * NULL.
- */
-
- /*
- * Two valid strings, compare normally.
- */
- if ((s1 != NULL) && (s2 != NULL)) {
- result = strcmp(s1, s2);
- }
- /*
- * Two NULL strings - primary keys are equal (fake it).
- */
- else if ((s1 == NULL) && (s2 == NULL)) {
- result = 0;
- }
- /*
- * s1 is NULL, but s2 is a string - so s2 wins.
- */
- else if (s1 == NULL) {
- result = -1;
- }
- /*
- * Last case: s1 is a string and s2 is NULL, so s1 wins.
- */
- else {
- result = 1;
+ if (compare_by_string) {
+ /*
+ * Take some care, here, in case one string or the other (or both) is
+ * NULL.
+ */
+
+ /*
+ * Two valid strings, compare normally.
+ */
+ if ((s1 != NULL) && (s2 != NULL)) {
+ result = strcmp(s1, s2);
+ }
+ /*
+ * Two NULL strings - primary keys are equal (fake it).
+ */
+ else if ((s1 == NULL) && (s2 == NULL)) {
+ result = 0;
+ }
+ /*
+ * s1 is NULL, but s2 is a string - so s2 wins.
+ */
+ else if (s1 == NULL) {
+ result = -1;
+ }
+ /*
+ * Last case: s1 is a string and s2 is NULL, so s1 wins.
+ */
+ else {
+ result = 1;
+ }
}
/*
* If the keys were equal, the file name is *always* the secondary key -
