Jeff King <peff@xxxxxxxx> wrote: > On Tue, Aug 09, 2016 at 11:47:31PM +0000, Eric Wong wrote: > > > Avoid waking up the readers for unnecessary context switches for > > each line of header data being written, as all the headers are > > written in short succession. > > > > It is unlikely any HTTP/1.x server would want to read a CGI > > response one-line-at-a-time and trickle each to the client. > > Instead, I'd expect HTTP servers want to minimize syscall and > > TCP/IP framing overhead by trying to send all of its response > > headers in a single syscall or even combining the headers and > > first chunk of the body with MSG_MORE or writev. > > > > Verified by strace-ing response parsing on the CGI side. > > I don't think this is wrong to do, but it does feel like it makes the > code slightly more brittle (you have to pass around the strbuf and > remember to initialize it and end_headers() when you're done), for not > much benefit. Yes, I was nervous about some of these changes and had to look it over several times. I think the problem was the code initially assumed global state while this change localizes it; so this ought to make future changes easier. Thanks to you and Junio for the extra eyes. > Using some kind of buffered I/O would be nicer, as then you would get > nice-sized chunks without having to impact the code. I wonder if just > using stdio here would be that bad. The place it usually sucks is in > complex error handling, but we don't care about that at all here (I > think we are basically happy to write until we get SIGPIPE). stdio to a non-blocking pipe (if so setup by a caller) could be problematic portability-wise. And reliance on SIGPIPE would hurt reusability if this were eventually factored into a standalone daemon using libmicrohttpd or similar. > I dunno. I suspect the performance improvement from your patch is > marginal, but it's not like the resulting code is all _that_ complex. So > I guess I am OK either way, just not enthused. Yes, marginal, but I still get annoyed to see extra lines from strace (maybe I trace syscalls too much :x). But I think it's also a baby step which opens up potential for the future. I have nothing planned at the moment, but who knows in a year or five... > > --- > > I admit I only noticed this because I was being lazy when > > implementing the reader-side on an HTTP server by making > > a single read(2) call :x > > The trouble is that your HTTP server is still broken. Now it's just > broken in an unpredictable and racy way, because the OS may still split > the write at PIPE_BUF boundaries. (Though given that this is not in the > commit message, I suspect you know this patch is not an excuse not to > fix your HTTP server). Of course :) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html