> From: Bill Stoddard [mailto:[EMAIL PROTECTED]] > > > From: Brian Pane [mailto:[EMAIL PROTECTED]] > > > Ryan Bloom wrote: > > > >>From: Brian Pane [mailto:[EMAIL PROTECTED]] > > > >>Ryan Bloom wrote:
> > Wait a second. Now you want to stop buffering to fix a completely > > differeny bug. The idea that we can't keep a file_bucket in the brigade > > across requests is only partially true. Take a closer look at what we > > are doing and why when we convert the file to an mmap. That logic was > > added so that we do not leak file descriptors across requests. > > > > However, there are multiple options to fix that. The first, and easiest > > one is to just have the core_output_filter call apr_file_close() after > > it has sent the file. The second is to migrate the apr_file_t to the > > conn_rec's pool if and only if the file needs to survive the request's > > pool being killed. Because we are only migrating file descriptors in > > the edge case, this shouldn't cause a big enough leak to cause a > > problem. > > > > > >You are trying to solve a problem that doesn't exist in the real > > world > > > >IIUIC. Think it through. The problem is that if the page is <1k and > > > >you request that page 8 times on the same connection, then Apache > > will > > > >buffer all of the responses. How often are those conditions going to > > > >happen in the real world? > > > > > > > > > > That's not the problem that I care about. The problem that matters > > > is the one that happens in the real world, as a side-effect of the > > > core_output_filter() code trying to be too clever: > > > - Client opens a keepalive connection to httpd > > > - Cclient requests a file smaller than 8KB > > > - core_output_filter(), upon seeing the file bucket followed by EOS, > > > decides to buffer the output because it has less than 8KB total. > > > - There isn't another request ready to be read (read returns EAGAIN) > > > because the client isn't pipelining its connections. So we then > > > do the writev of the file that we've just finished buffering. > > > > But, this case only happens if the headers + the file are less than 8k. > > If the file is >10k, then this problem doesn't actually exist at all. > > As I said above, there are better ways to fix this than removing all > > ability to pipeline responses. > > > > > Aside from slowing things down for the user, this hurts the > > scalability > > > of the httpd (mmap and munmap don't scale well on multiprocessor > > boxes). > > > > > > What we should be doing in this case is just doing the write > > immediately > > > upon seeing the EOS, rather than penalizing both the client and the > > > server. > > > > By doing that, you are removing ANY benefit to using pipelined requests > > when serving files. Multiple research projects have all found that > > pipelined requests show a performance benefit. In other words, you are > > fixing a performance problem by removing another performance enhancer. > > > > Ryan > > > > Ryan, > Solve the problem to enable setting aside the open fd just long enough to > check for a > pipelined request will nearly completely solve the worst part (the > mmap/munmap) of this > problem. On systems with expensive syscalls, we can do browser detection > and dynamically > determine whether we should attempt the pipelined optimization or not. Not > many browsers > today support pipelining requests, FWIW. That would be a trivial change. I'll have a patch posted for testing later today. Ryan
