On 25 Jul 2001 [EMAIL PROTECTED] wrote:

>   +/* APR_HAS_LARGE_FILES introduces the problem of spliting sendfile into
>   + * mutiple buckets, no greater than MAX(apr_size_t), and more granular
>   + * than that in case the brigade code/filters attempt to read it directly.
>   + * ### 4mb is an invention, no idea if it is reasonable.
>   + */
>   +#define AP_MAX_SENDFILE 4194304
>   +
>   +

It tend to think this should be at least as big as the size we're willing
to MMAP, which is 16MB (if not bigger).


>   +    fsize = r->finfo.size;
>   +    start = 0;
>   +#ifdef APR_HAS_LARGE_FILES

#if, not #ifdef, right?

>   +    while (fsize > AP_MAX_SENDFILE) {
>   +        /* APR_HAS_LARGE_FILES issue; must split into mutiple buckets,
>   +         * no greater than MAX(apr_size_t), and more granular than that
>   +         * in case the brigade code/filters attempt to read it directly.
>   +         */
>   +        e = apr_bucket_file_create(fd, start, AP_MAX_SENDFILE, r->pool);
>   +        APR_BRIGADE_INSERT_TAIL(bb, e);
>   +        fsize -= AP_MAX_SENDFILE;
>   +        start += AP_MAX_SENDFILE;
>   +    }
>   +#endif
>   +    e = apr_bucket_file_create(fd, start, (apr_size_t)fsize, r->pool);
>   +    APR_BRIGADE_INSERT_TAIL(bb, e);

You should probably just create a single file bucket and copy it a bunch
of times, adjusting the start/length on the copies.  Copying and adjusting
is much more efficient than making lots of whole file buckets.  Plus it's
more in tune with the intent of the refcounted bucket.

Something like this:

    fsize = r->finfo.size;
#if APR_HAS_LARGE_FILES
    if (fsize > AP_MAX_SENDFILE) {
        e = apr_bucket_file_create(fd, start, AP_MAX_SENDFILE, r->pool);
        APR_BRIGADE_INSERT_TAIL(bb, e);

        while (fsize > AP_MAX_SENDFILE) {
            fsize -= AP_MAX_SENDFILE;
            apr_bucket_copy(e, &e);
            e->start += AP_MAX_SENDFILE;
            APR_BRIGADE_INSERT_TAIL(bb, e);
        }
        e->length = fsize; /* fixup last bucket's length */
    }
    else {
#endif
        e = apr_bucket_file_create(fd, start, (apr_size_t)fsize, r->pool);
        APR_BRIGADE_INSERT_TAIL(bb, e);

#if APR_HAS_LARGE_FILES
    }
#endif


Does that seem sane?

--Cliff


--------------------------------------------------------------
   Cliff Woolley
   [EMAIL PROTECTED]
   Charlottesville, VA






Reply via email to