On Wed, 23 Nov 2005, Daniel Berlin wrote:
> One of my users noticed that if you specify files seperately to svn
> diff, the memory usage goes through the roof.
>
> IE if you do svn diff *.c */*.c on a gcc checkout in the gcc dir, it
> uses more than 300 meg of memory.
>
> This is caused by not actually passing the per-iteration pool to our
> children.
Whoa! This was a nice one:-)
> [[[
>
> Fix memory leak when diff is given multiple files on the command line.
>
> * subversion/clients/cmdlne/diff-cmd.c
"cmdlne":-)
(Do you know there is svn-dev.el with an svn-log-message command?)
> Index: subversion/clients/cmdline/diff-cmd.c
> ===================================================================
> --- subversion/clients/cmdline/diff-cmd.c (revision 17493)
> +++ subversion/clients/cmdline/diff-cmd.c (working copy)
> @@ -49,7 +49,7 @@ svn_cl__diff (apr_getopt_t *os,
> apr_file_t *outfile, *errfile;
> apr_status_t status;
> const char *old_target, *new_target;
> - apr_pool_t *subpool;
> + apr_pool_t *iterpool;
We use subpool for iterations all over the place, so I don't really
see the point. But if someone feels this is clearer, then I'm all for
it.
> svn_boolean_t pegged_diff = FALSE;
> int i;
>
> @@ -172,7 +172,7 @@ svn_cl__diff (apr_getopt_t *os,
>
> svn_opt_push_implicit_dot_target (targets, pool);
>
> - subpool = svn_pool_create (pool);
> + iterpool = svn_pool_create (pool);
> for (i = 0; i < targets->nelts; ++i)
> {
> const char *path = APR_ARRAY_IDX (targets, i, const char *);
> @@ -180,9 +180,9 @@ svn_cl__diff (apr_getopt_t *os,
>
> if (! pegged_diff)
> {
> - svn_pool_clear (subpool);
> - target1 = svn_path_join (old_target, path, subpool);
> - target2 = svn_path_join (new_target, path, subpool);
> + svn_pool_clear (iterpool);
Why is this pool clearing inside the if statement? This seems to leave
the leak in the pegged_diff case.
...
> }
> else
> {
> @@ -205,7 +205,7 @@ svn_cl__diff (apr_getopt_t *os,
> svn_opt_revision_t peg_revision;
>
> /* First check for a peg revision. */
> - SVN_ERR (svn_opt_parse_path (&peg_revision, &truepath, path, pool));
> + SVN_ERR (svn_opt_parse_path (&peg_revision, &truepath, path, iterpool));
I don't know if this line slips over the 80 cols limit without the
mailer and diff prefixes.
Else, looks great!
thanks,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Nov 24 08:41:38 2005