httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From r..@covalent.net
Subject Re: summary: issues for my filtering patch
Date Wed, 28 Jun 2000 19:37:11 GMT

> > A bucket doesn't grow.  You have lists of buckets, so if you need more
> > memory, you just add another bucket to the end.  So, it was a bit of a
> > misnomer to say the bucket can only get so big, in reality, it is the list
> > of buckets that can only provide so much memory.  The concept is the same
> > though.
> 
> Got it. Ideally, of course, the buckets are getting poured out onto
> the network before they ever accumulate like that. Its only the
> "bad" filters that need to see everything before sending anything
> along.

Ideally.  But......  I think more filters will actually accumulate data
than just pass it through.

> Even so, if your list of buckets has data from a variety of sources
> (some pointers to read-only memory allocated by some library, some
> to read-write memory, perhaps a file pointer or two), do you really
> want to write it all out to a file? How do you determine what is
> too much memory?

Well, no, but we don't need to.  If the data is already in a file on the
system, then the bucket knows that, and we just reference the file instead
of actually re-writing the data.  For example, if the data is already 
a file pointer, then instead of writing the data out to a file, we
use that in our bucket list:

So, if the original bucket list is:

    file pointer1 -->  read/write memory -->  file pointer2

The new bucket list becomes:

    file pointer1 -->  file pointer3 --> file pointer 2

As far as how much memory is too much, I really don't know.  Roy said
8K.  I would think this would need to be a compile time option, so that we
can experiment.  :-)

> > My impression after reviewing the patch is that Greg's patch has two
> > different filters.  One uses a char * to pass data.
> 
> My impression is that the char* version is just for people who want
> to be able to say "just give me raw char* data (that I can write
> to?) so I don't have to call bucket->get_data()".

That's not my impression from the code.  The where to write is whatever
the module wants to use, because he is using ap_l(write|put|...), so the
module could allocate more space for the data, or it could use what it was
passed (not if it wants to modify the data though).  The buckets are only
created in one of two instances:

1)  A module allocates the bucket, and it is passed down to the next
filter using ap_lsend_bucket (the function name may not be right).

2)  A module implements a bucket filter, and a char * was passed from the
previous filter.  In this case the core allocates a bucket for the filter.

This means that each modules MUST copy the data if it wants to modify
it.  Or if it wants to save it off to the side.  At least that is how I am
reading the code.

> I believe that in Greg's patch, the data being passed between layers
> is still a bucket, its just that some layers ask to get the data
> converted to something simpler before they touch it.

Again, that's not how I read it.  In fact, I read it just the
opposite.  The data is always a char *, but we'll wrap it up for you if
you really want us to.  These may seem similar, but they really aren't.

If the data is always in a bucket, then that implies that we know
something about the kind of memory used to store it.  We can't just lose
that information because we feel like it.  The information about the
memory that we store in the bucket is vitally important.

> Ideally, adding another type is as simple as allocating a ap_bucket_t
> and setting a few function pointers, right? You certainly don't want
> filters to have to know anything about the different bucket types
> (unless they choose to know, I guess).

Well, adding a new bucket type should be trivially simple yes.  I believe
the filters will need to know something about what kind of bucket they are
creating, but not what kind they are reading.  For example, if a module
wants to create a new bucket, it needs to know if that bucket is going to
store a file or a char * or an mmap'ed file.

Once a filter has been passed a bucket, then all buckets should
essentially look the same to the filter.

As far as implementing a new bucket type.  I expect it will be a matter of
creating a ap_create_bucket_foo function that sets up the correct
pointers.  It should be a five or ten line function, so it would be
trivial yes.

> Cool. I'm certainly not trying to pressure anyone to get something
> done *right now*. I thought if I asked enough dumb questions I
> wouldn't be the only one enlightened by the answers. :)

You are helping to enlighten everyone, and you are helping me to sharpen
my thoughts on this topic.  The more questions you ask, the clearer the
patch becomes in my mind.  I hope that in answering your questions I have
also helped to explain where my problems are with the current patch.  If
not, please keep asking.

> > Yes, I agree.  However starting down the wrong path can often make it
> > harder to find the right one.  This patch is IMO the wrong path.  I have
> > asked for proof that I am wrong and this morning I detailed exactly
> > what I need to see to be convinced.  The proof should be relatively easy
> > to create if I am wrong, and then I will step aside and let the patch go
> > in.
> 
> Also reasonable. I just wanted to make sure we knew we weren't
> looking for the perfect solution before we took *any* steps forward.

I do not want perfection, I just want the correct design/patch.  I expect
that my next patch will offer read/write memory buckets only.  This will
without a doubt not be a perfect patch.  It will however be in my mind the
correct design.

Ryan

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



Mime
View raw message