httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From r..@covalent.net
Subject Re: [PATCH] buckets take 2
Date Thu, 18 Jan 2001 20:41:42 GMT

> > I have spoken to Doug M, and as he
> > put it, the ap_r* API is basically staying as a compatability API.  If you
> > want to use the brigade API, then switch to it.  I am not at all
> > interested in allowing people to mix the API's, because it removes a lot
> > of features.
> 
> It isn't about a particular user or group of code mixing APIs, it is about
> indirectly mixing them. An example that I gave the other day: Handler H uses
> the bucket APIs to deliver content; H calls Utility Function F; F uses
> ap_r*. Oops... they got mixed.
> 
> I can easily see this occurring where a DAV sub-module or a PHP sub-module
> or whatever does some output, then calls back into mod_dav to handle some
> other output. The two output mechanisms need to be independent of each
> other (otherwise, we introduce too much coupling).

Please show us how to do this then.  So far, I haven't seen a patch that
will do this.

> > But, standardize() is a safe function to call.  It is perfectly valid to
> > call that function at any time, and nothing happens to the brigade unless
> > it is supposed to.  I do not mind asking people to work a bit harder if
> > they want to mix API's a bit.
> 
> Somebody who doesn't mix APIs may still need to call it because they don't
> know what somebody else has done. In fact, your latest patch puts a call
> right into ap_pass_brigade() to standardize every single brigade that passes
> through.

Actually that call to standardize has been there since the very first
patch.  It is a very simple way to ensure that there is no data in the
buffer before the brigade is passed to the next filter.  As I said,
sprinkling this function around is not a big performance issue.

I have two thoughts about this.

1)  Going to the direct bucket API requires a change in thinking,
requiring a specific call to ap_brigade_standardize doesn't bother me,
because it emphasizes that change in thinking.

2)  Think C Run-Time API's.  If you want to mix fwrite with write, you
have to call fflush, or the data will be out of order.  This is exactly
what we are doing here.  The ap_r* functions (and ap_brigade_*
functions) are a buffering API.  If you want to go to the non-bufferred
API, there is a flush call that you must use.

This is the exact same thing that happened in 1.3, if you wanted to use a
buffered API, you could use ap_r* or ap_b* and they just worked, but if
you didn't want to buffer, it was possible to write directly to the
network with write or send.  If you wanted to switch between the two, you
had call either ap_rflush or ap_bflush.  There is no difference with what
I have written.

> > Also, we can easily add more functions,
> > such as ap_brigade_sendfile, which basically does the checking and then
> > appends a file bucket to the end of the brigade.  I was more interested in
> > getting the basic stuff working before going on to the more complex stuff.
> 
> I think we're probably quite fine with the existing implementations of
> ap_send_fd() and ap_send_mmap(). The problem we're fixing deals with the
> "little bits" of ap_r*().

No, we aren't just fine with those functions.  Because those functions
need to work with ap_r*, there is a bit of work that needs to be done to
ensure that the data is inserted into the array in the correct
order.  If we apply the current patch, we will need to add calls to
ap_brigade_standardize.

> > But it doesn't work in general, because it easily allows for brigades like
> > the following:
> > 
> > 5 byte heap -> 10 byte transient -> 5 byte heap -> 10 byte transient
> > 
> > Also, to really make the idea work, the HEAP and POOL buckets were always
> > 4K buckets so we wasted a lot of memory.
> 
> The buckets would only need to be 4K if ap_r* were used. The above brigade
> wouldn't really happen if somebody was using ap_r*, as the content would be
> getting placed into HEAP bucket at the end of the brigade.
> 
> Again: using your original thought of appending-to-last-bucket will solve
> the "variant brigade" issue, and remove any need for the standardize()
> calls.

You lost me here.  The bucket has to be 4K regardless of the API, because
otherwise it won't have any extra space to grow into.  If you ever put a
heap bucket on the brigade that is the exact size of the data you are
adding, you remove the ability to append data to that bucket (unless you
want to call realloc, which you don't want to do).

Ryan

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


Mime
View raw message