Patchwork [8,of,9] bundle-ng: simplify lookup and state handling

login
register
mail settings
Submitter Sune Foldager
Date Feb. 14, 2013, 11:07 p.m.
Message ID <67f558991d99dca49cd5.1360883246@firefly.edlund.dk>
Download mbox | patch
Permalink /patch/1000/
State Superseded, archived
Commit a67e1380dfbd99a240e1d8b57e95d879248346a6
Headers show

Comments

Sune Foldager - Feb. 14, 2013, 11:07 p.m.
# HG changeset patch
# User Benoit Boissinot <benoit.boissinot@ens-lyon.org>
# Date 1360511730 -3600
# Node ID 67f558991d99dca49cd51485b0e9f96b70dc7301
# Parent  45c57eaf3b41b1493169ffe474926b793bb12db9
bundle-ng: simplify lookup and state handling
Pierre-Yves David - March 12, 2013, 4:55 p.m.
On Fri, Feb 15, 2013 at 12:07:26AM +0100, Sune Foldager wrote:
> # HG changeset patch
> # User Benoit Boissinot <benoit.boissinot@ens-lyon.org>
> # Date 1360511730 -3600
> # Node ID 67f558991d99dca49cd51485b0e9f96b70dc7301
> # Parent  45c57eaf3b41b1493169ffe474926b793bb12db9
> bundle-ng: simplify lookup and state handling

Please elaborate too.

> 
> diff -r 45c57eaf3b41 -r 67f558991d99 mercurial/changegroup.py
> --- a/mercurial/changegroup.py	Sun Feb 10 16:23:10 2013 +0100
> +++ b/mercurial/changegroup.py	Sun Feb 10 16:55:30 2013 +0100
> @@ -242,14 +242,13 @@
>              reorder = False
>          self._repo = repo
>          self._reorder = reorder
> -        self.count = [0, 0]
>      def close(self):
>          return closechunk()
>  
>      def fileheader(self, fname):
>          return chunkheader(len(fname)) + fname
>  
> -    def group(self, nodelist, revlog, reorder=None):
> +    def group(self, nodelist, revlog, lookup, reorder=None):
>          """Calculate a delta group, yielding a sequence of changegroup chunks
>          (strings).
>  
> @@ -282,7 +281,8 @@
>          # build deltas
>          for r in xrange(len(revs) - 1):
>              prev, curr = revs[r], revs[r + 1]
> -            for c in self.revchunk(revlog, curr, prev):
> +            linknode = lookup(revlog.node(curr))
> +            for c in self.revchunk(revlog, curr, prev, linknode):
>                  yield c
>  
>          yield self.close()
> @@ -294,7 +294,7 @@
>          mf = self._manifest
>          reorder = self._reorder
>          progress = repo.ui.progress
> -        count = self.count
> +        count = [0, 0]

What this count variable is for?

>          _bundling = _('bundling')
>          _changesets = _('changesets')
>          _manifests = _('manifests')
> @@ -303,7 +303,6 @@
>          mfs = {} # needed manifests
>          fnodes = {} # needed file nodes
>          changedfiles = set()
> -        fstate = ['', {}]
>  
>          # filter any nodes that claim to be part of the known set
>          def prune(revlog, missing):
> @@ -311,35 +310,29 @@
>              return [n for n in missing
>                      if rl(rr(n)) not in commonrevs]
>  
> -        def lookup(revlog, x):
> -            if revlog == cl:
> -                c = cl.read(x)
> -                changedfiles.update(c[3])
> -                mfs.setdefault(c[0], x)
> -                count[0] += 1
> -                progress(_bundling, count[0],
> -                         unit=_changesets, total=count[1])
> -                return x
> -            elif revlog == mf:
> -                clnode = mfs[x]
> -                if not fastpathlinkrev:
> -                    mdata = mf.readfast(x)
> -                    for f, n in mdata.iteritems():
> -                        if f in changedfiles:
> -                            fnodes[f].setdefault(n, clnode)
> -                count[0] += 1
> -                progress(_bundling, count[0],
> -                         unit=_manifests, total=count[1])
> -                return clnode
> -            else:
> -                progress(_bundling, count[0], item=fstate[0],
> -                         unit=_files, total=count[1])
> -                return fstate[1][x]
> +        def lookupcl(x):
> +            c = cl.read(x)
> +            changedfiles.update(c[3])
> +            mfs.setdefault(c[0], x)
> +            count[0] += 1
> +            progress(_bundling, count[0],
> +                     unit=_changesets, total=count[1])
> +            return x

Can you take this series as an occasion to document the goal of those small
helper?

>  
> -        self._lookup = lookup
> +        def lookupmf(x):
> +            clnode = mfs[x]
> +            if not fastpathlinkrev:
> +                mdata = mf.readfast(x)
> +                for f, n in mdata.iteritems():
> +                    if f in changedfiles:
> +                        fnodes[f].setdefault(n, clnode)
> +            count[0] += 1
> +            progress(_bundling, count[0],
> +                     unit=_manifests, total=count[1])
> +            return clnode

Can you take this series as an occasion to document the goal of those small
helper ?

Patch

diff -r 45c57eaf3b41 -r 67f558991d99 mercurial/changegroup.py
--- a/mercurial/changegroup.py	Sun Feb 10 16:23:10 2013 +0100
+++ b/mercurial/changegroup.py	Sun Feb 10 16:55:30 2013 +0100
@@ -242,14 +242,13 @@ 
             reorder = False
         self._repo = repo
         self._reorder = reorder
-        self.count = [0, 0]
     def close(self):
         return closechunk()
 
     def fileheader(self, fname):
         return chunkheader(len(fname)) + fname
 
-    def group(self, nodelist, revlog, reorder=None):
+    def group(self, nodelist, revlog, lookup, reorder=None):
         """Calculate a delta group, yielding a sequence of changegroup chunks
         (strings).
 
@@ -282,7 +281,8 @@ 
         # build deltas
         for r in xrange(len(revs) - 1):
             prev, curr = revs[r], revs[r + 1]
-            for c in self.revchunk(revlog, curr, prev):
+            linknode = lookup(revlog.node(curr))
+            for c in self.revchunk(revlog, curr, prev, linknode):
                 yield c
 
         yield self.close()
@@ -294,7 +294,7 @@ 
         mf = self._manifest
         reorder = self._reorder
         progress = repo.ui.progress
-        count = self.count
+        count = [0, 0]
         _bundling = _('bundling')
         _changesets = _('changesets')
         _manifests = _('manifests')
@@ -303,7 +303,6 @@ 
         mfs = {} # needed manifests
         fnodes = {} # needed file nodes
         changedfiles = set()
-        fstate = ['', {}]
 
         # filter any nodes that claim to be part of the known set
         def prune(revlog, missing):
@@ -311,35 +310,29 @@ 
             return [n for n in missing
                     if rl(rr(n)) not in commonrevs]
 
-        def lookup(revlog, x):
-            if revlog == cl:
-                c = cl.read(x)
-                changedfiles.update(c[3])
-                mfs.setdefault(c[0], x)
-                count[0] += 1
-                progress(_bundling, count[0],
-                         unit=_changesets, total=count[1])
-                return x
-            elif revlog == mf:
-                clnode = mfs[x]
-                if not fastpathlinkrev:
-                    mdata = mf.readfast(x)
-                    for f, n in mdata.iteritems():
-                        if f in changedfiles:
-                            fnodes[f].setdefault(n, clnode)
-                count[0] += 1
-                progress(_bundling, count[0],
-                         unit=_manifests, total=count[1])
-                return clnode
-            else:
-                progress(_bundling, count[0], item=fstate[0],
-                         unit=_files, total=count[1])
-                return fstate[1][x]
+        def lookupcl(x):
+            c = cl.read(x)
+            changedfiles.update(c[3])
+            mfs.setdefault(c[0], x)
+            count[0] += 1
+            progress(_bundling, count[0],
+                     unit=_changesets, total=count[1])
+            return x
 
-        self._lookup = lookup
+        def lookupmf(x):
+            clnode = mfs[x]
+            if not fastpathlinkrev:
+                mdata = mf.readfast(x)
+                for f, n in mdata.iteritems():
+                    if f in changedfiles:
+                        fnodes[f].setdefault(n, clnode)
+            count[0] += 1
+            progress(_bundling, count[0],
+                     unit=_manifests, total=count[1])
+            return clnode
 
         count[:] = [0, len(clnodes)]
-        for chunk in self.group(clnodes, cl, reorder=reorder):
+        for chunk in self.group(clnodes, cl, lookupcl, reorder=reorder):
             yield chunk
         progress(_bundling, None)
 
@@ -347,7 +340,7 @@ 
             fnodes[f] = {}
         count[:] = [0, len(mfs)]
         mfnodes = prune(mf, mfs)
-        for chunk in self.group(mfnodes, mf, reorder=reorder):
+        for chunk in self.group(mfnodes, mf, lookupmf, reorder=reorder):
             yield chunk
         progress(_bundling, None)
 
@@ -367,13 +360,20 @@ 
                         if linkrev not in commonrevs:
                             yield filerevlog.node(r), cl.node(linkrev)
                 fnodes[fname] = dict(genfilenodes())
-            fstate[0] = fname
-            fstate[1] = fnodes.pop(fname, {})
-            filenodes = prune(filerevlog, fstate[1])
+
+            linkrevnodes = fnodes.pop(fname, {})
+            # Lookup for filenodes.
+            def lookupfilelog(x):
+                progress(_bundling, count[0], item=fname,
+                         unit=_files, total=count[1])
+                return linkrevnodes[x]
+
+            filenodes = prune(filerevlog, linkrevnodes)
             if filenodes:
                 count[0] += 1
                 yield self.fileheader(fname)
-                for chunk in self.group(filenodes, filerevlog, reorder):
+                for chunk in self.group(filenodes, filerevlog, lookupfilelog,
+                                        reorder):
                     yield chunk
         yield self.close()
         progress(_bundling, None)
@@ -381,7 +381,7 @@ 
         if clnodes:
             repo.hook('outgoing', node=hex(clnodes[0]), source=source)
 
-    def revchunk(self, revlog, rev, prev):
+    def revchunk(self, revlog, rev, prev, linknode):
         node = revlog.node(rev)
         p1, p2 = revlog.parentrevs(rev)
         base = prev
@@ -392,7 +392,6 @@ 
             prefix = mdiff.trivialdiffheader(len(delta))
         else:
             delta = revlog.revdiff(base, rev)
-        linknode = self._lookup(revlog, node)
         p1n, p2n = revlog.parents(node)
         basenode = revlog.node(base)
         meta = self.builddeltaheader(node, p1n, p2n, basenode, linknode)