httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From r..@covalent.net
Subject Re: brigade patch, last time.
Date Mon, 29 Jan 2001 17:51:09 GMT

> > > 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.

I didn't say they weren't important, I said they weren't bugs.  And they
aren't, they are decisions that I made.  We can change the decision at any
time.

> > > 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.

a)  I don't worry about creating API's that don't allow mistakes.  I have
never seen such an API.  b)  If we document that filters can't use r->bb,
but handlers should, that should be enough.  Any mistakes made will be
very obvious very quickly.

> 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.

What you are saying, is that developers don't need to understand an API
before using it?  I disagree.  If I was writing a module that had a
filter, I would personally copy a filter, not a handler.  The "aren't I
handling the filter callback" thing is a question of terminology.  We have
always called those functions handlers, so any module writer should know
what we mean.  BTW, cut/paste from a handler to a filter would be
troublesome now.  Imagine copying code with an ap_r* call.  You now have
an infinite loop.  We can't protect programmers from themselves in all
cases, sometimes we have to accept that our programmers are intelligent
people.

> > 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.

This is untrue.  I happen to work with a guy (Doug MacEachern) who has a
couple of filters, and he has already implemented this logic
himself.  Take a look at mod_perl 2.0.  When you are done looking at that,
try looking at mod_snake written by Jon Travis.  It has the same
logic.  This is what I want to avoid.  If we release a beta with this API,
then more module writers will write for this API, and we will force them
to buffer their data in their own code.  If we fix it before we go beta,
we stop people from needing to solve this problem.

Ryan
_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Mime
View raw message