>> @@ -229,6 +229,11 @@
>> * It contains individual elements of type ' svn_merge_range_t *'.
>> apr_array_header_t *reflected_ranges;
>> + /* Before running the actual reflective rev merge we do get
>> + * a summary of merge and store the to be affected paths as keys in
>> + * this hash with value being of type svn_client_diff_summarize_kind_t *.
>> + */
> I'd rephrase this as something more like "A hash mapping paths to
> (svn_client_diff_summarize_kind_t *) values describing how they were
> affected by the reflective range under consideration."
Fixed in r29235.
>> +static svn_error_t *
>> +summarize_reflected_ranges(svn_depth_t depth,
>> + merge_cmd_baton_t *merge_b)
>> + int i;
>> + const char *target_url;
>> + apr_pool_t *iterpool = svn_pool_create(merge_b->pool);
>> + SVN_ERR(svn_ra_get_session_url(merge_b->target_ra_session,
>> + &target_url,
>> + merge_b->pool));
>> + svn_hash__clear(merge_b->reflective_rev_affected_paths);
> How often does this function get called? Since the keys and values of
> this hash are allocated in the top-level pool passed to do_merge, it
> seems like this might lead to inappropriate unbounded memory usage.
Only once per reflective revision.
>> + SVN_ERR(svn_client_diff_summarize_peg2(target_url, &peg_revision,
>> + &opt_revision1, &opt_revision2,
>> + depth,
>> + merge_b->ignore_ancestry,
>> + get_diff_summary_func_cb,
>> + merge_b->reflective_rev_affected_paths,
>> + merge_b->ctx,
>> + iterpool));
>> + }
> Others have pointed out that this should be documented as an expensive
> operation. I'd take it a step farther: it's worth considering whether
> a many-range-diff-summarize RA API would be helpful here.
Sounds Interesting. If I am correct if we had such an API(As I
uses common base code), we can do all the merges at one shot rather than
at multiple revision ranges as we do today.
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-02-08 16:03:26 CET