[Svnmerge] [PATCH] Performance patch for merging

[Raman Gupta]

Giovanni Bajo wrote:
> Raman Gupta <rocketraman at fastmail.fm> wrote:
> 
>>> The attached patch fixes a performance issue with the merge command. In
>>> the current version, the merge command does not remove phantom revisions
>>> from the list of revs that are used for 1) merging itself, and 2)
>>> obtaining the log messages for each merged revision to build the commit
>>> log message.
> 
> Alas, this patch is wrong.
> 
> You must not remove the phantom revisions from 1). In fact, it's done on
> purpose: with phantom revisions, you get more compact ranges and you call
> svn merge less times. For instance, if my property is "1-100", HEAD is 200,
> and I simply call "svnmerge merge" I want to invoke "svn merge" exactly
> *once*: to merge the interval 100-200. It doesn't matter if there are
> phantom revisions in that range: svn merge will ignore those. By calling
> svnmerge merge just once you get much faster results, and many less
> conflicts.

No, I don't believe it is wrong -- svnmerge merge is not called any more
times than it was before because the minimal_merge_intervals adds the
phantom revisions back in.

> In fact, I would *really* appreciate if you covered this usage with a
> testcase, so that somebody else does not try again to skip phantom revisions
> from merging.

The phantom revisions are not being skipped, as explained above.

> As for 2), I know that log construction is very slow, but the problem should
> be fixed otherwise. You should use minimal_merge_intervals() in
> construct_merged_log_message(), so to call "svn log" for ranges of revisions
> (instead of one revision at a time). Even here, the idea is to call "svn
> log" less times as possible, so it doesn't really matter if we call it once
> for a solid range like 100-200 including phantom revisions: those phantom
> revisions will not appear in the "svn log" output and thus will not be part
> of the commit message.

Yes, this is another optimization we can make. Once this optimization is
made, my simpler patch can be reverted as I think it will be unnecessary.

Cheers,
Raman