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 2012/10/08 04:03:51 UTC

svn commit: r1395434 - /subversion/trunk/subversion/libsvn_subr/mergeinfo.c

Author: stefan2
Date: Mon Oct  8 02:03:51 2012
New Revision: 1395434

URL: http://svn.apache.org/viewvc?rev=1395434&view=rev
Log:
Optimize frequently called mergeinfo-related operations.

* subversion/libsvn_subr/mergeinfo.c
  (svn_mergeinfo__equals): use expensive deltification only when
   a trivial comparison did not succeed
  (svn_mergeinfo_merge2): eliminate sort and copy operations
  (svn_rangelist_dup): reduce the number of allocations

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

Modified: subversion/trunk/subversion/libsvn_subr/mergeinfo.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/mergeinfo.c?rev=1395434&r1=1395433&r2=1395434&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/mergeinfo.c (original)
+++ subversion/trunk/subversion/libsvn_subr/mergeinfo.c Mon Oct  8 02:03:51 2012
@@ -1648,17 +1648,77 @@ svn_mergeinfo__equals(svn_boolean_t *is_
                       svn_boolean_t consider_inheritance,
                       apr_pool_t *pool)
 {
-  if (apr_hash_count(info1) == apr_hash_count(info2))
+  apr_hash_index_t *hi;
+
+  *is_equal = FALSE;
+
+  /* special cases: at least one side has no merge info */
+  if (info1 == NULL && info2 == NULL)
     {
-      svn_mergeinfo_t deleted, added;
-      SVN_ERR(svn_mergeinfo_diff2(&deleted, &added, info1, info2,
-                                  consider_inheritance, pool, pool));
-      *is_equal = apr_hash_count(deleted) == 0 && apr_hash_count(added) == 0;
+      *is_equal = TRUE;
+      return SVN_NO_ERROR;
     }
-  else
+
+  if (info1 == NULL ||  info2 == NULL)
+    return SVN_NO_ERROR;
+
+  /* trivial case: different number of paths -> unequal */
+  if (apr_hash_count(info1) != apr_hash_count(info2))
+    return SVN_NO_ERROR;
+
+  /* compare range lists for all paths */
+  for (hi = apr_hash_first(pool, info1); hi; hi = apr_hash_next(hi))
     {
-      *is_equal = FALSE;
+      const char *key;
+      apr_ssize_t key_length;
+      svn_rangelist_t *lhs, *rhs;
+      int i;
+      svn_rangelist_t *deleted, *added;
+
+      /* get both path lists */
+      apr_hash_this(hi, (const void**)&key, &key_length, (void **)&lhs);
+      rhs = apr_hash_get(info2, key, key_length);
+
+      /* missing on one side? */
+      if (rhs == NULL)
+        return SVN_NO_ERROR;
+
+      /* quick compare: the range lists will often be a perfect match */
+      if (lhs->nelts == rhs->nelts)
+        {
+          for (i = 0; i < lhs->nelts; ++i)
+            {
+              svn_merge_range_t *lrange
+                = APR_ARRAY_IDX(lhs, i, svn_merge_range_t *);
+              svn_merge_range_t *rrange
+                = APR_ARRAY_IDX(rhs, i, svn_merge_range_t *);
+
+              /* range mismatch? -> needs detailed comparison */
+              if (   lrange->start != rrange->start
+                  || lrange->end != rrange->end)
+                break;
+
+              /* inheritance mismatch? -> merge info differs */
+              if (   consider_inheritance
+                  && lrange->inheritable != rrange->inheritable)
+                return SVN_NO_ERROR;
+            }
+
+          /* all ranges found to match -> next path */
+          if (i == lhs->nelts)
+            continue;
+        }
+
+      /* range lists differ but there are many ways to sort and aggregate
+         revisions into ranges. Do a full diff on them. */
+      SVN_ERR(svn_rangelist_diff(&deleted, &added, lhs, rhs,
+                                  consider_inheritance, pool));
+      if (deleted->nelts || added->nelts)
+        return SVN_NO_ERROR;
     }
+
+  /* no mismatch found */
+  *is_equal = TRUE;
   return SVN_NO_ERROR;
 }
 
@@ -1668,62 +1728,37 @@ svn_mergeinfo_merge2(svn_mergeinfo_t mer
                      apr_pool_t *result_pool,
                      apr_pool_t *scratch_pool)
 {
-  apr_array_header_t *sorted1, *sorted2;
-  int i, j;
+  apr_hash_index_t *hi;
   apr_pool_t *iterpool;
 
   if (!apr_hash_count(changes))
     return SVN_NO_ERROR;
 
-  sorted1 = svn_sort__hash(mergeinfo, svn_sort_compare_items_as_paths,
-                           scratch_pool);
-  sorted2 = svn_sort__hash(changes, svn_sort_compare_items_as_paths,
-                           scratch_pool);
-
-  i = 0;
-  j = 0;
   iterpool = svn_pool_create(scratch_pool);
-  while (i < sorted1->nelts && j < sorted2->nelts)
+  for (hi = apr_hash_first(scratch_pool, changes); hi; hi = apr_hash_next(hi))
     {
-      svn_sort__item_t elt1, elt2;
-      int res;
-
-      svn_pool_clear(iterpool);
-
-      elt1 = APR_ARRAY_IDX(sorted1, i, svn_sort__item_t);
-      elt2 = APR_ARRAY_IDX(sorted2, j, svn_sort__item_t);
-      res = svn_sort_compare_items_as_paths(&elt1, &elt2);
-
-      if (res == 0)
-        {
-          svn_rangelist_t *rl1, *rl2;
-
-          rl1 = elt1.value;
-          rl2 = elt2.value;
-
-          SVN_ERR(svn_rangelist_merge2(rl1, rl2, result_pool, iterpool));
-          apr_hash_set(mergeinfo, elt1.key, elt1.klen, rl1);
-          i++;
-          j++;
-        }
-      else if (res < 0)
+      const char *key;
+      apr_ssize_t klen;
+      svn_rangelist_t *to_insert;
+      svn_rangelist_t *target;
+
+      /* get ranges to insert and the target ranges list of that insertion */
+      apr_hash_this(hi, (const void**)&key, &klen, (void*)&to_insert);
+      target = apr_hash_get(mergeinfo, key, klen);
+
+      /* if range list exists, just expand on it.
+       * Otherwise, add new hash entry. */
+      if (target)
         {
-          i++;
+          SVN_ERR(svn_rangelist_merge2(target, to_insert, result_pool,
+                                       iterpool));
+          apr_pool_clear(iterpool);
         }
       else
-        {
-          apr_hash_set(mergeinfo, elt2.key, elt2.klen, elt2.value);
-          j++;
-        }
+        apr_hash_set(mergeinfo, key, klen, to_insert);
     }
-  svn_pool_destroy(iterpool);
 
-  /* Copy back any remaining elements from the second hash. */
-  for (; j < sorted2->nelts; j++)
-    {
-      svn_sort__item_t elt = APR_ARRAY_IDX(sorted2, j, svn_sort__item_t);
-      apr_hash_set(mergeinfo, elt.key, elt.klen, elt.value);
-    }
+  svn_pool_destroy(iterpool);
 
   return SVN_NO_ERROR;
 }
@@ -2224,13 +2259,18 @@ svn_rangelist_dup(const svn_rangelist_t 
 {
   svn_rangelist_t *new_rl = apr_array_make(pool, rangelist->nelts,
                                            sizeof(svn_merge_range_t *));
+
+  /* allocate target range buffer with a single operation */
+  svn_merge_range_t *copy = apr_palloc(pool, sizeof(*copy) * rangelist->nelts);
   int i;
 
+  /* fill it iteratively and link it into the range list */
   for (i = 0; i < rangelist->nelts; i++)
     {
-      APR_ARRAY_PUSH(new_rl, svn_merge_range_t *) =
-        svn_merge_range_dup(APR_ARRAY_IDX(rangelist, i, svn_merge_range_t *),
-                            pool);
+      memcpy(copy + i,
+             APR_ARRAY_IDX(rangelist, i, svn_merge_range_t *),
+             sizeof(*copy));
+      APR_ARRAY_PUSH(new_rl, svn_merge_range_t *) = copy + i;
     }
 
   return new_rl;