On Tue, Oct 23, 2018 at 3:18 PM Plüm, Rüdiger, Vodafone Group <[email protected]> wrote: > > > -----Ursprüngliche Nachricht----- > > Von: Yann Ylavic <[email protected]> > > > > 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.
META usually have a length of 0, not -1 (so we shouldn't read them either). We pretty much everywhere detect morphing buckets with -1 only, META or not, and honestly if a META has length == -1 we probably should consider it's a morphing META and let read figure out :) > 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. Yeah, I have a version which checks only for (APR_BUCKET_IS_FLUSH(bucket) || cumulative_tmp_bb_len >= ap_get_core_module_config()->flush_max_threshold), but I thought I would not open code in the first version to show the idea... Ideally we'd have another ap_filter_xxx helper which could take a bucket (and offset) to start, and also account for f->next(s)' *bytes_in_brigade if asked to... Regards, Yann. > > > Regards > > Rüdiger >
