D4083: changegroup: control delta parent behavior via constructor
This revision was automatically updated to reflect the committed changes.
Closed by commit rHG23ae0c07a3e1: changegroup: control delta parent behavior
via constructor (authored by indygreg, committed by ).
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D4083?vs=9845=9958
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
@@ -521,8 +521,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
@@ -532,6 +532,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.
@@ -548,6 +551,7 @@
self._filematcher = filematcher
self.version = version
+self._useprevdelta = useprevdelta
self._builddeltaheader = builddeltaheader
self._manifestsend = manifestsend
self._sendtreemanifests = sendtreemanifests
@@ -950,9 +954,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'):
@@ -1125,51 +1171,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
-
D4083: changegroup: control delta parent behavior via constructor
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
