subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kot...@apache.org
Subject svn commit: r1724455 - in /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h request_body.c
Date Wed, 13 Jan 2016 16:27:10 GMT
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;
+}



Mime
View raw message