You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Cliff Woolley <cl...@yahoo.com> on 2001/07/26 00:24:37 UTC

Re: cvs commit: httpd-2.0/server core.c

On 25 Jul 2001 wrowe@apache.org 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
   cliffwoolley@yahoo.com
   Charlottesville, VA







Re: cvs commit: httpd-2.0/server core.c

Posted by Cliff Woolley <cl...@yahoo.com>.
Better yet:

    fsize = r->finfo.size;
    e = apr_bucket_file_create(fd, 0, AP_MAX_SENDFILE, r->pool);
    APR_BRIGADE_INSERT_TAIL(bb, e);

#if APR_HAS_LARGE_FILES
    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);
    }
#endif
    e->length = fsize; /* fixup last bucket's length */



--Cliff


--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA