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 2021/02/13 14:30:11 UTC

svn commit: r1886490 - in /subversion/trunk/subversion: include/private/ libsvn_subr/ libsvn_wc/ tests/cmdline/ tests/libsvn_subr/ tests/libsvn_wc/

Author: kotkov
Date: Sat Feb 13 14:30:10 2021
New Revision: 1886490

URL: http://svn.apache.org/viewvc?rev=1886490&view=rev
Log:
In the update editor, stream data to both pristine and working files.

Previously, the working files were installed in a separate step.  That step
happened per-directory and involved copying and possibly translating the
pristine contents into new working files.  For transports like HTTP having
a separate step with nothing being read from the connection puts an implicit
limit on the size of the directory that can be checked out without hitting
a timeout.

Assuming the default HTTP timeout = 60 seconds of httpd 2.4.x and a relatively
fast disk, the limit was around 6 GB for any directory.

If we stream data to both pristine and the (projected) working file, the
actual install can happen as an atomic rename.  Then we don't have to stop
reading from the connection, and the timeouts should no longer be an issue.

The new approach also has several nice properties, such as not having to
re-read the pristine files, not interfering with the network-level buffering,
TCP slow starts, and etc.  The implementation of the new approach gives up to
25% reduction in the amount of write I/O during checkouts.  On Windows, it
uses the existing mechanism of installing the files by handle, without having
to reopen them.

Several quick benchmarks:

  - Checking out subversion/trunk over https://

    Total time:  3.861 s  →  2.812 s
       Read IO:  57322 KB  →  316 KB
      Write IO:  455013 KB  →  359977 KB

  - Checking out 4 large binary files (7.4 GB) over https://

    Total time:   91.594 s   →  70.214 s
       Read IO:   7798883 KB  →  19 KB
      Write IO:   15598167 KB  →  15598005 KB


From the implementation standpoint, we factor out a utility layer that can
prepare and install working files, svn_wc__working_file_writer_t, and use it
in the part of the workqueue that handles OP_FILE_INSTALL.  We then rework
the update editor to make it stream the data to the working file during
apply_textdelta(), by using the introduced file writer utility layer.

* subversion/include/private/svn_io_private.h
  (svn_stream__install_stream_set_read_only,
   svn_stream__install_stream_set_executable,
   svn_stream__install_stream_set_affected_time,
   svn_io__win_set_file_basic_info): Declare.

* subversion/libsvn_subr/io.c
  (FILE_BASIC_INFO, FileBasicInfo): Declare these Win32 types for older SDKs.
  (APR_DELTA_EPOCH_IN_USEC): Unless defined, declare a copy of the constant
   from apr/arch/win32/apr_arch_atime.h.
  (svn_io__win_set_file_basic_info): New function that sets the basic file
   information using an existing file handle.

* subversion/libsvn_subr/stream.c
  (install_baton_t): Add new fields to remember if the target of the install
   stream should have specific mtime, read-only attribute or the executable
   bit.
  (svn_stream__create_for_install): Initialize the new fields to their
   default values.
  (svn_stream__install_stream_set_read_only,
   svn_stream__install_stream_set_executable,
   svn_stream__install_stream_set_affected_time): Update the new fields.
  (svn_stream__install_stream): Adjust the expected mtime and attributes of
   the to-be-installed file, if necessary.
  (svn_stream__install_get_info): If we are to set a specific mtime on the
   target, return it.  Otherwise, stat the file as before.

* subversion/libsvn_wc/working_file_writer.h: New file.
  (svn_wc__working_file_writer_t,
   svn_wc__working_file_writer_open,
   svn_wc__working_file_writer_get_stream,
   svn_wc__working_file_writer_get_info,
   svn_wc__working_file_writer_install,
   svn_wc__working_file_writer_close): Declare the new utility layer.

* subversion/libsvn_wc/working_file_writer.c: New file.
  (svn_wc__working_file_writer_t,
   cleanup_file_writer,
   svn_wc__working_file_writer_open,
   svn_wc__working_file_writer_get_stream,
   svn_wc__working_file_writer_get_info,
   svn_wc__working_file_writer_install,
   svn_wc__working_file_writer_close): Implement the new utility layer.
   Make use of the svn_stream__install_stream's new functionality.

* subversion/libsvn_wc/workqueue.c
  (): Include working_file_writer.h.
  (run_file_install): Reimplement by creating and delegating most of the
   work to the working file writer.

* subversion/libsvn_wc/wc_db.c
  (): Include working_file_writer.h.
  (svn_wc__db_base_add_file): Accept an optional working file writer.
   If it's passed in, this function will atomically update the db and complete
   the installation of the file.  Accept the new "record_fileinfo" parameter.
   If it's set, also record the fileinfo of the installed file.

* subversion/libsvn_wc/wc_db.c
  (db_record_fileinfo): Move this function so that it can be used without
   a forward declaration.
  (install_working_file): New helper.  Completes an installation of the
   working file and optionally records its fileinfo.
  (svn_wc__db_base_add_file): If the working file writer is passed in,
   atomically update the db and complete the installation of the file.
   If the "record_fileinfo" parameter is set, also record the fileinfo
   of the installed file.

* subversion/libsvn_wc/update_editor.c
  (): Include working_file_writer.h.
  (struct file_baton): Add new fields to hold the cached base properties
   of the file and an optional working file writer.
  (window_handler): In case of an error, explicitly close the working file
   writer.
  (get_file_base_props): New helper, mostly factored out from the
   close_file() function.  Returns the, possibly cached, base properties
   for a file context.
  (open_working_file_writer): New helper.  Opens a working file writer
   according to the properties in the specified file context.
  (lazy_open_target): If necessary, open the working file writer.
   Configure the incoming data to be written to both the pristine and
   the provisioned working file.  Introduce the local variable "stream"
   and rename the output argument to "stream_p" for clarity.
  (close_file): If we need to install the working file, ensure that we have
   an appropriate working file writer.  The writer may have been provisioned
   during apply_textdelta(), but also may need to be opened on-the-go.
   Pass the file writer to svn_wc__db_base_add_file() to complete the
   installation of the working file.  Don't schedule OP_FILE_INSTALL
   workqueue items.

* subversion/tests/libsvn_subr/io-test.c
  (test_install_stream_set_read_only,
   test_install_stream_set_affected_time): New tests.
  (test_funcs): Run the new tests.

* subversion/tests/libsvn_wc/db-test.c
  (test_inserting_nodes): Adjust the call to svn_wc__db_base_add_file().

* subversion/tests/cmdline/update_tests.py
  (update_add_missing_local_add): No longer fails.  Add comment.

Added:
    subversion/trunk/subversion/libsvn_wc/working_file_writer.c   (with props)
    subversion/trunk/subversion/libsvn_wc/working_file_writer.h   (with props)
Modified:
    subversion/trunk/subversion/include/private/svn_io_private.h
    subversion/trunk/subversion/libsvn_subr/io.c
    subversion/trunk/subversion/libsvn_subr/stream.c
    subversion/trunk/subversion/libsvn_wc/update_editor.c
    subversion/trunk/subversion/libsvn_wc/wc_db.c
    subversion/trunk/subversion/libsvn_wc/wc_db.h
    subversion/trunk/subversion/libsvn_wc/workqueue.c
    subversion/trunk/subversion/tests/cmdline/update_tests.py
    subversion/trunk/subversion/tests/libsvn_subr/io-test.c
    subversion/trunk/subversion/tests/libsvn_wc/db-test.c

Modified: subversion/trunk/subversion/include/private/svn_io_private.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_io_private.h?rev=1886490&r1=1886489&r2=1886490&view=diff
==============================================================================
--- subversion/trunk/subversion/include/private/svn_io_private.h (original)
+++ subversion/trunk/subversion/include/private/svn_io_private.h Sat Feb 13 14:30:10 2021
@@ -102,6 +102,24 @@ svn_stream__create_for_install(svn_strea
                                apr_pool_t *result_pool,
                                apr_pool_t *scratch_pool);
 
+/* Configure value of the read-only attribute that will be set when
+   the stream is installed. */
+void
+svn_stream__install_stream_set_read_only(svn_stream_t *install_stream,
+                                         svn_boolean_t read_only);
+
+/* Configure value of the executable bit that will be set when the
+   stream is installed. */
+void
+svn_stream__install_stream_set_executable(svn_stream_t *install_stream,
+                                          svn_boolean_t executable);
+
+/* Configure value of the last modification time that will be set
+   when the stream is installed. */
+void
+svn_stream__install_stream_set_affected_time(svn_stream_t *install_stream,
+                                             apr_time_t mtime);
+
 /* Installs a stream created with svn_stream__create_for_install in its final
    location FINAL_ABSPATH, potentially using platform specific optimizations.
 
@@ -172,6 +190,18 @@ svn_io__win_rename_open_file(apr_file_t
                              const char *to_path,
                              apr_pool_t *pool);
 
+/* This Windows-specific function sets the basic file information using an
+   existing file handle. If SET_MTIME is non-negative, it will be set as
+   the new LastWriteTime value. If SET_READ_ONLY is non-zero, the file
+   attributes will be updated to include FILE_ATTRIBUTE_READONLY.
+   Return SVN_ERR_UNSUPPORTED_FEATURE if not supported by OS. */
+svn_error_t *
+svn_io__win_set_file_basic_info(apr_file_t *file,
+                                const char *path,
+                                apr_time_t set_mtime,
+                                svn_boolean_t set_read_only,
+                                apr_pool_t *pool);
+
 #endif /* WIN32 */
 
 #ifdef __cplusplus

Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1886490&r1=1886489&r2=1886490&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Sat Feb 13 14:30:10 2021
@@ -144,6 +144,14 @@
 #ifdef WIN32
 
 #if _WIN32_WINNT < 0x600 /* Does the SDK assume Windows Vista+? */
+typedef struct _FILE_BASIC_INFO {
+  LARGE_INTEGER CreationTime;
+  LARGE_INTEGER LastAccessTime;
+  LARGE_INTEGER LastWriteTime;
+  LARGE_INTEGER ChangeTime;
+  DWORD FileAttributes;
+} FILE_BASIC_INFO, *PFILE_BASIC_INFO;
+
 typedef struct _FILE_RENAME_INFO {
   BOOL   ReplaceIfExists;
   HANDLE RootDirectory;
@@ -160,6 +168,7 @@ typedef struct _FILE_ATTRIBUTE_TAG_INFO
   DWORD ReparseTag;
 } FILE_ATTRIBUTE_TAG_INFO, *PFILE_ATTRIBUTE_TAG_INFO;
 
+#define FileBasicInfo 0
 #define FileRenameInfo 3
 #define FileDispositionInfo 4
 #define FileAttributeTagInfo 9
@@ -2319,6 +2328,62 @@ svn_io__win_rename_open_file(apr_file_t
     }
 
   return SVN_NO_ERROR;
+}
+
+/* Number of micro-seconds between the beginning of the Windows epoch
+ * (Jan. 1, 1601) and the Unix epoch (Jan. 1, 1970)
+ */
+#ifndef APR_DELTA_EPOCH_IN_USEC
+#define APR_DELTA_EPOCH_IN_USEC   APR_TIME_C(11644473600000000)
+#endif
+
+svn_error_t *
+svn_io__win_set_file_basic_info(apr_file_t *file,
+                                const char *path,
+                                apr_time_t set_mtime,
+                                svn_boolean_t set_read_only,
+                                apr_pool_t *pool)
+{
+  FILE_BASIC_INFO info;
+  HANDLE hFile;
+  apr_status_t status;
+
+  apr_os_file_get(&hFile, file);
+
+  if (set_read_only)
+    {
+      status = win32_get_file_information_by_handle(hFile, FileBasicInfo,
+                                                    &info, sizeof(info));
+      if (status)
+        {
+          return svn_error_wrap_apr(status, _("Can't get attributes of '%s'"),
+                                    svn_dirent_local_style(path, pool));
+        }
+    }
+
+  info.CreationTime.QuadPart = 0;
+  info.LastAccessTime.QuadPart = 0;
+  info.ChangeTime.QuadPart = 0;
+
+  if (set_mtime >= 0)
+    info.LastWriteTime.QuadPart = (set_mtime + APR_DELTA_EPOCH_IN_USEC) * 10;
+  else
+    info.LastWriteTime.QuadPart = 0;
+
+  if (set_read_only)
+    info.FileAttributes |= FILE_ATTRIBUTE_READONLY;
+  else
+    info.FileAttributes = 0;
+
+  status = win32_set_file_information_by_handle(hFile, FileBasicInfo,
+                                                &info, sizeof(info));
+  if (status)
+    {
+      return svn_error_wrap_apr(status, _("Can't set attributes of '%s'"),
+                                svn_dirent_local_style(path, pool));
+    }
+
+  return SVN_NO_ERROR;
 }
 
 #endif /* WIN32 */

Modified: subversion/trunk/subversion/libsvn_subr/stream.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/stream.c?rev=1886490&r1=1886489&r2=1886490&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/stream.c (original)
+++ subversion/trunk/subversion/libsvn_subr/stream.c Sat Feb 13 14:30:10 2021
@@ -2172,6 +2172,9 @@ struct install_baton_t
 {
   struct baton_apr baton_apr;
   const char *tmp_path;
+  svn_boolean_t set_read_only;
+  svn_boolean_t set_executable;
+  apr_time_t set_mtime;
 };
 
 #ifdef WIN32
@@ -2313,6 +2316,9 @@ svn_stream__create_for_install(svn_strea
   (*install_stream)->baton = ib;
 
   ib->tmp_path = tmp_path;
+  ib->set_read_only = FALSE;
+  ib->set_executable = FALSE;
+  ib->set_mtime = -1;
 
   /* Don't close the file on stream close; flush instead */
   svn_stream_set_close(*install_stream, install_close);
@@ -2320,6 +2326,33 @@ svn_stream__create_for_install(svn_strea
   return SVN_NO_ERROR;
 }
 
+void
+svn_stream__install_stream_set_read_only(svn_stream_t *install_stream,
+                                         svn_boolean_t read_only)
+{
+  struct install_baton_t *ib = install_stream->baton;
+
+  ib->set_read_only = read_only;
+}
+
+void
+svn_stream__install_stream_set_executable(svn_stream_t *install_stream,
+                                          svn_boolean_t executable)
+{
+  struct install_baton_t *ib = install_stream->baton;
+
+  ib->set_executable = executable;
+}
+
+void
+svn_stream__install_stream_set_affected_time(svn_stream_t *install_stream,
+                                             apr_time_t mtime)
+{
+  struct install_baton_t *ib = install_stream->baton;
+
+  ib->set_mtime = mtime;
+}
+
 svn_error_t *
 svn_stream__install_stream(svn_stream_t *install_stream,
                            const char *final_abspath,
@@ -2331,23 +2364,38 @@ svn_stream__install_stream(svn_stream_t
 
   SVN_ERR_ASSERT(svn_dirent_is_absolute(final_abspath));
 #ifdef WIN32
-  err = svn_io__win_rename_open_file(ib->baton_apr.file,  ib->tmp_path,
-                                     final_abspath, scratch_pool);
-  if (make_parents && err && APR_STATUS_IS_ENOENT(err->apr_err))
-    {
-      svn_error_t *err2;
-
-      err2 = svn_io_make_dir_recursively(svn_dirent_dirname(final_abspath,
-                                                    scratch_pool),
-                                         scratch_pool);
+  err = SVN_NO_ERROR;
 
-      if (err2)
-        return svn_error_trace(svn_error_compose_create(err, err2));
-      else
-        svn_error_clear(err);
+  /* Windows doesn't have an executable bit. */
+  if (ib->set_mtime >= 0 || ib->set_read_only)
+    {
+      err = svn_io__win_set_file_basic_info(ib->baton_apr.file,
+                                            ib->tmp_path,
+                                            ib->set_mtime,
+                                            ib->set_read_only,
+                                            scratch_pool);
+    }
 
+  if (!err)
+    {
       err = svn_io__win_rename_open_file(ib->baton_apr.file, ib->tmp_path,
                                          final_abspath, scratch_pool);
+      if (make_parents && err && APR_STATUS_IS_ENOENT(err->apr_err))
+        {
+          svn_error_t *err2;
+
+          err2 = svn_io_make_dir_recursively(svn_dirent_dirname(final_abspath,
+                                                                scratch_pool),
+                                             scratch_pool);
+
+          if (err2)
+            return svn_error_trace(svn_error_compose_create(err, err2));
+          else
+            svn_error_clear(err);
+
+          err = svn_io__win_rename_open_file(ib->baton_apr.file, ib->tmp_path,
+                                             final_abspath, scratch_pool);
+        }
     }
 
   /* ### rhuijben: I wouldn't be surprised if we later find out that we
@@ -2372,6 +2420,13 @@ svn_stream__install_stream(svn_stream_t
   /* Close temporary file. */
   SVN_ERR(svn_io_file_close(ib->baton_apr.file, scratch_pool));
 
+  if (ib->set_read_only)
+    SVN_ERR(svn_io_set_file_read_only(ib->tmp_path, FALSE, scratch_pool));
+  if (ib->set_executable)
+    SVN_ERR(svn_io_set_file_executable(ib->tmp_path, TRUE, FALSE, scratch_pool));
+  if (ib->set_mtime >= 0)
+    SVN_ERR(svn_io_set_file_affected_time(ib->set_mtime, ib->tmp_path, scratch_pool));
+
   err = svn_io_file_rename2(ib->tmp_path, final_abspath, FALSE, scratch_pool);
 
   /* A missing directory is too common to not cover here. */
@@ -2409,8 +2464,10 @@ svn_stream__install_get_info(apr_time_t
   apr_int32_t wanted;
   apr_finfo_t finfo;
 
+  /* If we are instructed to set a specific timestamp, return it.
+     For everything else, stat the file to get the information. */
   wanted = 0;
-  if (mtime_p)
+  if (mtime_p && ib->set_mtime < 0)
     wanted |= APR_FINFO_MTIME;
   if (size_p)
     wanted |= APR_FINFO_SIZE;
@@ -2422,7 +2479,9 @@ svn_stream__install_get_info(apr_time_t
         return svn_error_wrap_apr(status, NULL);
     }
 
-  if (mtime_p)
+  if (mtime_p && ib->set_mtime >= 0)
+    *mtime_p = ib->set_mtime;
+  else if (mtime_p)
     *mtime_p = finfo.mtime;
 
   if (size_p)

Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/update_editor.c?rev=1886490&r1=1886489&r2=1886490&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/update_editor.c (original)
+++ subversion/trunk/subversion/libsvn_wc/update_editor.c Sat Feb 13 14:30:10 2021
@@ -48,6 +48,7 @@
 #include "conflicts.h"
 #include "translate.h"
 #include "workqueue.h"
+#include "working_file_writer.h"
 
 #include "private/svn_subr_private.h"
 #include "private/svn_wc_private.h"
@@ -765,6 +766,12 @@ struct file_baton
 
   /* The tree conflict to install once the node is really edited */
   svn_skel_t *edit_conflict;
+
+  /* Cached copy of the base properties for this file. */
+  apr_hash_t *base_props;
+
+  /* Writer with the provisioned content of the working file.  May be NULL. */
+  svn_wc__working_file_writer_t *file_writer;
 };
 
 
@@ -1014,6 +1021,12 @@ window_handler(svn_txdelta_window_t *win
           svn_error_clear(svn_wc__db_pristine_install_abort(hb->install_data,
                                                             hb->pool));
         }
+
+      if (fb->file_writer)
+        {
+          svn_error_clear(svn_wc__working_file_writer_close(fb->file_writer));
+          fb->file_writer = NULL;
+        }
     }
   else
     {
@@ -3560,6 +3573,112 @@ open_file(const char *path,
   return SVN_NO_ERROR;
 }
 
+/* Return the, possibly cached, base properties for file associated
+   with the delta editor baton FB. */
+static svn_error_t *
+get_file_base_props(apr_hash_t **props_p,
+                    struct file_baton *fb,
+                    apr_pool_t *scratch_pool)
+{
+  if (!fb->base_props)
+    {
+      apr_hash_t *props;
+
+      if (fb->add_existed)
+        {
+          /* This node already exists. Grab the current pristine properties. */
+          SVN_ERR(svn_wc__db_read_pristine_props(&props,
+                                                 fb->edit_baton->db,
+                                                 fb->local_abspath,
+                                                 fb->pool, scratch_pool));
+        }
+      else if (!fb->adding_file)
+        {
+          /* Get the BASE properties for proper merging. */
+          SVN_ERR(svn_wc__db_base_get_props(&props,
+                                            fb->edit_baton->db,
+                                            fb->local_abspath,
+                                            fb->pool, scratch_pool));
+        }
+      else
+        props = apr_hash_make(fb->pool);
+
+      fb->base_props = props;
+    }
+
+  *props_p = fb->base_props;
+  return SVN_NO_ERROR;
+}
+
+/* Create the writer of the working file based on the state of
+   the file associated with the delta editor baton FB. */
+static svn_error_t *
+open_working_file_writer(svn_wc__working_file_writer_t **writer_p,
+                         struct file_baton *fb,
+                         apr_pool_t *result_pool,
+                         apr_pool_t *scratch_pool)
+{
+  apr_hash_t *base_props;
+  apr_hash_t *props;
+  const char *lock_token;
+  const char *temp_dir_abspath;
+  const char *cmt_rev_str;
+  svn_revnum_t cmt_rev;
+  const char *cmt_date_str;
+  apr_time_t cmt_date;
+  const char *cmt_author;
+  apr_time_t final_mtime;
+
+  SVN_ERR(get_file_base_props(&base_props, fb, scratch_pool));
+  props = svn_prop__patch(base_props, fb->propchanges, scratch_pool);
+
+  cmt_rev_str = svn_prop_get_value(props, SVN_PROP_ENTRY_COMMITTED_REV);
+  if (cmt_rev_str)
+    {
+      apr_int64_t rev;
+      SVN_ERR(svn_cstring_atoi64(&rev, cmt_rev_str));
+      cmt_rev = (svn_revnum_t)rev;
+    }
+  else
+    cmt_rev = SVN_INVALID_REVNUM;
+
+  cmt_date_str = svn_prop_get_value(props, SVN_PROP_ENTRY_COMMITTED_DATE);
+  if (cmt_date_str)
+    SVN_ERR(svn_time_from_cstring(&cmt_date, cmt_date_str, scratch_pool));
+  else
+    cmt_date = 0;
+
+  cmt_author = svn_prop_get_value(props, SVN_PROP_ENTRY_LAST_AUTHOR);
+
+  if (fb->edit_baton->use_commit_times && cmt_date)
+    final_mtime = cmt_date;
+  else
+    final_mtime = -1;
+
+  lock_token = svn_prop_get_value(props, SVN_PROP_ENTRY_LOCK_TOKEN);
+
+  SVN_ERR(svn_wc__db_temp_wcroot_tempdir(&temp_dir_abspath,
+                                         fb->edit_baton->db,
+                                         fb->edit_baton->wcroot_abspath,
+                                         scratch_pool, scratch_pool));
+
+  SVN_ERR(svn_wc__working_file_writer_open(writer_p,
+                                           temp_dir_abspath,
+                                           final_mtime,
+                                           props,
+                                           cmt_rev,
+                                           cmt_date,
+                                           cmt_author,
+                                           lock_token != NULL,
+                                           fb->adding_file,
+                                           fb->edit_baton->repos_root,
+                                           fb->new_repos_relpath,
+                                           fb->pool,
+                                           scratch_pool));
+
+  return SVN_NO_ERROR;
+}
+
 /* Implements svn_stream_lazyopen_func_t. */
 static svn_error_t *
 lazy_open_source(svn_stream_t **stream,
@@ -3580,29 +3699,61 @@ lazy_open_source(svn_stream_t **stream,
 
 /* Implements svn_stream_lazyopen_func_t. */
 static svn_error_t *
-lazy_open_target(svn_stream_t **stream,
+lazy_open_target(svn_stream_t **stream_p,
                  void *baton,
                  apr_pool_t *result_pool,
                  apr_pool_t *scratch_pool)
 {
   struct handler_baton *hb = baton;
-  svn_wc__db_install_data_t *install_data;
+  struct file_baton *fb = hb->fb;
+  svn_wc__db_install_data_t *pristine_install_data;
+  svn_stream_t *pristine_install_stream;
+  svn_wc__working_file_writer_t *file_writer;
+  svn_stream_t *stream;
 
   /* By convention return value is undefined on error, but we rely
      on HB->INSTALL_DATA value in window_handler() and abort
      INSTALL_STREAM if is not NULL on error.
      So we store INSTALL_DATA to local variable first, to leave
      HB->INSTALL_DATA unchanged on error. */
-  SVN_ERR(svn_wc__db_pristine_prepare_install(stream,
-                                              &install_data,
+  SVN_ERR(svn_wc__db_pristine_prepare_install(&pristine_install_stream,
+                                              &pristine_install_data,
                                               &hb->new_text_base_sha1_checksum,
                                               NULL,
-                                              hb->fb->edit_baton->db,
-                                              hb->fb->dir_baton->local_abspath,
+                                              fb->edit_baton->db,
+                                              fb->dir_baton->local_abspath,
                                               result_pool, scratch_pool));
 
-  hb->install_data = install_data;
+  if (fb->shadowed || fb->obstruction_found || fb->edit_obstructed)
+    {
+      /* No need to provision the working file. */
+      file_writer = NULL;
+    }
+  else
+    {
+      SVN_ERR(open_working_file_writer(&file_writer, fb, fb->pool,
+                                       scratch_pool));
+    }
+
+  hb->install_data = pristine_install_data;
+  fb->file_writer = file_writer;
 
+  /* Configure the incoming data to be written to both the pristine and the
+     provisioned working file.  This allows us to properly stream data and
+     avoid having to re-read and copy the contents of the pristine file as
+     some post-op.  Streaming is essential if we want the underlying transport
+     such as HTTP to work without timeouts and without any limits on the
+     sizes of the directories that can be checked out. */
+  stream = pristine_install_stream;
+  if (file_writer)
+    {
+      stream = svn_stream_tee(
+                 pristine_install_stream,
+                 svn_wc__working_file_writer_get_stream(file_writer),
+                 result_pool);
+    }
+
+  *stream_p = stream;
   return SVN_NO_ERROR;
 }
 
@@ -4214,6 +4365,9 @@ close_file(void *file_baton,
   svn_boolean_t keep_recorded_info = FALSE;
   const svn_checksum_t *new_checksum;
   apr_array_header_t *iprops = NULL;
+  svn_boolean_t install_pristine;
+  const char *install_from = NULL;
+  svn_boolean_t record_fileinfo;
 
   if (fb->skip_this)
     {
@@ -4318,22 +4472,8 @@ close_file(void *file_baton,
   if (local_actual_props == NULL)
     local_actual_props = apr_hash_make(scratch_pool);
 
-  if (fb->add_existed)
-    {
-      /* This node already exists. Grab the current pristine properties. */
-      SVN_ERR(svn_wc__db_read_pristine_props(&current_base_props,
-                                             eb->db, fb->local_abspath,
-                                             scratch_pool, scratch_pool));
-      current_actual_props = local_actual_props;
-    }
-  else if (!fb->adding_file)
-    {
-      /* Get the BASE properties for proper merging. */
-      SVN_ERR(svn_wc__db_base_get_props(&current_base_props,
-                                        eb->db, fb->local_abspath,
-                                        scratch_pool, scratch_pool));
-      current_actual_props = local_actual_props;
-    }
+  SVN_ERR(get_file_base_props(&current_base_props, fb, scratch_pool));
+  current_actual_props = local_actual_props;
 
   /* Note: even if the node existed before, it may not have
      pristine props (e.g a local-add)  */
@@ -4348,9 +4488,6 @@ close_file(void *file_baton,
 
   if (! fb->shadowed)
     {
-      svn_boolean_t install_pristine;
-      const char *install_from = NULL;
-
       /* Merge the 'regular' props into the existing working proplist. */
       /* This will merge the old and new props into a new prop db, and
          write <cp> commands to the logfile to install the merged
@@ -4419,29 +4556,8 @@ close_file(void *file_baton,
             content_state = svn_wc_notify_state_unchanged;
         }
 
-      if (install_pristine)
-        {
-          svn_boolean_t record_fileinfo;
-
-          /* If we are installing from the pristine contents, then go ahead and
-             record the fileinfo. That will be the "proper" values. Installing
-             from some random file means the fileinfo does NOT correspond to
-             the pristine (in which case, the fileinfo will be cleared for
-             safety's sake).  */
-          record_fileinfo = (install_from == NULL);
-
-          SVN_ERR(svn_wc__wq_build_file_install(&work_item,
-                                                eb->db,
-                                                fb->local_abspath,
-                                                install_from,
-                                                eb->use_commit_times,
-                                                record_fileinfo,
-                                                scratch_pool, scratch_pool));
-          all_work_items = svn_wc__wq_merge(all_work_items, work_item,
-                                            scratch_pool);
-        }
-      else if (lock_state == svn_wc_notify_lock_state_unlocked
-               && !fb->obstruction_found)
+      if (lock_state == svn_wc_notify_lock_state_unlocked
+          && !fb->obstruction_found)
         {
           /* If a lock was removed and we didn't update the text contents, we
              might need to set the file read-only.
@@ -4460,20 +4576,6 @@ close_file(void *file_baton,
           /* It is safe to keep the current recorded timestamp and size */
           keep_recorded_info = TRUE;
         }
-
-      /* Clean up any temporary files.  */
-
-      /* Remove the INSTALL_FROM file, as long as it doesn't refer to the
-         working file.  */
-      if (install_from != NULL
-          && strcmp(install_from, fb->local_abspath) != 0)
-        {
-          SVN_ERR(svn_wc__wq_build_file_remove(&work_item, eb->db,
-                                               fb->local_abspath, install_from,
-                                               scratch_pool, scratch_pool));
-          all_work_items = svn_wc__wq_merge(all_work_items, work_item,
-                                            scratch_pool);
-        }
     }
   else
     {
@@ -4505,6 +4607,8 @@ close_file(void *file_baton,
         content_state = svn_wc_notify_state_changed;
       else
         content_state = svn_wc_notify_state_unchanged;
+
+      install_pristine = FALSE;
     }
 
   /* Insert/replace the BASE node with all of the new metadata.  */
@@ -4558,6 +4662,54 @@ close_file(void *file_baton,
         svn_hash_sets(eb->wcroot_iprops, fb->local_abspath, NULL);
     }
 
+  /* We have to install the working file from the pristine, but we did not
+     receive it as the part of this edit.  Or we did receive it, but have to
+     install from a different file.  Either way, reset the writer and copy
+     appropriate contents to it.  */
+  if (install_pristine && (fb->file_writer == NULL || install_from))
+    {
+      svn_stream_t *src_stream;
+      svn_stream_t *dst_stream;
+
+      if (!fb->file_writer)
+        {
+          SVN_ERR(open_working_file_writer(&fb->file_writer, fb, fb->pool,
+                                           scratch_pool));
+        }
+
+      dst_stream = svn_wc__working_file_writer_get_stream(fb->file_writer);
+      SVN_ERR(svn_stream_reset(dst_stream));
+
+      if (install_from)
+        {
+          apr_file_t *file;
+          /* Open an unbuffered file, as svn_wc__db_pristine_read() does. */
+          SVN_ERR(svn_io_file_open(&file, install_from, APR_READ,
+                                   APR_OS_DEFAULT, scratch_pool));
+          src_stream = svn_stream_from_aprfile2(file, FALSE, scratch_pool);
+        }
+      else
+        {
+          SVN_ERR(svn_wc__db_pristine_read(&src_stream, NULL, eb->db,
+                                           eb->wcroot_abspath, new_checksum,
+                                           scratch_pool, scratch_pool));
+        }
+
+      SVN_ERR(svn_stream_copy3(src_stream, dst_stream, NULL, NULL, scratch_pool));
+
+      /* Remove the INSTALL_FROM file, as long as it doesn't refer to the
+         working file.  */
+      if (install_from && strcmp(install_from, fb->local_abspath) != 0)
+        SVN_ERR(svn_io_remove_file2(install_from, TRUE, scratch_pool));
+    }
+
+  /* If we are installing from the pristine contents, then go ahead and
+     record the fileinfo. That will be the "proper" values. Installing
+     from some random file means the fileinfo does NOT correspond to
+     the pristine (in which case, the fileinfo will be cleared for
+     safety's sake).  */
+  record_fileinfo = (install_from == NULL);
+
   SVN_ERR(svn_wc__db_base_add_file(eb->db, fb->local_abspath,
                                    eb->wcroot_abspath,
                                    fb->new_repos_relpath,
@@ -4580,9 +4732,17 @@ close_file(void *file_baton,
                                    keep_recorded_info,
                                    (fb->shadowed && fb->obstruction_found),
                                    conflict_skel,
+                                   install_pristine ? fb->file_writer : NULL,
+                                   record_fileinfo,
                                    all_work_items,
                                    scratch_pool));
 
+  if (fb->file_writer)
+    {
+      SVN_ERR(svn_wc__working_file_writer_close(fb->file_writer));
+      fb->file_writer = NULL;
+    }
+
   if (conflict_skel && eb->conflict_func)
     SVN_ERR(svn_wc__conflict_invoke_resolver(eb->db, fb->local_abspath,
                                              svn_node_file,

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1886490&r1=1886489&r2=1886490&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Sat Feb 13 14:30:10 2021
@@ -1752,6 +1752,59 @@ svn_wc__db_base_add_incomplete_directory
 }
 
 
+/* Record RECORDED_SIZE and RECORDED_TIME into top layer in NODES */
+static svn_error_t *
+db_record_fileinfo(svn_wc__db_wcroot_t *wcroot,
+                   const char *local_relpath,
+                   apr_int64_t recorded_size,
+                   apr_int64_t recorded_time,
+                   apr_pool_t *scratch_pool)
+{
+  svn_sqlite__stmt_t *stmt;
+  int affected_rows;
+
+  SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
+                                    STMT_UPDATE_NODE_FILEINFO));
+  SVN_ERR(svn_sqlite__bindf(stmt, "isii", wcroot->wc_id, local_relpath,
+                            recorded_size, recorded_time));
+  SVN_ERR(svn_sqlite__update(&affected_rows, stmt));
+
+  SVN_ERR_ASSERT(affected_rows == 1);
+
+  return SVN_NO_ERROR;
+}
+
+
+/* Install the working file provided by FILE_WRITER, optionally
+   recording its fileinfo. */
+static svn_error_t *
+install_working_file(svn_wc__db_wcroot_t *wcroot,
+                     const char *local_relpath,
+                     svn_wc__working_file_writer_t *file_writer,
+                     svn_boolean_t record_fileinfo,
+                     apr_pool_t *scratch_pool)
+{
+  const char *local_abspath;
+
+  if (record_fileinfo)
+    {
+      apr_time_t mtime;
+      apr_off_t size;
+
+      SVN_ERR(svn_wc__working_file_writer_get_info(&mtime, &size, file_writer,
+                                                   scratch_pool));
+      SVN_ERR(db_record_fileinfo(wcroot, local_relpath, size, mtime,
+                                 scratch_pool));
+    }
+
+  local_abspath = svn_dirent_join(wcroot->abspath, local_relpath, scratch_pool);
+  SVN_ERR(svn_wc__working_file_writer_install(file_writer, local_abspath,
+                                              scratch_pool));
+
+  return SVN_NO_ERROR;
+}
+
+
 svn_error_t *
 svn_wc__db_base_add_file(svn_wc__db_t *db,
                          const char *local_abspath,
@@ -1773,6 +1826,8 @@ svn_wc__db_base_add_file(svn_wc__db_t *d
                          svn_boolean_t keep_recorded_info,
                          svn_boolean_t insert_base_deleted,
                          const svn_skel_t *conflict,
+                         svn_wc__working_file_writer_t *file_writer,
+                         svn_boolean_t record_fileinfo,
                          const svn_skel_t *work_items,
                          apr_pool_t *scratch_pool)
 {
@@ -1828,9 +1883,22 @@ svn_wc__db_base_add_file(svn_wc__db_t *d
   ibb.conflict = conflict;
   ibb.work_items = work_items;
 
-  SVN_WC__DB_WITH_TXN(
-            insert_base_node(&ibb, wcroot, local_relpath, scratch_pool),
-            wcroot);
+  if (file_writer)
+    {
+      /* Atomically update the db and install the file (installation
+       * of the file is atomic, because it happens as a rename). */
+      SVN_WC__DB_WITH_TXN2(
+        insert_base_node(&ibb, wcroot, local_relpath, scratch_pool),
+        install_working_file(wcroot, local_relpath, file_writer,
+                             record_fileinfo, scratch_pool),
+        wcroot);
+    }
+  else
+    {
+      SVN_WC__DB_WITH_TXN(
+        insert_base_node(&ibb, wcroot, local_relpath, scratch_pool),
+        wcroot);
+    }
 
   /* If this used to be a directory we should remove children so pass
    * depth infinity. */
@@ -6045,28 +6113,6 @@ svn_wc__db_op_add_symlink(svn_wc__db_t *
 
   return SVN_NO_ERROR;
 }
-
-/* Record RECORDED_SIZE and RECORDED_TIME into top layer in NODES */
-static svn_error_t *
-db_record_fileinfo(svn_wc__db_wcroot_t *wcroot,
-                   const char *local_relpath,
-                   apr_int64_t recorded_size,
-                   apr_int64_t recorded_time,
-                   apr_pool_t *scratch_pool)
-{
-  svn_sqlite__stmt_t *stmt;
-  int affected_rows;
-
-  SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
-                                    STMT_UPDATE_NODE_FILEINFO));
-  SVN_ERR(svn_sqlite__bindf(stmt, "isii", wcroot->wc_id, local_relpath,
-                            recorded_size, recorded_time));
-  SVN_ERR(svn_sqlite__update(&affected_rows, stmt));
-
-  SVN_ERR_ASSERT(affected_rows == 1);
-
-  return SVN_NO_ERROR;
-}
 
 
 svn_error_t *

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.h?rev=1886490&r1=1886489&r2=1886490&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.h (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.h Sat Feb 13 14:30:10 2021
@@ -49,6 +49,8 @@
 
 #include "svn_private_config.h"
 
+#include "working_file_writer.h"
+
 #ifdef __cplusplus
 extern "C" {
 #endif /* __cplusplus */
@@ -503,6 +505,10 @@ svn_wc__db_base_add_incomplete_directory
    Unless KEEP_RECORDED_INFO is set to TRUE, recorded size and timestamp values
    will be cleared.
 
+   If FILE_WRITER is not NULL, atomically complete installation of the file to
+   LOCAL_ABSPATH together with updating the DB.  If RECORD_FILEINFO is non-zero,
+   also record the size and timestamp values of the installed file.
+
    All temporary allocations will be made in SCRATCH_POOL.
 */
 svn_error_t *
@@ -526,6 +532,8 @@ svn_wc__db_base_add_file(svn_wc__db_t *d
                          svn_boolean_t keep_recorded_info,
                          svn_boolean_t insert_base_deleted,
                          const svn_skel_t *conflict,
+                         svn_wc__working_file_writer_t *file_writer,
+                         svn_boolean_t record_fileinfo,
                          const svn_skel_t *work_items,
                          apr_pool_t *scratch_pool);
 

Added: subversion/trunk/subversion/libsvn_wc/working_file_writer.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/working_file_writer.c?rev=1886490&view=auto
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/working_file_writer.c (added)
+++ subversion/trunk/subversion/libsvn_wc/working_file_writer.c Sat Feb 13 14:30:10 2021
@@ -0,0 +1,223 @@
+/*
+ * working_file_writer.c :  utility to prepare and install working files
+ *
+ * ====================================================================
+ *    Licensed to the Apache Software Foundation (ASF) under one
+ *    or more contributor license agreements.  See the NOTICE file
+ *    distributed with this work for additional information
+ *    regarding copyright ownership.  The ASF licenses this file
+ *    to you under the Apache License, Version 2.0 (the
+ *    "License"); you may not use this file except in compliance
+ *    with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing,
+ *    software distributed under the License is distributed on an
+ *    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *    KIND, either express or implied.  See the License for the
+ *    specific language governing permissions and limitations
+ *    under the License.
+ * ====================================================================
+ */
+
+#include "svn_props.h"
+#include "svn_subst.h"
+#include "svn_time.h"
+#include "svn_path.h"
+
+#include "private/svn_io_private.h"
+
+#include "working_file_writer.h"
+
+struct svn_wc__working_file_writer_t
+{
+  apr_pool_t *pool;
+  const char *tmp_abspath;
+  svn_boolean_t is_special;
+  svn_stream_t *install_stream;
+  svn_stream_t *write_stream;
+};
+
+static apr_status_t
+cleanup_file_writer(void *baton)
+{
+  svn_wc__working_file_writer_t *writer = baton;
+
+  if (writer->install_stream)
+    {
+      svn_error_clear(svn_stream__install_delete(writer->install_stream,
+                                                 writer->pool));
+      writer->install_stream = NULL;
+    }
+
+  return APR_SUCCESS;
+}
+
+svn_error_t *
+svn_wc__working_file_writer_open(svn_wc__working_file_writer_t **writer_p,
+                                 const char *tmp_abspath,
+                                 apr_time_t final_mtime,
+                                 apr_hash_t *props,
+                                 svn_revnum_t changed_rev,
+                                 apr_time_t changed_date,
+                                 const char *changed_author,
+                                 svn_boolean_t has_lock,
+                                 svn_boolean_t is_added,
+                                 const char *repos_root_url,
+                                 const char *repos_relpath,
+                                 apr_pool_t *result_pool,
+                                 apr_pool_t *scratch_pool)
+{
+  svn_wc__working_file_writer_t *writer;
+  const char *url;
+  svn_boolean_t special;
+  svn_boolean_t executable;
+  svn_boolean_t needs_lock;
+  const char *keywords_propval;
+  apr_hash_t *keywords;
+  const char *eol_propval;
+  svn_subst_eol_style_t eol_style;
+  const char *eol;
+  svn_stream_t *install_stream;
+  svn_stream_t *write_stream;
+
+  url = svn_path_url_add_component2(repos_root_url, repos_relpath, scratch_pool);
+
+  special = svn_prop_get_value(props, SVN_PROP_SPECIAL) != NULL;
+  executable = svn_prop_get_value(props, SVN_PROP_EXECUTABLE) != NULL;
+  needs_lock = svn_prop_get_value(props, SVN_PROP_NEEDS_LOCK) != NULL;
+
+  eol_propval = svn_prop_get_value(props, SVN_PROP_EOL_STYLE);
+  svn_subst_eol_style_from_value(&eol_style, &eol, eol_propval);
+
+  keywords_propval = svn_prop_get_value(props, SVN_PROP_KEYWORDS);
+  if (keywords_propval)
+    {
+      SVN_ERR(svn_subst_build_keywords3(&keywords, keywords_propval,
+                                        apr_psprintf(scratch_pool, "%ld",
+                                                     changed_rev),
+                                        url, repos_root_url, changed_date,
+                                        changed_author, scratch_pool));
+      if (apr_hash_count(keywords) <= 0)
+        keywords = NULL;
+    }
+  else
+    keywords = NULL;
+
+  SVN_ERR(svn_stream__create_for_install(&install_stream, tmp_abspath,
+                                         result_pool, scratch_pool));
+
+  if (needs_lock && !is_added && !has_lock)
+    svn_stream__install_stream_set_read_only(install_stream, TRUE);
+  if (executable)
+    svn_stream__install_stream_set_executable(install_stream, TRUE);
+  if (final_mtime >= 0)
+    svn_stream__install_stream_set_affected_time(install_stream, final_mtime);
+
+  write_stream = install_stream;
+
+  if (svn_subst_translation_required(eol_style, eol, keywords,
+                                     FALSE /* special */,
+                                     TRUE /* force_eol_check */))
+    {
+      write_stream = svn_subst_stream_translated(write_stream,
+                                                 eol,
+                                                 TRUE /* repair */,
+                                                 keywords,
+                                                 TRUE /* expand */,
+                                                 result_pool);
+    }
+
+  writer = apr_pcalloc(result_pool, sizeof(*writer));
+  writer->pool = result_pool;
+  writer->tmp_abspath = apr_pstrdup(result_pool, tmp_abspath);
+  writer->is_special = special;
+  writer->install_stream = install_stream;
+  writer->write_stream = write_stream;
+
+  apr_pool_cleanup_register(result_pool, writer, cleanup_file_writer,
+                            apr_pool_cleanup_null);
+
+  *writer_p = writer;
+  return SVN_NO_ERROR;
+}
+
+svn_stream_t *
+svn_wc__working_file_writer_get_stream(svn_wc__working_file_writer_t *writer)
+{
+  return writer->write_stream;
+}
+
+svn_error_t *
+svn_wc__working_file_writer_get_info(apr_time_t *mtime_p,
+                                     apr_off_t *size_p,
+                                     svn_wc__working_file_writer_t *writer,
+                                     apr_pool_t *scratch_pool)
+{
+  SVN_ERR(svn_stream__install_get_info(mtime_p, size_p, writer->install_stream,
+                                       scratch_pool));
+
+  return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_wc__working_file_writer_install(svn_wc__working_file_writer_t *writer,
+                                    const char *target_abspath,
+                                    apr_pool_t *scratch_pool)
+{
+  if (writer->is_special)
+    {
+      const char *temp_path;
+      svn_stream_t *src_stream;
+      svn_stream_t *dst_stream;
+
+      /* Install the current contents to a temporary file, and use it to
+         create the resulting special file. */
+      SVN_ERR(svn_io_open_unique_file3(NULL, &temp_path, writer->tmp_abspath,
+                                       svn_io_file_del_on_pool_cleanup,
+                                       scratch_pool, scratch_pool));
+      SVN_ERR(svn_stream__install_stream(writer->install_stream, temp_path,
+                                         TRUE, scratch_pool));
+      writer->install_stream = NULL;
+
+      /* When this stream is closed, the resulting special file will
+         atomically be created/moved into place at LOCAL_ABSPATH. */
+      SVN_ERR(svn_subst_create_specialfile(&dst_stream, target_abspath,
+                                           scratch_pool, scratch_pool));
+      SVN_ERR(svn_stream_open_readonly(&src_stream, temp_path, scratch_pool,
+                                       scratch_pool));
+      SVN_ERR(svn_stream_copy3(src_stream, dst_stream, NULL, NULL,
+                               scratch_pool));
+      SVN_ERR(svn_io_remove_file2(temp_path, TRUE, scratch_pool));
+
+      return SVN_NO_ERROR;
+    }
+  else
+    {
+      /* With a single db we might want to install files in a missing directory.
+         Simply trying this scenario on error won't do any harm and at least
+         one user reported this problem on IRC. */
+      SVN_ERR(svn_stream__install_stream(writer->install_stream,
+                                         target_abspath, TRUE,
+                                         scratch_pool));
+      writer->install_stream = NULL;
+    }
+
+  return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_wc__working_file_writer_close(svn_wc__working_file_writer_t *writer)
+{
+  if (writer->install_stream)
+    {
+      svn_stream_t *stream = writer->install_stream;
+      /* Do not retry deleting if it fails, as the stream may already
+         be in an invalid state. */
+      writer->install_stream = NULL;
+      SVN_ERR(svn_stream__install_delete(stream, writer->pool));
+    }
+
+  return SVN_NO_ERROR;
+}

Propchange: subversion/trunk/subversion/libsvn_wc/working_file_writer.c
------------------------------------------------------------------------------
    svn:eol-style = native

Added: subversion/trunk/subversion/libsvn_wc/working_file_writer.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/working_file_writer.h?rev=1886490&view=auto
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/working_file_writer.h (added)
+++ subversion/trunk/subversion/libsvn_wc/working_file_writer.h Sat Feb 13 14:30:10 2021
@@ -0,0 +1,86 @@
+/*
+ * working_file_writer.h :  utility to prepare and install working files
+ *
+ * ====================================================================
+ *    Licensed to the Apache Software Foundation (ASF) under one
+ *    or more contributor license agreements.  See the NOTICE file
+ *    distributed with this work for additional information
+ *    regarding copyright ownership.  The ASF licenses this file
+ *    to you under the Apache License, Version 2.0 (the
+ *    "License"); you may not use this file except in compliance
+ *    with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing,
+ *    software distributed under the License is distributed on an
+ *    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *    KIND, either express or implied.  See the License for the
+ *    specific language governing permissions and limitations
+ *    under the License.
+ * ====================================================================
+ */
+
+#ifndef SVN_LIBSVN_WC_WORKING_FILE_WRITER_H
+#define SVN_LIBSVN_WC_WORKING_FILE_WRITER_H
+
+#include <apr_pools.h>
+#include "svn_types.h"
+#include "svn_io.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif /* __cplusplus */
+
+/* Context of the working file writer. */
+typedef struct svn_wc__working_file_writer_t svn_wc__working_file_writer_t;
+
+/* Create a write context for the (provisioned) working file with the specified
+   properties.  The file writer must be given data in the repository-normal
+   form and will handle its translation according to the specified properties.
+   Place the temporary file in the TMP_ABSPATH directory.  If FINAL_MTIME is
+   non-negative, it will be set as the last modification time on the installed
+   file. */
+svn_error_t *
+svn_wc__working_file_writer_open(svn_wc__working_file_writer_t **writer_p,
+                                 const char *tmp_abspath,
+                                 apr_time_t final_mtime,
+                                 apr_hash_t *props,
+                                 svn_revnum_t changed_rev,
+                                 apr_time_t changed_date,
+                                 const char *changed_author,
+                                 svn_boolean_t has_lock,
+                                 svn_boolean_t is_added,
+                                 const char *repos_root_url,
+                                 const char *repos_relpath,
+                                 apr_pool_t *result_pool,
+                                 apr_pool_t *scratch_pool);
+
+/* Get the writable stream for WRITER.  The returned stream supports reset
+   and is configured to be truncated on seek. */
+svn_stream_t *
+svn_wc__working_file_writer_get_stream(svn_wc__working_file_writer_t *writer);
+
+/* Retrieve file information for WRITER.
+   MTIME_P and SIZE_P both may be NULL to allow for partial queries. */
+svn_error_t *
+svn_wc__working_file_writer_get_info(apr_time_t *mtime_p,
+                                     apr_off_t *size_p,
+                                     svn_wc__working_file_writer_t *writer,
+                                     apr_pool_t *scratch_pool);
+
+/* Atomically install the contents of WRITER to TARGET_ABSPATH. */
+svn_error_t *
+svn_wc__working_file_writer_install(svn_wc__working_file_writer_t *writer,
+                                    const char *target_abspath,
+                                    apr_pool_t *scratch_pool);
+
+/* Cleanup WRITER by closing and removing the underlying file. */
+svn_error_t *
+svn_wc__working_file_writer_close(svn_wc__working_file_writer_t *writer);
+
+#ifdef __cplusplus
+}
+#endif /* __cplusplus */
+
+#endif /* SVN_LIBSVN_WC_WORKING_FILE_WRITER_H */

Propchange: subversion/trunk/subversion/libsvn_wc/working_file_writer.h
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: subversion/trunk/subversion/libsvn_wc/workqueue.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/workqueue.c?rev=1886490&r1=1886489&r2=1886490&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/workqueue.c (original)
+++ subversion/trunk/subversion/libsvn_wc/workqueue.c Sat Feb 13 14:30:10 2021
@@ -37,6 +37,7 @@
 #include "adm_files.h"
 #include "conflicts.h"
 #include "translate.h"
+#include "working_file_writer.h"
 
 #include "private/svn_io_private.h"
 #include "private/svn_skel.h"
@@ -463,19 +464,22 @@ run_file_install(work_item_baton_t *wqb,
   const char *local_abspath;
   svn_boolean_t use_commit_times;
   svn_boolean_t record_fileinfo;
-  svn_boolean_t special;
   svn_stream_t *src_stream;
-  svn_subst_eol_style_t style;
-  const char *eol;
-  apr_hash_t *keywords;
   const char *temp_dir_abspath;
-  svn_stream_t *dst_stream;
   apr_int64_t val;
   const char *wcroot_abspath;
   const char *source_abspath;
   const svn_checksum_t *checksum;
   apr_hash_t *props;
   apr_time_t changed_date;
+  svn_revnum_t changed_rev;
+  const char *changed_author;
+  apr_time_t final_mtime;
+  svn_wc__db_status_t status;
+  svn_wc__db_lock_t *lock;
+  const char *repos_relpath;
+  const char *repos_root_url;
+  svn_wc__working_file_writer_t *file_writer;
 
   local_relpath = apr_pstrmemdup(scratch_pool, arg1->data, arg1->len);
   SVN_ERR(svn_wc__db_from_relpath(&local_abspath, db, wri_abspath,
@@ -524,101 +528,54 @@ run_file_install(work_item_baton_t *wqb,
                                                   scratch_pool, scratch_pool));
     }
 
-  SVN_ERR(svn_stream_open_readonly(&src_stream, source_abspath,
-                                   scratch_pool, scratch_pool));
-
-  /* Fetch all the translation bits.  */
-  SVN_ERR(svn_wc__get_translate_info(&style, &eol,
-                                     &keywords,
-                                     &special, db, local_abspath,
-                                     props, FALSE,
-                                     scratch_pool, scratch_pool));
-  if (special)
-    {
-      /* When this stream is closed, the resulting special file will
-         atomically be created/moved into place at LOCAL_ABSPATH.  */
-      SVN_ERR(svn_subst_create_specialfile(&dst_stream, local_abspath,
-                                           scratch_pool, scratch_pool));
-
-      /* Copy the "repository normal" form of the special file into the
-         special stream.  */
-      SVN_ERR(svn_stream_copy3(src_stream, dst_stream,
-                               cancel_func, cancel_baton,
-                               scratch_pool));
-
-      /* No need to set exec or read-only flags on special files.  */
-
-      /* ### Shouldn't this record a timestamp and size, etc.? */
-      return SVN_NO_ERROR;
-    }
-
-  if (svn_subst_translation_required(style, eol, keywords,
-                                     FALSE /* special */,
-                                     TRUE /* force_eol_check */))
-    {
-      /* Wrap it in a translating (expanding) stream.  */
-      src_stream = svn_subst_stream_translated(src_stream, eol,
-                                               TRUE /* repair */,
-                                               keywords,
-                                               TRUE /* expand */,
-                                               scratch_pool);
-    }
-
   /* Where is the Right Place to put a temp file in this working copy?  */
   SVN_ERR(svn_wc__db_temp_wcroot_tempdir(&temp_dir_abspath,
                                          db, wcroot_abspath,
                                          scratch_pool, scratch_pool));
 
-  /* Translate to a temporary file. We don't want the user seeing a partial
-     file, nor let them muck with it while we translate. We may also need to
-     get its TRANSLATED_SIZE before the user can monkey it.  */
-  SVN_ERR(svn_stream__create_for_install(&dst_stream, temp_dir_abspath,
-                                         scratch_pool, scratch_pool));
+  SVN_ERR(svn_wc__db_read_info(&status, NULL, NULL, &repos_relpath,
+                               &repos_root_url, NULL, &changed_rev, NULL,
+                               &changed_author, NULL, NULL, NULL, NULL,
+                               NULL, NULL, NULL, &lock, NULL, NULL,
+                               NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+                               db, local_abspath,
+                               scratch_pool, scratch_pool));
+  /* Handle special statuses (e.g. added) */
+  if (!repos_relpath)
+     SVN_ERR(svn_wc__db_read_repos_info(NULL, &repos_relpath,
+                                        &repos_root_url, NULL,
+                                        db, local_abspath,
+                                        scratch_pool, scratch_pool));
 
-  /* Copy from the source to the dest, translating as we go. This will also
-     close both streams.  */
-  SVN_ERR(svn_stream_copy3(src_stream, dst_stream,
-                           cancel_func, cancel_baton,
-                           scratch_pool));
+  if (use_commit_times && changed_date)
+    final_mtime = changed_date;
+  else
+    final_mtime = -1;
 
-  /* All done. Move the file into place.  */
-  /* With a single db we might want to install files in a missing directory.
-     Simply trying this scenario on error won't do any harm and at least
-     one user reported this problem on IRC. */
-  SVN_ERR(svn_stream__install_stream(dst_stream, local_abspath,
-                                     TRUE /* make_parents*/, scratch_pool));
-
-  /* Tweak the on-disk file according to its properties.  */
-#ifndef WIN32
-  if (props && svn_hash_gets(props, SVN_PROP_EXECUTABLE))
-    SVN_ERR(svn_io_set_file_executable(local_abspath, TRUE, FALSE,
-                                       scratch_pool));
-#endif
-
-  /* Note that this explicitly checks the pristine properties, to make sure
-     that when the lock is locally set (=modification) it is not read only */
-  if (props && svn_hash_gets(props, SVN_PROP_NEEDS_LOCK))
-    {
-      svn_wc__db_status_t status;
-      svn_wc__db_lock_t *lock;
-      SVN_ERR(svn_wc__db_read_info(&status, NULL, NULL, NULL, NULL, NULL, NULL,
-                                   NULL, NULL, NULL, NULL, NULL, NULL, NULL,
-                                   NULL, NULL, &lock, NULL, NULL, NULL, NULL,
-                                   NULL, NULL, NULL, NULL, NULL, NULL,
-                                   db, local_abspath,
+  SVN_ERR(svn_wc__working_file_writer_open(&file_writer,
+                                           temp_dir_abspath,
+                                           final_mtime,
+                                           props,
+                                           changed_rev,
+                                           changed_date,
+                                           changed_author,
+                                           lock != NULL,
+                                           status == svn_wc__db_status_added,
+                                           repos_root_url,
+                                           repos_relpath,
+                                           scratch_pool,
+                                           scratch_pool));
+
+  SVN_ERR(svn_stream_open_readonly(&src_stream, source_abspath,
                                    scratch_pool, scratch_pool));
 
-      if (!lock && status != svn_wc__db_status_added)
-        SVN_ERR(svn_io_set_file_read_only(local_abspath, FALSE, scratch_pool));
-    }
+  SVN_ERR(svn_stream_copy3(src_stream,
+                           svn_wc__working_file_writer_get_stream(file_writer),
+                           cancel_func, cancel_baton,
+                           scratch_pool));
 
-  if (use_commit_times)
-    {
-      if (changed_date)
-        SVN_ERR(svn_io_set_file_affected_time(changed_date,
-                                              local_abspath,
+  SVN_ERR(svn_wc__working_file_writer_install(file_writer, local_abspath,
                                               scratch_pool));
-    }
 
   /* ### this should happen before we rename the file into place.  */
   if (record_fileinfo)

Modified: subversion/trunk/subversion/tests/cmdline/update_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/update_tests.py?rev=1886490&r1=1886489&r2=1886490&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/update_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/update_tests.py Sat Feb 13 14:30:10 2021
@@ -6840,12 +6840,14 @@ def update_delete_switched(sbox):
   svntest.actions.run_and_verify_update(wc_dir, None, None, expected_status,
                                         [], False, sbox.ospath('A'), '-r', 0)
 
-@XFail()
 def update_add_missing_local_add(sbox):
   "update adds missing local addition"
 
   sbox.build(read_only=True)
 
+  ### This used to insert an invalid workqueue item, but the issue vanished
+  ### when the update editor was changed.  Annotate this line for more info.
+
   # Note that updating 'A' to r0 doesn't reproduce this issue...
   sbox.simple_update('', revision='0')
   sbox.simple_mkdir('A')

Modified: subversion/trunk/subversion/tests/libsvn_subr/io-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/io-test.c?rev=1886490&r1=1886489&r2=1886490&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_subr/io-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_subr/io-test.c Sat Feb 13 14:30:10 2021
@@ -1043,6 +1043,82 @@ test_install_stream_over_readonly_file(a
 }
 
 static svn_error_t *
+test_install_stream_set_read_only(apr_pool_t *pool)
+{
+  const char *tmp_dir;
+  const char *final_abspath;
+  svn_stream_t *stream;
+  svn_stringbuf_t *actual_content;
+  apr_finfo_t finfo;
+  svn_boolean_t is_read_only;
+
+  /* Create an empty directory. */
+  SVN_ERR(svn_test_make_sandbox_dir(&tmp_dir,
+                                    "test_install_stream_set_read_only",
+                                    pool));
+
+  final_abspath = svn_dirent_join(tmp_dir, "stream1", pool);
+
+  SVN_ERR(svn_stream__create_for_install(&stream, tmp_dir, pool, pool));
+  SVN_ERR(svn_stream_puts(stream, "stream1 content"));
+  SVN_ERR(svn_stream_close(stream));
+  svn_stream__install_stream_set_read_only(stream, TRUE);
+  SVN_ERR(svn_stream__install_stream(stream,
+                                     final_abspath,
+                                     TRUE,
+                                     pool));
+
+  SVN_ERR(svn_stringbuf_from_file2(&actual_content,
+                                   final_abspath,
+                                   pool));
+
+  SVN_TEST_STRING_ASSERT(actual_content->data, "stream1 content");
+
+  SVN_ERR(svn_io_stat(&finfo, final_abspath, SVN__APR_FINFO_READONLY, pool));
+  SVN_ERR(svn_io__is_finfo_read_only(&is_read_only, &finfo, pool));
+  SVN_TEST_ASSERT(is_read_only);
+
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+test_install_stream_set_affected_time(apr_pool_t *pool)
+{
+  const char *tmp_dir;
+  const char *final_abspath;
+  svn_stream_t *stream;
+  svn_stringbuf_t *actual_content;
+  apr_finfo_t finfo;
+
+  /* Create an empty directory. */
+  SVN_ERR(svn_test_make_sandbox_dir(&tmp_dir,
+                                    "test_install_stream_set_affected_time",
+                                    pool));
+
+  final_abspath = svn_dirent_join(tmp_dir, "stream1", pool);
+
+  SVN_ERR(svn_stream__create_for_install(&stream, tmp_dir, pool, pool));
+  SVN_ERR(svn_stream_puts(stream, "stream1 content"));
+  SVN_ERR(svn_stream_close(stream));
+  svn_stream__install_stream_set_affected_time(stream, 123456789);
+  SVN_ERR(svn_stream__install_stream(stream,
+                                     final_abspath,
+                                     TRUE,
+                                     pool));
+
+  SVN_ERR(svn_stringbuf_from_file2(&actual_content,
+                                   final_abspath,
+                                   pool));
+
+  SVN_TEST_STRING_ASSERT(actual_content->data, "stream1 content");
+
+  SVN_ERR(svn_io_stat(&finfo, final_abspath, APR_FINFO_MTIME, pool));
+  SVN_TEST_INT_ASSERT(finfo.mtime, 123456789);
+
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
 test_file_size_get(apr_pool_t *pool)
 {
   const char *tmp_dir, *path;
@@ -1254,6 +1330,10 @@ static struct svn_test_descriptor_t test
                    "test svn_stream__install_stream to long path"),
     SVN_TEST_PASS2(test_install_stream_over_readonly_file,
                    "test svn_stream__install_stream over RO file"),
+    SVN_TEST_PASS2(test_install_stream_set_read_only,
+                   "test svn_stream__install_stream_set_read_only"),
+    SVN_TEST_PASS2(test_install_stream_set_affected_time,
+                   "test svn_stream__install_stream_set_affected_time"),
     SVN_TEST_PASS2(test_file_size_get,
                    "test svn_io_file_size_get"),
     SVN_TEST_PASS2(test_file_rename2,

Modified: subversion/trunk/subversion/tests/libsvn_wc/db-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_wc/db-test.c?rev=1886490&r1=1886489&r2=1886490&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_wc/db-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_wc/db-test.c Sat Feb 13 14:30:10 2021
@@ -635,7 +635,7 @@ test_inserting_nodes(apr_pool_t *pool)
             1, TIME_1a, AUTHOR_1,
             checksum,
             NULL, FALSE, FALSE, NULL, NULL, FALSE, FALSE,
-            NULL, NULL,
+            NULL, NULL, FALSE, NULL,
             pool));
 
   /* Create a new symlink node. */



Re: svn commit: r1886490 - in /subversion/trunk/subversion: include/private/ libsvn_subr/ libsvn_wc/ tests/cmdline/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Stefan Sperling <st...@elego.de>.
On Sat, Feb 13, 2021 at 04:50:10PM +0100, Branko Čibej wrote:
> On 13.02.2021 15:32, Evgeny Kotkov wrote:
> > My attempt to fix this is by making checkout stream data to both pristine and
> > the (projected) working file, so that the actual install would then happen as
> > just an atomic rename.  Since we now never stop reading from the connection,
> > the timeouts should no longer be an issue.  The new approach also has several
> > nice properties, such as not having to re-read the pristine files, not
> > interfering with the network-level buffering, TCP slow starts, and etc.
> > I see that it reduces the amount of both read and write I/O during all
> > checkouts, which should give a mild overall increase of how fast the
> > checkouts work.
> 
> I like this concept very much indeed.

Agreed, yes! I believe I've seen reports of connections timing out
during checkout/update, which weren't understood and couldn't be
solved in other ways than bumping up server-side limits.
This might explain what was going on there. Thanks Evgeny!

Re: svn commit: r1886490 - in /subversion/trunk/subversion: include/private/ libsvn_subr/ libsvn_wc/ tests/cmdline/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Branko Čibej <br...@apache.org>.
On 13.02.2021 15:32, Evgeny Kotkov wrote:
> Evgeny Kotkov <ko...@apache.org> writes:
>
>> URL: http://svn.apache.org/viewvc?rev=1886490&view=rev
>> Log:
>> In the update editor, stream data to both pristine and working files.
> ...
>> Several quick benchmarks:
>>
>>    - Checking out subversion/trunk over https://
>>
>>      Total time:  3.861 s  →  2.812 s
>>         Read IO:  57322 KB  →  316 KB
>>        Write IO:  455013 KB  →  359977 KB
>>
>>    - Checking out 4 large binary files (7.4 GB) over https://
>>
>>      Total time:   91.594 s   →  70.214 s
>>         Read IO:   7798883 KB  →  19 KB
>>        Write IO:   15598167 KB  →  15598005 KB
> Hey everyone,
>
> Here's an improvement I have been working on recently.
>
> Apparently, the client has an (implicit) limit on the size of directories that
> can be safely checked out over HTTP without hitting a timeout.  The problem is
> that when the client installs the new working files, it does so in a separate
> step.  This step happens per-directory and involves copying and possibly
> translating the pristine contents into new working files.  While that happens,
> nothing is read from the connection.  So the amount of work that can be done
> without hitting a timeout is limited.
>
> Assuming the default HTTP timeout = 60 seconds of httpd 2.4.x and a relatively
> fast disk, that puts the limit at around 6 GB for any directory.  Not cool.
>
> My attempt to fix this is by making checkout stream data to both pristine and
> the (projected) working file, so that the actual install would then happen as
> just an atomic rename.  Since we now never stop reading from the connection,
> the timeouts should no longer be an issue.  The new approach also has several
> nice properties, such as not having to re-read the pristine files, not
> interfering with the network-level buffering, TCP slow starts, and etc.
> I see that it reduces the amount of both read and write I/O during all
> checkouts, which should give a mild overall increase of how fast the
> checkouts work.

I like this concept very much indeed.

> Noting that this change only fixes "svn checkout", but not "svn export".

It also affects 'svn update', right, since 'checkout' is implemented as 
an update of an empty working copy.

> Export uses a separate implementation of the delta editor, and it should
> be possible to update it in a similar way — but I'm leaving that for future
> work for now.

The only thing that mildly worries me about the implementation is that 
the wc-db code is now responsible for installing working files. There's 
a bit of an abstraction leak here. On the other hand, you need to make 
the whole operation transactional with the sqlite updates, so ... maybe 
it's better this way.

-- Brane

Re: svn commit: r1886490 - in /subversion/trunk/subversion: include/private/ libsvn_subr/ libsvn_wc/ tests/cmdline/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Evgeny Kotkov wrote on Thu, 18 Feb 2021 20:53 +00:00:
> Daniel Shahaf <d....@daniel.shahaf.name> writes:
> 
> > Thanks for checking.  How about adding it somewhere, then?
> 
> Yes, that will probably be useful.  Would you have an opportunity to do so?

No.

Re: svn commit: r1886490 - in /subversion/trunk/subversion: include/private/ libsvn_subr/ libsvn_wc/ tests/cmdline/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Evgeny Kotkov <ev...@visualsvn.com>.
Daniel Shahaf <d....@daniel.shahaf.name> writes:

> Thanks for checking.  How about adding it somewhere, then?

Yes, that will probably be useful.  Would you have an opportunity to do so?


Thanks,
Evgeny Kotkov

Re: svn commit: r1886490 - in /subversion/trunk/subversion: include/private/ libsvn_subr/ libsvn_wc/ tests/cmdline/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Evgeny Kotkov wrote on Thu, 18 Feb 2021 11:23 +00:00:
> Daniel Shahaf <d....@daniel.shahaf.name> writes:
> 
> > > Noting that this change only fixes "svn checkout", but not "svn export".
> > > Export uses a separate implementation of the delta editor, and it should
> > > be possible to update it in a similar way — but I'm leaving that for
> > > future work for now.
> >
> > Are this problem and its proposed solution documented in a jira ticket?
> 
> No, I don't think so.
> 
> (Apparently, SVN-3264 [1] describes the checkout case, but I haven't done
> a thorough search to see if export is mentioned somewhere.)
> 

Thanks for checking.  How about adding it somewhere, then?

Cheers,

Daniel

> [1] https://issues.apache.org/jira/browse/SVN-3264
> 
> 
> Regards,
> Evgeny Kotkov
>

Re: svn commit: r1886490 - in /subversion/trunk/subversion: include/private/ libsvn_subr/ libsvn_wc/ tests/cmdline/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Evgeny Kotkov <ev...@visualsvn.com>.
Daniel Shahaf <d....@daniel.shahaf.name> writes:

> > Noting that this change only fixes "svn checkout", but not "svn export".
> > Export uses a separate implementation of the delta editor, and it should
> > be possible to update it in a similar way — but I'm leaving that for
> > future work for now.
>
> Are this problem and its proposed solution documented in a jira ticket?

No, I don't think so.

(Apparently, SVN-3264 [1] describes the checkout case, but I haven't done
a thorough search to see if export is mentioned somewhere.)

[1] https://issues.apache.org/jira/browse/SVN-3264


Regards,
Evgeny Kotkov

Re: svn commit: r1886490 - in /subversion/trunk/subversion: include/private/ libsvn_subr/ libsvn_wc/ tests/cmdline/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Evgeny Kotkov wrote on Sat, Feb 13, 2021 at 17:32:25 +0300:
> Apparently, the client has an (implicit) limit on the size of directories that
> can be safely checked out over HTTP without hitting a timeout.
[...]
> Noting that this change only fixes "svn checkout", but not "svn export".
> Export uses a separate implementation of the delta editor, and it should
> be possible to update it in a similar way — but I'm leaving that for future
> work for now.

Are this problem and its proposed solution documented in a jira ticket?

Re: svn commit: r1886490 - in /subversion/trunk/subversion: include/private/ libsvn_subr/ libsvn_wc/ tests/cmdline/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Evgeny Kotkov <ev...@visualsvn.com>.
Evgeny Kotkov <ev...@visualsvn.com> writes:

> My attempt to fix this is by making checkout stream data to both pristine
> and the (projected) working file, so that the actual install would then
> happen as just an atomic rename.

For the record, I made a few additional changes, including a significant
overhaul of the install stream's timestamp handling in r1886774 [1].

> Noting that this change only fixes "svn checkout", but not "svn export".
> Export uses a separate implementation of the delta editor, and it should
> be possible to update it in a similar way — but I'm leaving that for future
> work for now.

For this part with "svn export", r1886843 [2] should do the trick.

While here, r1887108 [3] begins using the new file writer utility also
when exporting from an existing working copy.

[1] https://svn.apache.org/r1886774
[2] https://svn.apache.org/r1886843
[3] https://svn.apache.org/r1887108


Thanks,
Evgeny Kotkov

Re: svn commit: r1886490 - in /subversion/trunk/subversion: include/private/ libsvn_subr/ libsvn_wc/ tests/cmdline/ tests/libsvn_subr/ tests/libsvn_wc/

Posted by Evgeny Kotkov <ev...@visualsvn.com>.
Evgeny Kotkov <ko...@apache.org> writes:

> URL: http://svn.apache.org/viewvc?rev=1886490&view=rev
> Log:
> In the update editor, stream data to both pristine and working files.
...
> Several quick benchmarks:
>
>   - Checking out subversion/trunk over https://
>
>     Total time:  3.861 s  →  2.812 s
>        Read IO:  57322 KB  →  316 KB
>       Write IO:  455013 KB  →  359977 KB
>
>   - Checking out 4 large binary files (7.4 GB) over https://
>
>     Total time:   91.594 s   →  70.214 s
>        Read IO:   7798883 KB  →  19 KB
>       Write IO:   15598167 KB  →  15598005 KB

Hey everyone,

Here's an improvement I have been working on recently.

Apparently, the client has an (implicit) limit on the size of directories that
can be safely checked out over HTTP without hitting a timeout.  The problem is
that when the client installs the new working files, it does so in a separate
step.  This step happens per-directory and involves copying and possibly
translating the pristine contents into new working files.  While that happens,
nothing is read from the connection.  So the amount of work that can be done
without hitting a timeout is limited.

Assuming the default HTTP timeout = 60 seconds of httpd 2.4.x and a relatively
fast disk, that puts the limit at around 6 GB for any directory.  Not cool.

My attempt to fix this is by making checkout stream data to both pristine and
the (projected) working file, so that the actual install would then happen as
just an atomic rename.  Since we now never stop reading from the connection,
the timeouts should no longer be an issue.  The new approach also has several
nice properties, such as not having to re-read the pristine files, not
interfering with the network-level buffering, TCP slow starts, and etc.
I see that it reduces the amount of both read and write I/O during all
checkouts, which should give a mild overall increase of how fast the
checkouts work.

Noting that this change only fixes "svn checkout", but not "svn export".
Export uses a separate implementation of the delta editor, and it should
be possible to update it in a similar way — but I'm leaving that for future
work for now.


Thanks,
Evgeny Kotkov