Mailing-List: contact new-httpd-help@apache.org; run by ezmlm
Precedence: bulk
Reply-To: new-httpd@apache.org
Date: Mon, 22 Jan 2001 19:53:18 -0800
From: Greg Stein <gstein@lyra.org>
To: new-httpd@apache.org
Subject: Re: ap_r* performance patch
Message-ID: <20010122195317.Z704@lyra.org>
Mail-Followup-To: new-httpd@apache.org
References: <20010122173705.R704@lyra.org>
 <Pine.LNX.4.21.0101221925320.32729-100000@koj>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.2i
In-Reply-To: <Pine.LNX.4.21.0101221925320.32729-100000@koj>;
 from rbb@covalent.net on Mon, Jan 22, 2001 at 07:38:10PM -0800

On Mon, Jan 22, 2001 at 07:38:10PM -0800, rbb@covalent.net wrote:
> > > If I missed anybody, please stand up.  I would really like to call this a
> > > vote at 8:00 PM PCT.  That is about 3hr 20m from now.  We have been
> > > holding this vote since last thursday or friday, so it is time we made a
> > > decision already.  If anybody thinkgs I should wait longer, I am perfectly
> > > willing to wait until tomorrow morning at 7:00am, or suggest a different
> > > time.
> > 
> > Your patch needs to solve these things first:
> 
> You can't dictate problems after a vote and decision has been made.

1) we're voting
2) no decision has been made

and:

3) I've been stating the ordering problem since you first posted your patch,
   and the large-write issue came up in connection with OtherBill's query.


I can certainly raise issues until the patch is applied and even afterwards.
I'm not trying to change the rules of the game, but I don't see that some of
the problems are being addressed adequately (so I restated them in a
(hopefully) clear list). The ordering and large-write issues exist and are
clearly present; why is it a problem to bring them up?

> > 1) ordering
>...
> > You've said (1) is possible (and you provided the "how") but don't want to
> > do it that way. I agree that calling apr_brigade_flush() inside the brigade
> > macros is bogus. I've also described the tail-bucket approach that will
> > completely eliminate the apr_brigade_flush() problem. If you switch to that,
> > it will completely remove the necessity for brigade_flush.
> 
> I completely disagree that we should create a bucket that can be written
> to.  We currently do not have any buckets that we can write to.  I

So? There is a lot of code that we didn't have before, yet we still end up
writing it :-)  Lack of something doesn't prove it isn't needed :-)

I could say that we don't have brigades that we can write to, so we
shouldn't do that :-)  [we create and insert buckets instead]

> thoroughly dislike adding a new bucket API that only works in certain
> situations.

IMO, it is an elegant way to solve the problem you're after: small writes
into a brigade (while maintaining ordering within that brigade).


All that aside: if you aren't intending to insert an apr_brigade_flush()
call into various brigade macros, then just *how* are you hoping to solve
the issue?

a) inserting calls inside the macros doesn't feel right
b) pushing the flush back onto the module author leads to problems


[---- for reference, inserting problem (2) here ----]
> > 2) large writes in ap_rputs, ap_rvputs, ap_rwrite, and ap_vrprintf
> >    specifically: how will you avoid copying the content into the brigade?
[---- ... ]
> > I suspect that when you go to solve (2), the ap_r* functions are going to
> > get quite a bit hairier, and the solution to those will be Apache specific
> > -- exactly what you are lobbying your patch avoids.
> 
> Please actually look at the code.  This has already been solved.

Huh? I haven't seen any handling in your patch for a 100k ap_rwrite. In
fact, the last patch that you posted corrupts the heap if you do that.

I will gladly review your solution to large writes, and clear my "large
write" issue off the table. However, I suspect you're going to end up with a
good chunk of code in ap_r* to deal with large writes.

Cheers,
-g

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