Patchwork [5,of,5,V5] context: floor adjustlinkrev graph walk during copy tracing

login
register
mail settings
Submitter Boris Feld
Date Oct. 10, 2018, 8:41 a.m.
Message ID <4a2a673868756ca9495a.1539160909@localhost.localdomain>
Download mbox | patch
Permalink /patch/35598/
State New
Headers show

Comments

Boris Feld - Oct. 10, 2018, 8:41 a.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1539125437 -7200
#      Wed Oct 10 00:50:37 2018 +0200
# Node ID 4a2a673868756ca9495a54b7686c19e22b6b21ab
# Parent  39593cc2e82294f379e9d7cfd0f9de7eca43e808
# EXP-Topic copy-perf
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 4a2a67386875
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.
Boris Feld - Oct. 18, 2018, 10:58 a.m.
There have been a bit a bit too much churn on this series and we would
like to re-validate the performance number. We won't have time before
the freeze so you can drop this series.

On 10/10/2018 10:41, 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 4a2a673868756ca9495a54b7686c19e22b6b21ab
> # Parent  39593cc2e82294f379e9d7cfd0f9de7eca43e808
> # EXP-Topic copy-perf
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 4a2a67386875
> 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.
>
> 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
> @@ -765,8 +771,12 @@ class basefilectx(object):
>      def isintroducedafter(self, changelogrev):
>          """True if a filectx have been introduced after a given floor revision
>          """
> -        return (self.linkrev() >= changelogrev
> -                or self._introrev() >= changelogrev)
> +        if self.linkrev() >= changelogrev:
> +            return True
> +        introrev = self._introrev(stoprev=changelogrev)
> +        if introrev is None:
> +            return False
> +        return introrev >= changelogrev
>  
>      def introrev(self):
>          """return the rev of the changeset which introduced this file revision
> @@ -779,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:
> @@ -790,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()
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> 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
@@ -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
@@ -765,8 +771,12 @@  class basefilectx(object):
     def isintroducedafter(self, changelogrev):
         """True if a filectx have been introduced after a given floor revision
         """
-        return (self.linkrev() >= changelogrev
-                or self._introrev() >= changelogrev)
+        if self.linkrev() >= changelogrev:
+            return True
+        introrev = self._introrev(stoprev=changelogrev)
+        if introrev is None:
+            return False
+        return introrev >= changelogrev
 
     def introrev(self):
         """return the rev of the changeset which introduced this file revision
@@ -779,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:
@@ -790,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()