httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
Subject Re: ap_r* performance patch
Date Tue, 23 Jan 2001 04:08:56 GMT

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

The problem here is one of priorities.  I find it to be much more
important that the solution works for modules other than the core.  Mine
does, your's can't.

The ordering isn't a problem with a bit of diligence.  The large-write
issue is either solved, or the solution is obvious from my last patch.

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

Except, buckets are defined as being read-only, so what you are saying is
buckets are read-only, oh except for this special one that is
read/write.  That is a poor API design, and it is inviting problems for
module writers.

> 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

b is not a problem.  It is a standard way of doing things.  I have never
seen a buffering API that tries to figure out how to switch from buffered
to non-buffered without help from the programmer.  Why are we trying to?

> > > 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'll take a closer look, but I don't see that problem.  I see a potential
optimization that will remove a single copy, but it is just that an

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

I'll have code in about an hour, or I may fix a seg fault before I do
this, I haven't decided yet.


Ryan Bloom               
406 29th St.
San Francisco, CA 94131

View raw message