You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ne...@apache.org on 2012/11/19 17:02:36 UTC

svn commit: r1411268 - /subversion/trunk/subversion/libsvn_wc/externals.c

Author: neels
Date: Mon Nov 19 16:02:35 2012
New Revision: 1411268

URL: http://svn.apache.org/viewvc?rev=1411268&view=rev
Log:
Externals parsing optimisation -- waste memory, not execution time.
Suggested by: danielsh

* subversion/libsvn_wc/externals.c (svn_wc_parse_externals_description3):
   Don't iterate the list to check for duplicates, use a hash table.


Modified:
    subversion/trunk/subversion/libsvn_wc/externals.c

Modified: subversion/trunk/subversion/libsvn_wc/externals.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/externals.c?rev=1411268&r1=1411267&r2=1411268&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/externals.c (original)
+++ subversion/trunk/subversion/libsvn_wc/externals.c Mon Nov 19 16:02:35 2012
@@ -164,16 +164,17 @@ svn_wc_parse_externals_description3(apr_
                                     apr_pool_t *pool)
 {
   int i;
-  int j;
+  int len;
   apr_array_header_t *externals = NULL;
   apr_array_header_t *lines = svn_cstring_split(desc, "\n\r", TRUE, pool);
+  apr_hash_t *duplicate_check = apr_hash_make(pool);
   const char *parent_directory_display = svn_path_is_url(parent_directory) ?
     parent_directory : svn_dirent_local_style(parent_directory, pool);
 
   /* If an error occurs halfway through parsing, *externals_p should stay
-   * untouched. So, store the list in a local var first. Since we are checking
-   * for duplicate targets, always compose the list regardless. */
-  externals = apr_array_make(pool, 1, sizeof(svn_wc_external_item2_t *));
+   * untouched. So, store the list in a local var first. */
+  if (externals_p)
+    externals = apr_array_make(pool, 1, sizeof(svn_wc_external_item2_t *));
 
   for (i = 0; i < lines->nelts; i++)
     {
@@ -332,22 +333,22 @@ svn_wc_parse_externals_description3(apr_
         }
 
       /* Has the same WC target path already been mentioned in this prop? */
-      for (j = 0; j < externals->nelts; j++)
+      len = apr_hash_count(duplicate_check);
+      apr_hash_set(duplicate_check, item->target_dir, APR_HASH_KEY_STRING, 1);
+      if (len == apr_hash_count(duplicate_check))
         {
-          svn_wc_external_item2_t *other_item =
-              APR_ARRAY_IDX(externals, j, svn_wc_external_item2_t *);
-
-          if (strcmp(item->target_dir, other_item->target_dir) == 0)
-            return svn_error_createf
-              (SVN_ERR_CLIENT_INVALID_EXTERNALS_DESCRIPTION, NULL,
-               _("Invalid %s property on '%s': "
-                 "target '%s' appears more than once"),
-               SVN_PROP_EXTERNALS,
-               parent_directory_display,
-               item->target_dir);
+          /* Hashtable length is unchanged. This must be a duplicate. */
+          return svn_error_createf
+            (SVN_ERR_CLIENT_INVALID_EXTERNALS_DESCRIPTION, NULL,
+             _("Invalid %s property on '%s': "
+               "target '%s' appears more than once"),
+             SVN_PROP_EXTERNALS,
+             parent_directory_display,
+             item->target_dir);
         }
 
-      APR_ARRAY_PUSH(externals, svn_wc_external_item2_t *) = item;
+      if (externals)
+        APR_ARRAY_PUSH(externals, svn_wc_external_item2_t *) = item;
     }
 
   if (externals_p)