Patchwork [14,of,19] snapshot: make sure we'll never refine delta base from a reused source

login
register
mail settings
Submitter Boris Feld
Date Sept. 8, 2018, 10:57 a.m.
Message ID <cc72a5b2135b94e1176e.1536404228@localhost.localdomain>
Download mbox | patch
Permalink /patch/34437/
State Accepted
Headers show

Comments

Boris Feld - Sept. 8, 2018, 10:57 a.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1536333454 14400
#      Fri Sep 07 11:17:34 2018 -0400
# Node ID cc72a5b2135b94e1176eaf7a71b5b577bb7b7a33
# Parent  4c849d34990166b1246426533efa517733d8a7fe
# EXP-Topic sparse-snapshot
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r cc72a5b2135b
snapshot: make sure we'll never refine delta base from a reused source

The point of reusing delta from the source is to avoid doing computation when
applying a bundle. Refining such delta would go against that spirit.

We do not have refining logic in place yet. This code needed to be moved out
of the way before we could start adding such logic.
Gregory Szorc - Sept. 10, 2018, 4:29 p.m.
On Sat, Sep 8, 2018 at 3:57 AM Boris Feld <boris.feld@octobus.net> wrote:

> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1536333454 14400
> #      Fri Sep 07 11:17:34 2018 -0400
> # Node ID cc72a5b2135b94e1176eaf7a71b5b577bb7b7a33
> # Parent  4c849d34990166b1246426533efa517733d8a7fe
> # EXP-Topic sparse-snapshot
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r
> cc72a5b2135b
> snapshot: make sure we'll never refine delta base from a reused source
>
> The point of reusing delta from the source is to avoid doing computation
> when
> applying a bundle. Refining such delta would go against that spirit.
>
> We do not have refining logic in place yet. This code needed to be moved
> out
> of the way before we could start adding such logic.
>
> diff --git a/mercurial/revlogutils/deltas.py
> b/mercurial/revlogutils/deltas.py
> --- a/mercurial/revlogutils/deltas.py
> +++ b/mercurial/revlogutils/deltas.py
> @@ -630,6 +630,19 @@ def _findsnapshots(revlog, cache, start_
>
>  def _refinedgroups(revlog, p1, p2, cachedelta):
>      good = None
> +    # First we try to reuse a the delta contained in the bundle.
> +    # (or from the source revlog)
> +    #
> +    # This logic only applies to general delta repositories and can be
> disabled
> +    # through configuration. Disabling reuse source delta is useful when
> +    # we want to make sure we recomputed "optimal" deltas.
> +    if cachedelta and revlog._generaldelta and revlog._lazydeltabase:
>

At some point we may want to pass a flag into this function (perhaps
attached to the cached delta object) saying whether it is appropriate to
reuse the delta. This would eliminate this code from having to query the
bowels of the revlog instance for low-level state. (I'm just trying to
think of ways of making this code more generic, as we'll want to employ
this advanced delta logic with alternate storage backends someday.)


> +        # Assume what we received from the server is a good choice
> +        # build delta will reuse the cache
> +        good = yield (cachedelta[0],)
> +        if good is not None:
> +            yield None
> +            return
>      for candidates in _rawgroups(revlog, p1, p2, cachedelta):
>          good = yield candidates
>          if good is not None:
> @@ -651,17 +664,6 @@ def _rawgroups(revlog, p1, p2, cachedelt
>      prev = curr - 1
>      deltachain = lambda rev: revlog._deltachain(rev)[0]
>
> -    # First we try to reuse a the delta contained in the bundle.
> -    # (or from the source revlog)
> -    #
> -    # This logic only applies to general delta repositories and can be
> disabled
> -    # through configuration. Disabling reuse of source delta is useful
> when
> -    # we want to make sure we recomputed "optimal" deltas.
> -    if cachedelta and gdelta and revlog._lazydeltabase:
> -        # Assume what we received from the server is a good choice
> -        # build delta will reuse the cache
> -        yield (cachedelta[0],)
> -
>      if gdelta:
>          # exclude already lazy tested base if any
>          parents = [p for p in (p1, p2) if p != nullrev]
>

Patch

diff --git a/mercurial/revlogutils/deltas.py b/mercurial/revlogutils/deltas.py
--- a/mercurial/revlogutils/deltas.py
+++ b/mercurial/revlogutils/deltas.py
@@ -630,6 +630,19 @@  def _findsnapshots(revlog, cache, start_
 
 def _refinedgroups(revlog, p1, p2, cachedelta):
     good = None
+    # First we try to reuse a the delta contained in the bundle.
+    # (or from the source revlog)
+    #
+    # This logic only applies to general delta repositories and can be disabled
+    # through configuration. Disabling reuse source delta is useful when
+    # we want to make sure we recomputed "optimal" deltas.
+    if cachedelta and revlog._generaldelta and revlog._lazydeltabase:
+        # Assume what we received from the server is a good choice
+        # build delta will reuse the cache
+        good = yield (cachedelta[0],)
+        if good is not None:
+            yield None
+            return
     for candidates in _rawgroups(revlog, p1, p2, cachedelta):
         good = yield candidates
         if good is not None:
@@ -651,17 +664,6 @@  def _rawgroups(revlog, p1, p2, cachedelt
     prev = curr - 1
     deltachain = lambda rev: revlog._deltachain(rev)[0]
 
-    # First we try to reuse a the delta contained in the bundle.
-    # (or from the source revlog)
-    #
-    # This logic only applies to general delta repositories and can be disabled
-    # through configuration. Disabling reuse of source delta is useful when
-    # we want to make sure we recomputed "optimal" deltas.
-    if cachedelta and gdelta and revlog._lazydeltabase:
-        # Assume what we received from the server is a good choice
-        # build delta will reuse the cache
-        yield (cachedelta[0],)
-
     if gdelta:
         # exclude already lazy tested base if any
         parents = [p for p in (p1, p2) if p != nullrev]