Important performance patch - Squid 1.2beta23 PL4

Hiya,

Please make sure this gets into the beta24 release Duane.

        The following patch fixes a problem I've long known existed but
didn't think would be an issue until our loads crept up so high. The
location of the combining writes method in diskHandleWrite() in disk.c
was badly located. Because it was called with every new piece of data
added to the write queue, it would combine multiple requests over and
over again until the current disk write completed. On a disk which is
taking 100ms to complete a write @ 100 TCP hits/sec, this roughly amounts
to copying 100K/sec more data than is needed. If the cache then becomes
loaded and CPU is an issue, this figure could blow out to about 500K/sec of
completely unneeded data copying. If CPU is an issue, then this is clearly
a waste. Further it GREATLY reduces the load on the malloc() library.

The below patch moves the write combining into a separate function, and then
calls this once on every COMPLETED write, so the function will only batch
together stuff once per disk write, hence turning a polynomial function
into a linear one.

Stew.

diff -u -r1.1 disk.c
--- disk.c 1998/08/20 22:37:13 1.1
+++ disk.c 1998/08/20 22:56:11
@@ -158,25 +158,28 @@
 }
 
 
-/* write handler */
+#if USE_ASYNC_IO
+/* This function has the purpose of combining multiple writes. This is to */
+/* facilitate the ASYNC_IO option since it can only guarantee 1 write to a */
+/* file per trip around the comm.c select() loop. That's bad because more */
+/* than 1 write can be made to the access.log file per trip, and so this */
+/* code is purely designed to help batch multiple sequential writes to */
+/* the access.log file. Squid will never issue multiple writes for any */
+/* other file type during 1 trip around the select() loop */
+
 static void
-diskHandleWrite(int fd, void *notused)
+diskCombineWrites(struct _fde_disk *fdd)
 {
     int len = 0;
- disk_ctrl_t *ctrlp;
     dwrite_q *q = NULL;
     dwrite_q *wq = NULL;
- fde *F = &fd_table[fd];
- struct _fde_disk *fdd = &F->disk;
- if (!fdd->write_q)
- return;
- debug(6, 3) ("diskHandleWrite: FD %d\n", fd);
- /* We need to combine subsequent write requests after the first */
+
+ /* We need to combine multiple write requests on an FD's write queue */
     /* But only if we don't need to seek() in between them, ugh! */
     /* XXX This currently ignores any seeks (file_offset) */
- if (fdd->write_q->next != NULL && fdd->write_q->next->next != NULL) {
+ if (fdd->write_q != NULL && fdd->write_q->next != NULL) {
        len = 0;
- for (q = fdd->write_q->next; q != NULL; q = q->next)
+ for (q = fdd->write_q; q != NULL; q = q->next)
            len += q->len - q->buf_offset;
        wq = xcalloc(1, sizeof(dwrite_q));
        wq->buf = xmalloc(len);
@@ -185,18 +188,35 @@
        wq->next = NULL;
        wq->free_func = xfree;
        do {
- q = fdd->write_q->next;
+ q = fdd->write_q;
            len = q->len - q->buf_offset;
            xmemcpy(wq->buf + wq->len, q->buf + q->buf_offset, len);
            wq->len += len;
- fdd->write_q->next = q->next;
+ fdd->write_q = q->next;
            if (q->free_func)
                (q->free_func) (q->buf);
            safe_free(q);
- } while (fdd->write_q->next != NULL);
+ } while (fdd->write_q != NULL);
        fdd->write_q_tail = wq;
- fdd->write_q->next = wq;
+ fdd->write_q = wq;
     }
+}
+#endif
+
+
+/* write handler */
+static void
+diskHandleWrite(int fd, void *notused)
+{
+ int len = 0;
+ disk_ctrl_t *ctrlp;
+ dwrite_q *q = NULL;
+ dwrite_q *wq = NULL;
+ fde *F = &fd_table[fd];
+ struct _fde_disk *fdd = &F->disk;
+ if (!fdd->write_q)
+ return;
+ debug(6, 3) ("diskHandleWrite: FD %d\n", fd);
     ctrlp = xcalloc(1, sizeof(disk_ctrl_t));
     ctrlp->fd = fd;
 #if USE_ASYNC_IO
@@ -325,6 +345,7 @@
        F->flags.write_daemon = 0;
     } else {
        /* another block is queued */
+ diskCombineWrites(fdd);
        cbdataLock(fdd->wrt_handle_data);
        commSetSelect(fd, COMM_SELECT_WRITE, diskHandleWrite, NULL, 0);
        F->flags.write_daemon = 1;
Received on Tue Jul 29 2003 - 13:15:51 MDT