Patchwork [4,of,5] adjustlinkrev: remove the inclusive parameter

login
register
mail settings
Submitter Jun Wu
Date Nov. 1, 2016, 8:51 a.m.
Message ID <f6c6598addaa1854f855.1477990265@x1c>
Download mbox | patch
Permalink /patch/17234/
State Changes Requested
Headers show

Comments

Jun Wu - Nov. 1, 2016, 8:51 a.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1477885221 0
#      Mon Oct 31 03:40:21 2016 +0000
# Node ID f6c6598addaa1854f8556bacb732347256964e3e
# Parent  eead0ed124cbc59b9bb5183035aa481af3547677
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r f6c6598addaa
adjustlinkrev: remove the inclusive parameter

The parameter is only set to False when srcrev=fctx._descendantrev, in which
case inclusive could also be True with little performance impact.

Drop this parameter to make the interface cleaner.
Pierre-Yves David - Nov. 1, 2016, 11:34 p.m.
On 11/01/2016 09:51 AM, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1477885221 0
> #      Mon Oct 31 03:40:21 2016 +0000
> # Node ID f6c6598addaa1854f8556bacb732347256964e3e
> # Parent  eead0ed124cbc59b9bb5183035aa481af3547677
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r f6c6598addaa
> adjustlinkrev: remove the inclusive parameter
>
> The parameter is only set to False when srcrev=fctx._descendantrev, in which
> case inclusive could also be True with little performance impact.

linkrev works in two steps:

1) find a revision that touch that file
2) check if the file version is the one we are looking for

If we drop inclusive here we'll alway have an initial match for (1) 
leading to a false check for (2). (2) is not cheap and we would rather 
avoid that.

> Drop this parameter to make the interface cleaner.
>
> diff --git a/mercurial/context.py b/mercurial/context.py
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -809,5 +809,5 @@ class basefilectx(object):
>          return True
>
> -    def _adjustlinkrev(self, path, filelog, fnode, srcrev, inclusive=False):
> +    def _adjustlinkrev(self, path, filelog, fnode, srcrev):
>          """return the first ancestor of <srcrev> introducing <fnode>
>
> @@ -821,5 +821,4 @@ class basefilectx(object):
>          :filelog: the filelog of this path
>          :srcrev: the changeset revision we search ancestors from
> -        :inclusive: if true, the src revision will also be checked
>          """
>          repo = self._repo
> @@ -834,5 +833,4 @@ class basefilectx(object):
>              # wctx case, used by workingfilectx during mergecopy
>              revs = [p.rev() for p in self._repo[None].parents()]
> -            inclusive = True # we skipped the real (revless) source
>          else:
>              revs = [srcrev]
> @@ -846,5 +844,5 @@ class basefilectx(object):
>                  return lkr
>          # fallback to walk through the changelog
> -        iteranc = cl.ancestors(revs, lkr, inclusive=inclusive)
> +        iteranc = cl.ancestors(revs, lkr, inclusive=True)
>          for a in iteranc:
>              ac = cl.read(a) # get changeset data (we avoid object creation)
> @@ -875,5 +873,5 @@ class basefilectx(object):
>              return self.linkrev()
>          return self._adjustlinkrev(self._path, self._filelog, self._filenode,
> -                                   self.rev(), inclusive=True)
> +                                   self.rev())
>
>      def _parentfilectx(self, path, fileid, filelog):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Jun Wu - Nov. 2, 2016, 2:35 a.m.
Excerpts from Pierre-Yves David's message of 2016-11-02 00:34:19 +0100:
> 
> On 11/01/2016 09:51 AM, Jun Wu wrote:
> > # HG changeset patch
> > # User Jun Wu <quark@fb.com>
> > # Date 1477885221 0
> > #      Mon Oct 31 03:40:21 2016 +0000
> > # Node ID f6c6598addaa1854f8556bacb732347256964e3e
> > # Parent  eead0ed124cbc59b9bb5183035aa481af3547677
> > # Available At https://bitbucket.org/quark-zju/hg-draft 
> > #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r f6c6598addaa
> > adjustlinkrev: remove the inclusive parameter
> >
> > The parameter is only set to False when srcrev=fctx._descendantrev, in which
> > case inclusive could also be True with little performance impact.
> 
> linkrev works in two steps:
> 
> 1) find a revision that touch that file
> 2) check if the file version is the one we are looking for
> 
> If we drop inclusive here we'll alway have an initial match for (1) 
> leading to a false check for (2). (2) is not cheap and we would rather 
> avoid that.

Good catch. I was focused on changelog.i walk but didn't think too much
about the changelog.d walk.

Feel free to drop this one while I'm thinking about other solutions.

> 
> > Drop this parameter to make the interface cleaner.
> >
> > diff --git a/mercurial/context.py b/mercurial/context.py
> > --- a/mercurial/context.py
> > +++ b/mercurial/context.py
> > @@ -809,5 +809,5 @@ class basefilectx(object):
> >          return True
> >
> > -    def _adjustlinkrev(self, path, filelog, fnode, srcrev, inclusive=False):
> > +    def _adjustlinkrev(self, path, filelog, fnode, srcrev):
> >          """return the first ancestor of <srcrev> introducing <fnode>
> >
> > @@ -821,5 +821,4 @@ class basefilectx(object):
> >          :filelog: the filelog of this path
> >          :srcrev: the changeset revision we search ancestors from
> > -        :inclusive: if true, the src revision will also be checked
> >          """
> >          repo = self._repo
> > @@ -834,5 +833,4 @@ class basefilectx(object):
> >              # wctx case, used by workingfilectx during mergecopy
> >              revs = [p.rev() for p in self._repo[None].parents()]
> > -            inclusive = True # we skipped the real (revless) source
> >          else:
> >              revs = [srcrev]
> > @@ -846,5 +844,5 @@ class basefilectx(object):
> >                  return lkr
> >          # fallback to walk through the changelog
> > -        iteranc = cl.ancestors(revs, lkr, inclusive=inclusive)
> > +        iteranc = cl.ancestors(revs, lkr, inclusive=True)
> >          for a in iteranc:
> >              ac = cl.read(a) # get changeset data (we avoid object creation)
> > @@ -875,5 +873,5 @@ class basefilectx(object):
> >              return self.linkrev()
> >          return self._adjustlinkrev(self._path, self._filelog, self._filenode,
> > -                                   self.rev(), inclusive=True)
> > +                                   self.rev())
> >
> >      def _parentfilectx(self, path, fileid, filelog):
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel 
> >
>

Patch

diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -809,5 +809,5 @@  class basefilectx(object):
         return True
 
-    def _adjustlinkrev(self, path, filelog, fnode, srcrev, inclusive=False):
+    def _adjustlinkrev(self, path, filelog, fnode, srcrev):
         """return the first ancestor of <srcrev> introducing <fnode>
 
@@ -821,5 +821,4 @@  class basefilectx(object):
         :filelog: the filelog of this path
         :srcrev: the changeset revision we search ancestors from
-        :inclusive: if true, the src revision will also be checked
         """
         repo = self._repo
@@ -834,5 +833,4 @@  class basefilectx(object):
             # wctx case, used by workingfilectx during mergecopy
             revs = [p.rev() for p in self._repo[None].parents()]
-            inclusive = True # we skipped the real (revless) source
         else:
             revs = [srcrev]
@@ -846,5 +844,5 @@  class basefilectx(object):
                 return lkr
         # fallback to walk through the changelog
-        iteranc = cl.ancestors(revs, lkr, inclusive=inclusive)
+        iteranc = cl.ancestors(revs, lkr, inclusive=True)
         for a in iteranc:
             ac = cl.read(a) # get changeset data (we avoid object creation)
@@ -875,5 +873,5 @@  class basefilectx(object):
             return self.linkrev()
         return self._adjustlinkrev(self._path, self._filelog, self._filenode,
-                                   self.rev(), inclusive=True)
+                                   self.rev())
 
     def _parentfilectx(self, path, fileid, filelog):