You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Jeff Trawick <tr...@attglobal.net> on 2002/04/09 20:33:45 UTC

[PATCH] lower CPU for sending data via sendfile()

(This should be extended to more than just sendfile().  apr_send() and
apr_writev() need it too.)

This patch isn't complete (only handles Linux sendfile(), and not very
smartly at that) but it shows how ripe this area is for some easy
improvements.  Any eager takers?

I tried to simulate the current apache.org scenario -- heavily loaded
with people downloading httpd-2.0.35.tar.gz.  I used a pretty decent
network connection (broadband via cable modem).  Typically 20K would
get acked every 140ms or so.

Before this patch: 556 syscalls required by the server to send Apache
After this patch : 381 syscalls required by the server to send Apache

A more effective but less portable patch for speeding up
apr_sendfile() would be to use the SO_SNDTIMEO socket option on those
relatively few systems which support it.  Since apr_sendfile() has
different implementations per platform anyway* it isn't much loss on
cleanness.

*but while OS/390 and AIX both have the same send_file() syscall, only
AIX of those two has SO_SNDTIMEO, so that code sharing would
potentially be lost/compromised

Index: srclib/apr/include/apr_network_io.h
===================================================================
RCS file: /home/cvs/apr/include/apr_network_io.h,v
retrieving revision 1.119
diff -u -r1.119 apr_network_io.h
--- srclib/apr/include/apr_network_io.h	13 Mar 2002 20:39:14 -0000	1.119
+++ srclib/apr/include/apr_network_io.h	9 Apr 2002 18:25:26 -0000
@@ -122,7 +122,8 @@
 				   * read, in cases where the app expects
 				   * that an immediate read would fail.)
 				   */
-
+#define APR_INCOMPLETE_WRITE 8192 /* like APR_INCOMPLETE_READ, but for write
+                                   */
 #define APR_POLLIN    0x001 
 #define APR_POLLPRI   0x002
 #define APR_POLLOUT   0x004
Index: srclib/apr/network_io/unix/sendrecv.c
===================================================================
RCS file: /home/cvs/apr/network_io/unix/sendrecv.c,v
retrieving revision 1.78
diff -u -r1.78 sendrecv.c
--- srclib/apr/network_io/unix/sendrecv.c	13 Mar 2002 20:39:25 -0000	1.78
+++ srclib/apr/network_io/unix/sendrecv.c	9 Apr 2002 18:25:26 -0000
@@ -323,6 +323,11 @@
         }
     }
 
+    if (sock->netmask & APR_INCOMPLETE_WRITE) {
+        sock->netmask &= ~APR_INCOMPLETE_WRITE;
+        goto do_select;
+    }
+
     do {
         rv = sendfile(sock->socketdes,	/* socket */
         	      file->filedes,	/* open file descriptor of the file to be sent */
@@ -334,6 +339,7 @@
     if (rv == -1 && 
         (errno == EAGAIN || errno == EWOULDBLOCK) && 
         sock->timeout > 0) {
+do_select:
 	arv = apr_wait_for_io_or_timeout(sock, 0);
 	if (arv != APR_SUCCESS) {
 	    *len = 0;
@@ -346,6 +352,9 @@
         		      &off,	/* where in the file to start */
         		      *len);	/* number of bytes to send */
             } while (rv == -1 && errno == EINTR);
+            if (sock->timeout && rv < *len) {
+                sock->netmask |= APR_INCOMPLETE_WRITE;
+            }
         }
     }
 

-- 
Jeff Trawick | trawick@attglobal.net
Born in Roswell... married an alien...

Re: [PATCH] lower CPU for sending data via sendfile()

Posted by Brian Pane <bp...@pacbell.net>.
Jeff Trawick wrote:

>(This should be extended to more than just sendfile().  apr_send() and
>apr_writev() need it too.)
>

I just checked in the changes for everything except the Solaris
sendfilev case.  I'll work on that one later today.

--Brian



Re: [PATCH] lower CPU for sending data via sendfile()

Posted by Brian Pane <bp...@pacbell.net>.
Jeff Trawick wrote:

>Brian Pane <bp...@pacbell.net> writes:
>
>>Jeff Trawick wrote:
>>
>>>+    if (sock->netmask & APR_INCOMPLETE_WRITE) {
>>>+        sock->netmask &= ~APR_INCOMPLETE_WRITE;
>>>+        goto do_select;
>>>+    }
>>>+
>>>    do {
>>>        rv = sendfile(sock->socketdes,	/* socket */
>>>        	      file->filedes,	/* open file descriptor of the file to be sent */
>>>
>>Without this patch, what was happening?  Did you get EAGAIN on the
>>sendfile call?
>>
>
>yeah, a wasted (EAGAIN) sendfile call for every good sendfile call
>

Got it...  I'll add the same fix for the writev case tomorrow
if nobody else gets to it first.

--Brian



Re: [PATCH] lower CPU for sending data via sendfile()

Posted by Jeff Trawick <tr...@attglobal.net>.
Brian Pane <bp...@pacbell.net> writes:

> Jeff Trawick wrote:
> 
> >+    if (sock->netmask & APR_INCOMPLETE_WRITE) {
> >+        sock->netmask &= ~APR_INCOMPLETE_WRITE;
> >+        goto do_select;
> >+    }
> >+
> >     do {
> >         rv = sendfile(sock->socketdes,	/* socket */
> >         	      file->filedes,	/* open file descriptor of the file to be sent */
> >
> 
> Without this patch, what was happening?  Did you get EAGAIN on the
> sendfile call?

yeah, a wasted (EAGAIN) sendfile call for every good sendfile call

-- 
Jeff Trawick | trawick@attglobal.net
Born in Roswell... married an alien...

Re: [PATCH] lower CPU for sending data via sendfile()

Posted by Brian Pane <bp...@pacbell.net>.
Jeff Trawick wrote:

>+    if (sock->netmask & APR_INCOMPLETE_WRITE) {
>+        sock->netmask &= ~APR_INCOMPLETE_WRITE;
>+        goto do_select;
>+    }
>+
>     do {
>         rv = sendfile(sock->socketdes,	/* socket */
>         	      file->filedes,	/* open file descriptor of the file to be sent */
>

Without this patch, what was happening?  Did you get EAGAIN on the
sendfile call?

--Brian