Patchwork [6,of,6] bundle: add optional 'tagsfnodecache' data to on disk bundle (issue5543)

login
register
mail settings
Submitter Pierre-Yves David
Date May 6, 2017, 8:05 a.m.
Message ID <3a04e2424b2821fa92d4.1494057900@nodosa.octopoid.net>
Download mbox | patch
Permalink /patch/20484/
State Accepted
Headers show

Comments

Pierre-Yves David - May 6, 2017, 8:05 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@octobus.net>
# Date 1493998275 -7200
#      Fri May 05 17:31:15 2017 +0200
# Node ID 3a04e2424b2821fa92d41ac9e40430aa25d4560d
# Parent  16b60a34def41f52e024d32d6e99e5d533cad02f
# EXP-Topic bundle2.tagsfnodecache
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
#              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 3a04e2424b28
bundle: add optional 'tagsfnodecache' data to on disk bundle (issue5543)

This should help performance when unbundling.
Augie Fackler - May 8, 2017, 7:26 p.m.
On Sat, May 06, 2017 at 10:05:00AM +0200, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@octobus.net>
> # Date 1493998275 -7200
> #      Fri May 05 17:31:15 2017 +0200
> # Node ID 3a04e2424b2821fa92d41ac9e40430aa25d4560d
> # Parent  16b60a34def41f52e024d32d6e99e5d533cad02f
> # EXP-Topic bundle2.tagsfnodecache
> # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
> #              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 3a04e2424b28
> bundle: add optional 'tagsfnodecache' data to on disk bundle (issue5543)

queued, thanks
Gregory Szorc - May 8, 2017, 10:48 p.m.
On Sat, May 6, 2017 at 1:05 AM, Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@octobus.net>
> # Date 1493998275 -7200
> #      Fri May 05 17:31:15 2017 +0200
> # Node ID 3a04e2424b2821fa92d41ac9e40430aa25d4560d
> # Parent  16b60a34def41f52e024d32d6e99e5d533cad02f
> # EXP-Topic bundle2.tagsfnodecache
> # Available At https://www.mercurial-scm.org/
> repo/users/marmoute/mercurial/
> #              hg pull https://www.mercurial-scm.org/
> repo/users/marmoute/mercurial/ -r 3a04e2424b28
> bundle: add optional 'tagsfnodecache' data to on disk bundle (issue5543)
>
> This should help performance when unbundling.
>
> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
> --- a/mercurial/bundle2.py
> +++ b/mercurial/bundle2.py
> @@ -1378,6 +1378,8 @@ def _addpartsfromopts(ui, repo, bundler,
>          part.addparam('nbchanges', str(cg.extras['clcount']),
>                        mandatory=False)
>
> +    addparttagsfnodescache(repo, bundler, outgoing)
> +
>  def addparttagsfnodescache(repo, bundler, outgoing):
>      # we include the tags fnode cache for the bundle changeset
>      # (as an optional parts)
> diff --git a/tests/test-tags.t b/tests/test-tags.t
> --- a/tests/test-tags.t
> +++ b/tests/test-tags.t
> @@ -716,3 +716,15 @@ Running hg tags should produce tags2* fi
>    0040: ff ff ff ff ff ff ff ff 40 f0 35 8c 19 e0 a7 d3 |........@.5.....|
>    0050: 8a 5c 6a 82 4d cf fb a5 87 d0 2f a3 1e 4f 2f 8a |.\j.M...../..O/.|
>
> +Check that the bundle includes cache data
> +
> +  $ hg -R tagsclient bundle --all ./test-cache-in-bundle-all-rev.hg
> +  4 changesets found
> +  $ hg debugbundle ./test-cache-in-bundle-all-rev.hg
> +  Stream params: sortdict([('Compression', 'BZ')])
> +  changegroup -- "sortdict([('version', '02'), ('nbchanges', '4')])"
> +      96ee1d7354c4ad7372047672c36a1f561e3a6a4c
> +      c4dab0c2fd337eb9191f80c3024830a4889a8f34
> +      f63cc8fe54e4d326f8d692805d70e092f851ddb1
> +      40f0358cb314c824a5929ee527308d90e023bc10
> +  hgtagsfnodes -- 'sortdict()'
>

This test would be more useful if it didn't send empty tags cache data. In
fact, I think you found a bug! Shouldn't we omit the part if it is empty?
Pierre-Yves David - May 9, 2017, 6:25 a.m.
On 05/09/2017 12:48 AM, Gregory Szorc wrote:
> On Sat, May 6, 2017 at 1:05 AM, Pierre-Yves David
> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
> wrote:
>
>     # HG changeset patch
>     # User Pierre-Yves David <pierre-yves.david@octobus.net
>     <mailto:pierre-yves.david@octobus.net>>
>     # Date 1493998275 -7200
>     #      Fri May 05 17:31:15 2017 +0200
>     # Node ID 3a04e2424b2821fa92d41ac9e40430aa25d4560d
>     # Parent  16b60a34def41f52e024d32d6e99e5d533cad02f
>     # EXP-Topic bundle2.tagsfnodecache
>     # Available At
>     https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>     <https://www.mercurial-scm.org/repo/users/marmoute/mercurial/>
>     #              hg pull
>     https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>     <https://www.mercurial-scm.org/repo/users/marmoute/mercurial/> -r
>     3a04e2424b28
>     bundle: add optional 'tagsfnodecache' data to on disk bundle (issue5543)
>
>     This should help performance when unbundling.
>
>     diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
>     --- a/mercurial/bundle2.py
>     +++ b/mercurial/bundle2.py
>     @@ -1378,6 +1378,8 @@ def _addpartsfromopts(ui, repo, bundler,
>              part.addparam('nbchanges', str(cg.extras['clcount']),
>                            mandatory=False)
>
>     +    addparttagsfnodescache(repo, bundler, outgoing)
>     +
>      def addparttagsfnodescache(repo, bundler, outgoing):
>          # we include the tags fnode cache for the bundle changeset
>          # (as an optional parts)
>     diff --git a/tests/test-tags.t b/tests/test-tags.t
>     --- a/tests/test-tags.t
>     +++ b/tests/test-tags.t
>     @@ -716,3 +716,15 @@ Running hg tags should produce tags2* fi
>        0040: ff ff ff ff ff ff ff ff 40 f0 35 8c 19 e0 a7 d3
>     |........@.5.....|
>        0050: 8a 5c 6a 82 4d cf fb a5 87 d0 2f a3 1e 4f 2f 8a
>     |.\j.M...../..O/.|
>
>     +Check that the bundle includes cache data
>     +
>     +  $ hg -R tagsclient bundle --all ./test-cache-in-bundle-all-rev.hg
>     +  4 changesets found
>     +  $ hg debugbundle ./test-cache-in-bundle-all-rev.hg
>     +  Stream params: sortdict([('Compression', 'BZ')])
>     +  changegroup -- "sortdict([('version', '02'), ('nbchanges', '4')])"
>     +      96ee1d7354c4ad7372047672c36a1f561e3a6a4c
>     +      c4dab0c2fd337eb9191f80c3024830a4889a8f34
>     +      f63cc8fe54e4d326f8d692805d70e092f851ddb1
>     +      40f0358cb314c824a5929ee527308d90e023bc10
>     +  hgtagsfnodes -- 'sortdict()'
>
>
> This test would be more useful if it didn't send empty tags cache data.
> In fact, I think you found a bug! Shouldn't we omit the part if it is
> empty?

The cache data is not empty, the `hg debugbundle` command just do not 
show it. The code is already checking for existing data and only 
generate the part if there are any.

So everything should be working as intended here.

(small note: In the future, I'll probably suggest we send data for all 
known cache entry since the storage for them is "free" and the overhead 
in the bundle should be negligible)

Cheers,
Gregory Szorc - May 9, 2017, 6:37 a.m.
On Mon, May 8, 2017 at 11:25 PM, Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

>
>
> On 05/09/2017 12:48 AM, Gregory Szorc wrote:
>
>> On Sat, May 6, 2017 at 1:05 AM, Pierre-Yves David
>> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
>> wrote:
>>
>>     # HG changeset patch
>>     # User Pierre-Yves David <pierre-yves.david@octobus.net
>>     <mailto:pierre-yves.david@octobus.net>>
>>
>>     # Date 1493998275 -7200
>>     #      Fri May 05 17:31:15 2017 +0200
>>     # Node ID 3a04e2424b2821fa92d41ac9e40430aa25d4560d
>>     # Parent  16b60a34def41f52e024d32d6e99e5d533cad02f
>>     # EXP-Topic bundle2.tagsfnodecache
>>     # Available At
>>     https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>>     <https://www.mercurial-scm.org/repo/users/marmoute/mercurial/>
>>     #              hg pull
>>     https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>>     <https://www.mercurial-scm.org/repo/users/marmoute/mercurial/> -r
>>     3a04e2424b28
>>     bundle: add optional 'tagsfnodecache' data to on disk bundle
>> (issue5543)
>>
>>     This should help performance when unbundling.
>>
>>     diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
>>     --- a/mercurial/bundle2.py
>>     +++ b/mercurial/bundle2.py
>>     @@ -1378,6 +1378,8 @@ def _addpartsfromopts(ui, repo, bundler,
>>              part.addparam('nbchanges', str(cg.extras['clcount']),
>>                            mandatory=False)
>>
>>     +    addparttagsfnodescache(repo, bundler, outgoing)
>>     +
>>      def addparttagsfnodescache(repo, bundler, outgoing):
>>          # we include the tags fnode cache for the bundle changeset
>>          # (as an optional parts)
>>     diff --git a/tests/test-tags.t b/tests/test-tags.t
>>     --- a/tests/test-tags.t
>>     +++ b/tests/test-tags.t
>>     @@ -716,3 +716,15 @@ Running hg tags should produce tags2* fi
>>        0040: ff ff ff ff ff ff ff ff 40 f0 35 8c 19 e0 a7 d3
>>     |........@.5.....|
>>        0050: 8a 5c 6a 82 4d cf fb a5 87 d0 2f a3 1e 4f 2f 8a
>>     |.\j.M...../..O/.|
>>
>>     +Check that the bundle includes cache data
>>     +
>>     +  $ hg -R tagsclient bundle --all ./test-cache-in-bundle-all-rev.hg
>>     +  4 changesets found
>>     +  $ hg debugbundle ./test-cache-in-bundle-all-rev.hg
>>     +  Stream params: sortdict([('Compression', 'BZ')])
>>     +  changegroup -- "sortdict([('version', '02'), ('nbchanges', '4')])"
>>     +      96ee1d7354c4ad7372047672c36a1f561e3a6a4c
>>     +      c4dab0c2fd337eb9191f80c3024830a4889a8f34
>>     +      f63cc8fe54e4d326f8d692805d70e092f851ddb1
>>     +      40f0358cb314c824a5929ee527308d90e023bc10
>>     +  hgtagsfnodes -- 'sortdict()'
>>
>>
>> This test would be more useful if it didn't send empty tags cache data.
>> In fact, I think you found a bug! Shouldn't we omit the part if it is
>> empty?
>>
>
> The cache data is not empty, the `hg debugbundle` command just do not show
> it. The code is already checking for existing data and only generate the
> part if there are any.
>
> So everything should be working as intended here.
>
> (small note: In the future, I'll probably suggest we send data for all
> known cache entry since the storage for them is "free" and the overhead in
> the bundle should be negligible)
>
>
I question the value of that. AFAIK we only ever resolve the .hgtags node
for heads. So sending down the cached fnode for changesets that are no
longer heads feels wasteful since the data will never be used.
Pierre-Yves David - May 9, 2017, 9:56 a.m.
On 05/09/2017 08:37 AM, Gregory Szorc wrote:
> On Mon, May 8, 2017 at 11:25 PM, Pierre-Yves David
> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
> wrote:
>
>
>
>     On 05/09/2017 12:48 AM, Gregory Szorc wrote:
>
>         On Sat, May 6, 2017 at 1:05 AM, Pierre-Yves David
>         <pierre-yves.david@ens-lyon.org
>         <mailto:pierre-yves.david@ens-lyon.org>
>         <mailto:pierre-yves.david@ens-lyon.org
>         <mailto:pierre-yves.david@ens-lyon.org>>>
>         wrote:
>
>             # HG changeset patch
>             # User Pierre-Yves David <pierre-yves.david@octobus.net
>         <mailto:pierre-yves.david@octobus.net>
>             <mailto:pierre-yves.david@octobus.net
>         <mailto:pierre-yves.david@octobus.net>>>
>
>             # Date 1493998275 -7200
>             #      Fri May 05 17:31:15 2017 +0200
>             # Node ID 3a04e2424b2821fa92d41ac9e40430aa25d4560d
>             # Parent  16b60a34def41f52e024d32d6e99e5d533cad02f
>             # EXP-Topic bundle2.tagsfnodecache
>             # Available At
>             https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>         <https://www.mercurial-scm.org/repo/users/marmoute/mercurial/>
>
>         <https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>         <https://www.mercurial-scm.org/repo/users/marmoute/mercurial/>>
>             #              hg pull
>             https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>         <https://www.mercurial-scm.org/repo/users/marmoute/mercurial/>
>
>         <https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>         <https://www.mercurial-scm.org/repo/users/marmoute/mercurial/>> -r
>             3a04e2424b28
>             bundle: add optional 'tagsfnodecache' data to on disk bundle
>         (issue5543)
>
>             This should help performance when unbundling.
>
>             diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
>             --- a/mercurial/bundle2.py
>             +++ b/mercurial/bundle2.py
>             @@ -1378,6 +1378,8 @@ def _addpartsfromopts(ui, repo, bundler,
>                      part.addparam('nbchanges', str(cg.extras['clcount']),
>                                    mandatory=False)
>
>             +    addparttagsfnodescache(repo, bundler, outgoing)
>             +
>              def addparttagsfnodescache(repo, bundler, outgoing):
>                  # we include the tags fnode cache for the bundle changeset
>                  # (as an optional parts)
>             diff --git a/tests/test-tags.t b/tests/test-tags.t
>             --- a/tests/test-tags.t
>             +++ b/tests/test-tags.t
>             @@ -716,3 +716,15 @@ Running hg tags should produce tags2* fi
>                0040: ff ff ff ff ff ff ff ff 40 f0 35 8c 19 e0 a7 d3
>             |........@.5.....|
>                0050: 8a 5c 6a 82 4d cf fb a5 87 d0 2f a3 1e 4f 2f 8a
>             |.\j.M...../..O/.|
>
>             +Check that the bundle includes cache data
>             +
>             +  $ hg -R tagsclient bundle --all
>         ./test-cache-in-bundle-all-rev.hg
>             +  4 changesets found
>             +  $ hg debugbundle ./test-cache-in-bundle-all-rev.hg
>             +  Stream params: sortdict([('Compression', 'BZ')])
>             +  changegroup -- "sortdict([('version', '02'),
>         ('nbchanges', '4')])"
>             +      96ee1d7354c4ad7372047672c36a1f561e3a6a4c
>             +      c4dab0c2fd337eb9191f80c3024830a4889a8f34
>             +      f63cc8fe54e4d326f8d692805d70e092f851ddb1
>             +      40f0358cb314c824a5929ee527308d90e023bc10
>             +  hgtagsfnodes -- 'sortdict()'
>
>
>         This test would be more useful if it didn't send empty tags
>         cache data.
>         In fact, I think you found a bug! Shouldn't we omit the part if
>         it is
>         empty?
>
>
>     The cache data is not empty, the `hg debugbundle` command just do
>     not show it. The code is already checking for existing data and only
>     generate the part if there are any.
>
>     So everything should be working as intended here.
>
>     (small note: In the future, I'll probably suggest we send data for
>     all known cache entry since the storage for them is "free" and the
>     overhead in the bundle should be negligible)
>
>
> I question the value of that. AFAIK we only ever resolve the .hgtags
> node for heads. So sending down the cached fnode for changesets that are
> no longer heads feels wasteful since the data will never be used.

The fnode cache is easy to "inherit" (new changeset that does not touch 
.hgtags can reuse the parent one at commit time) having more cache entry 
filled will helps here when creating new branch. Having an up to date 
cache will help with having efficient tags movement detections during 
transaction.

This will also helps in case of stripping.

Since each fnode cache entry is usually reused by many changeset we 
should be able to have a compat representation.

Cheers,

Patch

diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -1378,6 +1378,8 @@  def _addpartsfromopts(ui, repo, bundler,
         part.addparam('nbchanges', str(cg.extras['clcount']),
                       mandatory=False)
 
+    addparttagsfnodescache(repo, bundler, outgoing)
+
 def addparttagsfnodescache(repo, bundler, outgoing):
     # we include the tags fnode cache for the bundle changeset
     # (as an optional parts)
diff --git a/tests/test-tags.t b/tests/test-tags.t
--- a/tests/test-tags.t
+++ b/tests/test-tags.t
@@ -716,3 +716,15 @@  Running hg tags should produce tags2* fi
   0040: ff ff ff ff ff ff ff ff 40 f0 35 8c 19 e0 a7 d3 |........@.5.....|
   0050: 8a 5c 6a 82 4d cf fb a5 87 d0 2f a3 1e 4f 2f 8a |.\j.M...../..O/.|
 
+Check that the bundle includes cache data
+
+  $ hg -R tagsclient bundle --all ./test-cache-in-bundle-all-rev.hg
+  4 changesets found
+  $ hg debugbundle ./test-cache-in-bundle-all-rev.hg
+  Stream params: sortdict([('Compression', 'BZ')])
+  changegroup -- "sortdict([('version', '02'), ('nbchanges', '4')])"
+      96ee1d7354c4ad7372047672c36a1f561e3a6a4c
+      c4dab0c2fd337eb9191f80c3024830a4889a8f34
+      f63cc8fe54e4d326f8d692805d70e092f851ddb1
+      40f0358cb314c824a5929ee527308d90e023bc10
+  hgtagsfnodes -- 'sortdict()'