Re: [PATCH] Split up the reintegrate merge API: first find what to do, then do it - v2

I (Julian Foad) wrote:

>   The svn client reintegrate merge code calls:
>
>     svn_client_find_reintegrate_merge(&url1, &rev1, &url2, &rev2, ...);
>     svn_client_merge4(url1, rev1, url2, rev2, ...);

One issue is that there is some duplication of work in this formulation.  Both of these functions check that the target WC is suitable for merging into.  Both open up RA sessions.  Both determine the youngest common ancestor.

It looks like the amount of work duplicated is fairly constant,
suggesting it would not affect large merges very much, but I have yet to
 measure this.

We could avoid any or all of this duplication at the cost of making the API more complex, for example by exposing a cut-down merge function that only does part of what svn_client_merge4() does.  The 'svn_client_do_reintegrate_merge' that I showed in the previous version of this patch was one such option.  That was so cut-down that it was perhaps only suitable for doing a reintegrate merge.

Another way to address part of the issue is to expose the "check that the target WC is suitable for merging into" functionality as a separate API (like in v1 of this patch), which would be called by the client before doing any kind of merge (reintegrate or otherwise) instead of making each merge function do that check internally.  Another part of the issue could be addressed by pooling and reusing the client's RA connections (which is a totally separate project).  There are probably other ways of refactoring the merge APIs too.

I'll think on this a bit more.  I can say though that I'm more concerned with the large-scale shape of the merge APIs, and I'd strongly prefer general solutions such as RA session pooling over special-casing the APIs.

- Julian
Received on 2011-12-11 22:26:33 CET