Greg Hudson <ghudson@MIT.EDU> writes:
> I have looked into this problem after getting some more information
> from Stephan over IRC, and here are my findings:
Thanks for the excellent findings, Greg. Upon reading this
message, i realized this problem is probably the strange out of
memory problem our release engineers over here see when doing
weekly release branches.
This problem is what sent me on my last memory usage quest, and
the two resulting changes helped for a while, but the problem
eventually came back. Unfortunately, we were still dying in
purge_txn, which made no sense to me. I figured we must have
been coming *very* close to the resource limit somewhere else,
and then actually hitting the limit in purge_txn. I could never
figure out where, though.
With your findings, i finally knew how to reproduce the problem:
run two cp operations for our tree at the same time. Sure
enough, i can now reproduce it reliably. So, i iterpool-ified
tree.c:merge and dag.c:svn_fs_fs__dag_walk_predecessors, and the
problem is solved.
I've tested fairly extensively, and the test suite passes. If
there are no objections, i'll commit it.
Fix runaway memory usage when auto-merging a large tree.
* 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.
=== subversion/libsvn_fs_fs/dag.c
==================================================================
--- subversion/libsvn_fs_fs/dag.c (revision 1910)
+++ subversion/libsvn_fs_fs/dag.c (local)
@@ -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"
@@ -236,29 +237,34 @@
svn_fs_t *fs = svn_fs_fs__dag_get_fs (node);
dag_node_t *this_node;
svn_boolean_t done = FALSE;
+ apr_pool_t *iterpool;
+ iterpool = svn_pool_create (pool);
this_node = node;
while ((! done) && this_node)
{
node_revision_t *noderev;
+ 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);
return SVN_NO_ERROR;
}
=== subversion/libsvn_fs_fs/tree.c
==================================================================
--- subversion/libsvn_fs_fs/tree.c (revision 1910)
+++ subversion/libsvn_fs_fs/tree.c (local)
@@ -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? */
}
@@ -1724,6 +1728,7 @@
}
/* For each entry E in source but not in ancestor */
+ svn_pool_clear (iterpool);
for (hi = apr_hash_first (pool, s_entries);
hi;
hi = apr_hash_next (hi))
@@ -1734,6 +1739,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 +1754,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 +1768,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 +1776,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 +1784,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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Feb 27 04:56:44 2005