[PATCH 7 of 7] ancestors: return all ancestors - not just those who are furthest from a root

[Giovanni Gherdovich]

On Wed, Feb 26, 2014 at 12:54 AM, Mads Kiilerich <mads at kiilerich.com> wrote:
>
> On 02/24/2014 11:31 PM, Matt Mackall wrote:
>>
>> On Mon, 2014-02-24 at 23:19 +0100, Mads Kiilerich wrote:
>>>
>>> [...]
>>> The function seemed to solve a harder problem than necessary. It gave
some
>>> common ancestors priority over others. [...]
>>
>> I'll need to think on this. But you should probably share some
>> benchmarks.
>
>
> What benchmarks do you have in mind?
>
> My concern here is only correctness, not performance.
> The patch just removes a post processing step in
> the gca calculation [...]

When bos added the function you're removing,
he was very happy of the speedup he got:
http://selenic.com/hg/rev/2f7186400a07

The previous ancestor() code was renamed to genericancestor()
(dead code you are removing in another patch).
The reason that code wasn't deleted I guess
is in this 0/N message of that same series by bos:
http://markmail.org/message/64gbckmjmmwvdt3b

I think an important point is:
does mercurial need to bend the definition of Greater Common Ancestor,
which is heads(::X and ::Y), adding the clause that the base
of merges needs also to be "the furthest from the root" ?

One can say that it kinda make sense because "furthest from a root"
can mean "more recent" in the topological sense.
But if we say that we drop this, you're indeed saving some work
with your code removal.
Otherwise, it has to be done elsewhere and you might not have all info you
need at that point.

Cheers,
GGhh
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-devel/attachments/20140226/29848e0a/attachment-0002.html>