You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2013/07/22 14:15:00 UTC

svn commit: r1505671 - in /subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs: fs.h low_level.c temp_serializer.c transaction.c

Author: stefan2
Date: Mon Jul 22 12:15:00 2013
New Revision: 1505671

URL: http://svn.apache.org/r1505671
Log:
On the fsfs-improvements branch: Change the FSFS-internal change_t
structure such that it does no conversion at the svn_fs API level.

Simply make the svn_fs_path_change2_t struct a member of change_t
and turn the remaining path member into a svn_string_t for maximum
efficiency when constructing hashes at the svn_fs API level.
When writing change lists, we already use svn_fs_path_change2_t.

* subversion/libsvn_fs_fs/fs.h
  (change_t): reformulate as an extension to svn_fs_path_change2_t

* subversion/libsvn_fs_fs/low_level.c
  (read_change): update - mostly another level of indirection

* subversion/libsvn_fs_fs/temp_serializer.c
  (serialize_change,
   deserialize_change): ditto

* subversion/libsvn_fs_fs/transaction.c
  (fold_change): ditto; create new hash entries on demand only
  (process_changes): update; clean pool after usage only

Modified:
    subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/fs.h
    subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/low_level.c
    subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/temp_serializer.c
    subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/transaction.c

Modified: subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/fs.h
URL: http://svn.apache.org/viewvc/subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/fs.h?rev=1505671&r1=1505670&r2=1505671&view=diff
==============================================================================
--- subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/fs.h (original)
+++ subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/fs.h Mon Jul 22 12:15:00 2013
@@ -498,25 +498,10 @@ typedef struct node_revision_t
 typedef struct change_t
 {
   /* Path of the change. */
-  const char *path;
-
-  /* Node revision ID of the change. */
-  const svn_fs_id_t *noderev_id;
-
-  /* The kind of change. */
-  svn_fs_path_change_kind_t kind;
-
-  /* Text or property mods? */
-  svn_boolean_t text_mod;
-  svn_boolean_t prop_mod;
-
-  /* Node kind (possibly svn_node_unknown). */
-  svn_node_kind_t node_kind;
-
-  /* Copyfrom revision and path. */
-  svn_revnum_t copyfrom_rev;
-  const char * copyfrom_path;
+  svn_string_t path;
 
+  /* API compatible change description */
+  svn_fs_path_change2_t info;
 } change_t;
 
 

Modified: subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/low_level.c
URL: http://svn.apache.org/viewvc/subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/low_level.c?rev=1505671&r1=1505670&r2=1505671&view=diff
==============================================================================
--- subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/low_level.c (original)
+++ subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/low_level.c Mon Jul 22 12:15:00 2013
@@ -172,6 +172,7 @@ read_change(change_t **change_p,
   svn_boolean_t eof = TRUE;
   change_t *change;
   char *str, *last_str, *kind_str;
+  svn_fs_path_change2_t *info;
 
   /* Default return value. */
   *change_p = NULL;
@@ -183,6 +184,7 @@ read_change(change_t **change_p,
     return SVN_NO_ERROR;
 
   change = apr_pcalloc(pool, sizeof(*change));
+  info = &change->info;
   last_str = line->data;
 
   /* Get the node-id of the change. */
@@ -191,8 +193,8 @@ read_change(change_t **change_p,
     return svn_error_create(SVN_ERR_FS_CORRUPT, NULL,
                             _("Invalid changes line in rev-file"));
 
-  change->noderev_id = svn_fs_fs__id_parse(str, strlen(str), pool);
-  if (change->noderev_id == NULL)
+  info->node_rev_id = svn_fs_fs__id_parse(str, strlen(str), pool);
+  if (info->node_rev_id == NULL)
     return svn_error_create(SVN_ERR_FS_CORRUPT, NULL,
                             _("Invalid changes line in rev-file"));
 
@@ -204,7 +206,7 @@ read_change(change_t **change_p,
 
   /* Don't bother to check the format number before looking for
    * node-kinds: just read them if you find them. */
-  change->node_kind = svn_node_unknown;
+  info->node_kind = svn_node_unknown;
   kind_str = strchr(str, '-');
   if (kind_str)
     {
@@ -212,9 +214,9 @@ read_change(change_t **change_p,
       *kind_str = '\0';
       kind_str++;
       if (strcmp(kind_str, SVN_FS_FS__KIND_FILE) == 0)
-        change->node_kind = svn_node_file;
+        info->node_kind = svn_node_file;
       else if (strcmp(kind_str, SVN_FS_FS__KIND_DIR) == 0)
-        change->node_kind = svn_node_dir;
+        info->node_kind = svn_node_dir;
       else
         return svn_error_create(SVN_ERR_FS_CORRUPT, NULL,
                                 _("Invalid changes line in rev-file"));
@@ -222,23 +224,23 @@ read_change(change_t **change_p,
 
   if (strcmp(str, ACTION_MODIFY) == 0)
     {
-      change->kind = svn_fs_path_change_modify;
+      info->change_kind = svn_fs_path_change_modify;
     }
   else if (strcmp(str, ACTION_ADD) == 0)
     {
-      change->kind = svn_fs_path_change_add;
+      info->change_kind = svn_fs_path_change_add;
     }
   else if (strcmp(str, ACTION_DELETE) == 0)
     {
-      change->kind = svn_fs_path_change_delete;
+      info->change_kind = svn_fs_path_change_delete;
     }
   else if (strcmp(str, ACTION_REPLACE) == 0)
     {
-      change->kind = svn_fs_path_change_replace;
+      info->change_kind = svn_fs_path_change_replace;
     }
   else if (strcmp(str, ACTION_RESET) == 0)
     {
-      change->kind = svn_fs_path_change_reset;
+      info->change_kind = svn_fs_path_change_reset;
     }
   else
     {
@@ -254,11 +256,11 @@ read_change(change_t **change_p,
 
   if (strcmp(str, FLAG_TRUE) == 0)
     {
-      change->text_mod = TRUE;
+      info->text_mod = TRUE;
     }
   else if (strcmp(str, FLAG_FALSE) == 0)
     {
-      change->text_mod = FALSE;
+      info->text_mod = FALSE;
     }
   else
     {
@@ -274,11 +276,11 @@ read_change(change_t **change_p,
 
   if (strcmp(str, FLAG_TRUE) == 0)
     {
-      change->prop_mod = TRUE;
+      info->prop_mod = TRUE;
     }
   else if (strcmp(str, FLAG_FALSE) == 0)
     {
-      change->prop_mod = FALSE;
+      info->prop_mod = FALSE;
     }
   else
     {
@@ -287,15 +289,16 @@ read_change(change_t **change_p,
     }
 
   /* Get the changed path. */
-  change->path = apr_pstrdup(pool, last_str);
-
+  change->path.len = strlen(last_str);
+  change->path.data = apr_pstrmemdup(pool, last_str, change->path.len);
 
   /* Read the next line, the copyfrom line. */
   SVN_ERR(svn_stream_readline(stream, &line, "\n", &eof, pool));
+  info->copyfrom_known = TRUE;
   if (eof || line->len == 0)
     {
-      change->copyfrom_rev = SVN_INVALID_REVNUM;
-      change->copyfrom_path = NULL;
+      info->copyfrom_rev = SVN_INVALID_REVNUM;
+      info->copyfrom_path = NULL;
     }
   else
     {
@@ -304,13 +307,13 @@ read_change(change_t **change_p,
       if (! str)
         return svn_error_create(SVN_ERR_FS_CORRUPT, NULL,
                                 _("Invalid changes line in rev-file"));
-      change->copyfrom_rev = SVN_STR_TO_REV(str);
+      info->copyfrom_rev = SVN_STR_TO_REV(str);
 
       if (! last_str)
         return svn_error_create(SVN_ERR_FS_CORRUPT, NULL,
                                 _("Invalid changes line in rev-file"));
 
-      change->copyfrom_path = apr_pstrdup(pool, last_str);
+      info->copyfrom_path = apr_pstrdup(pool, last_str);
     }
 
   *change_p = change;

Modified: subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/temp_serializer.c
URL: http://svn.apache.org/viewvc/subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/temp_serializer.c?rev=1505671&r1=1505670&r2=1505671&view=diff
==============================================================================
--- subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/temp_serializer.c (original)
+++ subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/temp_serializer.c Mon Jul 22 12:15:00 2013
@@ -1073,10 +1073,10 @@ serialize_change(svn_temp_serializer__co
                             sizeof(*change));
 
   /* serialize sub-structures */
-  svn_fs_fs__id_serialize(context, &change->noderev_id);
+  svn_fs_fs__id_serialize(context, &change->info.node_rev_id);
 
-  svn_temp_serializer__add_string(context, &change->path);
-  svn_temp_serializer__add_string(context, &change->copyfrom_path);
+  svn_temp_serializer__add_string(context, &change->path.data);
+  svn_temp_serializer__add_string(context, &change->info.copyfrom_path);
 
   /* return to the caller's nesting level */
   svn_temp_serializer__pop(context);
@@ -1098,10 +1098,10 @@ deserialize_change(void *buffer, change_
     return;
 
   /* fix-up of sub-structures */
-  svn_fs_fs__id_deserialize(change, (svn_fs_id_t **)&change->noderev_id);
+  svn_fs_fs__id_deserialize(change, (svn_fs_id_t **)&change->info.node_rev_id);
 
-  svn_temp_deserializer__resolve(change, (void **)&change->path);
-  svn_temp_deserializer__resolve(change, (void **)&change->copyfrom_path);
+  svn_temp_deserializer__resolve(change, (void **)&change->path.data);
+  svn_temp_deserializer__resolve(change, (void **)&change->info.copyfrom_path);
 }
 
 /* Auxiliary structure representing the content of a change_t array.

Modified: subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/transaction.c
URL: http://svn.apache.org/viewvc/subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/transaction.c?rev=1505671&r1=1505670&r2=1505671&view=diff
==============================================================================
--- subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/transaction.c (original)
+++ subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/transaction.c Mon Jul 22 12:15:00 2013
@@ -592,17 +592,18 @@ fold_change(apr_hash_t *changes,
 {
   apr_pool_t *pool = apr_hash_pool_get(changes);
   svn_fs_path_change2_t *old_change, *new_change;
-  const char *path;
-  apr_size_t path_len = strlen(change->path);
+  const svn_string_t *path = &change->path;
+  const svn_fs_path_change2_t *info = &change->info;
 
-  if ((old_change = apr_hash_get(changes, change->path, path_len)))
+  if ((old_change = apr_hash_get(changes, path->data, path->len)))
     {
       /* This path already exists in the hash, so we have to merge
          this change into the already existing one. */
 
       /* Sanity check:  only allow NULL node revision ID in the
          `reset' case. */
-      if ((! change->noderev_id) && (change->kind != svn_fs_path_change_reset))
+      if ((! info->node_rev_id)
+           && (info->change_kind != svn_fs_path_change_reset))
         return svn_error_create
           (SVN_ERR_FS_CORRUPT, NULL,
            _("Missing required node revision ID"));
@@ -610,8 +611,8 @@ fold_change(apr_hash_t *changes,
       /* Sanity check: we should be talking about the same node
          revision ID as our last change except where the last change
          was a deletion. */
-      if (change->noderev_id
-          && (! svn_fs_fs__id_eq(old_change->node_rev_id, change->noderev_id))
+      if (info->node_rev_id
+          && (! svn_fs_fs__id_eq(old_change->node_rev_id, info->node_rev_id))
           && (old_change->change_kind != svn_fs_path_change_delete))
         return svn_error_create
           (SVN_ERR_FS_CORRUPT, NULL,
@@ -621,16 +622,16 @@ fold_change(apr_hash_t *changes,
       /* Sanity check: an add, replacement, or reset must be the first
          thing to follow a deletion. */
       if ((old_change->change_kind == svn_fs_path_change_delete)
-          && (! ((change->kind == svn_fs_path_change_replace)
-                 || (change->kind == svn_fs_path_change_reset)
-                 || (change->kind == svn_fs_path_change_add))))
+          && (! ((info->change_kind == svn_fs_path_change_replace)
+                 || (info->change_kind == svn_fs_path_change_reset)
+                 || (info->change_kind == svn_fs_path_change_add))))
         return svn_error_create
           (SVN_ERR_FS_CORRUPT, NULL,
            _("Invalid change ordering: non-add change on deleted path"));
 
       /* Sanity check: an add can't follow anything except
          a delete or reset.  */
-      if ((change->kind == svn_fs_path_change_add)
+      if ((info->change_kind == svn_fs_path_change_add)
           && (old_change->change_kind != svn_fs_path_change_delete)
           && (old_change->change_kind != svn_fs_path_change_reset))
         return svn_error_create
@@ -638,7 +639,7 @@ fold_change(apr_hash_t *changes,
            _("Invalid change ordering: add change on preexisting path"));
 
       /* Now, merge that change in. */
-      switch (change->kind)
+      switch (info->change_kind)
         {
         case svn_fs_path_change_reset:
           /* A reset here will simply remove the path change from the
@@ -658,8 +659,8 @@ fold_change(apr_hash_t *changes,
             {
               /* A deletion overrules all previous changes. */
               old_change->change_kind = svn_fs_path_change_delete;
-              old_change->text_mod = change->text_mod;
-              old_change->prop_mod = change->prop_mod;
+              old_change->text_mod = info->text_mod;
+              old_change->prop_mod = info->prop_mod;
               old_change->copyfrom_rev = SVN_INVALID_REVNUM;
               old_change->copyfrom_path = NULL;
             }
@@ -670,72 +671,54 @@ fold_change(apr_hash_t *changes,
           /* An add at this point must be following a previous delete,
              so treat it just like a replace. */
           old_change->change_kind = svn_fs_path_change_replace;
-          old_change->node_rev_id = svn_fs_fs__id_copy(change->noderev_id,
+          old_change->node_rev_id = svn_fs_fs__id_copy(info->node_rev_id,
                                                        pool);
-          old_change->text_mod = change->text_mod;
-          old_change->prop_mod = change->prop_mod;
-          if (change->copyfrom_rev == SVN_INVALID_REVNUM)
+          old_change->text_mod = info->text_mod;
+          old_change->prop_mod = info->prop_mod;
+          if (info->copyfrom_rev == SVN_INVALID_REVNUM)
             {
               old_change->copyfrom_rev = SVN_INVALID_REVNUM;
               old_change->copyfrom_path = NULL;
             }
           else
             {
-              old_change->copyfrom_rev = change->copyfrom_rev;
+              old_change->copyfrom_rev = info->copyfrom_rev;
               old_change->copyfrom_path = apr_pstrdup(pool,
-                                                      change->copyfrom_path);
+                                                      info->copyfrom_path);
             }
           break;
 
         case svn_fs_path_change_modify:
         default:
-          if (change->text_mod)
+          if (info->text_mod)
             old_change->text_mod = TRUE;
-          if (change->prop_mod)
+          if (info->prop_mod)
             old_change->prop_mod = TRUE;
           break;
         }
 
-      /* Point our new_change to our (possibly modified) old_change. */
-      new_change = old_change;
+      /* remove old_change from the cache if it is no longer needed. */
+      if (old_change == NULL)
+        apr_hash_set(changes, path->data, path->len, NULL);
     }
   else
     {
       /* This change is new to the hash, so make a new public change
          structure from the internal one (in the hash's pool), and dup
          the path into the hash's pool, too. */
-      new_change = apr_pcalloc(pool, sizeof(*new_change));
-      new_change->node_rev_id = svn_fs_fs__id_copy(change->noderev_id, pool);
-      new_change->change_kind = change->kind;
-      new_change->text_mod = change->text_mod;
-      new_change->prop_mod = change->prop_mod;
-      /* In FSFS, copyfrom_known is *always* true, since we've always
-       * stored copyfroms in changed paths lists. */
-      new_change->copyfrom_known = TRUE;
-      if (change->copyfrom_rev != SVN_INVALID_REVNUM)
-        {
-          new_change->copyfrom_rev = change->copyfrom_rev;
-          new_change->copyfrom_path = apr_pstrdup(pool, change->copyfrom_path);
-        }
-      else
-        {
-          new_change->copyfrom_rev = SVN_INVALID_REVNUM;
-          new_change->copyfrom_path = NULL;
-        }
+      new_change = apr_pmemdup(pool, info, sizeof(*new_change));
+      new_change->node_rev_id = svn_fs_fs__id_copy(info->node_rev_id, pool);
+      if (info->copyfrom_path)
+        new_change->copyfrom_path = apr_pstrdup(pool, info->copyfrom_path);
+
+      /* Add this path.  The API makes no guarantees that this (new) key
+        will not be retained.  Thus, we copy the key into the target pool
+        to ensure a proper lifetime.  */
+      apr_hash_set(changes,
+                   apr_pstrmemdup(pool, path->data, path->len), path->len,
+                   new_change);
     }
 
-  if (new_change)
-    new_change->node_kind = change->node_kind;
-
-  /* Add (or update) this path.
-
-     Note: this key might already be present, and it would be nice to
-     re-use its value, but there is no way to fetch it. The API makes no
-     guarantees that this (new) key will not be retained. Thus, we (again)
-     copy the key into the target pool to ensure a proper lifetime.  */
-  path = apr_pstrmemdup(pool, change->path, path_len);
-  apr_hash_set(changes, path, path_len, new_change);
-
   return SVN_NO_ERROR;
 }
 
@@ -773,8 +756,8 @@ process_changes(apr_hash_t *changed_path
          is already a temporary subpool.
       */
 
-      if (((change->kind == svn_fs_path_change_delete)
-           || (change->kind == svn_fs_path_change_replace))
+      if (((change->info.change_kind == svn_fs_path_change_delete)
+           || (change->info.change_kind == svn_fs_path_change_replace))
           && ! prefolded)
         {
           apr_hash_index_t *hi;
@@ -784,12 +767,12 @@ process_changes(apr_hash_t *changed_path
              Also, we should not assume that all paths have been normalized
              i.e. some might have trailing path separators.
           */
-          apr_ssize_t change_path_len = strlen(change->path);
-          apr_ssize_t min_child_len = change_path_len == 0
+          apr_ssize_t path_len = change->path.len;
+          apr_ssize_t min_child_len = path_len == 0
                                     ? 1
-                                    : change->path[change_path_len-1] == '/'
-                                        ? change_path_len + 1
-                                        : change_path_len + 2;
+                                    : change->path.data[path_len-1] == '/'
+                                        ? path_len + 1
+                                        : path_len + 2;
 
           /* CAUTION: This is the inner loop of an O(n^2) algorithm.
              The number of changes to process may be >> 1000.
@@ -809,13 +792,13 @@ process_changes(apr_hash_t *changed_path
                  this is actually a sub-path.
                */
               if (   klen >= min_child_len
-                  && svn_dirent_is_child(change->path, path, iterpool))
+                  && svn_dirent_is_child(change->path.data, path, iterpool))
                 apr_hash_set(changed_paths, path, klen, NULL);
             }
-        }
 
-      /* Clear the per-iteration subpool. */
-      svn_pool_clear(iterpool);
+          /* Clear the per-iteration subpool. */
+          svn_pool_clear(iterpool);
+        }
     }
 
   /* Destroy the per-iteration subpool. */