On 6 January 2014 15:23,  <rhuij...@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

Reply via email to