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

[Pierre-Yves David]

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 at ens-lyon.org <mailto:pierre-yves.david at 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 at ens-lyon.org
>         <mailto:pierre-yves.david at ens-lyon.org>
>         <mailto: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 octobus.net
>         <mailto:pierre-yves.david at octobus.net>
>             <mailto:pierre-yves.david at octobus.net
>         <mailto:pierre-yves.david at 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
>             |........ at .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,

-- 
Pierre-Yves David