[PATCH 2 of 4] changegroup: refactor emitrevision to use a `deltamode` argument
Gregory Szorc
gregory.szorc at gmail.com
Tue Oct 16 12:36:45 EDT 2018
On Tue, Oct 16, 2018 at 10:36 AM Boris Feld <boris.feld at octobus.net> wrote:
> # HG changeset patch
> # User Boris Feld <boris.feld at octobus.net>
> # Date 1539115321 -7200
> # Tue Oct 09 22:02:01 2018 +0200
> # Node ID a075ac3487d6d266ec5f2dbd6901da21c38d4ed9
> # Parent a69790aaac1946e80e35b28c8f41b14a7f6c0f64
> # EXP-Topic slim-bundle
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> # hg pull https://bitbucket.org/octobus/mercurial-devel/ -r
> a075ac3487d6
> changegroup: refactor emitrevision to use a `deltamode` argument
>
> This new argument gathers the semantic of `sendfulltext` and
> `deltaprevious`
> in a single value. We are about to introduce a new type of constraints.
> Avoiding yet another argument sounds like a plus.
>
I have mixed feelings about this patch. I agree the API is cleaner. It's
just that I feel with shallow clone, we'll need more flexibility in terms
of delta generation - too much flexibility to shoehorn into a single
"deltamode" argument. I think we'll need multiple arguments to influence
delta generation behavior.
But that is the future and this patch is now and is in support of a feature
to facilitate performance testing. We shouldn't let the future dictate
today. So I think I'll take this once the rest of the patches in this
series clear.
>
> diff --git a/mercurial/revlog.py b/mercurial/revlog.py
> --- a/mercurial/revlog.py
> +++ b/mercurial/revlog.py
> @@ -2229,6 +2229,12 @@ class revlog(object):
> if nodesorder is None and not self._generaldelta:
> nodesorder = 'storage'
>
> + deltamode = storageutil.DELTAMODE_STD
> + if deltaprevious:
> + deltamode = storageutil.DELTAMODE_PREV
> + elif not self._storedeltachains:
> + deltamode = storageutil.DELTAMODE_FULL
> +
> return storageutil.emitrevisions(
> self, nodes, nodesorder, revlogrevisiondelta,
> deltaparentfn=self.deltaparent,
> @@ -2236,10 +2242,9 @@ class revlog(object):
> rawsizefn=self.rawsize,
> revdifffn=self.revdiff,
> flagsfn=self.flags,
> - sendfulltext=not self._storedeltachains,
> + deltamode=deltamode,
> revisiondata=revisiondata,
> - assumehaveparentrevisions=assumehaveparentrevisions,
> - deltaprevious=deltaprevious)
> + assumehaveparentrevisions=assumehaveparentrevisions)
>
> DELTAREUSEALWAYS = 'always'
> DELTAREUSESAMEREVS = 'samerevs'
> diff --git a/mercurial/utils/storageutil.py
> b/mercurial/utils/storageutil.py
> --- a/mercurial/utils/storageutil.py
> +++ b/mercurial/utils/storageutil.py
> @@ -265,11 +265,14 @@ def resolvestripinfo(minlinkrev, tiprev,
>
> return strippoint, brokenrevs
>
> +DELTAMODE_STD = 'default'
> +DELTAMODE_PREV = 'previous'
> +DELTAMODE_FULL = 'fulltext'
> +
> def emitrevisions(store, nodes, nodesorder, resultcls, deltaparentfn=None,
> candeltafn=None, rawsizefn=None, revdifffn=None,
> flagsfn=None,
> - sendfulltext=False,
> - revisiondata=False, assumehaveparentrevisions=False,
> - deltaprevious=False):
> + deltamode=DELTAMODE_STD,
> + revisiondata=False, assumehaveparentrevisions=False):
> """Generic implementation of ifiledata.emitrevisions().
>
> Emitting revision data is subtly complex. This function attempts to
> @@ -320,14 +323,17 @@ def emitrevisions(store, nodes, nodesord
> Callable receiving a revision number and returns the integer flags
> value for it. If not defined, flags value will be 0.
>
> - ``sendfulltext``
> + ``deltamode``
> + constaint on delta to be sent:
> + * DELTAMODE_STD - normal mode, try to reuse storage deltas,
> + * DELTAMODE_PREV - only delta against "prev",
> + * DELTAMODE_FULL - only issue full snapshot.
> +
> Whether to send fulltext revisions instead of deltas, if allowed.
>
> ``nodesorder``
> ``revisiondata``
> ``assumehaveparentrevisions``
> - ``deltaprevious``
> - See ``ifiledata.emitrevisions()`` interface documentation.
> """
>
> fnode = store.node
> @@ -343,7 +349,7 @@ def emitrevisions(store, nodes, nodesord
>
> prevrev = None
>
> - if deltaprevious or assumehaveparentrevisions:
> + if deltamode == DELTAMODE_PREV or assumehaveparentrevisions:
> prevrev = store.parentrevs(revs[0])[0]
>
> # Set of revs available to delta against.
> @@ -362,11 +368,11 @@ def emitrevisions(store, nodes, nodesord
> deltaparentrev = nullrev
>
> # Forced delta against previous mode.
> - if deltaprevious:
> + if deltamode == DELTAMODE_PREV:
> baserev = prevrev
>
> # We're instructed to send fulltext. Honor that.
> - elif sendfulltext:
> + elif deltamode == DELTAMODE_FULL:
> baserev = nullrev
>
> # There is a delta in storage. We try to use that because it
> @@ -427,7 +433,7 @@ def emitrevisions(store, nodes, nodesord
> baserevisionsize = len(store.revision(baserev,
> raw=True))
>
> - elif baserev == nullrev and not deltaprevious:
> + elif baserev == nullrev and not deltamode == DELTAMODE_PREV:
> revision = store.revision(node, raw=True)
> available.add(rev)
> else:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20181016/ce010587/attachment.html>
More information about the Mercurial-devel
mailing list