[PATCH 4 of 6 v6] discovery: convert legacy part of checkheads to revs from nodes

[Joshua Redstone]

On 6/16/12 5:42 AM, "Pierre-Yves David" <pierre-yves.david at ens-lyon.org>
wrote:

>
>On 15 juin 2012, at 18:13, Joshua Redstone wrote:
>
>> My guess is that the intention was to sort based on revs, not nodes,
>>originally, but I'm also not sure.  Sorting nodes doesn't seem to have
>>much purpose, and also, wrt to the comment, is not consistent with
>> what _updatebranchcache does, which is sort by revs.
>> Josh
>
>outgoind.missing content comes from revlog.findcommonmissing.
>Revlog.findcommonmissing doc says:
>
>    The list is sorted by revision number, meaning it is
>    topologically sorted.
>
>So, sorting on rev is "correct". But it is also a bit useless anyway.
>It will be enough to just reverse the list.

I'm inclined to leave the extra sort in because there is a fair amount of
code and interfaces between the generation of the missing list and the
consumption in discovery, and seems like the kind of thing that would be
easy to break and not know it.  Also, the sort is very cheap.  But I'm
happy to take it out if you want.

>
>The list is reverse to ensure we meet missing head as soon as possible.
>Using heads helps to remove as much changeset as possible from the
>candidate
>set, preventing redundant reachable/ancestors call.
>**But outgoing.missingheads could be used directly**
>
>
>However we can be smarter here and avoid any graph traveling:
>
>   oldheads = [h for h in remoteheads if h in cl.nodemap]
>   # newheads = <pushedheads> + (<oldheads> without children in <pushed>)
>   rvset = repo.set('%ln - parents(%ln) + %ln', oldheads,
>outgoing.missing, outgoing.missingheads)
>   newheads = [c.node() for c in rvset]
>
>(Garantee un-tested code!)

I'm inclined to punt on this optimization given it's not currently a
performance problem.  Also, probably better for that kind of thing to be
in a separate diff?
Josh

>
>-- 
>Pierre-Yves David