You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2013/07/12 14:48:44 UTC
svn commit: r1502539 - in /subversion/trunk: ./ subversion/include/svn_io.h
subversion/libsvn_subr/io.c
Author: stefan2
Date: Fri Jul 12 12:48:44 2013
New Revision: 1502539
URL: http://svn.apache.org/r1502539
Log:
Cherrypick merge c1442910,1443171,1481447,1493042 from the
fsfs-format7 branch to /trunk and resolve a trivial tree conflict.
This introduces a new seek() function with proper APR file buffer
and read alignment that e.g. the fsfs-improvements branch needs.
Modified:
subversion/trunk/ (props changed)
subversion/trunk/subversion/include/svn_io.h
subversion/trunk/subversion/libsvn_subr/io.c
Propchange: subversion/trunk/
------------------------------------------------------------------------------
Merged /subversion/branches/fsfs-format7:r1442910,1443171,1481447,1493042
Modified: subversion/trunk/subversion/include/svn_io.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_io.h?rev=1502539&r1=1502538&r2=1502539&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_io.h (original)
+++ subversion/trunk/subversion/include/svn_io.h Fri Jul 12 12:48:44 2013
@@ -2065,6 +2065,26 @@ svn_io_file_seek(apr_file_t *file,
apr_off_t *offset,
apr_pool_t *pool);
+/** Set the file pointer of @a file to @a offset. In contrast to
+ * #svn_io_file_seek, this function will attempt to resize the internal
+ * data buffer to @a block_size bytes and to read data aligned to multiples
+ * of that value. The beginning of the block will be returned in
+ * @a buffer_start, if that is not NULL.
+ * Uses @a pool for temporary allocations.
+ *
+ * @note Due to limitations of the APR API, in particular pre-1.3 APR,
+ * the alignment may not be successful. If you never use any other seek
+ * function on @a file, you are, however, virtually guaranteed to get at
+ * least 4kByte alignments for all reads.
+ *
+ * @since New in 1.9
+ */
+svn_error_t *
+svn_io_file_aligned_seek(apr_file_t *file,
+ apr_off_t block_size,
+ apr_off_t *buffer_start,
+ apr_off_t offset,
+ apr_pool_t *pool);
/** Wrapper for apr_file_write(). */
svn_error_t *
Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1502539&r1=1502538&r2=1502539&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Fri Jul 12 12:48:44 2013
@@ -3557,6 +3557,91 @@ svn_io_file_seek(apr_file_t *file, apr_s
pool);
}
+svn_error_t *
+svn_io_file_aligned_seek(apr_file_t *file,
+ apr_off_t block_size,
+ apr_off_t *buffer_start,
+ apr_off_t offset,
+ apr_pool_t *pool)
+{
+ apr_size_t file_buffer_size = 4096;
+ apr_off_t desired_buffer = 0;
+ apr_off_t current = 0;
+ apr_off_t aligned_offset = 0;
+ svn_boolean_t fill_buffer = FALSE;
+
+ /* paranoia check: huge blocks on 32 bit machines may cause overflows */
+ SVN_ERR_ASSERT(block_size == (apr_size_t)block_size);
+
+ /* on old APRs, we are simply stuck with 4k blocks */
+#if APR_VERSION_AT_LEAST(1,3,0)
+ file_buffer_size = apr_file_buffer_size_get(file);
+ if (file_buffer_size != (apr_size_t)block_size)
+ {
+ /* FILE has the wrong buffer size. correct it */
+ char *buffer;
+ file_buffer_size = (apr_size_t)block_size;
+ buffer = apr_palloc(apr_file_pool_get(file), file_buffer_size);
+ apr_file_buffer_set(file, buffer, file_buffer_size);
+
+ /* seek to the start of the block and cause APR to read 1 block */
+ aligned_offset = offset - (offset % block_size);
+ fill_buffer = TRUE;
+ }
+ else
+#endif
+ {
+ aligned_offset = offset - (offset % file_buffer_size);
+
+ /* We have no way to determine the block start of an APR file.
+ Furthermore, we don't want to throw away the current buffer
+ contents. Thus, we re-align the buffer only if the CURRENT
+ offset definitely lies outside the desired, aligned buffer.
+ This covers the typical case of linear reads getting very
+ close to OFFSET but reading the previous / following block.
+
+ Note that ALIGNED_OFFSET may still be within the current
+ buffer and no I/O will actually happen in the FILL_BUFFER
+ section below.
+ */
+ SVN_ERR(svn_io_file_seek(file, SEEK_CUR, ¤t, pool));
+ fill_buffer = aligned_offset + file_buffer_size <= current
+ || current <= aligned_offset;
+ }
+
+ if (fill_buffer)
+ {
+ char dummy;
+ apr_status_t status;
+
+ /* seek to the start of the block and cause APR to read 1 block */
+ SVN_ERR(svn_io_file_seek(file, SEEK_SET, &aligned_offset, pool));
+ status = apr_file_getc(&dummy, file);
+
+ /* read may fail if we seek to or behind EOF. That's ok then. */
+ if (status != APR_SUCCESS && !APR_STATUS_IS_EOF(status))
+ return do_io_file_wrapper_cleanup(file, status,
+ N_("Can't read file '%s'"),
+ N_("Can't read stream"),
+ pool);
+ }
+
+ /* finally, seek to the OFFSET the caller wants */
+ desired_buffer = offset;
+ SVN_ERR(svn_io_file_seek(file, SEEK_SET, &offset, pool));
+ if (desired_buffer != offset)
+ return do_io_file_wrapper_cleanup(file, APR_EOF,
+ N_("Can't seek in file '%s'"),
+ N_("Can't seek in stream"),
+ pool);
+
+ /* return the buffer start that we (probably) enforced */
+ if (buffer_start)
+ *buffer_start = aligned_offset;
+
+ return SVN_NO_ERROR;
+}
+
svn_error_t *
svn_io_file_write(apr_file_t *file, const void *buf,
Re: svn commit: r1502539 - in /subversion/trunk: ./
subversion/include/svn_io.h subversion/libsvn_subr/io.c
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Sat, Jul 13, 2013 at 2:36 PM, Stefan Fuhrmann
<st...@wandisco.com> wrote:
> On Fri, Jul 12, 2013 at 2:52 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>>
>> On Fri, Jul 12, 2013 at 4:48 PM, <st...@apache.org> wrote:
>> > Author: stefan2
>> > Date: Fri Jul 12 12:48:44 2013
>> > New Revision: 1502539
>> >
>> > URL: http://svn.apache.org/r1502539
>> > Log:
>> > Cherrypick merge c1442910,1443171,1481447,1493042 from the
>> > fsfs-format7 branch to /trunk and resolve a trivial tree conflict.
>> > This introduces a new seek() function with proper APR file buffer
>> > and read alignment that e.g. the fsfs-improvements branch needs.
>> >
>> Hi Stefan,
>>
>> Function svn_io_file_aligned_seek() doesn't look trivial. Could you
>> please add several test for this function? Thanks!
>
>
> Done in r1502770.
>
Thanks!
I've spend some time with profiling APR file buffered I/O and found
that APR doesn't work well in terms in performance in buffered modes:
APR always tries to read data after getting EOF to fill buffer. So may
be it's worth to create Subversion own svn_io__paged_access_t object
with own buffer and use unbuffered APR files?
--
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com
Re: svn commit: r1502539 - in /subversion/trunk: ./
subversion/include/svn_io.h subversion/libsvn_subr/io.c
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Fri, Jul 12, 2013 at 2:52 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
> On Fri, Jul 12, 2013 at 4:48 PM, <st...@apache.org> wrote:
> > Author: stefan2
> > Date: Fri Jul 12 12:48:44 2013
> > New Revision: 1502539
> >
> > URL: http://svn.apache.org/r1502539
> > Log:
> > Cherrypick merge c1442910,1443171,1481447,1493042 from the
> > fsfs-format7 branch to /trunk and resolve a trivial tree conflict.
> > This introduces a new seek() function with proper APR file buffer
> > and read alignment that e.g. the fsfs-improvements branch needs.
> >
> Hi Stefan,
>
> Function svn_io_file_aligned_seek() doesn't look trivial. Could you
> please add several test for this function? Thanks!
>
Done in r1502770.
-- Stefan^2
Re: svn commit: r1502539 - in /subversion/trunk: ./
subversion/include/svn_io.h subversion/libsvn_subr/io.c
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Fri, Jul 12, 2013 at 4:48 PM, <st...@apache.org> wrote:
> Author: stefan2
> Date: Fri Jul 12 12:48:44 2013
> New Revision: 1502539
>
> URL: http://svn.apache.org/r1502539
> Log:
> Cherrypick merge c1442910,1443171,1481447,1493042 from the
> fsfs-format7 branch to /trunk and resolve a trivial tree conflict.
> This introduces a new seek() function with proper APR file buffer
> and read alignment that e.g. the fsfs-improvements branch needs.
>
Hi Stefan,
Function svn_io_file_aligned_seek() doesn't look trivial. Could you
please add several test for this function? Thanks!
--
Ivan Zhakov