httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Roy T. Fielding" <field...@kiwi.ICS.UCI.EDU>
Subject Re: Patch review: Re: [PATCH] hook based filters take 4
Date Tue, 27 Jun 2000 04:30:14 GMT
I don't think either of you will appreciate me saying so, but the basic
problem I have with both proposals is that they cannot be understood by
anyone but the author.  Using macros to define an interface is lame.
Use an ADT!  I honestly care a lot less about how the core implements
the sucker than I do about how a module writer is supposed to design,
debug, and deploy an application that uses these filters.  This code
should be *easier* to understand than the current buff.c.

However, that doesn't mean I think the core implementation isn't
important.  For example, I don't think it is safe to implement filters
in Apache without either a smart allocation system or a strict limiting
mechanism that prevents filters from buffering more than 8KB of
memory at a time (for the entire non-flushed stream).  It isn't possible
to create a robust server implementation using filters that allocate
memory from a pool (or the heap, or a stack, or whatever) without somehow
reclaiming and reusing the memory that gets written out to the network.
There is a certain level of "optimization" that must be present before
any filtering mechanism can be in Apache, and that means meeting the
requirement that the server not keel over and die the first time
a user requests a large filtered file.  XML tree manipulation is an
example where that can happen.  I don't think either patch has
addressed that problem yet, and I can't really evaluate them until
they do.

I also don't believe it is simpler to implement these "easy" forms of
filters than it is to implement the full-blown bucket brigades.  The
basic brigade interface is very simple -- a pointer to the stream head 
(the top filter on an output filter stack) and a pointer to the
bucket brigade (a linked list of typed memory buckets).  You can add
helper functions on top of that which take string/int/printf arguments
and translate those to a brigade pointer.  Memory allocation can be
specific to each bucket, so the application can decide for itself whether
it wants to allocate from the heap and donate the memory to the stream
or tell the stream to make a copy if it can't process it all in one go.

I don't understand how either scheme is going to work in terms of
placing filters on the output.  I expected to see multiple points
where filters may be added both on top of and within the filter stack.
For example, consider the HTTP processing code to be a filter.  I would
expect it to process the outgoing stream looking for indicators in
the metadata for such things as transfer encodings and simply insert
the encoding filters on its own output when needed.  I guess what
this is assuming is that there exists an r->outgoing pointer on the
request_rec that points to the current head of the stream and
the content generators simply call r->outgoing->lwrite_brigade(),
and further that any filter can place additional filters on (or pop
layers off) the stream.

Disabling content-length just because there are filters in the stream
is a blatant cop-out.  If you have to do that then the design is wrong.
At the very least the HTTP filter/buff should be capable of discovering
whether it knows the content length by examing whether it has the whole
response in buffer (or fd) before it sends out the headers.

Of course, it is also possible that the three of us are all thinking
of wildly different architectures from parallel universes and all the
argument is simply because we have no shared whiteboard to reach a
common understanding of the general model we are trying to implement.
But I still think we are making more progress than last week.



View raw message