[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1555716 - /subversion/trunk/subversion/libsvn_ra_serf/update.c

From: Ivan Zhakov <ivan_at_visualsvn.com>
Date: Tue, 7 Jan 2014 02:26:50 +0400

On 6 January 2014 15:23, <rhuijben_at_apache.org> wrote:
> Author: rhuijben
> Date: Mon Jan 6 11:23:05 2014
> New Revision: 1555716
>
> URL: http://svn.apache.org/r1555716
> Log:
> In the ra serf (update) editor, spool the report to a spillbuffer instead of
> to a tempfile to avoid creating a tempfile for small requests.
>

> + if (body_file != NULL)
> + {
> + /* The spillbuffer was spooled to disk. Use the most optimized way
> + * to send it to serf, like when we didn't spool to memory first */
> + apr_off_t offset;
> +
> + /* We need to flush the file, make it unbuffered (so that it can be
> + * zero-copied via mmap), and reset the position before attempting to
> + * deliver the file.
> + *
> + * N.B. If we have APR 1.3+, we can unbuffer the file to let us use mmap
> + * and zero-copy the PUT body. However, on older APR versions, we can't
> + * check the buffer status; but serf will fall through and create a file
> + * bucket for us on the buffered svndiff handle.
> + *
> + * ### Is this really a useful optimization for an update report?
> + */
> + SVN_ERR(svn_io_file_flush(body_file, pool));
> +#if APR_VERSION_AT_LEAST(1, 3, 0)
> + apr_file_buffer_set(body_file, NULL, 0);
> +#endif
>
> - *body_bkt = serf_bucket_file_create(report->body_file, alloc);
> + offset = 0;
> + SVN_ERR(svn_io_file_seek(body_file, APR_SET, &offset, pool));
> +
> + *body_bkt = serf_bucket_file_create(body_file, alloc);
> + }
> + else
> + {
> + /* Everything is already in memory. Just wrap as bucket.
> + * Note that this would just work for the file case if needed */
> + *body_bkt = svn_ra_serf__create_sb_bucket(report->body_sb, alloc,
> + report->pool, pool);
> + }
Hi Bert.

Probably this logic should be moved to svn_ra_serf__create_sb_bucket()?

Also I think that current implementation shouldn't work because
spillbuf file doesn't include in-memory content by default, because
SPILL_ALL_CONTENTS is FALSE. Did you test this scenario?

The proper implementation should create aggregate bucket with memory
and file buckets. But I'm not sure that all this optimization worth.

-- 
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com
Received on 2014-01-06 23:27:41 CET

This is an archived mail posted to the Subversion Dev mailing list.