Julian Foad wrote on Sun, Dec 11, 2011 at 21:25:55 +0000:
> 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.
I don't think "opening an RA session" is a problem for a function that's
about to do a merge operation :)
Checking the WC's sanity, presumably that's necessary since the WC may
be modified after svn_client_find_reintegrate_merge() but before
svn_client_merge4()? Especially if the user is prompted to +1 the
output of svn_client_find_reintegrate_merge() before the actual merge
is done. (and then goes to lunch, comes back, makes some commits, finds
a loose TSVN dialog and OKs it)
> 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-12 07:02:25 CET