[PATCH 08 of 10 V4] context: floor adjustlinkrev graph walk during copy tracing

[Martin von Zweigbergk]

On Thu, Oct 4, 2018 at 7:44 AM Boris Feld <boris.feld at octobus.net> wrote:

> # HG changeset patch
> # User Boris Feld <boris.feld at octobus.net>
> # Date 1536255188 14400
> #      Thu Sep 06 13:33:08 2018 -0400
> # Node ID 53c0bf99c013909bd628aa5254c26d301236ba26
> # Parent  462edcfdf6346b522eee9a64c5c1ca9ff566d7b8
> # EXP-Topic copy-perf
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r
> 53c0bf99c013
> 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.
>

When does this happen? In normal repos or only in broken ones? We don't
seem to have any such repos in our test suite (raising exception instead of
returning None does not fail any tests).


>
> 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.
>

Is that from just this patch or the entire series?


> diff --git a/mercurial/context.py b/mercurial/context.py
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -719,7 +719,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
> @@ -728,6 +728,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
> @@ -755,6 +759,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
> @@ -770,8 +776,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
> @@ -784,7 +794,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:
> @@ -795,11 +813,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()
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20181004/c53a3a8e/attachment.html>