Patchwork [8,of,8] changegroup: drop special-casing of flat manifests

login
register
mail settings
Submitter Martin von Zweigbergk
Date Feb. 23, 2016, 7:52 p.m.
Message ID <20697fa8d3d6ad8326fc.1456257153@martinvonz.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/13331/
State Accepted
Headers show

Comments

Martin von Zweigbergk - Feb. 23, 2016, 7:52 p.m.
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@google.com>
# Date 1456180994 28800
#      Mon Feb 22 14:43:14 2016 -0800
# Node ID 20697fa8d3d6ad8326fc3b1eb47da8bb61570f8a
# Parent  1ab4ec0c1cc67b33b069e41668bcf57215978df9
changegroup: drop special-casing of flat manifests

Since c08814b48ae5 (changegroup: avoid iterating the whole manifest,
2015-12-04), the manifest linkrev callback iterates over only the
files that were touched according the the changeset. Before that
change, we iterated over all files returned in
manifest.readfast(). That method returns the files in the delta, if
the delta parent is a parent, otherwise it returns the full
manifest. Most manifest revisions end up using one of the parents as
its delta parent, so most of the time, the method returns a short
manifest. It seems that that happens often enough that it doesn't
really matter; I could not reproduce the timings reported in that
change.

Since the treemanifest code now works quite differently, and since
that code also works correctly for flat manifests, let's drop the
special-casing of flat manifests.
Augie Fackler - Feb. 25, 2016, 5:55 p.m.
On Tue, Feb 23, 2016 at 11:52:33AM -0800, Martin von Zweigbergk wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1456180994 28800
> #      Mon Feb 22 14:43:14 2016 -0800
> # Node ID 20697fa8d3d6ad8326fc3b1eb47da8bb61570f8a
> # Parent  1ab4ec0c1cc67b33b069e41668bcf57215978df9
> changegroup: drop special-casing of flat manifests

Queued these two as well. Many thanks for cleaning up my mess in this code.

>
> Since c08814b48ae5 (changegroup: avoid iterating the whole manifest,
> 2015-12-04), the manifest linkrev callback iterates over only the
> files that were touched according the the changeset. Before that
> change, we iterated over all files returned in
> manifest.readfast(). That method returns the files in the delta, if
> the delta parent is a parent, otherwise it returns the full
> manifest. Most manifest revisions end up using one of the parents as
> its delta parent, so most of the time, the method returns a short
> manifest. It seems that that happens often enough that it doesn't
> really matter; I could not reproduce the timings reported in that
> change.
>
> Since the treemanifest code now works quite differently, and since
> that code also works correctly for flat manifests, let's drop the
> special-casing of flat manifests.
>
> diff -r 1ab4ec0c1cc6 -r 20697fa8d3d6 mercurial/changegroup.py
> --- a/mercurial/changegroup.py	Fri Feb 12 23:09:09 2016 -0800
> +++ b/mercurial/changegroup.py	Mon Feb 22 14:43:14 2016 -0800
> @@ -656,8 +656,7 @@
>          clrevorder = {}
>          mfs = {} # needed manifests
>          fnodes = {} # needed file nodes
> -        # maps manifest node id -> set(changed files)
> -        mfchangedfiles = {}
> +        changedfiles = set()
>
>          # Callback for the changelog, used to collect changed files and manifest
>          # nodes.
> @@ -670,7 +669,7 @@
>              mfs.setdefault(n, x)
>              # Record a complete list of potentially-changed files in
>              # this manifest.
> -            mfchangedfiles.setdefault(n, set()).update(c[3])
> +            changedfiles.update(c[3])
>              return x
>
>          self._verbosenote(_('uncompressed size of bundle content:\n'))
> @@ -703,7 +702,7 @@
>              'treemanifest' not in repo.requirements)
>
>          for chunk in self.generatemanifests(commonrevs, clrevorder,
> -                fastpathlinkrev, mfs, mfchangedfiles, fnodes):
> +                fastpathlinkrev, mfs, fnodes):
>              yield chunk
>          mfs.clear()
>          clrevs = set(cl.rev(x) for x in clnodes)
> @@ -719,9 +718,6 @@
>                  revs = ((r, llr(r)) for r in filerevlog)
>                  return dict((fln(r), cln(lr)) for r, lr in revs if lr in clrevs)
>
> -        changedfiles = set()
> -        for x in mfchangedfiles.itervalues():
> -            changedfiles.update(x)
>          for chunk in self.generatefiles(changedfiles, linknodes, commonrevs,
>                                          source):
>              yield chunk
> @@ -732,7 +728,7 @@
>              repo.hook('outgoing', node=hex(clnodes[0]), source=source)
>
>      def generatemanifests(self, commonrevs, clrevorder, fastpathlinkrev, mfs,
> -                          mfchangedfiles, fnodes):
> +                          fnodes):
>          repo = self._repo
>          dirlog = repo.manifest.dirlog
>          tmfnodes = {'': mfs}
> @@ -763,28 +759,15 @@
>                  """
>                  clnode = tmfnodes[dir][x]
>                  mdata = dirlog(dir).readshallowfast(x)
> -                if 'treemanifest' in repo.requirements:
> -                    for p, n, fl in mdata.iterentries():
> -                        if fl == 't': # subdirectory manifest
> -                            subdir = dir + p + '/'
> -                            tmfclnodes = tmfnodes.setdefault(subdir, {})
> -                            tmfclnode = tmfclnodes.setdefault(n, clnode)
> -                            if clrevorder[clnode] < clrevorder[tmfclnode]:
> -                                tmfclnodes[n] = clnode
> -                        else:
> -                            f = dir + p
> -                            fclnodes = fnodes.setdefault(f, {})
> -                            fclnode = fclnodes.setdefault(n, clnode)
> -                            if clrevorder[clnode] < clrevorder[fclnode]:
> -                                fclnodes[n] = clnode
> -                else:
> -                    for f in mfchangedfiles[x]:
> -                        try:
> -                            n = mdata[f]
> -                        except KeyError:
> -                            continue
> -                        # record the first changeset introducing this filelog
> -                        # version
> +                for p, n, fl in mdata.iterentries():
> +                    if fl == 't': # subdirectory manifest
> +                        subdir = dir + p + '/'
> +                        tmfclnodes = tmfnodes.setdefault(subdir, {})
> +                        tmfclnode = tmfclnodes.setdefault(n, clnode)
> +                        if clrevorder[clnode] < clrevorder[tmfclnode]:
> +                            tmfclnodes[n] = clnode
> +                    else:
> +                        f = dir + p
>                          fclnodes = fnodes.setdefault(f, {})
>                          fclnode = fclnodes.setdefault(n, clnode)
>                          if clrevorder[clnode] < clrevorder[fclnode]:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff -r 1ab4ec0c1cc6 -r 20697fa8d3d6 mercurial/changegroup.py
--- a/mercurial/changegroup.py	Fri Feb 12 23:09:09 2016 -0800
+++ b/mercurial/changegroup.py	Mon Feb 22 14:43:14 2016 -0800
@@ -656,8 +656,7 @@ 
         clrevorder = {}
         mfs = {} # needed manifests
         fnodes = {} # needed file nodes
-        # maps manifest node id -> set(changed files)
-        mfchangedfiles = {}
+        changedfiles = set()
 
         # Callback for the changelog, used to collect changed files and manifest
         # nodes.
@@ -670,7 +669,7 @@ 
             mfs.setdefault(n, x)
             # Record a complete list of potentially-changed files in
             # this manifest.
-            mfchangedfiles.setdefault(n, set()).update(c[3])
+            changedfiles.update(c[3])
             return x
 
         self._verbosenote(_('uncompressed size of bundle content:\n'))
@@ -703,7 +702,7 @@ 
             'treemanifest' not in repo.requirements)
 
         for chunk in self.generatemanifests(commonrevs, clrevorder,
-                fastpathlinkrev, mfs, mfchangedfiles, fnodes):
+                fastpathlinkrev, mfs, fnodes):
             yield chunk
         mfs.clear()
         clrevs = set(cl.rev(x) for x in clnodes)
@@ -719,9 +718,6 @@ 
                 revs = ((r, llr(r)) for r in filerevlog)
                 return dict((fln(r), cln(lr)) for r, lr in revs if lr in clrevs)
 
-        changedfiles = set()
-        for x in mfchangedfiles.itervalues():
-            changedfiles.update(x)
         for chunk in self.generatefiles(changedfiles, linknodes, commonrevs,
                                         source):
             yield chunk
@@ -732,7 +728,7 @@ 
             repo.hook('outgoing', node=hex(clnodes[0]), source=source)
 
     def generatemanifests(self, commonrevs, clrevorder, fastpathlinkrev, mfs,
-                          mfchangedfiles, fnodes):
+                          fnodes):
         repo = self._repo
         dirlog = repo.manifest.dirlog
         tmfnodes = {'': mfs}
@@ -763,28 +759,15 @@ 
                 """
                 clnode = tmfnodes[dir][x]
                 mdata = dirlog(dir).readshallowfast(x)
-                if 'treemanifest' in repo.requirements:
-                    for p, n, fl in mdata.iterentries():
-                        if fl == 't': # subdirectory manifest
-                            subdir = dir + p + '/'
-                            tmfclnodes = tmfnodes.setdefault(subdir, {})
-                            tmfclnode = tmfclnodes.setdefault(n, clnode)
-                            if clrevorder[clnode] < clrevorder[tmfclnode]:
-                                tmfclnodes[n] = clnode
-                        else:
-                            f = dir + p
-                            fclnodes = fnodes.setdefault(f, {})
-                            fclnode = fclnodes.setdefault(n, clnode)
-                            if clrevorder[clnode] < clrevorder[fclnode]:
-                                fclnodes[n] = clnode
-                else:
-                    for f in mfchangedfiles[x]:
-                        try:
-                            n = mdata[f]
-                        except KeyError:
-                            continue
-                        # record the first changeset introducing this filelog
-                        # version
+                for p, n, fl in mdata.iterentries():
+                    if fl == 't': # subdirectory manifest
+                        subdir = dir + p + '/'
+                        tmfclnodes = tmfnodes.setdefault(subdir, {})
+                        tmfclnode = tmfclnodes.setdefault(n, clnode)
+                        if clrevorder[clnode] < clrevorder[tmfclnode]:
+                            tmfclnodes[n] = clnode
+                    else:
+                        f = dir + p
                         fclnodes = fnodes.setdefault(f, {})
                         fclnode = fclnodes.setdefault(n, clnode)
                         if clrevorder[clnode] < clrevorder[fclnode]: