Patchwork [1,of,4] changegroup: if possible fallback to delta against parents instead of prev

login
register
mail settings
Submitter Boris Feld
Date Oct. 16, 2018, 8:33 a.m.
Message ID <a69790aaac1946e80e35.1539678810@localhost.localdomain>
Download mbox | patch
Permalink /patch/36031/
State New
Headers show

Comments

Boris Feld - Oct. 16, 2018, 8:33 a.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1539114828 -7200
#      Tue Oct 09 21:53:48 2018 +0200
# Node ID a69790aaac1946e80e35b28c8f41b14a7f6c0f64
# Parent  43f0a37bd9ed20b45d73f667acbb0c3035492e1a
# EXP-Topic slim-bundle
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r a69790aaac19
changegroup: if possible fallback to delta against parents instead of prev

Deltas against parent are likely to be smaller. This is especially useful
since we stop reordering changeset in the bundle.
Gregory Szorc - Oct. 16, 2018, 4:33 p.m.
On Tue, Oct 16, 2018 at 10:35 AM Boris Feld <boris.feld@octobus.net> wrote:

> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1539114828 -7200
> #      Tue Oct 09 21:53:48 2018 +0200
> # Node ID a69790aaac1946e80e35b28c8f41b14a7f6c0f64
> # Parent  43f0a37bd9ed20b45d73f667acbb0c3035492e1a
> # EXP-Topic slim-bundle
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r
> a69790aaac19
> changegroup: if possible fallback to delta against parents instead of prev
>
> Deltas against parent are likely to be smaller. This is especially useful
> since we stop reordering changeset in the bundle.
>
> diff --git a/mercurial/utils/storageutil.py
> b/mercurial/utils/storageutil.py
> --- a/mercurial/utils/storageutil.py
> +++ b/mercurial/utils/storageutil.py
> @@ -394,9 +394,11 @@ def emitrevisions(store, nodes, nodesord
>                  baserev = nullrev
>
>          # Storage has a fulltext revision.
> -
> -        # Let's use the previous revision, which is as good a guess as
> any.
> -        # There is definitely room to improve this logic.
> +        elif assumehaveparentrevisions and p1rev != nullrev:
> +            baserev = p1rev
> +        elif assumehaveparentrevisions and p2rev != nullrev:
> +            baserev = p2rev
> +        # revision is a root, try our luck against the previous revision
>

I'm worried this will kill performance on non-generaldelta revlogs. Yes,
this change is for the case where storage has a fulltext. So we need to
resolve the baserev's fulltext regardless. But since revlogs cache the last
fulltext and the last fulltext has a good chance of being "prev," this
change might take away some cache hits.

Could you please measure the impact of bundling performance on a
non-generaldelta repo? Preferably one with a lot of merges, like Netbeans.


>          elif prevrev is not None:
>              baserev = prevrev
>          else:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Boris FELD - Oct. 18, 2018, 10:47 a.m.
This patch is not essential to the series and opens various questions
regarding deltas in bundle. We are dropping it from the series for now.

On 16/10/2018 18:33, Gregory Szorc wrote:
> On Tue, Oct 16, 2018 at 10:35 AM Boris Feld <boris.feld@octobus.net
> <mailto:boris.feld@octobus.net>> wrote:
>
>     # HG changeset patch
>     # User Boris Feld <boris.feld@octobus.net
>     <mailto:boris.feld@octobus.net>>
>     # Date 1539114828 -7200
>     #      Tue Oct 09 21:53:48 2018 +0200
>     # Node ID a69790aaac1946e80e35b28c8f41b14a7f6c0f64
>     # Parent  43f0a37bd9ed20b45d73f667acbb0c3035492e1a
>     # EXP-Topic slim-bundle
>     # Available At https://bitbucket.org/octobus/mercurial-devel/
>     #              hg pull
>     https://bitbucket.org/octobus/mercurial-devel/ -r a69790aaac19
>     changegroup: if possible fallback to delta against parents instead
>     of prev
>
>     Deltas against parent are likely to be smaller. This is especially
>     useful
>     since we stop reordering changeset in the bundle.
>
>     diff --git a/mercurial/utils/storageutil.py
>     b/mercurial/utils/storageutil.py
>     --- a/mercurial/utils/storageutil.py
>     +++ b/mercurial/utils/storageutil.py
>     @@ -394,9 +394,11 @@ def emitrevisions(store, nodes, nodesord
>                      baserev = nullrev
>
>              # Storage has a fulltext revision.
>     -
>     -        # Let's use the previous revision, which is as good a
>     guess as any.
>     -        # There is definitely room to improve this logic.
>     +        elif assumehaveparentrevisions and p1rev != nullrev:
>     +            baserev = p1rev
>     +        elif assumehaveparentrevisions and p2rev != nullrev:
>     +            baserev = p2rev
>     +        # revision is a root, try our luck against the previous
>     revision
>
>
> I'm worried this will kill performance on non-generaldelta revlogs.
> Yes, this change is for the case where storage has a fulltext. So we
> need to resolve the baserev's fulltext regardless. But since revlogs
> cache the last fulltext and the last fulltext has a good chance of
> being "prev," this change might take away some cache hits.
>
> Could you please measure the impact of bundling performance on a
> non-generaldelta repo? Preferably one with a lot of merges, like Netbeans.
>  
>
>              elif prevrev is not None:
>                  baserev = prevrev
>              else:
>     _______________________________________________
>     Mercurial-devel mailing list
>     Mercurial-devel@mercurial-scm.org
>     <mailto:Mercurial-devel@mercurial-scm.org>
>     https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/utils/storageutil.py b/mercurial/utils/storageutil.py
--- a/mercurial/utils/storageutil.py
+++ b/mercurial/utils/storageutil.py
@@ -394,9 +394,11 @@  def emitrevisions(store, nodes, nodesord
                 baserev = nullrev
 
         # Storage has a fulltext revision.
-
-        # Let's use the previous revision, which is as good a guess as any.
-        # There is definitely room to improve this logic.
+        elif assumehaveparentrevisions and p1rev != nullrev:
+            baserev = p1rev
+        elif assumehaveparentrevisions and p2rev != nullrev:
+            baserev = p2rev
+        # revision is a root, try our luck against the previous revision
         elif prevrev is not None:
             baserev = prevrev
         else: