[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Incorrect handling of svn:mergeinfo during merging on the svnpatch-diff branch

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Thu, 11 Sep 2008 18:19:06 +0100

On Thu, 2008-09-11 at 12:26 -0400, Paul Burba wrote:
> On Thu, Sep 11, 2008 at 11:03 AM, Julian Foad
> > Paul explained to me on IRC...
> >
> >> Note that the "filtering", although currently implemented as a single
> >> function call, is performing two different logical functions:
> >>
> >> 1. Avoid adding any self-referential mergeinfo in the current merge.
> >> 2. Clean up any self-referential mergeinfo that may have existed.
> >
> > I was completely wrong about that. It only does (1), and (1) is what's
> > under discussion. That means it makes sense to avoid calling this
> > filtering function in a reverse merge. It still should not matter if we
> > did call it,
>
> I think it still matters as regards merge performance. Given how much
> slower merge performance has become in general with 1.5.x, I find any
> performance improvements (especially simple ones like this) to be at
> least a little compelling.
>
> FWIW I ran Arfrever's reverse merge of 'svn merge -c-32625
> ^/branches/svnpatch-diff' three times with filtering 'on' (trunk) for
> reverse merges and three times with it 'off' (my patch): The average
> time for the merge dropped from 00:05:52 to 00:04:01, a 31% reduction.

Ack: there is a performance benefit.

[...]
> > See the attached patch to replace "url" with a "merge_source" structure.
> >
> > Dou you think that would be better?
>
> For the purposes of my patch it is more than is needed, but if for
> conflict resolution it would be helpful I see no harm in including it
> now. Though I'd like to be explicit about how these values change as
> different merge sources are processed (similar to my comment for
> IS_ROLLBACK), something like:
>
> /* The left and right URLs and revs. The value of this field changes to
> reflect the merge_source_t *currently* being merged by do_merge(). */
> merge_source_t merge_source;

OK. Committing my patch (as soon as testing completes) with your
suggested doc string change... r33024.

> > In summary, I've no problem with you committing this if you could give
> > some sort of tweak to the comment I commented on, and all the rest of my
> > thoughts are just for your consideration!
>
> Seems that thought still holds, so I'll commit this later today
> (baring any objections of course).

+1.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-09-11 19:19:23 CEST

This is an archived mail posted to the Subversion Dev mailing list.