You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2013/05/17 23:33:09 UTC

svn commit: r1483999 - /subversion/trunk/subversion/libsvn_subr/io.c

Author: rhuijben
Date: Fri May 17 21:33:09 2013
New Revision: 1483999

URL: http://svn.apache.org/r1483999
Log:
* subversion/libsvn_subr/io.c
  (svn_io_file_flush_to_disk): Add note.
  (svn_io_write_unique): Differentiate file flush behavior on whether the file
    is temporary or not.

Modified:
    subversion/trunk/subversion/libsvn_subr/io.c

Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1483999&r1=1483998&r2=1483999&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Fri May 17 21:33:09 2013
@@ -2091,6 +2091,10 @@ svn_error_t *svn_io_file_flush_to_disk(a
 {
   apr_os_file_t filehand;
 
+  /* ### In apr 1.4+ we could delegate most of this function to
+         apr_file_sync(). The only major difference is that this doesn't
+         contain the retry loop for EINTR on linux. */
+
   /* First make sure that any user-space buffered data is flushed. */
   SVN_ERR(do_io_file_wrapper_cleanup(file, apr_file_flush(file),
                                      N_("Can't flush file '%s'"),
@@ -3483,7 +3487,15 @@ svn_io_write_unique(const char **tmp_pat
   err = svn_io_file_write_full(new_file, buf, nbytes, NULL, pool);
 
   if (!err)
-    err = svn_io_file_flush_to_disk(new_file, pool);
+    {
+      /* svn_io_file_flush_to_disk() can be very expensive, so use the
+         cheaper standard flush if the file is created as temporary file
+         anyway */
+      if (delete_when == svn_io_file_del_none)
+        err = svn_io_file_flush_to_disk(new_file, pool);
+      else
+        err = svn_io_file_flush(new_file, pool);
+    }
 
   return svn_error_trace(
                   svn_error_compose_create(err,



Re: svn commit: r1483999 - /subversion/trunk/subversion/libsvn_subr/io.c

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Sat, May 18, 2013 at 1:33 AM,  <rh...@apache.org> wrote:
> Author: rhuijben
> Date: Fri May 17 21:33:09 2013
> New Revision: 1483999
>
> URL: http://svn.apache.org/r1483999
> Log:
> * subversion/libsvn_subr/io.c
>   (svn_io_file_flush_to_disk): Add note.
>   (svn_io_write_unique): Differentiate file flush behavior on whether the file
>     is temporary or not.
>
> Modified:
>     subversion/trunk/subversion/libsvn_subr/io.c
>
> Modified: subversion/trunk/subversion/libsvn_subr/io.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1483999&r1=1483998&r2=1483999&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/io.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/io.c Fri May 17 21:33:09 2013
> @@ -2091,6 +2091,10 @@ svn_error_t *svn_io_file_flush_to_disk(a
>  {
>    apr_os_file_t filehand;
>
> +  /* ### In apr 1.4+ we could delegate most of this function to
> +         apr_file_sync(). The only major difference is that this doesn't
> +         contain the retry loop for EINTR on linux. */
> +
>    /* First make sure that any user-space buffered data is flushed. */
>    SVN_ERR(do_io_file_wrapper_cleanup(file, apr_file_flush(file),
>                                       N_("Can't flush file '%s'"),
> @@ -3483,7 +3487,15 @@ svn_io_write_unique(const char **tmp_pat
>    err = svn_io_file_write_full(new_file, buf, nbytes, NULL, pool);
>
>    if (!err)
> -    err = svn_io_file_flush_to_disk(new_file, pool);
> +    {
> +      /* svn_io_file_flush_to_disk() can be very expensive, so use the
> +         cheaper standard flush if the file is created as temporary file
> +         anyway */
> +      if (delete_when == svn_io_file_del_none)
> +        err = svn_io_file_flush_to_disk(new_file, pool);
> +      else
> +        err = svn_io_file_flush(new_file, pool);
> +    }
Using delete_when for differentiate flush behavior is looks very
strange. Why you didn't add separate boolean argument for this?

-- 
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com