Patchwork [STABLE] context: optimize linkrev adjustment in blockancestors() (issue5538)

login
register
mail settings
Submitter Denis Laxalde
Date April 25, 2017, 7:20 a.m.
Message ID <b159c904ca10bf48afe2.1493104818@sh77.tls.logilab.fr>
Download mbox | patch
Permalink /patch/20289/
State Accepted
Headers show

Comments

Denis Laxalde - April 25, 2017, 7:20 a.m.
# HG changeset patch
# User Denis Laxalde <denis.laxalde@logilab.fr>
# Date 1493051603 -7200
#      Mon Apr 24 18:33:23 2017 +0200
# Branch stable
# Node ID b159c904ca10bf48afe271ad4d16ade9d1ca3c40
# Parent  40cf693fc07d846502f9c15a1602880ca99d7b56
# Available At http://hg.logilab.org/users/dlaxalde/hg
#              hg pull http://hg.logilab.org/users/dlaxalde/hg -r b159c904ca10
context: optimize linkrev adjustment in blockancestors() (issue5538)

We set parent._descendantrev = child.rev() when walking parents in
blockancestors() so that, when linkrev adjustment is perform for these, it
starts from a close descendant instead of possibly topmost introrev. (See
`self._adjustlinkrev(self._descendantrev)` in filectx._changeid().)

This is similar to changeset c82d88dfaf59, which added a "f._changeid"
instruction in annotate() for the same purpose.
However, here, we set _descendantrev explicitly instead of relying on the
'_changeid' cached property being accessed (with effect to set _changeid
attribute) so that, in _parentfilectx() (called from parents()), we go through
`if '_changeid' in vars(self) [...]` branch in which instruction
`fctx._descendantrev = self.rev()` finally appears and does what we want.

With this, we can roughly get a 3x speedup (including in example of issue5538
from mozilla-central repository) on usage of followlines revset (and
equivalent hgweb request).
Gregory Szorc - April 25, 2017, 7:29 a.m.
On Tue, Apr 25, 2017 at 12:20 AM, Denis Laxalde <denis@laxalde.org> wrote:

> # HG changeset patch
> # User Denis Laxalde <denis.laxalde@logilab.fr>
> # Date 1493051603 -7200
> #      Mon Apr 24 18:33:23 2017 +0200
> # Branch stable
> # Node ID b159c904ca10bf48afe271ad4d16ade9d1ca3c40
> # Parent  40cf693fc07d846502f9c15a1602880ca99d7b56
> # Available At http://hg.logilab.org/users/dlaxalde/hg
> #              hg pull http://hg.logilab.org/users/dlaxalde/hg -r
> b159c904ca10
> context: optimize linkrev adjustment in blockancestors() (issue5538)
>
> We set parent._descendantrev = child.rev() when walking parents in
> blockancestors() so that, when linkrev adjustment is perform for these, it
> starts from a close descendant instead of possibly topmost introrev. (See
> `self._adjustlinkrev(self._descendantrev)` in filectx._changeid().)
>
> This is similar to changeset c82d88dfaf59, which added a "f._changeid"
> instruction in annotate() for the same purpose.
> However, here, we set _descendantrev explicitly instead of relying on the
> '_changeid' cached property being accessed (with effect to set _changeid
> attribute) so that, in _parentfilectx() (called from parents()), we go
> through
> `if '_changeid' in vars(self) [...]` branch in which instruction
> `fctx._descendantrev = self.rev()` finally appears and does what we want.
>
> With this, we can roughly get a 3x speedup (including in example of
> issue5538
> from mozilla-central repository) on usage of followlines revset (and
> equivalent hgweb request).
>

I get a 5.4x speedup and the new code is spending 2/3 of CPU time reading
revlogs and ~26% doing this |p._descendantrev = c.rev()| assignment. I'd
say that's a clear win :)

Thank you for improving this.


>
> diff --git a/mercurial/context.py b/mercurial/context.py
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -1214,6 +1214,10 @@ def blockancestors(fctx, fromline, tolin
>                  # introduced in this revision; no need to go futher in
> this
>                  # branch.
>                  continue
> +            # Set _descendantrev with 'c' (a known descendant) so that,
> when
> +            # _adjustlinkrev is called for 'p', it receives this
> descendant
> +            # (as srcrev) instead possibly topmost introrev.
> +            p._descendantrev = c.rev()
>              visit[p.linkrev(), p.filenode()] = p, linerange1
>          if inrange:
>              yield c, linerange2
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Yuya Nishihara - April 25, 2017, 2:20 p.m.
On Tue, 25 Apr 2017 00:29:31 -0700, Gregory Szorc wrote:
> On Tue, Apr 25, 2017 at 12:20 AM, Denis Laxalde <denis@laxalde.org> wrote:
> 
> > # HG changeset patch
> > # User Denis Laxalde <denis.laxalde@logilab.fr>
> > # Date 1493051603 -7200
> > #      Mon Apr 24 18:33:23 2017 +0200
> > # Branch stable
> > # Node ID b159c904ca10bf48afe271ad4d16ade9d1ca3c40
> > # Parent  40cf693fc07d846502f9c15a1602880ca99d7b56
> > # Available At http://hg.logilab.org/users/dlaxalde/hg
> > #              hg pull http://hg.logilab.org/users/dlaxalde/hg -r
> > b159c904ca10
> > context: optimize linkrev adjustment in blockancestors() (issue5538)
> >
> > We set parent._descendantrev = child.rev() when walking parents in
> > blockancestors() so that, when linkrev adjustment is perform for these, it
> > starts from a close descendant instead of possibly topmost introrev. (See
> > `self._adjustlinkrev(self._descendantrev)` in filectx._changeid().)
> >
> > This is similar to changeset c82d88dfaf59, which added a "f._changeid"
> > instruction in annotate() for the same purpose.
> > However, here, we set _descendantrev explicitly instead of relying on the
> > '_changeid' cached property being accessed (with effect to set _changeid
> > attribute) so that, in _parentfilectx() (called from parents()), we go
> > through
> > `if '_changeid' in vars(self) [...]` branch in which instruction
> > `fctx._descendantrev = self.rev()` finally appears and does what we want.
> >
> > With this, we can roughly get a 3x speedup (including in example of
> > issue5538
> > from mozilla-central repository) on usage of followlines revset (and
> > equivalent hgweb request).
> >
> 
> I get a 5.4x speedup and the new code is spending 2/3 of CPU time reading
> revlogs and ~26% doing this |p._descendantrev = c.rev()| assignment. I'd
> say that's a clear win :)

Yeah, looks good. Queued, thanks.

linkrev is hard.

Patch

diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -1214,6 +1214,10 @@  def blockancestors(fctx, fromline, tolin
                 # introduced in this revision; no need to go futher in this
                 # branch.
                 continue
+            # Set _descendantrev with 'c' (a known descendant) so that, when
+            # _adjustlinkrev is called for 'p', it receives this descendant
+            # (as srcrev) instead possibly topmost introrev.
+            p._descendantrev = c.rev()
             visit[p.linkrev(), p.filenode()] = p, linerange1
         if inrange:
             yield c, linerange2