Patchwork [02,of,10,V4] context: drop a redundant fast path in introrev

login
register
mail settings
Submitter Boris Feld
Date Oct. 4, 2018, 2:44 p.m.
Message ID <afeaa54d6b18c6582e46.1538664276@localhost.localdomain>
Download mbox | patch
Permalink /patch/35451/
State Accepted
Headers show

Comments

Boris Feld - Oct. 4, 2018, 2:44 p.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1538635201 -7200
#      Thu Oct 04 08:40:01 2018 +0200
# Node ID afeaa54d6b18c6582e461a65000f506bc1d7b9d8
# Parent  8ae36e75e785542f415e0b1daa6f2c840542e17e
# EXP-Topic copy-perf
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r afeaa54d6b18
context: drop a redundant fast path in introrev

Now that _adjustlinkrev fast path this case itself, we no longer need an extra
conditional.

A nice side effect is that we are no longer calling `self.rev()`. In case
where `_descendantrev` is set, calling `self.rev` will trigger a potentially
expensive `_adjustlinkrev` call. So blindly calling `self.rev()` to avoid
another `_adjustlinkrev` call can be counterproductive.

Note that `_descendantrev` is currently never taken into account in `introrev`
so far which is wrong. We'll fix that in changeset later in this series.
via Mercurial-devel - Oct. 4, 2018, 4:03 p.m.
On Thu, Oct 4, 2018 at 7:44 AM Boris Feld <boris.feld@octobus.net> wrote:

> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1538635201 -7200
> #      Thu Oct 04 08:40:01 2018 +0200
> # Node ID afeaa54d6b18c6582e461a65000f506bc1d7b9d8
> # Parent  8ae36e75e785542f415e0b1daa6f2c840542e17e
> # EXP-Topic copy-perf
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r
> afeaa54d6b18
> context: drop a redundant fast path in introrev
>
> Now that _adjustlinkrev fast path this case itself, we no longer need an
> extra
> conditional.
>
> A nice side effect is that we are no longer calling `self.rev()`. In case
> where `_descendantrev` is set, calling `self.rev` will trigger a
> potentially
> expensive `_adjustlinkrev` call. So blindly calling `self.rev()` to avoid
> another `_adjustlinkrev` call can be counterproductive.
>
> Note that `_descendantrev` is currently never taken into account in
> `introrev`
> so far which is wrong. We'll fix that in changeset later in this series.
>
> diff --git a/mercurial/context.py b/mercurial/context.py
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -779,7 +779,7 @@ class basefilectx(object):
>          lkr = self.linkrev()
>

This now becomes unused. I'll either drop this line in flight or change
"self.linkrev()" below to be "lkr", depending on whether it's used in later
patches (I haven't checked yet).


>          attrs = vars(self)
>          noctx = not (r'_changeid' in attrs or r'_changectx' in attrs)
> -        if noctx or self.rev() == lkr:
> +        if noctx:
>              return self.linkrev()
>          return self._adjustlinkrev(self.rev(), inclusive=True)
>
>

Patch

diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -779,7 +779,7 @@  class basefilectx(object):
         lkr = self.linkrev()
         attrs = vars(self)
         noctx = not (r'_changeid' in attrs or r'_changectx' in attrs)
-        if noctx or self.rev() == lkr:
+        if noctx:
             return self.linkrev()
         return self._adjustlinkrev(self.rev(), inclusive=True)