httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
Subject Re: [PATCH] bucket problems.
Date Wed, 17 Jan 2001 16:03:06 GMT

> > was required to make mod_autoindex work.  That is the other half of this
> > patch, I have modified mod_autoindex to use those functions instead of
> > ap_r* to actually write the data for the request.
> One obvious question: is there a _really_ good reason to change the name
> of ap_rputs?  This dumps unnecessary work onto the module authors IMO,
> at a time we are trying to stabilize the code base.  If this code is
> solid, why not just hijack the ap_rputs entry point?

A couple of reasons.

1)  ap_r* functions are httpd specific, these need to be brigade

2)  ap_r* functions work on a request_rec, these work on a bucket_brigade,
so calling them ap_r* doesn't make any sense, because there is no r.

3)  Even if we kept the function name the same, the API is different, and
if somebody is going to go in and modify the ap_rputs call to pass a
bucket_brigade instead of a request_rec, they can change the name too.  I
also fully expect that the ap_r* functions will go away completely, but
this allows us to have a working server for the day it will take to make
that happen.

> > Finally, module writers can use both ap_brigade_* functions and
> > ap_bucket_create_* functions at the same time, but they must be
> > diligent.  
> I think you are saying you have a solution for Greg Stein's concern
> about mixing the APIs.  Cool!  
> But, as stated above, I would like to maintain the ap_rputs API to make
> life easier for module writers and to minimize changes to innocent
> modules.  

Modules are going to change regardless if we implement this.  We are no
longer passing around a request_rec, we are passing around a
bucket_brigade, and that should be obvious just by looking at the function

> One thing that I will be looking for is what will cost of the *average*
> ap_rputs/ap_brigade_puts call be with the new close will it
> come to 1.3?  It needs to be not much more than one or two function
> calls, a few tests, and a copy into the buffer.

The average ap_brigade_puts call is a simple memcpy, and that is it.  At
either end of the scale (buffer non-existant [see below], buffer
full) this becomes a palloc and a memcpy, but even that is much less
expensive than what we have now.

I have found one problem with the patch, and a simple solution.  Right
now, allocating a brigade is cheap, with this patch, it becomes a 4K+
allocation, which is unacceptable to me.  The solution is easy, we just
delay creating the buffer until the first call to one of the ap_brigade_*
functions.  When I commit the real thing, this will be implemented as


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

View raw message