On Fri, Jul 22, 2011 at 12:29:33PM +0200, Bert Huijben wrote:
> > @@ -1952,9 +1959,9 @@ svn_client_mergeinfo_log(svn_boolean_t f
> > if (finding_merged)
> > {
> > /* Roll all the merged revisions into one rangelist. */
> > - SVN_ERR(svn_rangelist_merge(&master_inheritable_rangelist,
> > - master_noninheritable_rangelist,
> > - scratch_pool));
> > + SVN_ERR(svn_rangelist_merge2(master_inheritable_rangelist,
> > + master_noninheritable_rangelist,
> > + scratch_pool, scratch_pool));
>
> You can use iterpool as scratch_pool here.
> >
> > }
> > else
> > @@ -1970,9 +1977,10 @@ svn_client_mergeinfo_log(svn_boolean_t f
> > apr_array_header_t *subtree_merged_rangelist =
> > svn__apr_hash_index_val(hi);
> >
> > - SVN_ERR(svn_rangelist_merge(&source_master_rangelist,
> > - subtree_merged_rangelist,
> > - iterpool));
> > + svn_pool_clear(iterpool);
> > + SVN_ERR(svn_rangelist_merge2(source_master_rangelist,
> > + subtree_merged_rangelist,
> > + scratch_pool, iterpool));
> > }
> >
> > /* From what might be eligible subtract what we know is partially merged
> > @@ -1981,9 +1989,9 @@ svn_client_mergeinfo_log(svn_boolean_t f
> > master_noninheritable_rangelist,
> > source_master_rangelist,
> > FALSE, scratch_pool));
> > - SVN_ERR(svn_rangelist_merge(&source_master_rangelist,
> > - master_noninheritable_rangelist,
> > - scratch_pool));
> > + SVN_ERR(svn_rangelist_merge2(source_master_rangelist,
> > + master_noninheritable_rangelist,
> > + scratch_pool, scratch_pool));
> > SVN_ERR(svn_rangelist_remove(&master_inheritable_rangelist,
> > master_inheritable_rangelist,
> > source_master_rangelist,
> >
>
> (same) You can use iterpool for the scratch pool here, as that pool hasn't been destroyed at this scope.
I thought about these two points while I was changing the code.
This function is very long. It should really be split up into
several pieces. The more complex the dependencies between variables
the harder it will be to untangle it. That's why I chose not to reuse
the iterpool outside of loops. Doing so won't save that much memory anyway.
The main point of this commit is to make the number of new allocations
independent of the number of loop iterations.
> And at the very bottom of this function we run a log operation where we could also pass iterpool as a scratch pool if we extend the scope a bit.
I'd say let's split the function up first.
It's already complex enough as it is.
Received on 2011-07-22 13:09:44 CEST