PATCH: fix unnecessary flushes in hisv6

Chris Caputo ccaputo at alt.net
Thu Jun 2 04:45:21 UTC 2005


My expire process was running slow.  I ran an strace while it was running
and noticed that it was doing ~8k read()s and 48 byte write()s.  A write()  
call for each line in the history file is bad, especially during an
expire.

Turns out hisv6_writeline() calls ftello() and ftello() in glibc was
causing buffered writes to flush.

hisv6 has its own flush behavior, based on things like icdsynccount or
HIS_INCORE flag, but these were being superceded by ftello()'s
implementation.

The following patch makes it so ftello() isn't used, but rather an
internal offset is maintained.

Now when I run expire, the write()s are ~4k at a time, rather than 48
bytes at a time.  Net gain is 1/85th the number of write() system calls
and no llseek() system calls.  This appears to have shaved a minute or two
off my expire, while lowering CPU usage, but I have not done extensive
benchmarking.

I've tested it with my system and it appears to work.  I'd appreciate if
others would test it.  Russ please feel free to integrate this if it looks
good.

Patch at http://www.caputo.com/foss/inn-2.4.2-hisv6_syncfix.patch or
below.

Thanks,
Chris

-----

--- inn-2.4.2-stock/history/hisv6/hisv6-private.h	2004-12-22 04:21:19.000000000 +0000
+++ inn-2.4.2/history/hisv6/hisv6-private.h	2005-06-02 04:24:51.818453837 +0000
@@ -40,6 +40,7 @@
 struct hisv6 {
     char *histpath;
     FILE *writefp;
+    off_t offset;  /* relating to writefp */
     unsigned long nextcheck;
     struct history *history;
     time_t statinterval;
--- inn-2.4.2-stock/history/hisv6/hisv6.c	2004-12-22 04:21:19.000000000 +0000
+++ inn-2.4.2/history/hisv6/hisv6.c	2005-06-02 04:25:43.134788109 +0000
@@ -260,6 +260,7 @@
 	    r = false;
 	}
 	h->writefp = NULL;
+        h->offset = 0;
     }
 
     h->nextcheck = 0;
@@ -308,6 +309,13 @@
 	    hisv6_closefiles(h);
 	    goto fail;
 	}
+	if ((h->offset = ftello(h->writefp)) == -1) {
+	    hisv6_seterror(h, concat("can't ftello ",
+				      h->histpath, " ",
+				      strerror(errno), NULL));
+	    hisv6_closefiles(h);
+	    goto fail;
+	}
 	close_on_exec(fileno(h->writefp), true);
     }
 
@@ -462,6 +470,7 @@
     h->histpath = path ? xstrdup(path) : NULL;
     h->flags = flags;
     h->writefp = NULL;
+    h->offset = 0;
     h->history = history;
     h->readfd = -1;
     h->nextcheck = 0;
@@ -718,7 +727,7 @@
 **  format a history line; `s' should be of at least HISV6_MAXLINE+1
 **  characters (to allow for the \0).
 */
-static bool
+static int
 hisv6_formatline(char *s, const HASH *hash, time_t arrived,
 		  time_t posted, time_t expires, const TOKEN *token)
 {
@@ -750,8 +759,8 @@
 	}
     }
     if (i < 0 || i >= HISV6_MAXLINE)
-	return false;
-    return true;
+	return 0;
+    return i;
 }
 
 
@@ -807,7 +816,7 @@
 {
     int i;
     bool r;
-    off_t offset;
+    int linelen;
     char hisline[HISV6_MAXLINE + 1];
     char location[HISV6_MAX_LOCATION];
 
@@ -823,18 +832,17 @@
 	return false;
     }
 
-    if (hisv6_formatline(hisline, hash, arrived, posted, expires,
-			  token) != true) {
+    if ((linelen = hisv6_formatline(hisline, hash, arrived, posted, expires,
+			  token)) == 0) {
 	hisv6_seterror(h, concat("error formatting history line ",
 				  h->histpath, NULL));
 	return false;
     }	
 
-    offset = ftello(h->writefp);
-    i = fputs(hisline, h->writefp);
-    if (i == EOF ||
+    i = fwrite(hisline, 1, linelen, h->writefp);
+    if (i < linelen ||
 	(!(h->flags & HIS_INCORE) && fflush(h->writefp) == EOF)) {
-	hisv6_errloc(location, (size_t)-1, offset);
+	hisv6_errloc(location, (size_t)-1, h->offset);
 	/* The history line is now an orphan... */
 	hisv6_seterror(h, concat("can't write history ", h->histpath,
 				  location, " ", strerror(errno), NULL));
@@ -842,7 +850,8 @@
 	goto fail;
     }
 
-    r = hisv6_writedbz(h, hash, offset);
+    r = hisv6_writedbz(h, hash, h->offset);
+    h->offset += linelen;  /* increment regardless of error from writedbz */
  fail:
     return r;
 }
@@ -915,7 +924,7 @@
 	char new[HISV6_MAXLINE + 1];
 
 	if (hisv6_formatline(new, &hash, arrived, posted, expires,
-			      token) != true) {
+			      token) == 0) {
  	    hisv6_seterror(h, concat("error formatting history line ",
 				      h->histpath, NULL));
 	    r = false;



More information about the inn-workers mailing list