D4083: changegroup: control delta parent behavior via constructor
indygreg (Gregory Szorc)
phabricator at mercurial-scm.org
Fri Aug 3 21:05:24 UTC 2018
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
The last remaining override on cg2packer related to parent delta
computation. We pass a parameter to the constructor to control
whether to delta against the previous revision and we inline all
parent delta logic into a single function.
With this change, cg2packer is empty, so it has been deleted.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D4083
AFFECTED FILES
mercurial/changegroup.py
CHANGE DETAILS
diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
--- a/mercurial/changegroup.py
+++ b/mercurial/changegroup.py
@@ -524,8 +524,8 @@
class cg1packer(object):
def __init__(self, repo, filematcher, version, allowreorder,
- builddeltaheader, manifestsend, sendtreemanifests,
- bundlecaps=None):
+ useprevdelta, builddeltaheader, manifestsend,
+ sendtreemanifests, bundlecaps=None):
"""Given a source repo, construct a bundler.
filematcher is a matcher that matches on files to include in the
@@ -535,6 +535,9 @@
This value is used when ``bundle.reorder`` is ``auto`` or isn't
set.
+ useprevdelta controls whether revisions should always delta against
+ the previous revision in the changegroup.
+
builddeltaheader is a callable that constructs the header for a group
delta.
@@ -551,6 +554,7 @@
self._filematcher = filematcher
self.version = version
+ self._useprevdelta = useprevdelta
self._builddeltaheader = builddeltaheader
self._manifestsend = manifestsend
self._sendtreemanifests = sendtreemanifests
@@ -953,9 +957,51 @@
progress.complete()
def deltaparent(self, store, rev, p1, p2, prev):
- if not store.candelta(prev, rev):
- raise error.ProgrammingError('cg1 should not be used in this case')
- return prev
+ if self._useprevdelta:
+ if not store.candelta(prev, rev):
+ raise error.ProgrammingError(
+ 'cg1 should not be used in this case')
+ return prev
+
+ # Narrow ellipses mode.
+ if util.safehasattr(self, 'full_nodes'):
+ # TODO: send better deltas when in narrow mode.
+ #
+ # changegroup.group() loops over revisions to send,
+ # including revisions we'll skip. What this means is that
+ # `prev` will be a potentially useless delta base for all
+ # ellipsis nodes, as the client likely won't have it. In
+ # the future we should do bookkeeping about which nodes
+ # have been sent to the client, and try to be
+ # significantly smarter about delta bases. This is
+ # slightly tricky because this same code has to work for
+ # all revlogs, and we don't have the linkrev/linknode here.
+ return p1
+
+ dp = store.deltaparent(rev)
+ if dp == nullrev and store.storedeltachains:
+ # Avoid sending full revisions when delta parent is null. Pick prev
+ # in that case. It's tempting to pick p1 in this case, as p1 will
+ # be smaller in the common case. However, computing a delta against
+ # p1 may require resolving the raw text of p1, which could be
+ # expensive. The revlog caches should have prev cached, meaning
+ # less CPU for changegroup generation. There is likely room to add
+ # a flag and/or config option to control this behavior.
+ base = prev
+ elif dp == nullrev:
+ # revlog is configured to use full snapshot for a reason,
+ # stick to full snapshot.
+ base = nullrev
+ elif dp not in (p1, p2, prev):
+ # Pick prev when we can't be sure remote has the base revision.
+ return prev
+ else:
+ base = dp
+
+ if base != nullrev and not store.candelta(base, rev):
+ base = nullrev
+
+ return base
def revchunk(self, store, rev, prev, linknode):
if util.safehasattr(self, 'full_nodes'):
@@ -1128,51 +1174,13 @@
deltachunks=(diffheader, data),
)
-class cg2packer(cg1packer):
- def deltaparent(self, store, rev, p1, p2, prev):
- # Narrow ellipses mode.
- if util.safehasattr(self, 'full_nodes'):
- # TODO: send better deltas when in narrow mode.
- #
- # changegroup.group() loops over revisions to send,
- # including revisions we'll skip. What this means is that
- # `prev` will be a potentially useless delta base for all
- # ellipsis nodes, as the client likely won't have it. In
- # the future we should do bookkeeping about which nodes
- # have been sent to the client, and try to be
- # significantly smarter about delta bases. This is
- # slightly tricky because this same code has to work for
- # all revlogs, and we don't have the linkrev/linknode here.
- return p1
-
- dp = store.deltaparent(rev)
- if dp == nullrev and store.storedeltachains:
- # Avoid sending full revisions when delta parent is null. Pick prev
- # in that case. It's tempting to pick p1 in this case, as p1 will
- # be smaller in the common case. However, computing a delta against
- # p1 may require resolving the raw text of p1, which could be
- # expensive. The revlog caches should have prev cached, meaning
- # less CPU for changegroup generation. There is likely room to add
- # a flag and/or config option to control this behavior.
- base = prev
- elif dp == nullrev:
- # revlog is configured to use full snapshot for a reason,
- # stick to full snapshot.
- base = nullrev
- elif dp not in (p1, p2, prev):
- # Pick prev when we can't be sure remote has the base revision.
- return prev
- else:
- base = dp
- if base != nullrev and not store.candelta(base, rev):
- base = nullrev
- return base
-
def _makecg1packer(repo, filematcher, bundlecaps):
builddeltaheader = lambda d: _CHANGEGROUPV1_DELTA_HEADER.pack(
d.node, d.p1node, d.p2node, d.linknode)
- return cg1packer(repo, filematcher, b'01', allowreorder=None,
+ return cg1packer(repo, filematcher, b'01',
+ useprevdelta=True,
+ allowreorder=None,
builddeltaheader=builddeltaheader,
manifestsend=b'', sendtreemanifests=False,
bundlecaps=bundlecaps)
@@ -1184,16 +1192,20 @@
# Since generaldelta is directly supported by cg2, reordering
# generally doesn't help, so we disable it by default (treating
# bundle.reorder=auto just like bundle.reorder=False).
- return cg2packer(repo, filematcher, b'02', allowreorder=False,
+ return cg1packer(repo, filematcher, b'02',
+ useprevdelta=False,
+ allowreorder=False,
builddeltaheader=builddeltaheader,
manifestsend=b'', sendtreemanifests=False,
bundlecaps=bundlecaps)
def _makecg3packer(repo, filematcher, bundlecaps):
builddeltaheader = lambda d: _CHANGEGROUPV3_DELTA_HEADER.pack(
d.node, d.p1node, d.p2node, d.basenode, d.linknode, d.flags)
- return cg2packer(repo, filematcher, b'03', allowreorder=False,
+ return cg1packer(repo, filematcher, b'03',
+ useprevdelta=False,
+ allowreorder=False,
builddeltaheader=builddeltaheader,
manifestsend=closechunk(), sendtreemanifests=True,
bundlecaps=bundlecaps)
To: indygreg, #hg-reviewers
Cc: mercurial-devel
More information about the Mercurial-devel
mailing list