httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject Re: brigade patch, last time.
Date Mon, 29 Jan 2001 12:39:50 GMT
On Sat, Jan 27, 2001 at 09:42:40AM -0800, rbb@covalent.net wrote:
>...
> > 1) large writes consume large amounts of working set (copy into a bucket)
> > 2) medium-ish writes cause extra copies (anything larger than 9k cause an
> >    extra copy to the heap, rather than delivery down the chain)
> > 3) two ordering problems: within the brigade, and between ap_r* and
> >    ap_pass_brigade.
> 
> The problems you have with 1 are decisions made, not design.  So let's
> ignore those.  2 is also a decision, not a design flaw.

They're important to me since I know they can be solved reasonably (and the
existing code deals *very* well with them for ap_r*). Any solution needs to
deal with these issues, whether in the initial patch or a follow-on patch.

>...
> > B) The use of r->bb is always going to cause an ordering problem between
> >    ap_r* and ap_pass_brigade.
> > 
> >    Answer: The only solution I see there is to teach ap_pass_brigade about
> >            r->bb. Not sure how I feel about that.
> 
> Let me give another option.  Stop using your own brigade in handlers.  If
> you are re-writing your handler to use brigades, you have two options, use
> your own brigade, or just use r->bb.  If you use your own brigade, then
> you had better be sure that r->bb is never used, or that you account for
> it.  The core shouldn't have to deal with that.

Hmm. Possible, although I'm not sure that I like it. It still feels a bit
like a trap door. Specifically: how is this communicated to module
developers? If we make it so that a module developer can't make an ordering
mistake no matter how he calls the Apache API, then everybody will be
happier. I also worry about filter writers beliving they should use r->bb,
but discovering they're stomping on the brigade that was passed to them.

Imagine the following:

my_filter(bb):
    b = apr_bucket_create_immortal("please wait while your data is loaded");
    APR_BRIGADE_INSERT_TAIL(flush_bb, b);
    b = apr_bucket_create_flush();
    APR_BRIGADE_INSERT_TAIL(flush_bb, b);
    ap_pass_brigade(flush_bb, b);
    
    b = apr_bucket_create_pipe(...)
    APR_BRIGADE_INSERT_HEAD(bb, b)
    ap_pass_brigade(f->next, bb);

If r->bb is used in there, then things fall apart pretty quick :-) Telling
people to "handlers should always use r->bb or live at their peril," could
confuse them ("aren't I handling the filter callback?"). Not to mention that
most people never read the docs on this stuff :-) A lot of monkey-see,
monkey-do. Cut/paste from a handler to filter would be troublesome.

etc etc. Not saying I'm hard and fast against it, but it still sounds like
trouble. I'd like to find a way that we automagically sync between r->bb and
arbitrary brigade output.

>...
> If this is acceptable to everybody, I will fix the code tonight, and
> commit today.  This lets us work on fixing any bugs or change decisions
> over the next week or so.  As soon as i see 3 +1's, I will write the code
> and commit.  If I get the 3 +1's today, then you can expect the commit to
> happen about 10:00 tonight.  BTW, I believe this should happen before any
> kind of beta, because this fixes performance for a filters as well as
> handlers, and it allows us to simplify the header handling a bit more.

I don't think we need to go in and revamp this stuff for beta. You have been
pushing for solid code. Changes like this won't be solid at all. Our
performance is completely fine at this point for our beta. We don't need to
spiff up filter performance because there aren't any, other than
mod_include. ... and we don't have perf measurements to determine *if* there
is a performance problem in mod_include that needs to be solved *before* the
beta.

IMO, let's ship another alpha or beta, and *then* complete this discussion.

Your second sentence above, "lets us work on fixing any bugs" can be done
right now. No need to wait for this stuff. [which I note you and OtherBill
are doing an exemplary job of :-)]

Let's see a release before we worry about (unknown) filter performance.

I'm going to have very spotty access until Wed night while I'm in Seattle
for some business. I would request that we don't have any of these "please
comment within 24 hours or it goes in" types of patches. Especially if they
aren't going to handle the large-write problem; the existing OLD_WRITE
handles that, and I won't be able to assist with rebuilding that capability
on top of new brigade features if it gets tossed.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Mime
View raw message