[PATCH 1 of 5] revlog: make commonancestorsheads accept revision numbers

[Jun Wu]

Excerpts from Gregory Szorc's message of 2016-11-01 19:47:07 -0700:
> On Tue, Nov 1, 2016 at 7:25 PM, Jun Wu <quark at fb.com> wrote:
> 
> > Excerpts from Pierre-Yves David's message of 2016-11-02 00:05:32 +0100:
> > > I think I would rather see a two methods, one for revs and one for nodes
> > > that would keep the signature of each function clearer. One of the
> > > function would probably all into the other one to keep the code common.
> > >
> > > What do you think?
> >
> > If we have two "commonancestorsheads"s, we will have to write two
> > "isancestor"s. I'd avoid the contagious pattern that enforces the caller
> > ("isancestor" in this case) to do things otherwise unnecessary.
> >
> > Given the fact that "repo.rev(x)" takes both node and rev. And repo[x]
> > takes
> > everything. I'd prefer shorter code. It can be seen as a reasonable
> > function
> > overloading in C++.
> >
> 
> I agree with Pierre-Yves here: it is too easy to fall into performance
> traps where functions accept either node or rev and needlessly perform type
> checking or dip into the index for unneeded lookups. Let's standardize on
> one. With list comprehensions, it's easy enough to ensure arguments are
> either all revs or all nodes.
> 
> I do disagree about the need for 2 methods. Just standardize on whatever
> one makes sense and have the caller convert to that type. This may entail a

The convert is not free (coverting from node -> rev requires walking through
the whole index for the first 2 times). This patch is all about avoiding
converting from node -> rev -> node (commonancestorsheads), and the return
value is a bool (isancestor). Comparing with converting between node and
rev, I think "isinstance" is a cheaper operation. It should be even much
faster comparing with commonancestorsheads's C implementation. When the
commonancestorsheads C implementation is the bottleneck, I don't think
it's necessary to optimize those "isinstance" out is necessary.

> `revlog.revstonodes(revs)` or `revlog.nodestorevs(nodes)` to facilitate
> bulk converting iterables of 1 type to the other.