httpd-cvs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From r..@covalent.net
Subject Re: cvs commit: apache-2.0/src/main http_protocol.c
Date Wed, 08 Nov 2000 14:34:13 GMT

>    typedef struct byterange_ctx {
>        ap_bucket_brigade *bb;
>   +    const char *bound_head;
>   +    int num_ranges;
>    } byterange_ctx;
>    
>   -        curr_length = range_end - range_start + 1;
>   -        loc = range = apr_pcalloc(r->pool, curr_length + 1);
>   +        /* ### this is so bogus, but not dealing with it right now */
>   +        range = loc = apr_pcalloc(r->pool, range_length + 1);

Why is this bogus?  We have no choice but to copy the buffer.

>   +        /* ### we should split() buckets rather than read() them. this
>   +           ### will allow us to avoid reading files or custom buckets
>   +           ### into memory. for example: we REALLY don't want to cycle
>   +           ### a 10gig file into memory just to send out 100 bytes from
>   +           ### the end of the file.
>   +           ###
>   +           ### content generators that used to call ap_each_byterange()
>   +           ### manually (thus, optimizing the output) can replace their
>   +           ### behavior with a new bucket type that understands split().
>   +           ### they can then defer reading actual content until a read()
>   +           ### occurs, using the split() as an internal "seek".
>   +        */
>   +
>            ap_bucket_read(e, &str, &n, AP_NONBLOCK_READ);
>   +        /* ### using e->length doesn't account for pipes */

Yes it does, because the read that is just above it converted the pipes
into blocks of actual data.  The thing is that our file reading is bogus,
but I can't figure out how to fix it, so it has to stay as it is for now.

>            while (range_start > (curr_offset + e->length)) {
>                curr_offset += e->length;
>                e = AP_BUCKET_NEXT(e);
>   +
>                if (e == AP_BRIGADE_SENTINEL(bb)) {
>                    break;
>                }
>   +
>   +            /* ### eventually we can avoid this */
>   +            ap_bucket_read(e, &str, &n, AP_NONBLOCK_READ);

How?  We have just found the start of the data, so we need to have it in
memory so that we can copy it.

>   -            apr_cpystrn(loc, str + (range_start - curr_offset), 
>   -                        MIN_LENGTH(curr_length + 1, e->length));
>   -            loc += MIN_LENGTH(curr_length + 1, e->length);
>   -            curr_length -= MIN_LENGTH(curr_length + 1, e->length);
>   +
>   +            /* ### we should use 'n', not e->length */
>   +            segment_length = MIN_LENGTH(curr_length + 1, e->length);

It doesn't matter if we use n or e->length, because the reads that you
want to get rid of make them equivalent.

Can I assume that the only difference in the output is that we now add the
content length correctly?  If so, I had an idea last night that I think is
slightly cleaner than this.  If we put back the original byterange filter,
and add it to the filter stack before the content-length filter for all
requests, it can check very quickly to determine if it should be
active.  If not, it just removes itself.  If so, it does the byterange
magic.  Then, we can rely on the content-length filter to compute the C-L
correctly everytime.

I figured this out late last night, and I kind of like it because it
re-uses code, and allows us to get aways with just having a single place
to calculate the C-L, which is a VERY good thing IMHO.

If there were more differences, (other than general cleanup), please let
me know.  I would love to know what the originl problem was.

Overall, very nice patch.  :-)

Ryan

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




Mime
View raw message