svn_rangelist_dup -> ptr_array_dup?

Stefan,

In svn.apache.org/r1395434 and svn.apache.org/r1483310, you optimised svn_rangelist_dup().

An svn_rangelist_t is an APR array of pointers to small simple objects.

I guessed we would have other places where we want to duplicate an
array of pointers to simple objects, but I guessed wrong and
can't find any. Nevertheless, how about we factor out the generic array-duplication code like this, in order to maintain a clear separation between rangelist code and generic array code? I think that makes it easier for a reader to see that there's nothing special happening there that concerns rangelists specifically.

I have at the back of my mind that this function could very well be proposed for inclusion in APR, but I am not presently thinking of doing so.

Of course, parameterizing the 'object size' like this may make it slightly slower. I don't imagine it would be significant but haven't measured it. Does that bother you in this case?

[[[
Index: subversion/libsvn_subr/mergeinfo.c
===================================================================
--- subversion/libsvn_subr/mergeinfo.c    (revision 1570152)
+++ subversion/libsvn_subr/mergeinfo.c    (working copy)
@@ -2237,35 +2237,48 @@ svn_mergeinfo__add_suffix_to_mergeinfo(s
                     rangelist);
     }

   return SVN_NO_ERROR;
 }

-svn_rangelist_t *
-svn_rangelist_dup(const svn_rangelist_t *rangelist, apr_pool_t *pool)
+/* Deep-copy an array of pointers to simple objects.
+ *
+ * Return a duplicate in POOL of the array ARRAY of pointers to objects
+ * of size OBJECT_SIZE bytes. Duplicate each object bytewise.
+ */
+static apr_array_header_t *
+ptr_array_dup(const apr_array_header_t *array,
+              size_t object_size,
+              apr_pool_t *pool)
 {
-  svn_rangelist_t *new_rl = apr_array_make(pool, rangelist->nelts,
-                                           sizeof(svn_merge_range_t *));
+  apr_array_header_t *new_array = apr_array_make(pool, array->nelts,
+                                                 sizeof(void *));

   /* allocate target range buffer with a single operation */
-  svn_merge_range_t *copy = apr_palloc(pool, sizeof(*copy) * rangelist->nelts);
+  char *copy = apr_palloc(pool, object_size * array->nelts);

   /* for efficiency, directly address source and target reference buffers */
-  svn_merge_range_t **source = (svn_merge_range_t **)(rangelist->elts);
-  svn_merge_range_t **target = (svn_merge_range_t **)(new_rl->elts);
+  void **source = (void **)(array->elts);
+  void **target = (void **)(new_array->elts);
   int i;

   /* copy ranges iteratively and link them into the target range list */
-  for (i = 0; i < rangelist->nelts; i++)
+  for (i = 0; i < array->nelts; i++)
     {
-      copy[i] = *source[i];
-      target[i] = &copy[i];
+      target[i] = &copy[i * object_size];
+      memcpy(target[i], source[i], object_size);
     }
-  new_rl->nelts = rangelist->nelts;
+  new_array->nelts = array->nelts;
+
+  return new_array;
+}

-  return new_rl;
+svn_rangelist_t *
+svn_rangelist_dup(const svn_rangelist_t *rangelist, apr_pool_t *pool)
+{
+  return ptr_array_dup(rangelist, sizeof(svn_merge_range_t), pool);
 }

 svn_merge_range_t *
 svn_merge_range_dup(const svn_merge_range_t *range, apr_pool_t *pool)
 {
   svn_merge_range_t *new_range = apr_palloc(pool, sizeof(*new_range));
]]]

- Julian
Received on 2014-02-21 16:59:09 CET