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

Re: Problems Reintegrating the fsfs-format7 branch

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Thu, 4 Apr 2013 16:08:02 +0100 (BST)

Paul Burba wrote:
> On Fri, Mar 22, 2013 at 2:39 PM, Julian Foad wrote:
>> Paul Burba wrote:
>>> On Fri, Mar 8, 2013 at 4:17 PM, Paul Burba <ptburba_at_gmail.com> wrote:
>>>> The attached patch accomplishes, let's call it "2.5", because it
>>>> handles the more general case I outlined above, but not the case
>>>> represented by issue #4255.  Would you mind taking a look?
>>>
>>> fyi http://subversion.tigris.org/issues/show_bug.cgi?id=4329#desc7
>>> explains my proposed patch in terms of how it changes the symmetric
>>> merge algorithm.
>>
>> Hi Paul.  That did help, yes.
>>
>> It took me a while to get my head back into this stuff, but yes it looks like
>> this hangs together theoretically as well as fixing the immediate use case.
>>
>> As for the implementation, in find_last_merged_location() the task is to
>> find the last location on SOURCE_BRANCH such that all changes on SOURCE_BRANCH
>> after YCA up to and including *BASE_P have already been merged into the target
>> branch.  The function finds the first eligible revision, considering mergeinfo
>> only on the branch root, and then you add a call to svn_client_mergeinfo_log2()
>> to find the first eligible revision after that, considering subtree mergeinfo
>> and considering only operative changes.  Two questions:
>>
>>    - Why do you only call svn_client_mergeinfo_log2() if there has been a
>> merge from SOURCE to TARGET recorded at the root of the branch?  That would
>> seem to miss the cases where there have only been subtree merges.
>
> You are quite right.  This can be done entirely with
> svn_client_mergeinfo_log2() and catch the case where only subtree
> merges have been performed, but are "complete" (do we have an official
> term to describe the situation where a given subtree merge of rX is
> effectively the same as a merge of rX to the branch root?).
>
>>    - Why not just use svn_client_mergeinfo_log2(), if it answers the
>> question, and delete all the prior code that first looks at just the branch-root
>> mergeinfo?  If the initial look at branch-root mergeinfo is a performance
>> optimization, then it should be inside svn_client_mergeinfo_log2(), I would
>> assume.
>
> I reworked the patch to do just this and committed it in r1464102.
>
>> You might consider committing the part that adds a 'limit' argument to a
>> mergeinfo API as a separate change, as it looks like a fairly large part of
>> the patch.  But if we can use svn_client_mergeinfo_log2() to answer the whole
>> question in one go, then it probably won't need the limit argument anyway.
>
> r1464102 simply raises a SVN_ERR_CEASE_INVOCATION error in the
> svn_log_entry_receiver_t callback.  This, combined with my earlier
> commit (r1463237) to allow to ask for revisions in youngest:oldest or
> oldest:younger order, means that there is no need for the limit code;
> we can call svn_client_mergeinfo_log2() in such a way as to find the
> youngest merged revision or the oldest elibigle revision in the first
> callback.

Fantastic.  Thanks, Paul.

For the record, this resolves the two issues <http://subversion.tigris.org/issues/show_bug.cgi?id=4258> "Automatic merge after subtree merge in opposite direction" and <http://subversion.tigris.org/issues/show_bug.cgi?id=4329> "automatic merge uses reintegrate type merge if source is fully synced".  (You've already updated them, I just want to mention them here.)

> What I've struggled with is here is the (not unexpected) catch: Using
> svn_client_mergeinfo_log2(), rather than simply looking at the
> mergeinfo on the branch roots, may be more correct, but obviously
> takes more time.  During my ad hoc testing with some of our own
> branches, it adds anywhere from 5 to 25 seconds of real time.
>
> I'm still looking if there are any places we can claw back some of
> that performance loss before 1.8...but after a long history of
> speeding up merge since 1.5, we might have to take a performance hit
> in the name of correctness.

One candidate for optimisation is that I added a check whether the source and target branches are related, which is called from 'svn' before the merge API is called.  This check involves retrieving the location-segment histories of the two branch root dirs.  We then throw away these location histories and retrieve them again later in the merge.  We should store them and re-use them.  (I'm pretty sure they were already being fetched more than once even before I added this relatedness check.)

- Julian
Received on 2013-04-04 17:08:50 CEST

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