[PATCH] localrepo: avoid unnecessary conversion from node to rev

[Gregory Szorc]

> On Feb 3, 2017, at 05:16, Yuya Nishihara <yuya at tcha.org> wrote:
> 
>> On Thu, 2 Feb 2017 15:06:46 +0000, Jun Wu wrote:
>> This patch looks good to me. See inline comment about how to make it faster.
>> That could probably be fixed in flight.
>> 
>> Excerpts from Stanislau Hlebik's message of 2017-02-02 02:57:24 -0800:
>>> # HG changeset patch
>>> # User Stanislau Hlebik <stash at fb.com>
>>> # Date 1486032998 28800
>>> #      Thu Feb 02 02:56:38 2017 -0800
>>> # Node ID 13a528b72173b1228f6eeb0ffc2346e7b78d1d78
>>> # Parent  abf029200e198878a4576a87e095bd8d77d9cea9
>>> localrepo: avoid unnecessary conversion from node to rev
>>> 
>>> changelog.heads() first calls headrevs then converts them to nodes.
>>> localrepo.heads() then sorts them using self.changelog.rev function and makes
>>> useless conversion back to revs. Instead let's call changelog.headrevs() from
>>> localrepo.heads(), sort the output and then convert to nodes. Because headrevs
>>> does not support start parameter this optimization only works if start is None.
>>> 
>>> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
>>> --- a/mercurial/localrepo.py
>>> +++ b/mercurial/localrepo.py
>>> @@ -1852,6 +1852,10 @@
>>>                                   listsubrepos)
>>> 
>>>     def heads(self, start=None):
>>> +        if start is None:
>>> +            headrevs = sorted(self.changelog.headrevs(), reverse=True)
>> 
>> headrevs() is already sorted in both C and Python implementations, so
>> "sorted(..., reverse=True)" could be replaced by "reversed(...)".
> 
> Yeah.
> 
>>> +            return [self.changelog.node(rev) for rev in headrevs]
> 
> The function call "self.changelog" wasn't instant because of repoview, but I
> don't remember if that's still true. Pierre-Yves?

Pierre-Yves made it substantially faster a few releases ago. But there is still enough overhead for it to be problematic.

FWIW, just doing a plain attribute lookup in a simple loop like this can introduce measurable overhead. We go so far as to alias list.append in a local to avoid this in some cases!