Patchwork changegroup: extract method that sorts nodes to send

login
register
mail settings
Submitter Augie Fackler
Date May 16, 2016, 1:44 p.m.
Message ID <72621a4b75d4fcfd2251.1463406244@imladris.local>
Download mbox | patch
Permalink /patch/15148/
State Accepted
Headers show

Comments

Augie Fackler - May 16, 2016, 1:44 p.m.
# HG changeset patch
# User Augie Fackler <augie@google.com>
# Date 1463106545 14400
#      Thu May 12 22:29:05 2016 -0400
# Node ID 72621a4b75d4fcfd2251a34bbc44913d893d4005
# Parent  983874e22594cebf97f24f7c4d2d28480a90a54f
changegroup: extract method that sorts nodes to send

The current implementation of narrowhg needs to influence the order in
which nodes are sent to the client. adgar@ and I think this is
fixable, but it's going to require pretty substantial time investment,
so in the interim we'd like to extract this method.

I think it makes the group() code a little more obvious, as it took us
a couple of tries to isolate the exact behavior we were observing.
Yuya Nishihara - May 25, 2016, 12:51 p.m.
On Mon, 16 May 2016 09:44:04 -0400, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1463106545 14400
> #      Thu May 12 22:29:05 2016 -0400
> # Node ID 72621a4b75d4fcfd2251a34bbc44913d893d4005
> # Parent  983874e22594cebf97f24f7c4d2d28480a90a54f
> changegroup: extract method that sorts nodes to send

LGTM, pushed to the committed repo, thanks.

Patch

diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
--- a/mercurial/changegroup.py
+++ b/mercurial/changegroup.py
@@ -530,6 +530,17 @@  class cg1packer(object):
     def fileheader(self, fname):
         return chunkheader(len(fname)) + fname
 
+    # Extracted both for clarity and for overriding in extensions.
+    def _sortgroup(self, revlog, nodelist, lookup):
+        """Sort nodes for change group and turn them into revnums."""
+        # for generaldelta revlogs, we linearize the revs; this will both be
+        # much quicker and generate a much smaller bundle
+        if (revlog._generaldelta and self._reorder is None) or self._reorder:
+            dag = dagutil.revlogdag(revlog)
+            return dag.linearize(set(revlog.rev(n) for n in nodelist))
+        else:
+            return sorted([revlog.rev(n) for n in nodelist])
+
     def group(self, nodelist, revlog, lookup, units=None):
         """Calculate a delta group, yielding a sequence of changegroup chunks
         (strings).
@@ -549,14 +560,7 @@  class cg1packer(object):
             yield self.close()
             return
 
-        # for generaldelta revlogs, we linearize the revs; this will both be
-        # much quicker and generate a much smaller bundle
-        if (revlog._generaldelta and self._reorder is None) or self._reorder:
-            dag = dagutil.revlogdag(revlog)
-            revs = set(revlog.rev(n) for n in nodelist)
-            revs = dag.linearize(revs)
-        else:
-            revs = sorted([revlog.rev(n) for n in nodelist])
+        revs = self._sortgroup(revlog, nodelist, lookup)
 
         # add the parent of the first rev
         p = revlog.parentrevs(revs[0])[0]