[PATCH 3 of 3] addrevision: use general delta when the incoming base delta is bad

[Pierre-Yves David]

On 12/02/2015 11:15 PM, Martin von Zweigbergk wrote:
>
>
> On Wed, Dec 2, 2015 at 9:56 AM Pierre-Yves David
> <pierre-yves.david at ens-lyon.org <mailto:pierre-yves.david at ens-lyon.org>>
> wrote:
>
>     # HG changeset patch
>     # User Pierre-Yves David <pierre-yves.david at fb.com
>     <mailto:pierre-yves.david at fb.com>>
>     # Date 1449015359 28800
>     #      Tue Dec 01 16:15:59 2015 -0800
>     # Node ID 64e48bdff2461e793c963d677407d35fc32c30a9
>     # Parent  49f5bfa7043ae0c797536dab93260aee92fcbaa8
>     # EXP-Topic generaldelta
>     # Available At http://hg.netv6.net/marmoute-wip/mercurial/
>     #              hg pull http://hg.netv6.net/marmoute-wip/mercurial/
>     -r 64e48bdff246
>     addrevision: use general delta when the incoming base delta is bad
>
>     We unify the delta selection process to be a simple three options
>     process:
>
>     - try to use the incoming delta (if lazydeltabase is on)
>     - try to find a suitable parents to delta against (if gd is on)
>     - try to delta against the tipmost revision
>
>     The first of this option that yield a valid delta will be used.
>
>     The test change in 'test-generaldelta.t' show this behavior as we
>     use a delta
>     against the parent instead of a full delta when the incoming delta
>     is not
>     suitable.
>
>     This as some impact on 'test-bundle.t' because a delta somewhere
>     changes. It
>     does not seems to change the test semantic and have been ignored.
>
>     diff --git a/mercurial/revlog.py b/mercurial/revlog.py
>     --- a/mercurial/revlog.py
>     +++ b/mercurial/revlog.py
>     @@ -1422,38 +1422,37 @@ class revlog(object):
>               else:
>                   textlen = len(text)
>
>               # should we try to build a delta?
>               if prev != nullrev:
>     +            tested = set()
>                   if cachedelta and self._generaldelta and
>     self._lazydeltabase:
>                       # Assume what we received from the server is a
>     good choice
>                       # build delta will reuse the cache
>                       candidatedelta = builddelta(cachedelta[0])
>     +                tested.add(candidatedelta[3])
>                       if self._isgooddelta(candidatedelta, textlen):
>                           delta = candidatedelta
>     -                elif prev != candidatedelta[3]:
>     -                    # Try against prev to hopefully save us a fulltext.
>     -                    delta = builddelta(prev)
>     -            elif self._generaldelta:
>     +            if delta is None and self._generaldelta:
>                       parents = [p1r, p2r]
>     -                if not self._aggressivemergedeltas:
>     +                # exclude already lazy tested base if any
>     +                parents = [p for p in parents if p not in tested]
>     +                if parents and not self._aggressivemergedeltas:
>                           # Pick whichever parent is closer to us (to
>     minimize the
>     -                    # chance of having to build a fulltext). Since
>     -                    # nullrev == -1, any non-merge commit will
>     always pick p1r.
>     +                    # chance of having to build a fulltext).
>                           parents = [max(parents)]
>
>
> Before this patch, there were always 2 revisions in parents before this
> line, so the nullrev would never be tested only for root commits. After
> this patch, nullrev will also be tested for non-merge commits where the
> first parent was the incoming delta. I suppose it's unlikely to make a
> difference, since those will almost always probably not be good deltas,
> but it also seems unnecessary to even check. Is the testing of nullrev
> even intentional?

earlier version of this patch was testing for nullrev explicitly, but I 
figured out that builddelta could also we feed nullrev (root commit) and 
therefore was able to handle it.

-- 
Pierre-Yves David