You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ko...@apache.org on 2016/01/13 17:27:10 UTC
svn commit: r1724455 - in /subversion/trunk/subversion/libsvn_ra_serf:
commit.c ra_serf.h request_body.c
Author: kotkov
Date: Wed Jan 13 16:27:10 2016
New Revision: 1724455
URL: http://svn.apache.org/viewvc?rev=1724455&view=rev
Log:
In ra_serf, keep small svndiffs in memory during commit, instead of always
spilling them to temporary files.
This avoids a certain part of the disk I/O — for instance, importing around
2000 files on a A-Tier Azure VM becomes slightly faster:
47.945 s → 41.547 s
What's probably more important, we don't create/open/close/delete files
per every change in the commit. On Windows this can be heavy with a virus
scanner working in background. If that's the case, the performance penalty
of this work for the same 2000 files is much more visible:
107.644 s → 53.901 s
* subversion/libsvn_ra_serf/ra_serf.h
(svn_ra_serf__request_body_cleanup): Declare.
* subversion/libsvn_ra_serf/request_body.c
(svn_ra_serf__request_body_cleanup): New function. Explicitly closes
the handle to a temporary file in case it's being used.
* subversion/libsvn_ra_serf/commit.c
(struct file_context_t.stream): Adjust the comment.
(struct file_context_t.svndiff): Now is a svn_ra_serf__request_body_t.
(create_put_body): Remove, since we'll ask svn_ra_serf__request_body_t
for the delegate.
(delayed_commit_stream_open): Remove, since svn_ra_serf__request_body_t
knows how to avoid creating temporary files unless they're needed.
Therefore, we no longer need a lazyopen stream.
(apply_textdelta): Create a svn_ra_serf__request_body_t, ask it for the
writable stream. Disown the stream when calling svn_txdelta_to_svndiff3()
so that we could close it explicitly in close_file(), instead of relying
on the stream to be closed due to encountering the final (null) delta
window.
(close_file): If we had a stream of changes, close the stream and ask
svn_ra_serf__request_body_t for a request body delegate. Explicitly
cleanup the request body after the PUT request.
Modified:
subversion/trunk/subversion/libsvn_ra_serf/commit.c
subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
subversion/trunk/subversion/libsvn_ra_serf/request_body.c
Modified: subversion/trunk/subversion/libsvn_ra_serf/commit.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/commit.c?rev=1724455&r1=1724454&r2=1724455&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/commit.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/commit.c Wed Jan 13 16:27:10 2016
@@ -170,11 +170,11 @@ typedef struct file_context_t {
const char *copy_path;
svn_revnum_t copy_revision;
- /* stream */
+ /* Stream for collecting the svndiff. */
svn_stream_t *stream;
- /* Temporary file containing the svndiff. */
- apr_file_t *svndiff;
+ /* Buffer holding the svndiff (can spill to disk). */
+ svn_ra_serf__request_body_t *svndiff;
/* Our base checksum as reported by the WC. */
const char *base_checksum;
@@ -869,35 +869,6 @@ proppatch_resource(svn_ra_serf__session_
/* Implements svn_ra_serf__request_body_delegate_t */
static svn_error_t *
-create_put_body(serf_bucket_t **body_bkt,
- void *baton,
- serf_bucket_alloc_t *alloc,
- apr_pool_t *pool /* request pool */,
- apr_pool_t *scratch_pool)
-{
- file_context_t *ctx = baton;
- 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.
- */
- SVN_ERR(svn_io_file_flush(ctx->svndiff, pool));
- apr_file_buffer_set(ctx->svndiff, NULL, 0);
- offset = 0;
- SVN_ERR(svn_io_file_seek(ctx->svndiff, APR_SET, &offset, pool));
-
- *body_bkt = serf_bucket_file_create(ctx->svndiff, alloc);
- return SVN_NO_ERROR;
-}
-
-/* Implements svn_ra_serf__request_body_delegate_t */
-static svn_error_t *
create_empty_put_body(serf_bucket_t **body_bkt,
void *baton,
serf_bucket_alloc_t *alloc,
@@ -1877,23 +1848,6 @@ open_file(const char *path,
return SVN_NO_ERROR;
}
-/* Implements svn_stream_lazyopen_func_t for apply_textdelta */
-static svn_error_t *
-delayed_commit_stream_open(svn_stream_t **stream,
- void *baton,
- apr_pool_t *result_pool,
- apr_pool_t *scratch_pool)
-{
- file_context_t *file_ctx = baton;
-
- SVN_ERR(svn_io_open_unique_file3(&file_ctx->svndiff, NULL, NULL,
- svn_io_file_del_on_pool_cleanup,
- file_ctx->pool, scratch_pool));
-
- *stream = svn_stream_from_aprfile2(file_ctx->svndiff, TRUE, result_pool);
- return SVN_NO_ERROR;
-}
-
static svn_error_t *
apply_textdelta(void *file_baton,
const char *base_checksum,
@@ -1905,12 +1859,12 @@ apply_textdelta(void *file_baton,
int svndiff_version;
int compression_level;
- /* Store the stream in a temporary file; we'll give it to serf when we
+ /* Construct a holder for the request body; we'll give it to serf when we
* close this file.
*
* TODO: There should be a way we can stream the request body instead of
- * writing to a temporary file (ugh). A special svn stream serf bucket
- * that returns EAGAIN until we receive the done call? But, when
+ * possibly writing to a temporary file (ugh). A special svn stream serf
+ * bucket that returns EAGAIN until we receive the done call? But, when
* would we run through the serf context? Grr.
*
* BH: If you wait to a specific event... why not use that event to
@@ -1923,8 +1877,10 @@ apply_textdelta(void *file_baton,
* for sure after the request is completely available.
*/
- ctx->stream = svn_stream_lazyopen_create(delayed_commit_stream_open,
- ctx, FALSE, ctx->pool);
+ ctx->svndiff =
+ svn_ra_serf__request_body_create(SVN_RA_SERF__REQUEST_BODY_IN_MEM_SIZE,
+ ctx->pool);
+ ctx->stream = svn_ra_serf__request_body_get_stream(ctx->svndiff);
if (ctx->commit_ctx->session->supports_svndiff1 &&
ctx->commit_ctx->session->using_compression)
@@ -1949,7 +1905,9 @@ apply_textdelta(void *file_baton,
compression_level = SVN_DELTA_COMPRESSION_LEVEL_NONE;
}
- svn_txdelta_to_svndiff3(handler, handler_baton, ctx->stream,
+ /* Disown the stream; we'll close it explicitly in close_file(). */
+ svn_txdelta_to_svndiff3(handler, handler_baton,
+ svn_stream_disown(ctx->stream, pool),
svndiff_version, compression_level, pool);
if (base_checksum)
@@ -2016,8 +1974,11 @@ close_file(void *file_baton,
}
else
{
- handler->body_delegate = create_put_body;
- handler->body_delegate_baton = ctx;
+ SVN_ERR(svn_stream_close(ctx->stream));
+
+ svn_ra_serf__request_body_get_delegate(&handler->body_delegate,
+ &handler->body_delegate_baton,
+ ctx->svndiff);
handler->body_type = SVN_SVNDIFF_MIME_TYPE;
}
@@ -2035,8 +1996,9 @@ close_file(void *file_baton,
return svn_error_trace(svn_ra_serf__unexpected_status(handler));
}
+ /* Don't keep open file handles longer than necessary. */
if (ctx->svndiff)
- SVN_ERR(svn_io_file_close(ctx->svndiff, scratch_pool));
+ SVN_ERR(svn_ra_serf__request_body_cleanup(ctx->svndiff, scratch_pool));
/* If we had any prop changes, push them via PROPPATCH. */
if (apr_hash_count(ctx->prop_changes))
Modified: subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h?rev=1724455&r1=1724454&r2=1724455&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h Wed Jan 13 16:27:10 2016
@@ -1594,6 +1594,14 @@ svn_ra_serf__request_body_get_delegate(s
void **baton,
svn_ra_serf__request_body_t *body);
+/* Release intermediate resources associated with BODY. These resources
+ (such as open file handles) will be automatically released when the
+ pool used to construct BODY is cleared or destroyed, but this optional
+ function allows doing that explicitly. */
+svn_error_t *
+svn_ra_serf__request_body_cleanup(svn_ra_serf__request_body_t *body,
+ apr_pool_t *scratch_pool);
+
#if defined(SVN_DEBUG)
/* Wrapper macros to collect file and line information */
Modified: subversion/trunk/subversion/libsvn_ra_serf/request_body.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/request_body.c?rev=1724455&r1=1724454&r2=1724455&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/request_body.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/request_body.c Wed Jan 13 16:27:10 2016
@@ -220,3 +220,13 @@ svn_ra_serf__request_body_get_delegate(s
*del = request_body_delegate;
*baton = body;
}
+
+svn_error_t *
+svn_ra_serf__request_body_cleanup(svn_ra_serf__request_body_t *body,
+ apr_pool_t *scratch_pool)
+{
+ if (body->file)
+ SVN_ERR(svn_io_file_close(body->file, scratch_pool));
+
+ return SVN_NO_ERROR;
+}