[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[PATCH] Execute groups of log-files, was (Re: Improving the performance of libsvn_wc for checkouts/updates/switches)

From: Josh Pieper <jjp_at_pobox.com>
Date: 2004-05-23 04:29:28 CEST

Philip Martin wrote:
> Josh Pieper <jjp@pobox.com> writes:
>
> > So, how about we execute all the log files in order, then erase them
> > in reverse order? That should take care of any unforseen crashes,
> > yes?
>
> Probably. I think we could also delete log files as we go provided
> cleanup did apr_dir_read to identify the lowest numbered log file.

Ok Philip, since you're the resident libsvn_wc expert, think you could
look over this patch and make sure everything looks OK before I
commit? It performs as well as anything I've done before and should
behave identically to the current trunk as far as resumed updates.

-Josh

-----------------------

Improve the performance of the libsvn_wc update editor by letting it
run multiple log files at the same time. When running a group of log
files, the resulting entries file is written out only once at the
end. Previously, the entries file was read and written at the
beginning and end of every log file.

As a result of this change, all logs in the working copy directory
have a numerical suffix that indicates which number they are in a
sequence. During cleanup, the log files are executed in order, but
they are deleted in reverse order so that stale log files cannot be
left behind.

* subversion/libsvn_wc/props.c
  (svn_wc_merge_prop_diffs): Append a ".1" to the log files created
    here since they are not part of the update editor.

* subversion/libsvn_wc/adm_ops.c
  (svn_wc_process_committed): Append a ".1" to the log files here too.

* subversion/libsvn_wc/lock.c
  (svn_wc__adm_is_cleanup_required): Once again append a ".1" to the
    checked for log file.
    
* subversion/libsvn_wc/log.c
  (svn_wc__run_log): Loop through all files named "log.1", "log.2",
    etc. until a non-existant file is found. After executing this log
    file group, erase them in reverse order.

* subversion/libsvn_wc/update_editor.c
  (struct dir_baton): Add a field to keep track of the next available
    log file number for this directory.
  (cleanup_dir_baton, cleanup_dir_baton_child): New, called when a
    dir_baton is destroyed. They run any left over log files so that
    updates are easily resumable if a soft-interrupt caused the editor
    to end.
  (make_dir_baton): Initialize the new log file number field.
  (struct file_baton): Add a field to record the directory that this
    file is located in.
  (make_file_baton): Initialize the dir_baton field.
  (do_entry_deletion): Add a parameter that accepts a pointer to an
    integer that contains the next log file number to use. Reset this
    log number after running the log file group.
  (delete_entry): Pass the log file number to do_entry_deletion.
  (close_directory): Always run the current log file group.
  (install_file): Accept a log file number, use it, and also perform a
    merge dry run if necessary so that the correct conflict status can
    be reported to the caller. Previously running the log file would
    generate this information.
  (close_edit): Pass a log number of 1 to do_entry_deletion.
  (svn_wc_add_repos_file): Pass a log number of 1 to install file and
    run the resulting log file.

Index: subversion/libsvn_wc/props.c
===================================================================
--- subversion/libsvn_wc/props.c (revision 9860)
+++ subversion/libsvn_wc/props.c (working copy)
@@ -299,7 +299,7 @@
 
   if (! dry_run)
     {
- SVN_ERR (svn_wc__open_adm_file (&log_fp, parent, SVN_WC__ADM_LOG,
+ SVN_ERR (svn_wc__open_adm_file (&log_fp, parent, SVN_WC__ADM_LOG ".1",
                                       (APR_WRITE | APR_CREATE), /* not excl */
                                       pool));
       log_accum = svn_stringbuf_create ("", pool);
@@ -317,7 +317,7 @@
                                          log_accum->len, NULL, pool),
                  apr_psprintf (pool, _("Error writing log for '%s'"), path));
 
- SVN_ERR (svn_wc__close_adm_file (log_fp, parent, SVN_WC__ADM_LOG,
+ SVN_ERR (svn_wc__close_adm_file (log_fp, parent, SVN_WC__ADM_LOG ".1",
                                        1, /* sync */ pool));
       SVN_ERR (svn_wc__run_log (adm_access, NULL, pool));
     }
Index: subversion/libsvn_wc/log.c
===================================================================
--- subversion/libsvn_wc/log.c (revision 9860)
+++ subversion/libsvn_wc/log.c (working copy)
@@ -1274,6 +1274,9 @@
   char buf[BUFSIZ];
   apr_size_t buf_len;
   apr_file_t *f = NULL;
+ const char *logfile_path;
+ int log_number;
+ apr_pool_t *iterpool = svn_pool_create (pool);
 
   /* kff todo: use the tag-making functions here, now. */
   const char *log_start
@@ -1293,29 +1296,45 @@
      a ghost open tag. */
   SVN_ERR (svn_xml_parse (parser, log_start, strlen (log_start), 0));
 
- /* Parse the log file's contents. */
- SVN_ERR_W (svn_wc__open_adm_file (&f, svn_wc_adm_access_path (adm_access),
- SVN_WC__ADM_LOG, APR_READ, pool),
- _("Couldn't open log"));
-
- do {
- buf_len = sizeof (buf);
+ for (log_number = 1; ; log_number++)
+ {
+ svn_pool_clear (iterpool);
+ logfile_path = apr_psprintf (iterpool, "log.%d", log_number);
+ /* Parse the log file's contents. */
+ err = svn_wc__open_adm_file (&f, svn_wc_adm_access_path (adm_access),
+ logfile_path, APR_READ, iterpool);
+ if (err)
+ {
+ if (APR_STATUS_IS_ENOENT (err->apr_err))
+ {
+ svn_error_clear (err);
+ break;
+ }
+ else
+ {
+ SVN_ERR_W (err, _("Couldn't open log"));
+ }
+ }
+
+ do {
+ buf_len = sizeof (buf);
+
+ err = svn_io_file_read (f, buf, &buf_len, iterpool);
+ if (err && !APR_STATUS_IS_EOF(err->apr_err))
+ return svn_error_createf
+ (err->apr_err, err,
+ _("Error reading administrative log file in '%s'"),
+ svn_wc_adm_access_path (adm_access));
+
+ SVN_ERR (svn_xml_parse (parser, buf, buf_len, 0));
+
+ } while (! err);
+
+ svn_error_clear (err);
+ SVN_ERR (svn_io_file_close (f, iterpool));
+ }
 
- err = svn_io_file_read (f, buf, &buf_len, pool);
- if (err && !APR_STATUS_IS_EOF(err->apr_err))
- return svn_error_createf
- (err->apr_err, err,
- _("Error reading administrative log file in '%s'"),
- svn_wc_adm_access_path (adm_access));
 
- SVN_ERR (svn_xml_parse (parser, buf, buf_len, 0));
-
- } while (! err);
-
- svn_error_clear (err);
- SVN_ERR (svn_io_file_close (f, pool));
-
-
   /* Pacify Expat with a pointless closing element tag. */
   SVN_ERR (svn_xml_parse (parser, log_end, strlen (log_end), 1));
 
@@ -1336,9 +1355,15 @@
     }
   else
     {
- /* No 'killme'? Remove the logfile; its commands have been executed. */
- SVN_ERR (svn_wc__remove_adm_file (svn_wc_adm_access_path (adm_access),
- pool, SVN_WC__ADM_LOG, NULL));
+ for (log_number--; log_number > 0; log_number--)
+ {
+ svn_pool_clear (iterpool);
+ logfile_path = apr_psprintf (iterpool, "log.%d", log_number);
+
+ /* No 'killme'? Remove the logfile; its commands have been executed. */
+ SVN_ERR (svn_wc__remove_adm_file (svn_wc_adm_access_path (adm_access),
+ iterpool, logfile_path, NULL));
+ }
     }
 
   return SVN_NO_ERROR;
Index: subversion/libsvn_wc/adm_ops.c
===================================================================
--- subversion/libsvn_wc/adm_ops.c (revision 9860)
+++ subversion/libsvn_wc/adm_ops.c (working copy)
@@ -242,7 +242,7 @@
   /* Open a log file in the administrative directory */
   SVN_ERR (svn_wc__open_adm_file (&log_fp,
                                   svn_wc_adm_access_path (adm_access),
- SVN_WC__ADM_LOG,
+ SVN_WC__ADM_LOG ".1",
                                   (APR_WRITE | APR_CREATE),
                                   pool));
 
@@ -378,7 +378,7 @@
              apr_psprintf (pool, _("Error writing log file for '%s'"), path));
       
   SVN_ERR (svn_wc__close_adm_file (log_fp, svn_wc_adm_access_path (adm_access),
- SVN_WC__ADM_LOG,
+ SVN_WC__ADM_LOG ".1",
                                    TRUE, /* sync */
                                    pool));
 
Index: subversion/libsvn_wc/lock.c
===================================================================
--- subversion/libsvn_wc/lock.c (revision 9860)
+++ subversion/libsvn_wc/lock.c (working copy)
@@ -893,7 +893,8 @@
 {
   svn_node_kind_t kind;
   const char *log_path = svn_wc__adm_path (svn_wc_adm_access_path (adm_access),
- FALSE, pool, SVN_WC__ADM_LOG, NULL);
+ FALSE, pool, SVN_WC__ADM_LOG ".1",
+ NULL);
 
   /* The presence of a log file demands cleanup */
   SVN_ERR (svn_io_check_path (log_path, &kind, pool));
Index: subversion/libsvn_wc/update_editor.c
===================================================================
--- subversion/libsvn_wc/update_editor.c (revision 9860)
+++ subversion/libsvn_wc/update_editor.c (working copy)
@@ -141,6 +141,9 @@
   /* The bump information for this directory. */
   struct bump_dir_info *bump_info;
 
+ /* The current log file number. */
+ int log_number;
+
   /* The pool in which this baton itself is allocated. */
   apr_pool_t *pool;
 };
@@ -211,7 +214,53 @@
   return entry->url;
 }
 
+/* An APR pool cleanup handler. This runs the log file for a
+ directory baton. */
+static apr_status_t
+cleanup_dir_baton (void *dir_baton)
+{
+ struct dir_baton *db = dir_baton;
+ svn_error_t *err;
+ apr_status_t apr_err;
+ svn_wc_adm_access_t *adm_access;
 
+ /* If there are no log files to write, return immediately. */
+ if (db->log_number == 1)
+ return APR_SUCCESS;
+
+ err = svn_wc_adm_retrieve (&adm_access, db->edit_baton->adm_access,
+ db->path, apr_pool_parent_get (db->pool));
+
+ if (err)
+ {
+ apr_err = err->apr_err;
+ svn_error_clear (err);
+ return apr_err;
+ }
+
+ err = svn_wc__run_log (adm_access, NULL, apr_pool_parent_get (db->pool));
+
+ if (err)
+ {
+ apr_err = err->apr_err;
+ svn_error_clear (err);
+ return apr_err;
+ }
+
+ return APR_SUCCESS;
+}
+
+/* An APR pool cleanup handler. This is a child handler, it removes
+ the mail pool handler. */
+static apr_status_t
+cleanup_dir_baton_child (void *dir_baton)
+{
+ struct dir_baton *db = dir_baton;
+ apr_pool_cleanup_kill (db->pool, db, cleanup_dir_baton);
+ return APR_SUCCESS;
+}
+
+
 /* Return a new dir_baton to represent NAME (a subdirectory of
    PARENT_BATON). If PATH is NULL, this is the root directory of the
    edit. */
@@ -291,11 +340,15 @@
 
   d->edit_baton = eb;
   d->parent_baton = pb;
- d->pool = pool;
+ d->pool = svn_pool_create (pool);
   d->propchanges = apr_array_make (pool, 1, sizeof (svn_prop_t));
   d->added = added;
   d->bump_info = bdi;
+ d->log_number = 1;
 
+ apr_pool_cleanup_register (d->pool, d, cleanup_dir_baton,
+ cleanup_dir_baton_child);
+
   return d;
 }
 
@@ -443,6 +496,9 @@
   /* The global edit baton. */
   struct edit_baton *edit_baton;
 
+ /* The parent directory of this file. */
+ struct dir_baton *dir_baton;
+
   /* Pool specific to this file_baton. */
   apr_pool_t *pool;
 
@@ -520,6 +576,7 @@
   f->propchanges = apr_array_make (pool, 1, sizeof (svn_prop_t));
   f->bump_info = pb->bump_info;
   f->added = adding;
+ f->dir_baton = pb;
 
   /* No need to initialize f->digest, since we used pcalloc(). */
 
@@ -789,7 +846,8 @@
 static svn_error_t *
 do_entry_deletion (struct edit_baton *eb,
                    const char *parent_path,
- const char *path,
+ const char *path,
+ int *log_number,
                    apr_pool_t *pool)
 {
   apr_file_t *log_fp = NULL;
@@ -811,7 +869,8 @@
 
   SVN_ERR (svn_wc__open_adm_file (&log_fp,
                                   parent_path,
- SVN_WC__ADM_LOG,
+ apr_psprintf (pool, SVN_WC__ADM_LOG
+ ".%d", *log_number),
                                   (APR_WRITE | APR_CREATE), /* not excl */
                                   pool));
 
@@ -862,7 +921,8 @@
 
   SVN_ERR (svn_wc__close_adm_file (log_fp,
                                    parent_path,
- SVN_WC__ADM_LOG,
+ apr_psprintf (pool, SVN_WC__ADM_LOG
+ ".%d", (*log_number)++),
                                    TRUE, /* sync */
                                    pool));
     
@@ -906,6 +966,7 @@
 
   SVN_ERR (leftmod_error_chain (svn_wc__run_log (adm_access, NULL, pool),
                                 logfile_path, parent_path, pool));
+ *log_number = 1;
 
   /* The passed-in `path' is relative to the anchor of the edit, so if
    * the operation was invoked on something other than ".", then
@@ -934,7 +995,8 @@
               apr_pool_t *pool)
 {
   struct dir_baton *pb = parent_baton;
- return do_entry_deletion (pb->edit_baton, pb->path, path, pool);
+ return do_entry_deletion (pb->edit_baton, pb->path, path, &pb->log_number,
+ pool);
 }
 
 
@@ -1125,27 +1187,28 @@
   struct dir_baton *db = dir_baton;
   svn_wc_notify_state_t prop_state = svn_wc_notify_state_unknown;
   apr_array_header_t *entry_props, *wc_props, *regular_props;
-
+ svn_wc_adm_access_t *adm_access;
+
   SVN_ERR (svn_categorize_props (db->propchanges, &entry_props, &wc_props,
                                  &regular_props, pool));
 
+ SVN_ERR (svn_wc_adm_retrieve (&adm_access, db->edit_baton->adm_access,
+ db->path, db->pool));
+
   /* If this directory has property changes stored up, now is the time
      to deal with them. */
   if (regular_props->nelts || entry_props->nelts || wc_props->nelts)
     {
- svn_wc_adm_access_t *adm_access;
       apr_file_t *log_fp = NULL;
 
       /* to hold log messages: */
       svn_stringbuf_t *entry_accum = svn_stringbuf_create ("", db->pool);
 
- SVN_ERR (svn_wc_adm_retrieve (&adm_access, db->edit_baton->adm_access,
- db->path, db->pool));
-
       /* Open log file */
       SVN_ERR (svn_wc__open_adm_file (&log_fp,
                                       db->path,
- SVN_WC__ADM_LOG,
+ apr_psprintf (db->pool, SVN_WC__ADM_LOG
+ ".%d", db->log_number),
                                       (APR_WRITE | APR_CREATE), /* not excl */
                                       db->pool));
 
@@ -1244,14 +1307,17 @@
       /* The log is ready to run, close it. */
       SVN_ERR (svn_wc__close_adm_file (log_fp,
                                        db->path,
- SVN_WC__ADM_LOG,
+ apr_psprintf (db->pool, SVN_WC__ADM_LOG
+ ".%d", db->log_number),
                                        TRUE, /* sync */
                                        db->pool));
 
- /* Run the log. */
- SVN_ERR (svn_wc__run_log (adm_access, NULL, db->pool));
     }
 
+ /* Run the log. */
+ SVN_ERR (svn_wc__run_log (adm_access, NULL, db->pool));
+ db->log_number = 1;
+
   /* We're done with this directory, so remove one reference from the
      bump information. This may trigger a number of actions. See
      maybe_bump_dir_info() for more information. */
@@ -1710,6 +1776,7 @@
 install_file (svn_wc_notify_state_t *content_state,
               svn_wc_notify_state_t *prop_state,
               svn_wc_adm_access_t *adm_access,
+ int *log_number,
               const char *file_path,
               svn_revnum_t new_revision,
               const char *new_text_path,
@@ -1731,6 +1798,7 @@
   svn_boolean_t magic_props_changed = FALSE;
   apr_array_header_t *regular_props = NULL, *wc_props = NULL,
     *entry_props = NULL;
+ enum svn_wc_merge_outcome_t merge_outcome;
 
   /* The code flow does not depend upon these being set to NULL, but
      it removes a gcc 3.1 `might be used uninitialized in this
@@ -1765,7 +1833,8 @@
      right now. */
   SVN_ERR (svn_wc__open_adm_file (&log_fp,
                                   parent_dir,
- SVN_WC__ADM_LOG,
+ apr_psprintf (pool, SVN_WC__ADM_LOG ".%d",
+ *log_number),
                                   (APR_WRITE | APR_CREATE), /* not excl */
                                   pool));
 
@@ -2050,6 +2119,7 @@
                  textual changes into the working file. */
               const char *oldrev_str, *newrev_str;
               const svn_wc_entry_t *e;
+ const char *base;
               
               /* Create strings representing the revisions of the
                  old and new text-bases. */
@@ -2083,6 +2153,19 @@
               /* If a conflict happens, then the entry will be
                  marked "Conflicted" and will track either 2 or 3 new
                  temporary fulltext files that resulted. */
+
+ /* Run a dry-run of the merge to see if a conflict will
+ occur. This is needed so we can report back to the
+ client as the changes come in. */
+ base = svn_wc_adm_access_path (adm_access);
+
+ SVN_ERR (svn_wc_merge (svn_path_join (base, txtb, pool),
+ svn_path_join (base, tmp_txtb, pool),
+ svn_path_join (base, base_name, pool),
+ adm_access,
+ oldrev_str, newrev_str, ".mine",
+ TRUE, &merge_outcome, diff3_cmd,
+ pool));
               
             } /* end: working file exists and has mods */
         } /* end: working file has mods */
@@ -2190,27 +2273,17 @@
                                     log_accum->len, NULL, pool),
              apr_psprintf (pool, _("Error writing log for '%s'"), file_path));
 
- /* The log is ready to run. Close it and run it! */
- SVN_ERR (svn_wc__close_adm_file (log_fp, parent_dir, SVN_WC__ADM_LOG,
+ /* The log is done, close it. */
+ SVN_ERR (svn_wc__close_adm_file (log_fp, parent_dir,
+ apr_psprintf (pool, SVN_WC__ADM_LOG ".%d",
+ (*log_number)++),
                                    TRUE, /* sync */ pool));
- SVN_ERR (svn_wc__run_log (adm_access, diff3_cmd, pool));
 
   if (content_state)
     {
- const svn_wc_entry_t *entry;
- svn_boolean_t tc, pc;
-
       /* Initialize the state of our returned value. */
       *content_state = svn_wc_notify_state_unknown;
       
- /* ### There should be a more efficient way of finding out whether
- or not the file is modified|merged|conflicted. If the
- svn_wc__run_log() call above could return a special error code
- in case of a conflict or something, that would work. */
-
- SVN_ERR (svn_wc_entry (&entry, file_path, adm_access, TRUE, pool));
- SVN_ERR (svn_wc_conflicted_p (&tc, &pc, parent_dir, entry, pool));
-
       /* This is kind of interesting. Even if no new text was
          installed (i.e., new_text_path was null), we could still
          report a pre-existing conflict state. Say a file, already
@@ -2218,7 +2291,7 @@
          update. Then we'll notify that it has text conflicts. This
          seems okay to me. I guess. I dunno. You? */
 
- if (tc)
+ if (merge_outcome == svn_wc_merge_conflict)
         *content_state = svn_wc_notify_state_conflicted;
       else if (new_text_path)
         {
@@ -2277,6 +2350,7 @@
   SVN_ERR (install_file (&content_state,
                          &prop_state,
                          adm_access,
+ &fb->dir_baton->log_number,
                          fb->path,
                          (*eb->target_revision),
                          new_text_path,
@@ -2317,13 +2391,15 @@
 {
   struct edit_baton *eb = edit_baton;
   const char *target_path = svn_path_join (eb->anchor, eb->target, pool);
+ int log_number = 1;
 
   /* If there is a target and that target is missing, then it
      apparently wasn't re-added by the update process, so we'll
      pretend that the editor deleted the entry. The helper function
      do_entry_deletion() will take care of the necessary steps. */
   if ((*eb->target) && (svn_wc__adm_missing (eb->adm_access, target_path)))
- SVN_ERR (do_entry_deletion (eb, eb->anchor, eb->target, pool));
+ SVN_ERR (do_entry_deletion (eb, eb->anchor, eb->target, &log_number,
+ pool));
 
   /* The editor didn't even open the root; we have to take care of
      some cleanup stuffs. */
@@ -2737,6 +2813,7 @@
 {
   const char *new_URL;
   apr_array_header_t *propchanges;
+ int log_number = 1;
 
   /* Fabricate the anticipated new URL of the target. */
   {
@@ -2757,6 +2834,7 @@
   SVN_ERR (install_file (NULL,
                          NULL,
                          adm_access,
+ &log_number,
                          dst_path,
                          0,
                          new_text_path,
@@ -2770,5 +2848,7 @@
                          NULL,
                          pool));
 
+ SVN_ERR (svn_wc__run_log (adm_access, NULL, pool));
+
   return SVN_NO_ERROR;
 }
    

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun May 23 04:30:25 2004

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.