svn commit: r13193 - in trunk: . subversion/libsvn_fs_fs

Author: epg
Date: Mon Feb 28 13:18:28 2005
New Revision: 13193

Modified:
   trunk/   (props changed)
   trunk/subversion/libsvn_fs_fs/dag.c
   trunk/subversion/libsvn_fs_fs/tree.c
Log:
Fix runaway memory usage when auto-merging a large tree.  Before,
a simple cp of a tree of 70,000 files would die when running into
a 512 MB resource limit; now it uses only 8 MB.

Thanks to Julian Foad and Philip Martin for review and suggestions.

* subversion/libsvn_fs_fs/dag.c
  (svn_fs_fs__dag_walk_predecessors): Use an iterpool for the loop.

* subversion/libsvn_fs_fs/tree.c
  (merge): Use an iterpool for the two loops.


Modified: trunk/subversion/libsvn_fs_fs/dag.c
Url: 
http://svn.collab.net/viewcvs/svn/trunk/subversion/libsvn_fs_fs/dag.c?view=diff&rev=13193&p1=trunk/subversion/libsvn_fs_fs/dag.c&r1=13192&p2=trunk/subversion/libsvn_fs_fs/dag.c&r2=13193
==============================================================================
--- trunk/subversion/libsvn_fs_fs/dag.c (original)
+++ trunk/subversion/libsvn_fs_fs/dag.c Mon Feb 28 13:18:28 2005
@@ -23,6 +23,7 @@
 #include "svn_error.h"
 #include "svn_fs.h"
 #include "svn_props.h"
+#include "svn_pools.h"
 
 #include "dag.h"
 #include "err.h"
@@ -238,29 +239,42 @@
   svn_fs_t *fs = svn_fs_fs__dag_get_fs (node);
   dag_node_t *this_node;
   svn_boolean_t done = FALSE;
+  apr_pool_t *last_iterpool, *iterpool, *tmp_iterpool;
 
+  last_iterpool = svn_pool_create (pool);
+  iterpool = svn_pool_create (pool);
   this_node = node;
   while ((! done) && this_node)
     {
       node_revision_t *noderev;
 
+      /* Cycle the pools so iterpool will remain valid on the next
+       * iteration. */
+      tmp_iterpool = last_iterpool;
+      last_iterpool = iterpool;
+      iterpool = tmp_iterpool;
+      svn_pool_clear (iterpool);
+
       /* Get the node revision for THIS_NODE so we can examine its
          predecessor id.  */
-      SVN_ERR (get_node_revision (&noderev, this_node, pool));
+      SVN_ERR (get_node_revision (&noderev, this_node, iterpool));
 
       /* If THIS_NODE has a predecessor, replace THIS_NODE with the
          precessor, else set it to NULL.  */
       if (noderev->predecessor_id)
         SVN_ERR (svn_fs_fs__dag_get_node (&this_node, fs, 
-                                          noderev->predecessor_id, pool));
+                                          noderev->predecessor_id,
+                                          iterpool));
       else
         this_node = NULL;
 
       /* Now call the user-supplied callback with our predecessor
          node. */
       if (callback)
-        SVN_ERR (callback (baton, this_node, &done, pool));
+        SVN_ERR (callback (baton, this_node, &done, iterpool));
     }
+  apr_pool_destroy (iterpool);
+  apr_pool_destroy (last_iterpool);
 
   return SVN_NO_ERROR;
 }

Modified: trunk/subversion/libsvn_fs_fs/tree.c
Url: 
http://svn.collab.net/viewcvs/svn/trunk/subversion/libsvn_fs_fs/tree.c?view=diff&rev=13193&p1=trunk/subversion/libsvn_fs_fs/tree.c&r1=13192&p2=trunk/subversion/libsvn_fs_fs/tree.c&r2=13193
==============================================================================
--- trunk/subversion/libsvn_fs_fs/tree.c        (original)
+++ trunk/subversion/libsvn_fs_fs/tree.c        Mon Feb 28 13:18:28 2005
@@ -1300,6 +1300,7 @@
   apr_hash_t *s_entries, *t_entries, *a_entries;
   apr_hash_index_t *hi;
   svn_fs_t *fs;
+  apr_pool_t *iterpool;
 
   /* Make sure everyone comes from the same filesystem. */
   fs = svn_fs_fs__dag_get_fs (ancestor);
@@ -1488,6 +1489,7 @@
   a_entries = svn_fs_fs__copy_dir_entries (a_entries, pool);
 
   /* for each entry E in a_entries... */
+  iterpool = svn_pool_create (pool);
   for (hi = apr_hash_first (pool, a_entries); 
        hi; 
        hi = apr_hash_next (hi))
@@ -1498,6 +1500,8 @@
       void *val;
       apr_ssize_t klen;
           
+      svn_pool_clear (iterpool);
+
       /* KEY will be the entry name in ancestor, VAL the dirent */
       apr_hash_this (hi, &key, &klen, &val);
       a_entry = val;
@@ -1532,13 +1536,13 @@
               else
                 {
                   SVN_ERR (id_check_ancestor (&a_ancestorof_t, fs, a_entry->id,
-                                              t_entry->id, pool));
+                                              t_entry->id, iterpool));
                   if (a_ancestorof_t)
                     {
                       /* this is an &&, so we need both ancestor checks. */
                       SVN_ERR (id_check_ancestor (&t_ancestorof_s, fs, 
                                                   t_entry->id, s_entry->id, 
-                                                  pool));
+                                                  iterpool));
                       if (t_ancestorof_s)
                         {
                           /* This is Case 1.  */
@@ -1552,7 +1556,7 @@
                 {
                   SVN_ERR (id_check_ancestor (&s_ancestorof_t, fs, 
                                               s_entry->id, t_entry->id, 
-                                              pool));
+                                              iterpool));
                   if (! s_ancestorof_t)
                     {
                       /* This is Case 2. */
@@ -1575,7 +1579,7 @@
 
                   SVN_ERR (svn_fs_fs__dag_set_entry
                            (target, t_entry->name, s_entry->id, s_entry->kind,
-                            txn_id, pool));
+                            txn_id, iterpool));
                 }
               /* or if target entry is different from both and
                  unrelated to source, and all three entries are
@@ -1588,11 +1592,11 @@
                   svn_node_kind_t s_kind, t_kind, a_kind;
                       
                   SVN_ERR (svn_fs_fs__dag_get_node (&s_ent_node, fs,
-                                                    s_entry->id, pool));
+                                                    s_entry->id, iterpool));
                   SVN_ERR (svn_fs_fs__dag_get_node (&t_ent_node, fs,
-                                                    t_entry->id, pool));
+                                                    t_entry->id, iterpool));
                   SVN_ERR (svn_fs_fs__dag_get_node (&a_ent_node, fs,
-                                                    a_entry->id, pool));
+                                                    a_entry->id, iterpool));
 
                   s_kind = svn_fs_fs__dag_node_kind (s_ent_node);
                   t_kind = svn_fs_fs__dag_node_kind (t_ent_node);
@@ -1605,19 +1609,19 @@
                       return conflict_err (conflict_p,
                                            svn_path_join (target_path,
                                                           a_entry->name,
-                                                          pool));
+                                                          iterpool));
                     }
 
                   /* ... just recurse. */
                   new_tpath = svn_path_join (target_path, t_entry->name,
-                                             pool);
+                                             iterpool);
                   SVN_ERR (merge (conflict_p, new_tpath,
                                   t_ent_node, s_ent_node, a_ent_node,
-                                  txn_id, pool));
+                                  txn_id, iterpool));
 
                   SVN_ERR (svn_fs_fs__dag_get_predecessor_count (&pred_count,
                                                                  s_ent_node,
-                                                                 pool));
+                                                                 iterpool));
 
                   /* If target is an immediate descendant of ancestor,
                      and source is also a descendant of ancestor, we
@@ -1625,7 +1629,7 @@
                      source. */
                   SVN_ERR (update_ancestry (fs, s_entry->id,
                                             t_entry->id, txn_id, 
-                                            new_tpath, pred_count, pool));
+                                            new_tpath, pred_count, iterpool));
                 }
               /* Else target entry has changed since ancestor entry,
                  but it changed either to source entry or to a
@@ -1643,7 +1647,7 @@
               return conflict_err (conflict_p,
                                    svn_path_join (target_path,
                                                   a_entry->name,
-                                                  pool));
+                                                  iterpool));
             }
 
           /* Else if E did not change between ancestor and source,
@@ -1666,7 +1670,7 @@
                    _("Unexpected immutable node at '%s'"), target_path);
               
               SVN_ERR (svn_fs_fs__dag_delete (target, t_entry->name, 
-                                              txn_id, pool));
+                                              txn_id, iterpool));
 
               /* Seems cleanest to remove it from the target entries
                  hash now, even though no code would break if we
@@ -1688,7 +1692,7 @@
               return conflict_err (conflict_p,
                                    svn_path_join (target_path,
                                                   t_entry->name,
-                                                  pool));
+                                                  iterpool));
             }
           else
             {
@@ -1698,8 +1702,8 @@
                  this change. */
               SVN_ERR (undelete_change (fs, svn_path_join (target_path, 
                                                            t_entry->name, 
-                                                           pool),
-                                        txn_id, pool));
+                                                           iterpool),
+                                        txn_id, iterpool));
             }
         }
       /* E exists in neither target nor source */
@@ -1710,8 +1714,8 @@
              for that change. */
           SVN_ERR (undelete_change (fs, svn_path_join (target_path, 
                                                        a_entry->name, 
-                                                       pool),
-                                    txn_id, pool));
+                                                       iterpool),
+                                    txn_id, iterpool));
 
           /* ### kff todo: what about the rename case? */
         }
@@ -1734,6 +1738,8 @@
       apr_ssize_t klen;
       svn_boolean_t s_ancestorof_t = FALSE;
 
+      svn_pool_clear (iterpool);
+
       apr_hash_this (hi, &key, &klen, &val);
       s_entry = val;
       t_entry = apr_hash_get (t_entries, key, klen);
@@ -1747,7 +1753,7 @@
       if (t_entry)
         {
           SVN_ERR (id_check_ancestor (&s_ancestorof_t, fs, s_entry->id,
-                                      t_entry->id, pool));
+                                      t_entry->id, iterpool));
         }
 
       /* E does not exist in target */
@@ -1761,7 +1767,7 @@
               
           SVN_ERR (svn_fs_fs__dag_set_entry
                    (target, s_entry->name, s_entry->id, s_entry->kind,
-                    txn_id, pool));
+                    txn_id, iterpool));
         }
       /* E exists in target but is different from E in source */
       else if (! s_ancestorof_t)
@@ -1769,7 +1775,7 @@
           return conflict_err (conflict_p,
                                svn_path_join (target_path,
                                               t_entry->name,
-                                              pool));
+                                              iterpool));
 
           /* The remaining case would be: E exists in target and is
            * same as in source.  This implies a twin add, so target
@@ -1777,7 +1783,8 @@
            */
         }
     }
-      
+  apr_pool_destroy (iterpool);
+
   /* All entries in ancestor and source have been accounted for.
    *
    * Any entry E in target that does not exist in ancestor or source