[PATCH 1 of 2] localrepo: cache self.changelog in local variable

Bryan O'Sullivan bos at serpentine.com
Mon Feb 13 13:20:21 EST 2017


On Mon, Feb 13, 2017 at 2:32 AM, Stanislau Hlebik <stash at fb.com> wrote:

> localrepo: cache self.changelog in local variable
>
> Repeated self.changelog lookups can incur overhead. Let's cache it in the
> separate variable.
>

I'm not suggesting that you should change this since Yuya has already
queued it, but a few notes for the future...


> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -1853,8 +1853,9 @@
>
>      def heads(self, start=None):
>          if start is None:
> -            headrevs = sorted(self.changelog.headrevs(), reverse=True)
> -            return [self.changelog.node(rev) for rev in headrevs]
> +            cl = self.changelog
> +            headrevs = sorted(cl.headrevs(), reverse=True)
> +            return [cl.node(rev) for rev in headrevs]
>
>          heads = self.changelog.heads(start)
>          # sort the output in rev descending order
>

For local nano-optimizations like this, it's worth being clear about
whether you saw this show up in a performance profile or not. (And if you
did, to explain why.) Most of the other changes of this form have happened
because of observed performance effects, but as these edits harm
readability a little, they deserve at least a little explicit justification.

Since the only place that the chained access could be hurting in your patch
is inside the list comprehension, you could further micro-optimize away
another chained access:

node = self.changelog.node
headrevs = sorted(self.changelog.headrevs(), reverse=True)
return [node(rev) for rev in headrevs]
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170213/dbcdc52f/attachment.html>


More information about the Mercurial-devel mailing list