Patchwork [8,of,8,V6] context: floor adjustlinkrev graph walk during copy tracing

login
register
mail settings
Submitter Boris Feld
Date Nov. 19, 2018, 4:49 p.m.
Message ID <62fe8adca90eeba238fa.1542646187@localhost.localdomain>
Download mbox | patch
Permalink /patch/36656/
State Accepted
Headers show

Comments

Boris Feld - Nov. 19, 2018, 4:49 p.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1539125437 -7200
#      Wed Oct 10 00:50:37 2018 +0200
# Node ID 62fe8adca90eeba238fa313827f4714fed6c34a5
# Parent  3b75faab24d72c1e5689d352ff13b87b9f9faa51
# EXP-Topic copy-perf
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 62fe8adca90e
context: floor adjustlinkrev graph walk during copy tracing

The `_adjustlinkrev` method gains an optional "stoprev" argument. The linkrev
adjustment will give up once this floor is reached. The relevant functions
using `_adjustlinkrev` are updated to pass an appropriate value in the copy
tracing code.

In some private repository, about 10% of the status call triggered the
pathological case addressed by this change. The speedup varies from one call
to another, the best-observed win is moving from 170s to 11s.

The effect of this change can be seen in the public pypy repository, running the
following command:

    hg perftracecopies --source 83c9ff0c0206 --destination 59c79103d5b0

before: 3.401753 seconds
after:  2.634897 seconds (-23%)
via Mercurial-devel - Nov. 23, 2018, 7:15 a.m.
On Mon, Nov 19, 2018 at 8:49 AM Boris Feld <boris.feld@octobus.net> wrote:

> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1539125437 -7200
> #      Wed Oct 10 00:50:37 2018 +0200
> # Node ID 62fe8adca90eeba238fa313827f4714fed6c34a5
> # Parent  3b75faab24d72c1e5689d352ff13b87b9f9faa51
> # EXP-Topic copy-perf
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r
> 62fe8adca90e
> context: floor adjustlinkrev graph walk during copy tracing
>

Queued this, thanks. Thanks for updating the series to make it easier to
follow. I found this version much easier to follow.
Yuya Nishihara - Nov. 24, 2018, 2:53 a.m.
On Mon, 19 Nov 2018 17:49:47 +0100, Boris Feld wrote:
> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1539125437 -7200
> #      Wed Oct 10 00:50:37 2018 +0200
> # Node ID 62fe8adca90eeba238fa313827f4714fed6c34a5
> # Parent  3b75faab24d72c1e5689d352ff13b87b9f9faa51
> # EXP-Topic copy-perf
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 62fe8adca90e
> context: floor adjustlinkrev graph walk during copy tracing

> @@ -750,6 +754,8 @@ class basefilectx(object):
>              fnode = self._filenode
>              path = self._path
>              for a in iteranc:
> +                if stoprev is not None and a < stoprev:
> +                    return None

So, we rely on the fact that iteranc is sorted by revision numbers. Can you
update the docstring of revlog.ancestors? It only says "reverse topological
order", which is weaker constraint.

Patch

diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -714,7 +714,7 @@  class basefilectx(object):
 
         return True
 
-    def _adjustlinkrev(self, srcrev, inclusive=False):
+    def _adjustlinkrev(self, srcrev, inclusive=False, stoprev=None):
         """return the first ancestor of <srcrev> introducing <fnode>
 
         If the linkrev of the file revision does not point to an ancestor of
@@ -723,6 +723,10 @@  class basefilectx(object):
 
         :srcrev: the changeset revision we search ancestors from
         :inclusive: if true, the src revision will also be checked
+        :stoprev: an optional revision to stop the walk at. If no introduction
+                  of this file content could be found before this floor
+                  revision, the function will returns "None" and stops its
+                  iteration.
         """
         repo = self._repo
         cl = repo.unfiltered().changelog
@@ -750,6 +754,8 @@  class basefilectx(object):
             fnode = self._filenode
             path = self._path
             for a in iteranc:
+                if stoprev is not None and a < stoprev:
+                    return None
                 ac = cl.read(a) # get changeset data (we avoid object creation)
                 if path in ac[3]: # checking the 'files' field.
                     # The file has been touched, check if the content is
@@ -767,7 +773,9 @@  class basefilectx(object):
         """
         if self.linkrev() >= changelogrev:
             return True
-        introrev = self._introrev()
+        introrev = self._introrev(stoprev=changelogrev)
+        if introrev is None:
+            return False
         return introrev >= changelogrev
 
     def introrev(self):
@@ -781,7 +789,15 @@  class basefilectx(object):
         """
         return self._introrev()
 
-    def _introrev(self):
+    def _introrev(self, stoprev=None):
+        """
+        Same as `introrev` but, with an extra argument to limit changelog
+        iteration range in some internal usecase.
+
+        If `stoprev` is set, the `introrev` will not be searched past that
+        `stoprev` revision and "None" might be returned. This is useful to
+        limit the iteration range.
+        """
         toprev = None
         attrs = vars(self)
         if r'_changeid' in attrs:
@@ -792,11 +808,12 @@  class basefilectx(object):
             toprev = self._changectx.rev()
 
         if toprev is not None:
-            return self._adjustlinkrev(toprev, inclusive=True)
+            return self._adjustlinkrev(toprev, inclusive=True, stoprev=stoprev)
         elif r'_descendantrev' in attrs:
-            introrev = self._adjustlinkrev(self._descendantrev)
+            introrev = self._adjustlinkrev(self._descendantrev, stoprev=stoprev)
             # be nice and cache the result of the computation
-            self._changeid = introrev
+            if introrev is not None:
+                self._changeid = introrev
             return introrev
         else:
             return self.linkrev()