Patchwork [06,of,10,V4] context: introduce an `isintroducedafter` method and use it in copies

login
register
mail settings
Submitter Boris Feld
Date Oct. 4, 2018, 2:44 p.m.
Message ID <1ef29e4089dde437729f.1538664280@localhost.localdomain>
Download mbox | patch
Permalink /patch/35454/
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 1536252767 14400
#      Thu Sep 06 12:52:47 2018 -0400
# Node ID 1ef29e4089dde437729f46bb6b8acb451e11a7f5
# Parent  36a3f6fc8dbd727e1369fb29c5bda044a9b44754
# EXP-Topic copy-perf
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 1ef29e4089dd
context: introduce an `isintroducedafter` method and use it in copies

Right now, copy tracing make effort to not traverse the graph too much to save
performance. It uses a "limit" acting as a floor revision past which data are
no longer relevant to the current copy tracing.

However, to enforce this limit, it uses a call to `filectx.rev()`, that call
can trigger a graph traversal on its own. That extra graph traversal is
unaware of the current limit and can become very expensive. That cost is
increased by the nature of work done in adjust link rev, we are not only
walking down the graph, we are also checking the affected file for each
revision we walk through. Something significantly more expensive than the walk
itself.

To work around this we need to make the `filectx` operation aware of the
current limit. The first step is to introduce a dedicated method:
`isintroducedafter`. We'll then rework that method logic to stop traversal as
soon as possible.
via Mercurial-devel - Oct. 4, 2018, 5:24 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 1536252767 14400
> #      Thu Sep 06 12:52:47 2018 -0400
> # Node ID 1ef29e4089dde437729f46bb6b8acb451e11a7f5
> # Parent  36a3f6fc8dbd727e1369fb29c5bda044a9b44754
> # EXP-Topic copy-perf
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r
> 1ef29e4089dd
> context: introduce an `isintroducedafter` method and use it in copies
>
> Right now, copy tracing make effort to not traverse the graph too much to
> save
> performance. It uses a "limit" acting as a floor revision past which data
> are
> no longer relevant to the current copy tracing.
>
> However, to enforce this limit, it uses a call to `filectx.rev()`, that
> call
> can trigger a graph traversal on its own. That extra graph traversal is
> unaware of the current limit and can become very expensive. That cost is
> increased by the nature of work done in adjust link rev, we are not only
> walking down the graph, we are also checking the affected file for each
> revision we walk through. Something significantly more expensive than the
> walk
> itself.
>
> To work around this we need to make the `filectx` operation aware of the
> current limit. The first step is to introduce a dedicated method:
> `isintroducedafter`. We'll then rework that method logic to stop traversal
> as
> soon as possible.
>
> diff --git a/mercurial/context.py b/mercurial/context.py
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -767,6 +767,12 @@ class basefilectx(object):
>              # result is crash somewhere else at to some point.
>          return lkr
>
> +    def isintroducedafter(self, changelogrev):
> +        """True if a filectx have been introduced after a given floor
> revision
> +        """
> +        return (self.linkrev() > changelogrev
> +                or self.introrev() > changelogrev)
>

test-mv-cp-st-diff.t and test-mq-merge.t fail with this patch. I assume
it's because .rev() in copies.py changed to .introrev() here, but I haven't
confirmed that.


> +
>      def introrev(self):
>          """return the rev of the changeset which introduced this file
> revision
>
> diff --git a/mercurial/copies.py b/mercurial/copies.py
> --- a/mercurial/copies.py
> +++ b/mercurial/copies.py
> @@ -139,7 +139,7 @@ def _tracefile(fctx, am, limit=-1):
>      for f in fctx.ancestors():
>          if am.get(f.path(), None) == f.filenode():
>              return f
> -        if limit >= 0 and f.linkrev() < limit and f.rev() < limit:
> +        if limit >= 0 and not f.isintroducedafter(limit):
>              return None
>
>  def _dirstatecopies(d, match=None):
>
Boris Feld - Oct. 10, 2018, 8:43 a.m.
On 04/10/2018 19:24, Martin von Zweigbergk via Mercurial-devel wrote:
>
>
> On Thu, Oct 4, 2018 at 7:44 AM Boris Feld <boris.feld@octobus.net
> <mailto:boris.feld@octobus.net>> wrote:
>
>     # HG changeset patch
>     # User Boris Feld <boris.feld@octobus.net
>     <mailto:boris.feld@octobus.net>>
>     # Date 1536252767 14400
>     #      Thu Sep 06 12:52:47 2018 -0400
>     # Node ID 1ef29e4089dde437729f46bb6b8acb451e11a7f5
>     # Parent  36a3f6fc8dbd727e1369fb29c5bda044a9b44754
>     # EXP-Topic copy-perf
>     # Available At https://bitbucket.org/octobus/mercurial-devel/
>     #              hg pull
>     https://bitbucket.org/octobus/mercurial-devel/ -r 1ef29e4089dd
>     context: introduce an `isintroducedafter` method and use it in copies
>
>     Right now, copy tracing make effort to not traverse the graph too
>     much to save
>     performance. It uses a "limit" acting as a floor revision past
>     which data are
>     no longer relevant to the current copy tracing.
>
>     However, to enforce this limit, it uses a call to `filectx.rev()`,
>     that call
>     can trigger a graph traversal on its own. That extra graph
>     traversal is
>     unaware of the current limit and can become very expensive. That
>     cost is
>     increased by the nature of work done in adjust link rev, we are
>     not only
>     walking down the graph, we are also checking the affected file for
>     each
>     revision we walk through. Something significantly more expensive
>     than the walk
>     itself.
>
>     To work around this we need to make the `filectx` operation aware
>     of the
>     current limit. The first step is to introduce a dedicated method:
>     `isintroducedafter`. We'll then rework that method logic to stop
>     traversal as
>     soon as possible.
>
>     diff --git a/mercurial/context.py b/mercurial/context.py
>     --- a/mercurial/context.py
>     +++ b/mercurial/context.py
>     @@ -767,6 +767,12 @@ class basefilectx(object):
>                  # result is crash somewhere else at to some point.
>              return lkr
>
>     +    def isintroducedafter(self, changelogrev):
>     +        """True if a filectx have been introduced after a given
>     floor revision
>     +        """
>     +        return (self.linkrev() > changelogrev
>     +                or self.introrev() > changelogrev)
>
>
> test-mv-cp-st-diff.t and test-mq-merge.t fail with this patch. I
> assume it's because .rev() in copies.py changed to .introrev() here,
> but I haven't confirmed that.
We have sent a V5 series that fixes the tests, it was caused by the
condition that was `>` instead of `>=`
>  
>
>     +
>          def introrev(self):
>              """return the rev of the changeset which introduced this
>     file revision
>
>     diff --git a/mercurial/copies.py b/mercurial/copies.py
>     --- a/mercurial/copies.py
>     +++ b/mercurial/copies.py
>     @@ -139,7 +139,7 @@ def _tracefile(fctx, am, limit=-1):
>          for f in fctx.ancestors():
>              if am.get(f.path(), None) == f.filenode():
>                  return f
>     -        if limit >= 0 and f.linkrev() < limit and f.rev() < limit:
>     +        if limit >= 0 and not f.isintroducedafter(limit):
>                  return None
>
>      def _dirstatecopies(d, match=None):
>
>
> _______________________________________________
> 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
@@ -767,6 +767,12 @@  class basefilectx(object):
             # result is crash somewhere else at to some point.
         return lkr
 
+    def isintroducedafter(self, changelogrev):
+        """True if a filectx have been introduced after a given floor revision
+        """
+        return (self.linkrev() > changelogrev
+                or self.introrev() > changelogrev)
+
     def introrev(self):
         """return the rev of the changeset which introduced this file revision
 
diff --git a/mercurial/copies.py b/mercurial/copies.py
--- a/mercurial/copies.py
+++ b/mercurial/copies.py
@@ -139,7 +139,7 @@  def _tracefile(fctx, am, limit=-1):
     for f in fctx.ancestors():
         if am.get(f.path(), None) == f.filenode():
             return f
-        if limit >= 0 and f.linkrev() < limit and f.rev() < limit:
+        if limit >= 0 and not f.isintroducedafter(limit):
             return None
 
 def _dirstatecopies(d, match=None):