dgaudet 98/01/26 21:35:34
Modified: . STATUS src CHANGES src/main buff.c Log: really fix Rasmus' chunking problem this time introduce LARGE_WRITE_THRESHOLD tunable for determining when large_write() is a good idea Reviewed by: Rasmus Lerdorf Revision Changes Path 1.132 +1 -1 apachen/STATUS Index: STATUS =================================================================== RCS file: /export/home/cvs/apachen/STATUS,v retrieving revision 1.131 retrieving revision 1.132 diff -u -r1.131 -r1.132 --- STATUS 1998/01/26 19:50:05 1.131 +++ STATUS 1998/01/27 05:35:22 1.132 @@ -70,7 +70,7 @@ * Dean's [PATCH] ap_snprintf should be more sane (fwd) * Jim's/Ken's move of main/util_snprintf.c to ap/ap_snprintf.c * [PATCH] Re: [BUGFIXES] Wrong GID for PID file and UMASK for logs - * Dean's [PATCH] fix Rasmus' chunking error + * Dean's [PATCH] fix Rasmus' chunking error (take 2, really fix it) * [PATCH] PR#1366: fix result of send_fd_length * Ben Hyde's [PATCH] Finish suite of mutex ops for non-threaded platforms * Ben Hyde's [PATCH] Serialize the update to pool.sub_* in destroy_pool 1.601 +15 -5 apachen/src/CHANGES Index: CHANGES =================================================================== RCS file: /export/home/cvs/apachen/src/CHANGES,v retrieving revision 1.600 retrieving revision 1.601 diff -u -r1.600 -r1.601 --- CHANGES 1998/01/26 19:50:08 1.600 +++ CHANGES 1998/01/27 05:35:27 1.601 @@ -241,11 +241,21 @@ *) send_fd_length() did not calculate total_bytes_sent properly. [Ben Reser <[EMAIL PROTECTED]>] PR#1366 - *) The large_write() changes tickled a bug in bputc(), this would - show up as certain modules not working with Internet Explorer 4.0. - Fix this bug, and also fix a performance bug related to bputc() - causing a large_write() -- don't do large_write() unless there's - at least two bytes to write. [Dean Gaudet] + *) The bputc() macro was not properly integrated with the chunking + code; in many cases modules using bputc() could cause completely + bogus chunked output. (Typically this will show up as problems + with Internet Explorer 4.0 reading a page, but other browsers + having no problem.) [Dean Gaudet] + + *) Create LARGE_WRITE_THRESHOLD define which determines how many + bytes have to be supplied to bwrite() before it will consider + doing a writev() to assemble multiple buffers in one system + call. This is critical for modules such as mod_include, + mod_autoindex, mod_php3 which all use bputc()/bputs() of smaller + strings in some cases. The result would be extra effort + setting up writev(), and in many cases extra effort building + chunks. The default is 31, it can be override at compile + time. [Dean Gaudet] *) Move the gid switching code into the child so that log files and pid files are opened with the root gid. 1.60 +39 -12 apachen/src/main/buff.c Index: buff.c =================================================================== RCS file: /export/home/cvs/apachen/src/main/buff.c,v retrieving revision 1.59 retrieving revision 1.60 diff -u -r1.59 -r1.60 --- buff.c 1998/01/26 16:46:07 1.59 +++ buff.c 1998/01/27 05:35:32 1.60 @@ -71,7 +71,21 @@ #include <bstring.h> /* for IRIX, FD_SET calls bzero() */ #endif +#ifndef DEFAULT_BUFSIZE #define DEFAULT_BUFSIZE (4096) +#endif + +/* bwrite()s of greater than this size can result in a large_write() call, + * which can result in a writev(). It's a little more work to set up the + * writev() rather than copy bytes into the buffer, so we don't do it for small + * writes. This is especially important when chunking (which is a very likely + * source of small writes if it's a module using bputc/bputs)... because we + * have the expense of actually building two chunks for each writev(). + */ +#ifndef LARGE_WRITE_THRESHOLD +#define LARGE_WRITE_THRESHOLD 31 +#endif + /* * Buffered I/O routines. @@ -347,7 +361,13 @@ } /* - * start chunked encoding + * Start chunked encoding. + * + * Note that in order for bputc() to be an efficient macro we have to guarantee + * that start_chunk() has always been called on the buffer before we leave any + * routine in this file. Said another way, if a routine here uses end_chunk() + * and writes something on the wire, then it has to call start_chunk() or set + * an error condition before returning. */ static void start_chunk(BUFF *fb) { @@ -865,16 +885,9 @@ API_EXPORT(int) bflsbuf(int c, BUFF *fb) { char ss[1]; - int rc; ss[0] = c; - rc = bwrite(fb, ss, 1); - /* We do start_chunk() here so that the bputc macro can be smaller - * and faster - */ - if (rc == 1 && (fb->flags & B_CHUNK)) - start_chunk(fb); - return rc; + return bwrite(fb, ss, 1); } /* @@ -1069,7 +1082,6 @@ int nvec; char chunksize[16]; - nvec = 0; /* it's easiest to end the current chunk */ if (fb->flags & B_CHUNK) { end_chunk(fb); @@ -1103,7 +1115,13 @@ } fb->outcnt = 0; - return writev_it_all(fb, vec, nvec) ? -1 : nbyte; + if (writev_it_all(fb, vec, nvec)) { + return -1; + } + else if (fb->flags & B_CHUNK) { + start_chunk(fb); + } + return nbyte; } #endif @@ -1154,7 +1172,8 @@ * us to use writev() too frequently. In those cases we really should just * start a new buffer. */ - if (fb->outcnt > 0 && nbyte > 1 && nbyte + fb->outcnt >= fb->bufsiz) { + if (fb->outcnt > 0 && nbyte > LARGE_WRITE_THRESHOLD + && nbyte + fb->outcnt >= fb->bufsiz) { return large_write(fb, buf, nbyte); } #endif @@ -1215,6 +1234,10 @@ * original buffer until there is less than bufsiz left. Note that we * use bcwrite() to do this for us, it will do the chunking so that * we don't have to dink around building a chunk in our own buffer. + * + * Note also that bcwrite never does a partial write if we're chunking, + * so we're guaranteed to either end in an error state, or make it + * out of this loop and call start_chunk() below. */ while (nbyte >= fb->bufsiz) { i = bcwrite(fb, buf, nbyte); @@ -1279,6 +1302,10 @@ */ if (fb->flags & B_EOUT) return -1; + } + + if (fb->flags & B_CHUNK) { + start_chunk(fb); } return 0;