> Before discussing the patch I want make two quick suggestions re patch
> submissions in general (I know this isn't technically a patch
> submission, more of an RFC, but you sent a patch):
>
> 1) New functions should have doc strings, even if they are private. I
> know there are examples in the code of functions with no or poor
> documentation, heck I'm probably responsible for some, but it makes
> reviewing a lot easier.
>
> 2) Include a log message in the recommended format, see
> http://subversion.tigris.org/hacking#log-messages. Again this makes
> reviewing easier and increases the chances that someone will review in
> the first place.
>
> Ok, sorry for that bit of nagging, now onto the patch!
I didn't expect that this patch will be committed (because it's naive
and maybe incomplete). That's why there was no of log message. Looks
like omitting log message wasn't a good idea :)
>> - significantly improve merge performance
> Could you explain how this significantly improves merge performance?
> All it does is not update some subtree mergeinfo at the end of a
> merge. Is there some follow-on effect with subsequent merges you are
> envisioning?
Looks like it's my error too. After repeated measures I found that
there is no of *significant* performance improvements.
> If we pursue this idea we need to do one of the following:
>
> Note: Most of this has already been discussed here
> http://svn.haxx.se/dev/archive-2008-03/0914.shtml, but I don't object
> to restating it.
I can't find any references to 'no-op merges' in the specification at
http://subversion.tigris.org/merge-tracking/
It's hard to investigate and understand this sophisticated piece of
functionality without up-to-date specification. Moreover, some other
aspects such as "empty-mergeinfo-on-wc-to-wc-copy" aren't specified in
detail.
> 1) Make the elision code smarter, currently it is rather stupid (some
> might say cheap or efficient :-), simply comparing mergeinfo on path X
> and it's nearest parent P and seeing if the mergeinfo is identical
> when accounting for the relative path differences between X and P.
> This obviously won't work if we don't don't update all subtrees with
> mergeinfo.
>
> 2) Just get rid of elision. This would mean that once we have subtree
> mergeinfo, we have it forever. And we might need to special case
> reverse merges so we don't end up with empty mergeinfo when we should
> have none. But from what I've seen getting rid of elision might not
> be too big of a deal, it hasn't seemed that useful in practice (at
> least in our own repository).
Am I understand properly, that you think that mergeinfo elision isn't
very useful (from your practice)?
> There are some other considerations with your patch in place:
>
> For the next two items let's say we have a working copy merge target
> named BRANCH_TARGET with no explicit mergeinfo and with a single
> subtree named BRANCH_TARGET/SUBTREE that has some of its own explicit
> mergeinfo:
>
> Properties on 'BRANCH_TARGET\SUBTREE':
> svn:mergeinfo
> /TRUNK/SUBTREE:4
>
> A) Individual merges equivalent to a single merge can produce
> different mergeinfo results.
>
> Say we merge ROOT_URL/TRUNK BRANCH_TARGET -r10:20. Let's assume that
> the SUBTREE is affected by r15 and r18. If we did a merge like this:
>
> svn merge ROOT_URL/TRUNK BRANCH_TARGET -r10:20 (SUBTREE unaffected
> so no mergeinfo change)
>
> We'd expect mergeinfo from the source for r11-20 to be added to both
> BRANCH_TARGET and BRANCH_TARGET/SUBTREE with or without your patch
> yes? Like this:
>
> Properties on 'BRANCH_TARGET':
> svn:mergeinfo
> /TRUNK:11-20
> Properties on 'BRANCH_TARGET\SUBTREE':
> svn:mergeinfo
> /TRUNK/SUBTREE:4,11-20
>
> But what if we did several individual merges equivalent to the single merge:
>
> svn merge ROOT_URL/TRUNK BRANCH_TARGET -r10:14 (SUBTREE unaffected
> so no mergeinfo change)
> svn merge ROOT_URL/TRUNK BRANCH_TARGET -c15 (SUBTREE affected
> mergeinfo updated)
> svn merge ROOT_URL/TRUNK BRANCH_TARGET -r15:18 (SUBTREE affected
> mergeinfo updated)
> svn merge ROOT_URL/TRUNK BRANCH_TARGET -r18:20 (SUBTREE unaffected
> so no mergeinfo change)
>
> The tree changes wrought by these individual merges will be the same,
> but the resulting merginfo would be:
>
> Properties on 'BRANCH_TARGET':
> svn:mergeinfo
> /TRUNK:11-20
> Properties on 'BRANCH_TARGET\SUBTREE':
> svn:mergeinfo
> /TRUNK/SUBTREE:4,15-18
>
Is it anywhere mentioned in the specification, that merges that
produce equal results should produce equal mergeinfo?
As far as I can understand, specification states the opposite:
[[
Q: Are there many different "spellings" of the same merge info?
A: Yes. Depending on the URLs and target you specify for merges, I
believe it is possible to end up with merge info in different places,
or with slightly different revision lines that have the same semantic
effect (e.g. info like /trunk:1-9 vs /trunk:1-8\n/trunk/bar:9 when
revision 9 on /trunk only affected /trunk/bar), so you can end up with
merge info in different places, even though the semantic result will
be the same in all cases.
]]
Moreover, there are some signs that original specification doesn't
allow no-op merges to modify mergeinfo:
[[
Each will store the full, complete list of current merged-in changes,
as far as it knows...
Directly merged into the item means changes from any merge that have
affected this path, which includes merges into parents, grandparents,
etc., that had some affect on the path.
]]
At http://svn.haxx.se/dev/archive-2007-03/0000.shtml you mention that
'the mergeinfo property store(s) the *full, complete* list of
revisions that are directly merged into the item."
Please excuse me for referencing the specification (usually I'm not so
pedantic). But I'm trying to understand what is conceptual and what is
just a band-aid.
> B) "Subtree" merges and "Root" merges can produce inconsistent mergeinfo.
>
> Hopefully it will be obvious from context what I mean by "subtree" and
> "root" merges...
>
> With or without your patch, no-op merges still record mergeinfo on the
> merge target. Did you intend to stop this as well?
I don't know for sure. From the paranoid point of view, there are
should be no mergeinfo changes if nothing is affected. But I
understand that there can be some problems with other features like
--reintegrate.
> Because if we
> record mergeinfo for no-op merges:
>
> When we do a "root" merge...
>
> svn merge ROOT_URL/TRUNK BRANCH_TARGET -r10:14
>
> ...SUBTREE is unaffected so no mergeinfo change there, we get:
>
> Properties on 'BRANCH_TARGET':
> svn:mergeinfo
> /TRUNK:11-14
> Properties on 'BRANCH_TARGET\SUBTREE':
> svn:mergeinfo
> /TRUNK/SUBTREE:4
>
> But is we do a "subtree" merge...
>
> svn merge ROOT_URL/TRUNK/SUBTREE BRANCH_TARGET/SUBTREE -r10:14
>
> ...SUBTREE is unaffected, but SUBTREE is the target so it gets updated
> mergeinfo:
>
> Properties on 'BRANCH_TARGET\SUBTREE':
> svn:mergeinfo
> /TRUNK/SUBTREE:4,11-14
>
> I'm not arguing that A and B are big problems, but this very
> inconsistency is one of the reasons we decided to record subtree
> mergeinfo all the time. That, and it also to facilitate "cheap"
> elision.
I don't think that this is an optimal way. I prefer the standard and
well know pattern when user "doesn't pay for something he doesn't
use". Actually, our team doesn't use cherry picking, local branches,
partially switched working copies etc. All we have is a trunk plus
several plain stabilization/feature branches.
But after 1.5 was released our life is turned to hell. Now we're
requested to worry about no-op merges regardless of the fact that we
don't need mergeinfo elision at all!
> So the real question is not so much should we stop recording mergeinfo
> on subtrees unaffected by a merge, but rather do we care about elision
> and can we live with the inconsistencies in mergeinfo produced in
> cases like A and B.
I think that mergeinfo elision should be explicit process executed by
something like svnadmin mergeinfo-optimize. Just as proposed in the
specification.
--
Danil
---------------------------------------------------------------------
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-15 17:27:53 CEST