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
