Patchwork [4,of,4,V2] changegroup: refactor changegroup generation into a separate class

login
register
mail settings
Submitter Durham Goode
Date May 7, 2013, 3:14 a.m.
Message ID <c86e8d104d87bf958827.1367896490@dev350.prn1.facebook.com>
Download mbox | patch
Permalink /patch/1581/
State Changes Requested, archived
Delegated to: Augie Fackler
Headers show

Comments

Durham Goode - May 7, 2013, 3:14 a.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1367430936 25200
#      Wed May 01 10:55:36 2013 -0700
# Node ID c86e8d104d87bf958827d9713d4e2f22f7535117
# Parent  dbdfdfcc9b14728cee940dd9f96398561bed0802
changegroup: refactor changegroup generation into a separate class

Previously changegroup generation was confined to _changegroup() and
_changegroupsubset(). Both were massive functions and they had lots of
duplicate code.

I've moved all their logic into the changegroup module as new classes:
ChangeGroupGen and SubsetChangeGroupGen. This breaks up all the different
logic into separate functions so extensions can modify them individually
and allows us to share most of the logic with the subset version. This
also helps make localrepo less of a god class (-156 lines from localrepo,
+157 lines to changegroup).

There should be no behavior changes introduced by this.
Augie Fackler - May 7, 2013, 5:15 p.m.
On Mon, May 06, 2013 at 08:14:50PM -0700, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1367430936 25200
> #      Wed May 01 10:55:36 2013 -0700
> # Node ID c86e8d104d87bf958827d9713d4e2f22f7535117
> # Parent  dbdfdfcc9b14728cee940dd9f96398561bed0802
> changegroup: refactor changegroup generation into a separate class

Series LGTM, anyone else want a look?

>
> Previously changegroup generation was confined to _changegroup() and
> _changegroupsubset(). Both were massive functions and they had lots of
> duplicate code.
>
> I've moved all their logic into the changegroup module as new classes:
> ChangeGroupGen and SubsetChangeGroupGen. This breaks up all the different
> logic into separate functions so extensions can modify them individually
> and allows us to share most of the logic with the subset version. This
> also helps make localrepo less of a god class (-156 lines from localrepo,
> +157 lines to changegroup).

Code removed from localrepo. You get a cookie.

>
> There should be no behavior changes introduced by this.
>
>
> diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
> --- a/mercurial/changegroup.py
> +++ b/mercurial/changegroup.py
> @@ -6,7 +6,7 @@
>  # GNU General Public License version 2 or any later version.
>
>  from i18n import _
> -from node import nullrev
> +from node import nullrev, hex
>  import mdiff, util
>  import struct, os, bz2, zlib, tempfile
>
> @@ -254,3 +254,160 @@
>      def builddeltaheader(self, node, p1n, p2n, basenode, linknode):
>          # do nothing with basenode, it is implicitly the previous one in HG10
>          return struct.pack(self.deltaheader, node, p1n, p2n, linknode)
> +
> +_bundling = _('bundling')
> +_changesets = _('changesets')
> +_manifests = _('manifests')
> +_files = _('files')

I like this, but mpm may not be a fan of this underbar style. Matt?

> +
> +class changegroupgen(object):
> +    def __init__(self, repo, csets, source, reorder):
> +        self.repo = repo
> +        self.cl = repo.changelog
> +        self.mf = repo.manifest
> +        self.csets = csets
> +        self.changedfiles = set()
> +        self.source = source
> +        self.neededmfs = {}
> +        self.reorder = reorder
> +        self.bundler = bundle10(self.lookup)
> +        self.fstate = ['', {}]
> +        self.count = [0, 0]
> +        self.progress = repo.ui.progress
> +
> +    def gengroup(self):
> +        for chunk in self.addcommitgroups():
> +            yield chunk
> +        for chunk in self.addmanifestgroups():
> +            yield chunk
> +        for chunk in self.addfilegroups():
> +            yield chunk
> +
> +        # Signal that no more groups are left.
> +        yield self.bundler.close()
> +        self.progress(_bundling, None)
> +
> +        if self.csets:
> +            self.repo.hook('outgoing',
> +                           node = hex(self.csets[0]),
> +                           source = self.source)
> +
> +    def addcommitgroups(self):
> +        # Create a changenode group generator that will call our functions
> +        # back to lookup the owning changenode and collect information.
> +        self.count[:] = [0, len(self.csets)]
> +        for chunk in self.cl.group(self.csets,
> +                                   self.bundler,
> +                                   reorder=self.reorder):
> +            yield chunk
> +        self.progress(_bundling, None)
> +
> +    def addmanifestgroups(self):
> +        # Create a generator for the manifestnodes that calls our lookup
> +        # and data collection functions back.
> +        self.count[:] = [0, len(self.neededmfs)]
> +        outgoingmfs = self.outgoingmanifests(self.mf, self.neededmfs)
> +
> +        for chunk in self.mf.group(outgoingmfs, self.bundler,
> +                                   reorder=self.reorder):
> +            yield chunk
> +        self.progress(_bundling, None)
> +
> +    def addfilegroups(self):
> +        # Go through all our files in order sorted by name.
> +        repo = self.repo
> +        fstate = self.fstate
> +        self.count[:] = [0, len(self.changedfiles)]
> +        for fname in sorted(self.changedfiles):
> +            filerevlog = repo.file(fname)
> +
> +            fstate[0] = fname
> +            fstate[1] = self.outgoingfilemap(filerevlog, fname)
> +
> +            nodelist = fstate[1]
> +            if nodelist:
> +                self.count[0] += 1
> +                yield self.bundler.fileheader(fname)
> +                for chunk in filerevlog.group(nodelist,
> +                                              self.bundler,
> +                                              self.reorder):
> +                    yield chunk
> +
> +    def outgoingmanifests(self, manifest, nodes):
> +        # return the manifest nodes that are outgoing
> +        rr, rl = manifest.rev, manifest.linkrev
> +        clnode = self.cl.node
> +        return [n for n in nodes if clnode(rl(rr(n))) in self.csets]
> +
> +    def outgoingfilemap(self, filerevlog, fname):
> +        # map of outgoing file nodes to changelog nodes
> +        mapping = {}
> +        for r in filerevlog:
> +            clnode = self.cl.node(filerevlog.linkrev(r))
> +            if clnode in self.csets:
> +                mapping[filerevlog.node(r)] = clnode
> +        return mapping
> +
> +    def lookup(self, revlog, node):
> +        if revlog == self.cl:
> +            return self.lookupcommit(node)
> +        elif revlog == self.mf:
> +            return self.lookupmanifest(node)
> +        else:
> +            return self.lookupfile(revlog, node)
> +
> +    def lookupcommit(self, node):
> +        c = self.cl.read(node)
> +        self.changedfiles.update(c[3])
> +        self.neededmfs.setdefault(c[0], node)
> +        self.count[0] += 1
> +        self.progress(_bundling, self.count[0],
> +                      unit=_changesets, total=self.count[1])
> +        return node
> +
> +    def lookupmanifest(self, node):
> +        self.count[0] += 1
> +        self.progress(_bundling, self.count[0],
> +                      unit=_manifests, total=self.count[1])
> +
> +        return self.cl.node(self.mf.linkrev(self.mf.rev(node)))
> +
> +    def lookupfile(self, filelog, node):
> +        self.progress(_bundling, self.count[0], item=self.fstate[0],
> +                      unit=_files, total=self.count[1])
> +        return self.fstate[1][node]
> +
> +class subsetchangegroupgen(changegroupgen):
> +    def __init__(self, repo, csets, commonrevs, source, reorder):
> +        super(subsetchangegroupgen, self).__init__(repo, csets, source, reorder)
> +        self.commonrevs = commonrevs
> +        self.fnodes = {}
> +
> +    def lookupmanifest(self, node):
> +        clnode = self.neededmfs[node]
> +        mdata = self.mf.readfast(node)
> +        changedfiles = self.changedfiles
> +        fnodes = self.fnodes
> +        for f, n in mdata.iteritems():
> +            if f in changedfiles:
> +                nodes = fnodes.setdefault(f, {})
> +                nodes.setdefault(n, clnode)
> +
> +        self.count[0] += 1
> +        self.progress(_bundling, self.count[0],
> +                      unit=_manifests, total=self.count[1])
> +        return clnode
> +
> +    def outgoingmanifests(self, manifest, nodes):
> +        rr, rl = manifest.rev, manifest.linkrev
> +        return [n for n in nodes if rl(rr(n)) not in self.commonrevs]
> +
> +    def outgoingfilemap(self, filerevlog, fname):
> +        # map of outgoing file nodes to changelog nodes
> +        mapping = {}
> +        for fnode, clnode in self.fnodes.pop(fname, {}).iteritems():
> +            # filter any nodes that claim to be part of the known set
> +            clrev = filerevlog.linkrev(filerevlog.rev(fnode))
> +            if clrev not in self.commonrevs:
> +                mapping[fnode] = clnode
> +        return mapping
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -2022,15 +2022,6 @@
>
>      @unfilteredmethod
>      def _changegroupsubset(self, commonrevs, csets, heads, source):
> -
> -        cl = self.changelog
> -        mf = self.manifest
> -        mfs = {} # needed manifests
> -        fnodes = {} # needed file nodes
> -        changedfiles = set()
> -        fstate = ['', {}]
> -        count = [0, 0]
> -
>          # can we go through the fast path ?
>          heads.sort()
>          if heads == sorted(self.heads()):
> @@ -2040,93 +2031,16 @@
>          self.hook('preoutgoing', throw=True, source=source)
>          self.changegroupinfo(csets, source)
>
> -        # filter any nodes that claim to be part of the known set
> -        def prune(revlog, missing):
> -            rr, rl = revlog.rev, revlog.linkrev
> -            return [n for n in missing
> -                    if rl(rr(n)) not in commonrevs]
> -
> -        progress = self.ui.progress
> -        _bundling = _('bundling')
> -        _changesets = _('changesets')
> -        _manifests = _('manifests')
> -        _files = _('files')
> -
> -        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]
> -                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]
> -
> -        bundler = changegroup.bundle10(lookup)
>          reorder = self.ui.config('bundle', 'reorder', 'auto')
>          if reorder == 'auto':
>              reorder = None
>          else:
>              reorder = util.parsebool(reorder)
>
> -        def gengroup():
> -            # Create a changenode group generator that will call our functions
> -            # back to lookup the owning changenode and collect information.
> -            count[:] = [0, len(csets)]
> -            for chunk in cl.group(csets, bundler, reorder=reorder):
> -                yield chunk
> -            progress(_bundling, None)
> +        gen = changegroup.subsetchangegroupgen(self, csets, commonrevs,
> +                                               source, reorder)
>
> -            # Create a generator for the manifestnodes that calls our lookup
> -            # and data collection functions back.
> -            for f in changedfiles:
> -                fnodes[f] = {}
> -            count[:] = [0, len(mfs)]
> -            for chunk in mf.group(prune(mf, mfs), bundler, reorder=reorder):
> -                yield chunk
> -            progress(_bundling, None)
> -
> -            mfs.clear()
> -
> -            # Go through all our files in order sorted by name.
> -            count[:] = [0, len(changedfiles)]
> -            for fname in sorted(changedfiles):
> -                filerevlog = self.file(fname)
> -                if not len(filerevlog):
> -                    raise util.Abort(_("empty or missing revlog for %s")
> -                                     % fname)
> -                fstate[0] = fname
> -                fstate[1] = fnodes.pop(fname, {})
> -
> -                nodelist = prune(filerevlog, fstate[1])
> -                if nodelist:
> -                    count[0] += 1
> -                    yield bundler.fileheader(fname)
> -                    for chunk in filerevlog.group(nodelist, bundler, reorder):
> -                        yield chunk
> -
> -            # Signal that no more groups are left.
> -            yield bundler.close()
> -            progress(_bundling, None)
> -
> -            if csets:
> -                self.hook('outgoing', node=hex(csets[0]), source=source)
> -
> -        return changegroup.unbundle10(util.chunkbuffer(gengroup()), 'UN')
> +        return changegroup.unbundle10(util.chunkbuffer(gen.gengroup()), 'UN')
>
>      def changegroup(self, basenodes, source):
>          # to avoid a race we use changegroupsubset() (issue1320)
> @@ -2143,88 +2057,18 @@
>
>          nodes is the set of nodes to send"""
>
> -        cl = self.changelog
> -        mf = self.manifest
> -        mfs = {}
> -        changedfiles = set()
> -        fstate = ['']
> -        count = [0, 0]
> -
>          self.hook('preoutgoing', throw=True, source=source)
>          self.changegroupinfo(nodes, source)
>
> -        revset = set([cl.rev(n) for n in nodes])
> -
> -        def gennodelst(log):
> -            ln, llr = log.node, log.linkrev
> -            return [ln(r) for r in log if llr(r) in revset]
> -
> -        progress = self.ui.progress
> -        _bundling = _('bundling')
> -        _changesets = _('changesets')
> -        _manifests = _('manifests')
> -        _files = _('files')
> -
> -        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:
> -                count[0] += 1
> -                progress(_bundling, count[0],
> -                         unit=_manifests, total=count[1])
> -                return cl.node(revlog.linkrev(revlog.rev(x)))
> -            else:
> -                progress(_bundling, count[0], item=fstate[0],
> -                    total=count[1], unit=_files)
> -                return cl.node(revlog.linkrev(revlog.rev(x)))
> -
> -        bundler = changegroup.bundle10(lookup)
>          reorder = self.ui.config('bundle', 'reorder', 'auto')
>          if reorder == 'auto':
>              reorder = None
>          else:
>              reorder = util.parsebool(reorder)
>
> -        def gengroup():
> -            '''yield a sequence of changegroup chunks (strings)'''
> -            # construct a list of all changed files
> +        gen = changegroup.changegroupgen(self, nodes, source, reorder)
>
> -            count[:] = [0, len(nodes)]
> -            for chunk in cl.group(nodes, bundler, reorder=reorder):
> -                yield chunk
> -            progress(_bundling, None)
> -
> -            count[:] = [0, len(mfs)]
> -            for chunk in mf.group(gennodelst(mf), bundler, reorder=reorder):
> -                yield chunk
> -            progress(_bundling, None)
> -
> -            count[:] = [0, len(changedfiles)]
> -            for fname in sorted(changedfiles):
> -                filerevlog = self.file(fname)
> -                if not len(filerevlog):
> -                    raise util.Abort(_("empty or missing revlog for %s")
> -                                     % fname)
> -                fstate[0] = fname
> -                nodelist = gennodelst(filerevlog)
> -                if nodelist:
> -                    count[0] += 1
> -                    yield bundler.fileheader(fname)
> -                    for chunk in filerevlog.group(nodelist, bundler, reorder):
> -                        yield chunk
> -            yield bundler.close()
> -            progress(_bundling, None)
> -
> -            if nodes:
> -                self.hook('outgoing', node=hex(nodes[0]), source=source)
> -
> -        return changegroup.unbundle10(util.chunkbuffer(gengroup()), 'UN')
> +        return changegroup.unbundle10(util.chunkbuffer(gen.gengroup()), 'UN')
>
>      @unfilteredmethod
>      def addchangegroup(self, source, srctype, url, emptyok=False):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
--- a/mercurial/changegroup.py
+++ b/mercurial/changegroup.py
@@ -6,7 +6,7 @@ 
 # GNU General Public License version 2 or any later version.
 
 from i18n import _
-from node import nullrev
+from node import nullrev, hex
 import mdiff, util
 import struct, os, bz2, zlib, tempfile
 
@@ -254,3 +254,160 @@ 
     def builddeltaheader(self, node, p1n, p2n, basenode, linknode):
         # do nothing with basenode, it is implicitly the previous one in HG10
         return struct.pack(self.deltaheader, node, p1n, p2n, linknode)
+
+_bundling = _('bundling')
+_changesets = _('changesets')
+_manifests = _('manifests')
+_files = _('files')
+
+class changegroupgen(object):
+    def __init__(self, repo, csets, source, reorder):
+        self.repo = repo
+        self.cl = repo.changelog
+        self.mf = repo.manifest
+        self.csets = csets
+        self.changedfiles = set()
+        self.source = source
+        self.neededmfs = {}
+        self.reorder = reorder
+        self.bundler = bundle10(self.lookup)
+        self.fstate = ['', {}]
+        self.count = [0, 0]
+        self.progress = repo.ui.progress
+
+    def gengroup(self):
+        for chunk in self.addcommitgroups():
+            yield chunk
+        for chunk in self.addmanifestgroups():
+            yield chunk
+        for chunk in self.addfilegroups():
+            yield chunk
+
+        # Signal that no more groups are left.
+        yield self.bundler.close()
+        self.progress(_bundling, None)
+
+        if self.csets:
+            self.repo.hook('outgoing',
+                           node = hex(self.csets[0]),
+                           source = self.source)
+
+    def addcommitgroups(self):
+        # Create a changenode group generator that will call our functions
+        # back to lookup the owning changenode and collect information.
+        self.count[:] = [0, len(self.csets)]
+        for chunk in self.cl.group(self.csets,
+                                   self.bundler,
+                                   reorder=self.reorder):
+            yield chunk
+        self.progress(_bundling, None)
+
+    def addmanifestgroups(self):
+        # Create a generator for the manifestnodes that calls our lookup
+        # and data collection functions back.
+        self.count[:] = [0, len(self.neededmfs)]
+        outgoingmfs = self.outgoingmanifests(self.mf, self.neededmfs)
+
+        for chunk in self.mf.group(outgoingmfs, self.bundler,
+                                   reorder=self.reorder):
+            yield chunk
+        self.progress(_bundling, None)
+
+    def addfilegroups(self):
+        # Go through all our files in order sorted by name.
+        repo = self.repo
+        fstate = self.fstate
+        self.count[:] = [0, len(self.changedfiles)]
+        for fname in sorted(self.changedfiles):
+            filerevlog = repo.file(fname)
+
+            fstate[0] = fname
+            fstate[1] = self.outgoingfilemap(filerevlog, fname)
+
+            nodelist = fstate[1]
+            if nodelist:
+                self.count[0] += 1
+                yield self.bundler.fileheader(fname)
+                for chunk in filerevlog.group(nodelist,
+                                              self.bundler,
+                                              self.reorder):
+                    yield chunk
+
+    def outgoingmanifests(self, manifest, nodes):
+        # return the manifest nodes that are outgoing
+        rr, rl = manifest.rev, manifest.linkrev
+        clnode = self.cl.node
+        return [n for n in nodes if clnode(rl(rr(n))) in self.csets]
+
+    def outgoingfilemap(self, filerevlog, fname):
+        # map of outgoing file nodes to changelog nodes
+        mapping = {}
+        for r in filerevlog:
+            clnode = self.cl.node(filerevlog.linkrev(r))
+            if clnode in self.csets:
+                mapping[filerevlog.node(r)] = clnode
+        return mapping
+
+    def lookup(self, revlog, node):
+        if revlog == self.cl:
+            return self.lookupcommit(node)
+        elif revlog == self.mf:
+            return self.lookupmanifest(node)
+        else:
+            return self.lookupfile(revlog, node)
+
+    def lookupcommit(self, node):
+        c = self.cl.read(node)
+        self.changedfiles.update(c[3])
+        self.neededmfs.setdefault(c[0], node)
+        self.count[0] += 1
+        self.progress(_bundling, self.count[0],
+                      unit=_changesets, total=self.count[1])
+        return node
+
+    def lookupmanifest(self, node):
+        self.count[0] += 1
+        self.progress(_bundling, self.count[0],
+                      unit=_manifests, total=self.count[1])
+
+        return self.cl.node(self.mf.linkrev(self.mf.rev(node)))
+
+    def lookupfile(self, filelog, node):
+        self.progress(_bundling, self.count[0], item=self.fstate[0],
+                      unit=_files, total=self.count[1])
+        return self.fstate[1][node]
+
+class subsetchangegroupgen(changegroupgen):
+    def __init__(self, repo, csets, commonrevs, source, reorder):
+        super(subsetchangegroupgen, self).__init__(repo, csets, source, reorder)
+        self.commonrevs = commonrevs
+        self.fnodes = {}
+
+    def lookupmanifest(self, node):
+        clnode = self.neededmfs[node]
+        mdata = self.mf.readfast(node)
+        changedfiles = self.changedfiles
+        fnodes = self.fnodes
+        for f, n in mdata.iteritems():
+            if f in changedfiles:
+                nodes = fnodes.setdefault(f, {})
+                nodes.setdefault(n, clnode)
+
+        self.count[0] += 1
+        self.progress(_bundling, self.count[0],
+                      unit=_manifests, total=self.count[1])
+        return clnode
+
+    def outgoingmanifests(self, manifest, nodes):
+        rr, rl = manifest.rev, manifest.linkrev
+        return [n for n in nodes if rl(rr(n)) not in self.commonrevs]
+
+    def outgoingfilemap(self, filerevlog, fname):
+        # map of outgoing file nodes to changelog nodes
+        mapping = {}
+        for fnode, clnode in self.fnodes.pop(fname, {}).iteritems():
+            # filter any nodes that claim to be part of the known set
+            clrev = filerevlog.linkrev(filerevlog.rev(fnode))
+            if clrev not in self.commonrevs:
+                mapping[fnode] = clnode
+        return mapping
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -2022,15 +2022,6 @@ 
 
     @unfilteredmethod
     def _changegroupsubset(self, commonrevs, csets, heads, source):
-
-        cl = self.changelog
-        mf = self.manifest
-        mfs = {} # needed manifests
-        fnodes = {} # needed file nodes
-        changedfiles = set()
-        fstate = ['', {}]
-        count = [0, 0]
-
         # can we go through the fast path ?
         heads.sort()
         if heads == sorted(self.heads()):
@@ -2040,93 +2031,16 @@ 
         self.hook('preoutgoing', throw=True, source=source)
         self.changegroupinfo(csets, source)
 
-        # filter any nodes that claim to be part of the known set
-        def prune(revlog, missing):
-            rr, rl = revlog.rev, revlog.linkrev
-            return [n for n in missing
-                    if rl(rr(n)) not in commonrevs]
-
-        progress = self.ui.progress
-        _bundling = _('bundling')
-        _changesets = _('changesets')
-        _manifests = _('manifests')
-        _files = _('files')
-
-        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]
-                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]
-
-        bundler = changegroup.bundle10(lookup)
         reorder = self.ui.config('bundle', 'reorder', 'auto')
         if reorder == 'auto':
             reorder = None
         else:
             reorder = util.parsebool(reorder)
 
-        def gengroup():
-            # Create a changenode group generator that will call our functions
-            # back to lookup the owning changenode and collect information.
-            count[:] = [0, len(csets)]
-            for chunk in cl.group(csets, bundler, reorder=reorder):
-                yield chunk
-            progress(_bundling, None)
+        gen = changegroup.subsetchangegroupgen(self, csets, commonrevs,
+                                               source, reorder)
 
-            # Create a generator for the manifestnodes that calls our lookup
-            # and data collection functions back.
-            for f in changedfiles:
-                fnodes[f] = {}
-            count[:] = [0, len(mfs)]
-            for chunk in mf.group(prune(mf, mfs), bundler, reorder=reorder):
-                yield chunk
-            progress(_bundling, None)
-
-            mfs.clear()
-
-            # Go through all our files in order sorted by name.
-            count[:] = [0, len(changedfiles)]
-            for fname in sorted(changedfiles):
-                filerevlog = self.file(fname)
-                if not len(filerevlog):
-                    raise util.Abort(_("empty or missing revlog for %s")
-                                     % fname)
-                fstate[0] = fname
-                fstate[1] = fnodes.pop(fname, {})
-
-                nodelist = prune(filerevlog, fstate[1])
-                if nodelist:
-                    count[0] += 1
-                    yield bundler.fileheader(fname)
-                    for chunk in filerevlog.group(nodelist, bundler, reorder):
-                        yield chunk
-
-            # Signal that no more groups are left.
-            yield bundler.close()
-            progress(_bundling, None)
-
-            if csets:
-                self.hook('outgoing', node=hex(csets[0]), source=source)
-
-        return changegroup.unbundle10(util.chunkbuffer(gengroup()), 'UN')
+        return changegroup.unbundle10(util.chunkbuffer(gen.gengroup()), 'UN')
 
     def changegroup(self, basenodes, source):
         # to avoid a race we use changegroupsubset() (issue1320)
@@ -2143,88 +2057,18 @@ 
 
         nodes is the set of nodes to send"""
 
-        cl = self.changelog
-        mf = self.manifest
-        mfs = {}
-        changedfiles = set()
-        fstate = ['']
-        count = [0, 0]
-
         self.hook('preoutgoing', throw=True, source=source)
         self.changegroupinfo(nodes, source)
 
-        revset = set([cl.rev(n) for n in nodes])
-
-        def gennodelst(log):
-            ln, llr = log.node, log.linkrev
-            return [ln(r) for r in log if llr(r) in revset]
-
-        progress = self.ui.progress
-        _bundling = _('bundling')
-        _changesets = _('changesets')
-        _manifests = _('manifests')
-        _files = _('files')
-
-        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:
-                count[0] += 1
-                progress(_bundling, count[0],
-                         unit=_manifests, total=count[1])
-                return cl.node(revlog.linkrev(revlog.rev(x)))
-            else:
-                progress(_bundling, count[0], item=fstate[0],
-                    total=count[1], unit=_files)
-                return cl.node(revlog.linkrev(revlog.rev(x)))
-
-        bundler = changegroup.bundle10(lookup)
         reorder = self.ui.config('bundle', 'reorder', 'auto')
         if reorder == 'auto':
             reorder = None
         else:
             reorder = util.parsebool(reorder)
 
-        def gengroup():
-            '''yield a sequence of changegroup chunks (strings)'''
-            # construct a list of all changed files
+        gen = changegroup.changegroupgen(self, nodes, source, reorder)
 
-            count[:] = [0, len(nodes)]
-            for chunk in cl.group(nodes, bundler, reorder=reorder):
-                yield chunk
-            progress(_bundling, None)
-
-            count[:] = [0, len(mfs)]
-            for chunk in mf.group(gennodelst(mf), bundler, reorder=reorder):
-                yield chunk
-            progress(_bundling, None)
-
-            count[:] = [0, len(changedfiles)]
-            for fname in sorted(changedfiles):
-                filerevlog = self.file(fname)
-                if not len(filerevlog):
-                    raise util.Abort(_("empty or missing revlog for %s")
-                                     % fname)
-                fstate[0] = fname
-                nodelist = gennodelst(filerevlog)
-                if nodelist:
-                    count[0] += 1
-                    yield bundler.fileheader(fname)
-                    for chunk in filerevlog.group(nodelist, bundler, reorder):
-                        yield chunk
-            yield bundler.close()
-            progress(_bundling, None)
-
-            if nodes:
-                self.hook('outgoing', node=hex(nodes[0]), source=source)
-
-        return changegroup.unbundle10(util.chunkbuffer(gengroup()), 'UN')
+        return changegroup.unbundle10(util.chunkbuffer(gen.gengroup()), 'UN')
 
     @unfilteredmethod
     def addchangegroup(self, source, srctype, url, emptyok=False):