Re: [PATCH] avoid bflush() when pipelining

[Randy Terbush <ra...@zyzzyva.com>]

I'd vote to get this in today. It would be nice if we can try
to make 1.2b6 a release candidate, and I'm not sure that we
can if we don't get changes like this, and some of the others
I am seeing in the past 24 hours, into 1.2b5.


> As promised, here's a patch which avoids calling bflush() when the client
> is pipelining requests.  In limited testing it appears to work fine for
> non-pipelined clients, and works as advertised for pipelined clients.
> It reduced a 139 packet exchange to only 40 packets.  The test page was
> a page with <img src=""> for each of the icons in the shipped /icons
> directory.
> 
> This and the previous buff.c patch can easily wait for 1.2b6, it'd be nice
> to have them in for 1.2 however.
> 
> Dean
> 
> Index: buff.c
> ===================================================================
> RCS file: /export/home/cvs/apache/src/buff.c,v
> retrieving revision 1.14
> diff -c -3 -r1.14 buff.c
> *** buff.c	1997/01/20 04:28:07	1.14
> --- buff.c	1997/01/25 09:32:58
> ***************
> *** 402,407 ****
> --- 402,438 ----
>   }
>   
>   /*
> +  * Tests if there is data to be read.  Returns 0 if there is data,
> +  * -1 otherwise.
> +  */
> + int
> + btestread(BUFF *fb)
> + {
> +     fd_set fds;
> +     struct timeval tv;
> +     int rv;
> + 
> +     /* the simple case, we've already got data in the buffer */
> +     if( fb->incnt ) {
> + 	return( 0 );
> +     }
> + 
> +     /* otherwise see if the descriptor would block if we try to read it */
> +     FD_ZERO( &fds );
> +     FD_SET( fb->fd_in, &fds );
> +     tv.tv_sec = 0;
> +     tv.tv_usec = 0;
> +     do {
> + #ifdef HPUX
> + 	rv = select( fb->fd_in + 1, (int *)&fds, NULL, NULL, &tv );
> + #else
> + 	rv = select( fb->fd_in + 1, &fds, NULL, NULL, &tv );
> + #endif
> +     } while( rv < 0 && errno == EINTR );
> +     return( rv == 1 ? 0 : -1 );
> + }
> + 
> + /*
>    * Skip data until a linefeed character is read
>    * Returns 1 on success, 0 if no LF found, or -1 on error
>    */
> Index: buff.h
> ===================================================================
> RCS file: /export/home/cvs/apache/src/buff.h,v
> retrieving revision 1.9
> diff -c -3 -r1.9 buff.h
> *** buff.h	1997/01/01 18:10:15	1.9
> --- buff.h	1997/01/25 09:32:58
> ***************
> *** 120,125 ****
> --- 120,126 ----
>   extern int bvputs(BUFF *fb, ...);
>   extern int bprintf(BUFF *fb,const char *fmt,...);
>   extern int vbprintf(BUFF *fb,const char *fmt,va_list vlist);
> + extern int btestread(BUFF *fb);
>   
>   /* Internal routines */
>   extern int bflsbuf(int c, BUFF *fb);
> Index: http_main.c
> ===================================================================
> RCS file: /export/home/cvs/apache/src/http_main.c,v
> retrieving revision 1.110
> diff -c -3 -r1.110 http_main.c
> *** http_main.c	1997/01/24 21:06:33	1.110
> --- http_main.c	1997/01/25 09:32:59
> ***************
> *** 1673,1679 ****
>           if (r) increment_counts(child_num,r,1);
>   #endif
>   	while (r && current_conn->keepalive) {
> ! 	    bflush(conn_io);
>   	    destroy_pool(r->pool);
>   	    (void)update_child_status (child_num, SERVER_BUSY_KEEPALIVE,
>   	     (request_rec*)NULL);
> --- 1673,1682 ----
>           if (r) increment_counts(child_num,r,1);
>   #endif
>   	while (r && current_conn->keepalive) {
> ! 	    /* If there's no request waiting for us to read then flush the
> ! 	     * socket.  This is to avoid tickling a bug in older Netscape
> ! 	     * clients. */
> ! 	    if( btestread(conn_io) == -1 ) bflush(conn_io);
>   	    destroy_pool(r->pool);
>   	    (void)update_child_status (child_num, SERVER_BUSY_KEEPALIVE,
>   	     (request_rec*)NULL);
> ***************
> *** 2142,2148 ****
>   	if (r) process_request (r); /* else premature EOF (ignore) */
>   
>           while (r && conn->keepalive) {
> ! 	    bflush(cio);
>   	    destroy_pool(r->pool);
>               r = read_request (conn);
>               if (r) process_request (r);
> --- 2145,2151 ----
>   	if (r) process_request (r); /* else premature EOF (ignore) */
>   
>           while (r && conn->keepalive) {
> ! 	    if( btestread(cio) == -1 ) bflush(cio);
>   	    destroy_pool(r->pool);
>               r = read_request (conn);
>               if (r) process_request (r);