> From: Brian Pane [mailto:[EMAIL PROTECTED]] > Ryan Bloom wrote: > > >>From: Brian Pane [mailto:[EMAIL PROTECTED]] > >> > >>Ryan Bloom wrote: > >> > >> > >> > >>>I think we should leave it alone. This is the difference between > >>>benchmarks and the real world. How often do people have 8 requests > >>> > >>> > >in a > > > > > >>>row that total less than 8K? > >>> > >>>As a compromise, there are two other options. You could have the > >>>core_output_filter refuse to buffer more than 2 requests, or you > >>> > >>> > >could > > > > > >>>have the core_output_filter not buffer if the full request is in the > >>>buffer. > >>> > >>> > >>> > >>In your second option, do you mean "full response" rather than "full > >>request"? If so, it's equivalent to what I was proposing yesterday: > >>send the response for each request as soon as we see EOS for that > >> > >> > >request. > > > > > >>I like that approach a lot because it keeps us from doing an extra > >>mmap/memcpy/memunmap write before the write in the real-world case > >>where a client sends a non-pipelined request for a small (<8KB) file > >>over a keepalive connection. > >> > >> > > > >I do mean full response, and that is NOT equivalent to sending the > >response as soon as you see the EOS for that request. EVERY request > >gets its own EOS. If you send the response when you see the EOS, then > >you will have removed all of the buffering for pipelined requests. > > > > -1 on buffering across requests, because the performance problems > caused by the extra mmap+munmap will offset the gain you're trying > to achieve with pipelining.
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
