OSDir


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

AW: ap_request_core_filter issues



> -----Ursprüngliche Nachricht-----
> Von: Yann Ylavic <ylavic.dev@xxxxxxxxx>
> Gesendet: Dienstag, 23. Oktober 2018 13:09
> An: httpd-dev <dev@xxxxxxxxxxxxxxxx>
> Betreff: Re: ap_request_core_filter issues
> 
> On Thu, Oct 18, 2018 at 1:52 PM Joe Orton <jorton@xxxxxxxxxx> wrote:
> >
> > On Thu, Oct 18, 2018 at 12:51:06PM +0200, Ruediger Pluem wrote:
> > > >>>
> > > >>> Curiously inefficient writev use when stracing the process,
> though,
> > > >>> dunno what's going on there (trunk/prefork):
> > > >>>
> > > >>> writev(46, [{iov_base="\r\n", iov_len=2}], 1) = 2
> > > >>> writev(46, [{iov_base="1f84\r\n", iov_len=6}], 1) = 6
> > > >>> writev(46, [{iov_base="<D:lockdiscovery/>\n<D:getcontent"...,
> iov_len=7820}], 1) = 7820
> > > >>> writev(46, [{iov_base="<D:supportedlock>\n<D:lockentry>\n"...,
> iov_len=248}], 1) = 248
> > > >>
> > > >> The reason is ap_request_core_filter. It iterates over the
> brigade and hands over each bucket alone to
> > > >> ap_core_output_filter. IMHO a bug.
> > > >
> > > > How about the attached patch for fixing?
> > >
> > > Scratch that. I guess it is much more complex.
> >
> > Hmm, well it works for me and looks correct, it batches up those the
> > writev(,,1) calls as expected, but it seems YMMV :)
> 
> After a closer look into this, Rüdiger's patch seems indeed incomplete.
> What we possibly want is keep buckets in tmp_bb until it's time to
> pass it down the chain, per ap_filter_reinstate_brigade() logic (i.e.
> FLUSH bucket, size limit, ...).
> 
> I tried to implement that in the attached patch, WDYT?

1. Why removing the META_BUCKET check when reading buckets of length -1? I don't see any sense in
   doing an apr_bucket_read on a META_BUCKET.
2. Apart from 1. I think the flow is correct, but I am a little bit worried if calling ap_filter_reinstate_brigade
   inside the loop iterating over the brigade has a too large performance impact as ap_filter_reinstate_brigade
   iterates itself over the brigade. So we are in O(n^2) with n being the size of the brigade. But to be honest I have
   no good idea yet how to do without. The only thing I can think of is to duplicate and merge the logic of ap_filter_reinstate_brigade
   inside our loop to avoid this as we could gather the stats needed for deciding up to which bucket we need to write while
   doing our iteration of the brigade.


Regards

Rüdiger