This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1417

prefetch: merge prefetchtrees command into prefetch
ClosedPublic

Authored by singhsrb on Nov 14 2017, 3:43 PM.
Tags
None
Subscribers
quark, durham

Details

Reviewers
durham
Group Reviewers
Restricted Project
Commits
rFBHGXc99011005e70: prefetch: merge prefetchtrees command into prefetch
Summary

Currently,

  • hg prefetch prefetches files.
  • hg prefetchtrees prefetches trees.

This commit removes prefetchtrees and makes prefetch responsible for
everything i.e. prefetch will prefetch whatever it can prefetch be it files,
trees, or both.

Test Plan

Ran all the tests.

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

singhsrb created this revision.Nov 14 2017, 3:43 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 14 2017, 3:43 PM
durham requested changes to this revision.Nov 14 2017, 7:44 PM
durham added a subscriber: durham.

Looks good. Just requesting changes for clarification on the test permutations.

tests/test-treemanifest-prefetch.t
27

I don't understand the three cases here. Shouldn't it just be remotefilelog on or off? Maybe have a comment if explaining them if they are all needed.

treemanifest/__init__.py
1026

I'd add comments to these two functions, since it's not obvious from the names how they differ.

1031

Kinda awkward that this is done in three locations now (here, below, and in remotefilelog). Maybe in _prefetchwrapper we delete the repack option from opts and then have a if opts.get('repack'): backgroundrepack(..) call in there. At least then the pattern is a little clearer. Versus the code below which requires that we know repack is an option and that not calling orig means we don't execute that option.

Not much cleaner, but a good start for now.

This revision now requires changes to proceed.Nov 14 2017, 7:44 PM
singhsrb planned changes to this revision.Nov 14 2017, 8:24 PM
singhsrb added inline comments.
tests/test-treemanifest-prefetch.t
27

There are three cases of interest:

  1. remotefilelog enabled and the repo is shallow: We prefetch trees and files in this case.
  2. remotefilelog enabled and the repo is not shallow: We only prefetch trees in this case.
  3. remotefilelog disabled: We only prefetch trees in this case.

I can add more documentation to explain these.

treemanifest/__init__.py
1026

I was hoping the comment at line 188 would help in sorting that out. But no harm in adding more comments.

1031

I would prefer that too. This crossed my mind at some point and then I just ignored it. Thanks for noticing this!

singhsrb updated this revision to Diff 3536.Nov 15 2017, 4:44 PM
durham accepted this revision.Nov 15 2017, 6:14 PM

Not sure how I feel about the wide spread use of the test conditions here, but let's see if it causes problems in the future.

tests/test-treemanifest-prefetch.t
52

We should be able to set remotefilelog.server=True regardless, since normal clones from clients will still work just fine.

109

Maybe not as part of this diff, but we should make treemanifest add the debug*pack commands if treemanifest is not enabled. So we wouldn't need this change.

128

Why drop this line?

387

What about the remotefilelog.false case?

This revision is now accepted and ready to land.Nov 15 2017, 6:14 PM
singhsrb added inline comments.Nov 15 2017, 7:37 PM
tests/test-treemanifest-prefetch.t
52

I wanted to highlight its not really required for the other tests. In hindsight, I would prefer your suggestion after all the other conditional blocks. I will update this!

109

Yeah, I considered that. I even considered having a separate extension for the debugcommands since they seem fairly independent. I will take care of this eventually because I hate the pattern too.

128

When we prefetch both trees and files, we end up fetching more trees in case we actually don't have the base commit. That's just because of how the code is structured for now. I thought we don't care about that even though its a stronger check and therefore, removed the line. The --base option states that the base revision is assumed to be local. If its not, the behavior is undefined.

387

I actually did not check because I was primarily looking at the prefetch command. I would expect pull to behave the same way with/without my changes with remotefilelog disabled on the client. I can check if that's the case.

durham added inline comments.Nov 16 2017, 2:33 PM
tests/test-treemanifest-prefetch.t
128

If we don't clear the cache, it's not necessarily obvious that the reason we only downloaded some of the data is because of --base, and not because the data is already in the repo.

387

Does (capability !) means "this line applies if that capability is not enabled"? Or "this line applies if that capability is enabled"? If it's the latter, I would've expected a line here for when remotefilelog.false is on, since a normal non-remotefilelog hg pull should have output here.

singhsrb added a subscriber: quark.Nov 16 2017, 5:26 PM
singhsrb added inline comments.
tests/test-treemanifest-prefetch.t
128

Okay, I will separate out the cases and add a comment.

387

@quark helped out with it this! The summary is that it turns out this was a feature (or bug based on what you make of it) of capability ! feature. I thought it indicates that the this line applies if that capability is enabled but it actually means this line applies if that capability is enabled. Otherwise, it is optional. This was making the line added 2 changesets with 2 changes to 1 files pass for both remotefilelog.true.shallowrepo.false (because we asked it to match) and also for remotefilelog.false (because it was optional). I will fix this to a more appropriate syntax.

singhsrb updated this revision to Diff 3579.Nov 16 2017, 5:29 PM

Revision Contents
Changeset List

Diff 3637

tests/test-treemanifest-prefetch.t

There are three cases which are of interest in this test:
- client remotefilelog enabled and the client repo is a shallowrepo
In this case, the expectation is that the prefetch command will be able to
prefetch both trees and files.
- client remotefilelog enabled and the client repo is not a shallowrepo
In this case, the expectation is that the prefetch command will be able to only
prefetch trees.
- client remotefilelog disabled
In this case, the expectation is that the prefetch command will be able to only
prefetch trees and will not be able to perform operations like repack which
require remotefilelog.
#testcases remotefilelog.true.shallowrepo.true remotefilelog.true.shallowrepo.false remotefilelog.false
$ CACHEDIR=`pwd`/hgcache $ CACHEDIR=`pwd`/hgcache
$ PYTHONPATH=$TESTDIR/..:$PYTHONPATH $ PYTHONPATH=$TESTDIR/..:$PYTHONPATH
$ export PYTHONPATH $ export PYTHONPATH
$ . "$TESTDIR/library.sh" $ . "$TESTDIR/library.sh"
$ hg init master $ hginit master
$ cd master $ cd master
$ mkdir dir $ mkdir dir
$ echo x > dir/x $ echo x > dir/x
durhamUnsubmitted
Not Done

I don't understand the three cases here. Shouldn't it just be remotefilelog on or off? Maybe have a comment if explaining them if they are all needed.

durham: I don't understand the three cases here. Shouldn't it just be remotefilelog on or off? Maybe…
singhsrbAuthorUnsubmitted
Not Done

There are three cases of interest:

  1. remotefilelog enabled and the repo is shallow: We prefetch trees and files in this case.
  2. remotefilelog enabled and the repo is not shallow: We only prefetch trees in this case.
  3. remotefilelog disabled: We only prefetch trees in this case.

I can add more documentation to explain these.

singhsrb: There are three cases of interest: # remotefilelog enabled and the repo is shallow: We…
$ hg commit -qAm 'add x' $ hg commit -qAm 'add x'
$ mkdir subdir $ mkdir subdir
$ echo z > subdir/z $ echo z > subdir/z
$ hg commit -qAm 'add subdir/z' $ hg commit -qAm 'add subdir/z'
$ echo x >> dir/x $ echo x >> dir/x
$ hg commit -Am 'modify x' $ hg commit -Am 'modify x'
$ cat >> .hg/hgrc <<EOF $ cat >> .hg/hgrc <<EOF
> [extensions] > [extensions]
> treemanifest=$TESTDIR/../treemanifest > treemanifest=$TESTDIR/../treemanifest
> >
> [remotefilelog] > [remotefilelog]
> server=True
> name=master > name=master
> cachepath=$CACHEDIR > cachepath=$CACHEDIR
> usefastdatapack=True > usefastdatapack=True
> >
> [fastmanifest] > [fastmanifest]
> usetree=True > usetree=True
> usecache=False > usecache=False
> >
> [treemanifest] > [treemanifest]
> server=True > server=True
> EOF > EOF
$ cd .. $ cd ..
durhamUnsubmitted
Not Done

We should be able to set remotefilelog.server=True regardless, since normal clones from clients will still work just fine.

durham: We should be able to set remotefilelog.server=True regardless, since normal clones from clients…
singhsrbAuthorUnsubmitted
Not Done

I wanted to highlight its not really required for the other tests. In hindsight, I would prefer your suggestion after all the other conditional blocks. I will update this!

singhsrb: I wanted to highlight its not really required for the other tests. In hindsight, I would prefer…
#if remotefilelog.true.shallowrepo.true
$ hgcloneshallow ssh://user@dummy/master client
streaming all changes
2 files to transfer, 749 bytes of data
transferred 749 bytes in * seconds (*) (glob)
searching for changes
no changes found
updating to branch default
2 files updated, 0 files merged, 0 files removed, 0 files unresolved
2 files fetched over 1 fetches - (2 misses, 0.00% hit ratio) over * (glob)
#else
$ hg clone ssh://user@dummy/master client $ hg clone ssh://user@dummy/master client
streaming all changes streaming all changes
4 files to transfer, 952 bytes of data 4 files to transfer, 952 bytes of data
transferred 952 bytes in * seconds (*) (glob) transferred 952 bytes in * seconds (*) (glob)
searching for changes searching for changes
no changes found no changes found
updating to branch default updating to branch default
2 files updated, 0 files merged, 0 files removed, 0 files unresolved 2 files updated, 0 files merged, 0 files removed, 0 files unresolved
#endif
$ cd master $ cd master
$ hg backfilltree $ hg backfilltree
$ cd ../client $ cd ../client
$ cat >> .hg/hgrc <<EOF $ cat >> .hg/hgrc <<EOF
> [extensions] > [extensions]
> treemanifest=$TESTDIR/../treemanifest > treemanifest=$TESTDIR/../treemanifest
> fastmanifest=$TESTDIR/../fastmanifest > fastmanifest=$TESTDIR/../fastmanifest
> [remotefilelog] > [remotefilelog]
> reponame = master > reponame = master
> cachepath = $CACHEDIR > cachepath = $CACHEDIR
> [fastmanifest] > [fastmanifest]
> usetree = True > usetree = True
> usecache = False > usecache = False
> EOF > EOF
Test prefetchtrees #if remotefilelog.false
$ hg prefetchtrees -r '0 + 1 + 2' $ cat >> .hg/hgrc <<EOF
>
> [extensions]
> remotefilelog=!
> EOF
#endif
Test prefetch
$ hg prefetch -r '0 + 1 + 2'
6 trees fetched over * (glob) 6 trees fetched over * (glob)
1 files fetched over 1 fetches - (1 misses, 0.00% hit ratio) over * (glob) (remotefilelog.true.shallowrepo.true !)
$ ls $CACHEDIR/master/packs/manifests $ ls $CACHEDIR/master/packs/manifests
29938257d506f677320d5abec8e34a1a9ed635fe.histidx 29938257d506f677320d5abec8e34a1a9ed635fe.histidx
29938257d506f677320d5abec8e34a1a9ed635fe.histpack 29938257d506f677320d5abec8e34a1a9ed635fe.histpack
8adc618d23082c0a5311a4bbf9ac08b9b9672471.dataidx 8adc618d23082c0a5311a4bbf9ac08b9b9672471.dataidx
8adc618d23082c0a5311a4bbf9ac08b9b9672471.datapack 8adc618d23082c0a5311a4bbf9ac08b9b9672471.datapack
$ hg debugdatapack --long $CACHEDIR/master/packs/manifests/*.dataidx $ hg debugdatapack --config extensions.remotefilelog=$TESTDIR/../remotefilelog \
> --long $CACHEDIR/master/packs/manifests/*.dataidx
durhamUnsubmitted
Not Done

Maybe not as part of this diff, but we should make treemanifest add the debug*pack commands if treemanifest is not enabled. So we wouldn't need this change.

durham: Maybe not as part of this diff, but we should make treemanifest add the debug*pack commands if…
singhsrbAuthorUnsubmitted
Not Done

Yeah, I considered that. I even considered having a separate extension for the debugcommands since they seem fairly independent. I will take care of this eventually because I hate the pattern too.

singhsrb: Yeah, I considered that. I even considered having a separate extension for the debugcommands…
$TESTTMP/hgcache/master/packs/manifests/8adc618d23082c0a5311a4bbf9ac08b9b9672471: $TESTTMP/hgcache/master/packs/manifests/8adc618d23082c0a5311a4bbf9ac08b9b9672471:
subdir: subdir:
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
ddb35f099a648a43a997aef53123bce309c794fd 0000000000000000000000000000000000000000 43 (missing) ddb35f099a648a43a997aef53123bce309c794fd 0000000000000000000000000000000000000000 43 (missing)
(empty name): (empty name):
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
1be4ab2126dd2252dcae6be2aac2561dd3ddcda0 0000000000000000000000000000000000000000 95 (missing) 1be4ab2126dd2252dcae6be2aac2561dd3ddcda0 0000000000000000000000000000000000000000 95 (missing)
dir: dir:
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
bc0c2c938b929f98b1c31a8c5994396ebb096bf0 0000000000000000000000000000000000000000 43 (missing) bc0c2c938b929f98b1c31a8c5994396ebb096bf0 0000000000000000000000000000000000000000 43 (missing)
(empty name): (empty name):
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
ef362f8bbe8aa457b0cfc49f200cbeb7747984ed 0000000000000000000000000000000000000000 46 (missing) ef362f8bbe8aa457b0cfc49f200cbeb7747984ed 0000000000000000000000000000000000000000 46 (missing)
$ hg debughistorypack $CACHEDIR/master/packs/manifests/*.histidx $ hg debughistorypack --config extensions.remotefilelog=$TESTDIR/../remotefilelog \
> $CACHEDIR/master/packs/manifests/*.histidx
Node P1 Node P2 Node Link Node Copy From Node P1 Node P2 Node Link Node Copy From
60a7f7acb6bb 1be4ab2126dd 000000000000 bd6f9b289c01 60a7f7acb6bb 1be4ab2126dd 000000000000 bd6f9b289c01
1be4ab2126dd ef362f8bbe8a 000000000000 f15c65c6e9bd 1be4ab2126dd ef362f8bbe8a 000000000000 f15c65c6e9bd
ef362f8bbe8a 000000000000 000000000000 ecfb693caff5 ef362f8bbe8a 000000000000 000000000000 ecfb693caff5
dir dir
Node P1 Node P2 Node Link Node Copy From Node P1 Node P2 Node Link Node Copy From
a18d21674e76 bc0c2c938b92 000000000000 bd6f9b289c01 a18d21674e76 bc0c2c938b92 000000000000 bd6f9b289c01
bc0c2c938b92 000000000000 000000000000 ecfb693caff5 bc0c2c938b92 000000000000 000000000000 ecfb693caff5
subdir subdir
Node P1 Node P2 Node Link Node Copy From Node P1 Node P2 Node Link Node Copy From
ddb35f099a64 000000000000 000000000000 f15c65c6e9bd ddb35f099a64 000000000000 000000000000 f15c65c6e9bd
$ hg debugdatapack --node ef362f8bbe8aa457b0cfc49f200cbeb7747984ed $CACHEDIR/master/packs/manifests/*.dataidx $ hg debugdatapack --config extensions.remotefilelog=$TESTDIR/../remotefilelog \
> --node ef362f8bbe8aa457b0cfc49f200cbeb7747984ed $CACHEDIR/master/packs/manifests/*.dataidx
$TESTTMP/hgcache/master/packs/manifests/8adc618d23082c0a5311a4bbf9ac08b9b9672471: $TESTTMP/hgcache/master/packs/manifests/8adc618d23082c0a5311a4bbf9ac08b9b9672471:
Node Delta Base Delta SHA1 Delta Length Node Delta Base Delta SHA1 Delta Length
ef362f8bbe8aa457b0cfc49f200cbeb7747984ed 0000000000000000000000000000000000000000 3b295111780498d177793f9228bf736b915f0255 46 ef362f8bbe8aa457b0cfc49f200cbeb7747984ed 0000000000000000000000000000000000000000 3b295111780498d177793f9228bf736b915f0255 46
$ hg -R ../master debugindex ../master/.hg/store/00manifesttree.i $ hg -R ../master debugindex ../master/.hg/store/00manifesttree.i
rev offset length delta linkrev nodeid p1 p2 rev offset length delta linkrev nodeid p1 p2
0 0 47 -1 0 ef362f8bbe8a 000000000000 000000000000 0 0 47 -1 0 ef362f8bbe8a 000000000000 000000000000
1 47 61 0 1 1be4ab2126dd ef362f8bbe8a 000000000000 1 47 61 0 1 1be4ab2126dd ef362f8bbe8a 000000000000
2 108 58 1 2 60a7f7acb6bb 1be4ab2126dd 000000000000 2 108 58 1 2 60a7f7acb6bb 1be4ab2126dd 000000000000
$ hg -R ../master debugindex ../master/.hg/store/00manifest.i $ hg -R ../master debugindex ../master/.hg/store/00manifest.i
rev offset length delta linkrev nodeid p1 p2 rev offset length delta linkrev nodeid p1 p2
0 0 48 -1 0 ef362f8bbe8a 000000000000 000000000000 0 0 48 -1 0 ef362f8bbe8a 000000000000 000000000000
1 48 62 0 1 1be4ab2126dd ef362f8bbe8a 000000000000 1 48 62 0 1 1be4ab2126dd ef362f8bbe8a 000000000000
2 110 59 1 2 60a7f7acb6bb 1be4ab2126dd 000000000000 2 110 59 1 2 60a7f7acb6bb 1be4ab2126dd 000000000000
Test prefetch with base node (subdir/ shouldn't show up in the pack) Test prefetch with base node (subdir/ shouldn't show up in the pack)
$ rm -rf $CACHEDIR/master $ rm -rf $CACHEDIR/master
durhamUnsubmitted
Not Done

Why drop this line?

durham: Why drop this line?
singhsrbAuthorUnsubmitted
Not Done

When we prefetch both trees and files, we end up fetching more trees in case we actually don't have the base commit. That's just because of how the code is structured for now. I thought we don't care about that even though its a stronger check and therefore, removed the line. The --base option states that the base revision is assumed to be local. If its not, the behavior is undefined.

singhsrb: When we prefetch both trees and files, we end up fetching more trees in case we actually don't…
durhamUnsubmitted
Not Done

If we don't clear the cache, it's not necessarily obvious that the reason we only downloaded some of the data is because of --base, and not because the data is already in the repo.

durham: If we don't clear the cache, it's not necessarily obvious that the reason we only downloaded…
singhsrbAuthorUnsubmitted
Not Done

Okay, I will separate out the cases and add a comment.

singhsrb: Okay, I will separate out the cases and add a comment.
$ hg prefetchtrees -r '2' --base '1'
#if remotefilelog.true.shallowrepo.true
Multiple trees are fetched in this case because the file prefetching code path
requires tree manifest for the base commit.
$ hg prefetch -r '2' --base '1'
2 trees fetched over * (glob) 2 trees fetched over * (glob)
2 trees fetched over * (glob)
3 trees fetched over * (glob)
1 files fetched over 1 fetches - (1 misses, 0.00% hit ratio) over * (glob)
$ ls $CACHEDIR/master/packs/manifests/*.dataidx $ ls $CACHEDIR/master/packs/manifests/*.dataidx
$TESTTMP/hgcache/master/packs/manifests/148e9eb32f473ea522c591c95be0f9e772be9675.dataidx
$TESTTMP/hgcache/master/packs/manifests/3fb59713808147bda39cbd97b9cd862406f5865c.dataidx $TESTTMP/hgcache/master/packs/manifests/3fb59713808147bda39cbd97b9cd862406f5865c.dataidx
$TESTTMP/hgcache/master/packs/manifests/5f14647c5653622d4c2682648ec82c7193d2a9ab.dataidx
#else
$ hg prefetch -r '2' --base '1'
2 trees fetched over * (glob)
$ ls $CACHEDIR/master/packs/manifests/*.dataidx
$TESTTMP/hgcache/master/packs/manifests/3fb59713808147bda39cbd97b9cd862406f5865c.dataidx
#endif
$ hg debugdatapack $CACHEDIR/master/packs/manifests/3fb59713808147bda39cbd97b9cd862406f5865c.dataidx $ hg debugdatapack --config extensions.remotefilelog=$TESTDIR/../remotefilelog \
> $CACHEDIR/master/packs/manifests/3fb59713808147bda39cbd97b9cd862406f5865c.dataidx
$TESTTMP/hgcache/master/packs/manifests/3fb59713808147bda39cbd97b9cd862406f5865c: $TESTTMP/hgcache/master/packs/manifests/3fb59713808147bda39cbd97b9cd862406f5865c:
dir: dir:
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
a18d21674e76 000000000000 43 (missing) a18d21674e76 000000000000 43 (missing)
(empty name): (empty name):
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
60a7f7acb6bb 000000000000 95 (missing) 60a7f7acb6bb 000000000000 95 (missing)
tag: tip tag: tip
user: test user: test
date: Thu Jan 01 00:00:00 1970 +0000 date: Thu Jan 01 00:00:00 1970 +0000
summary: modify x summary: modify x
dir/x | 1 + dir/x | 1 +
1 files changed, 1 insertions(+), 0 deletions(-) 1 files changed, 1 insertions(+), 0 deletions(-)
2 files fetched over 1 fetches - (2 misses, 0.00% hit ratio) over * (glob) (remotefilelog.true.shallowrepo.true !)
$ ls $CACHEDIR/master/packs/manifests $ ls $CACHEDIR/master/packs/manifests
148e9eb32f473ea522c591c95be0f9e772be9675.dataidx 148e9eb32f473ea522c591c95be0f9e772be9675.dataidx
148e9eb32f473ea522c591c95be0f9e772be9675.datapack 148e9eb32f473ea522c591c95be0f9e772be9675.datapack
3fb59713808147bda39cbd97b9cd862406f5865c.dataidx 3fb59713808147bda39cbd97b9cd862406f5865c.dataidx
3fb59713808147bda39cbd97b9cd862406f5865c.datapack 3fb59713808147bda39cbd97b9cd862406f5865c.datapack
524ab81400c6bc8449e3e720d81f836ebacec539.histidx 524ab81400c6bc8449e3e720d81f836ebacec539.histidx
524ab81400c6bc8449e3e720d81f836ebacec539.histpack 524ab81400c6bc8449e3e720d81f836ebacec539.histpack
e5c44a5c1bbfd8841df1c6c4b7cca54536e016db.histidx e5c44a5c1bbfd8841df1c6c4b7cca54536e016db.histidx
e5c44a5c1bbfd8841df1c6c4b7cca54536e016db.histpack e5c44a5c1bbfd8841df1c6c4b7cca54536e016db.histpack
$ hg debugdatapack $CACHEDIR/master/packs/manifests/148e9eb32f473ea522c591c95be0f9e772be9675 $ hg debugdatapack --config extensions.remotefilelog=$TESTDIR/../remotefilelog \
> $CACHEDIR/master/packs/manifests/148e9eb32f473ea522c591c95be0f9e772be9675
$TESTTMP/hgcache/master/packs/manifests/148e9eb32f473ea522c591c95be0f9e772be9675: $TESTTMP/hgcache/master/packs/manifests/148e9eb32f473ea522c591c95be0f9e772be9675:
dir: dir:
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
bc0c2c938b92 000000000000 43 (missing) bc0c2c938b92 000000000000 43 (missing)
subdir: subdir:
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
ddb35f099a64 000000000000 43 (missing) ddb35f099a64 000000000000 43 (missing)
(empty name): (empty name):
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
1be4ab2126dd 000000000000 95 (missing) 1be4ab2126dd 000000000000 95 (missing)
- Note that subdir/ is not downloaded again - Note that subdir/ is not downloaded again
$ hg debugdatapack $CACHEDIR/master/packs/manifests/3fb59713808147bda39cbd97b9cd862406f5865c $ hg debugdatapack --config extensions.remotefilelog=$TESTDIR/../remotefilelog \
> $CACHEDIR/master/packs/manifests/3fb59713808147bda39cbd97b9cd862406f5865c
$TESTTMP/hgcache/master/packs/manifests/3fb59713808147bda39cbd97b9cd862406f5865c: $TESTTMP/hgcache/master/packs/manifests/3fb59713808147bda39cbd97b9cd862406f5865c:
dir: dir:
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
a18d21674e76 000000000000 43 (missing) a18d21674e76 000000000000 43 (missing)
(empty name): (empty name):
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
60a7f7acb6bb 000000000000 95 (missing) 60a7f7acb6bb 000000000000 95 (missing)
Test that auto prefetch scans up the changelog for base trees Test that auto prefetch scans up the changelog for base trees
$ rm -rf $CACHEDIR/master $ rm -rf $CACHEDIR/master
$ hg prefetchtrees -r 'tip^' $ hg prefetch -r 'tip^'
3 trees fetched over * (glob) 3 trees fetched over * (glob)
2 files fetched over 1 fetches - (2 misses, 0.00% hit ratio) over * (glob) (remotefilelog.true.shallowrepo.true !)
$ rm -rf $CACHEDIR/master $ rm -rf $CACHEDIR/master
$ hg prefetchtrees -r tip $ hg prefetch -r tip
3 trees fetched over * (glob) 3 trees fetched over * (glob)
2 files fetched over 1 fetches - (2 misses, 0.00% hit ratio) over * (glob) (remotefilelog.true.shallowrepo.true !)
- Only 2 of the 3 trees from tip^ are downloaded as part of --stat's fetch - Only 2 of the 3 trees from tip^ are downloaded as part of --stat's fetch
$ hg log -r tip --stat --pager=off > /dev/null $ hg log -r tip --stat --pager=off > /dev/null
2 trees fetched over * (glob) 2 trees fetched over * (glob)
1 files fetched over 1 fetches - (1 misses, 0.00% hit ratio) over * (glob) (remotefilelog.true.shallowrepo.true !)
Test auto prefetch during pull Test auto prefetch during pull
- Prefetch everything - Prefetch everything
$ echo a >> a $ echo a >> a
$ hg commit -Aqm 'draft commit that shouldnt affect prefetch' $ hg commit -Aqm 'draft commit that shouldnt affect prefetch'
$ rm -rf $CACHEDIR/master $ rm -rf $CACHEDIR/master
$ hg pull --config treemanifest.pullprefetchcount=10 --traceback $ hg pull --config treemanifest.pullprefetchcount=10 --traceback
pulling from ssh://user@dummy/master pulling from ssh://user@dummy/master
searching for changes searching for changes
no changes found no changes found
prefetching trees prefetching trees
6 trees fetched over * (glob) 6 trees fetched over * (glob)
$ hg debugdatapack $CACHEDIR/master/packs/manifests/*.dataidx $ hg debugdatapack --config extensions.remotefilelog=$TESTDIR/../remotefilelog \
> $CACHEDIR/master/packs/manifests/*.dataidx
$TESTTMP/hgcache/master/packs/manifests/8adc618d23082c0a5311a4bbf9ac08b9b9672471: $TESTTMP/hgcache/master/packs/manifests/8adc618d23082c0a5311a4bbf9ac08b9b9672471:
subdir: subdir:
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
ddb35f099a64 000000000000 43 (missing) ddb35f099a64 000000000000 43 (missing)
(empty name): (empty name):
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
1be4ab2126dd 000000000000 95 (missing) 1be4ab2126dd 000000000000 95 (missing)
- Prefetch just the top manifest (but the full one) - Prefetch just the top manifest (but the full one)
$ rm -rf $CACHEDIR/master $ rm -rf $CACHEDIR/master
$ hg pull --config treemanifest.pullprefetchcount=1 --traceback $ hg pull --config treemanifest.pullprefetchcount=1 --traceback
pulling from ssh://user@dummy/master pulling from ssh://user@dummy/master
searching for changes searching for changes
no changes found no changes found
prefetching trees prefetching trees
3 trees fetched over * (glob) 3 trees fetched over * (glob)
$ hg debugdatapack $CACHEDIR/master/packs/manifests/*.dataidx $ hg debugdatapack --config extensions.remotefilelog=$TESTDIR/../remotefilelog \
> $CACHEDIR/master/packs/manifests/*.dataidx
$TESTTMP/hgcache/master/packs/manifests/4ee15de76c068ec1c80e3e61f2c3c476a779078a: $TESTTMP/hgcache/master/packs/manifests/4ee15de76c068ec1c80e3e61f2c3c476a779078a:
dir: dir:
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
a18d21674e76 000000000000 43 (missing) a18d21674e76 000000000000 43 (missing)
subdir: subdir:
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
ddb35f099a64 000000000000 43 (missing) ddb35f099a64 000000000000 43 (missing)
(empty name): (empty name):
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
60a7f7acb6bb 000000000000 95 (missing) 60a7f7acb6bb 000000000000 95 (missing)
- Prefetch commit 1 then minimally prefetch commit 2 - Prefetch commit 1 then minimally prefetch commit 2
$ rm -rf $CACHEDIR/master $ rm -rf $CACHEDIR/master
$ hg prefetchtrees -r 1 $ hg prefetch -r 1
3 trees fetched over * (glob) 3 trees fetched over * (glob)
2 files fetched over 1 fetches - (2 misses, 0.00% hit ratio) over * (glob) (remotefilelog.true.shallowrepo.true !)
$ ls $CACHEDIR/master/packs/manifests/*dataidx $ ls $CACHEDIR/master/packs/manifests/*dataidx
$TESTTMP/hgcache/master/packs/manifests/148e9eb32f473ea522c591c95be0f9e772be9675.dataidx $TESTTMP/hgcache/master/packs/manifests/148e9eb32f473ea522c591c95be0f9e772be9675.dataidx
$ hg pull --config treemanifest.pullprefetchcount=1 --traceback $ hg pull --config treemanifest.pullprefetchcount=1 --traceback
pulling from ssh://user@dummy/master pulling from ssh://user@dummy/master
searching for changes searching for changes
no changes found no changes found
prefetching trees prefetching trees
2 trees fetched over * (glob) 2 trees fetched over * (glob)
$ ls $CACHEDIR/master/packs/manifests/*dataidx $ ls $CACHEDIR/master/packs/manifests/*dataidx
$TESTTMP/hgcache/master/packs/manifests/148e9eb32f473ea522c591c95be0f9e772be9675.dataidx $TESTTMP/hgcache/master/packs/manifests/148e9eb32f473ea522c591c95be0f9e772be9675.dataidx
$TESTTMP/hgcache/master/packs/manifests/3fb59713808147bda39cbd97b9cd862406f5865c.dataidx $TESTTMP/hgcache/master/packs/manifests/3fb59713808147bda39cbd97b9cd862406f5865c.dataidx
$ hg debugdatapack $CACHEDIR/master/packs/manifests/3fb59713808147bda39cbd97b9cd862406f5865c.dataidx $ hg debugdatapack --config extensions.remotefilelog=$TESTDIR/../remotefilelog \
> $CACHEDIR/master/packs/manifests/3fb59713808147bda39cbd97b9cd862406f5865c.dataidx
$TESTTMP/hgcache/master/packs/manifests/3fb59713808147bda39cbd97b9cd862406f5865c: $TESTTMP/hgcache/master/packs/manifests/3fb59713808147bda39cbd97b9cd862406f5865c:
dir: dir:
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
a18d21674e76 000000000000 43 (missing) a18d21674e76 000000000000 43 (missing)
(empty name): (empty name):
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
60a7f7acb6bb 000000000000 95 (missing) 60a7f7acb6bb 000000000000 95 (missing)
$ cd ../client $ cd ../client
$ rm -rf $CACHEDIR/master $ rm -rf $CACHEDIR/master
$ hg pull --config treemanifest.pullprefetchrevs='tip~2' $ hg pull --config treemanifest.pullprefetchrevs='tip~2'
pulling from ssh://user@dummy/master pulling from ssh://user@dummy/master
searching for changes searching for changes
adding changesets adding changesets
adding manifests adding manifests
adding file changes adding file changes
added 2 changesets with 2 changes to 1 files added 2 changesets with 0 changes to 0 files (remotefilelog.true.shallowrepo.true !)
added 2 changesets with 2 changes to 1 files (no-remotefilelog.true.shallowrepo.true !)
durhamUnsubmitted
Not Done

What about the remotefilelog.false case?

durham: What about the remotefilelog.false case?
singhsrbAuthorUnsubmitted
Not Done

I actually did not check because I was primarily looking at the prefetch command. I would expect pull to behave the same way with/without my changes with remotefilelog disabled on the client. I can check if that's the case.

singhsrb: I actually did not check because I was primarily looking at the prefetch command. I would…
durhamUnsubmitted
Not Done

Does (capability !) means "this line applies if that capability is not enabled"? Or "this line applies if that capability is enabled"? If it's the latter, I would've expected a line here for when remotefilelog.false is on, since a normal non-remotefilelog hg pull should have output here.

durham: Does `(capability !)` means "this line applies if that capability is not enabled"? Or "this…
singhsrbAuthorUnsubmitted
Not Done

@quark helped out with it this! The summary is that it turns out this was a feature (or bug based on what you make of it) of capability ! feature. I thought it indicates that the this line applies if that capability is enabled but it actually means this line applies if that capability is enabled. Otherwise, it is optional. This was making the line added 2 changesets with 2 changes to 1 files pass for both remotefilelog.true.shallowrepo.false (because we asked it to match) and also for remotefilelog.false (because it was optional). I will fix this to a more appropriate syntax.

singhsrb: @quark helped out with it this! The summary is that it turns out this was a feature (or bug…
new changesets dece825f8add:cfacdcc4cee5 new changesets dece825f8add:cfacdcc4cee5
(run 'hg update' to get a working copy) (run 'hg update' to get a working copy)
prefetching trees prefetching trees
3 trees fetched over * (glob) 3 trees fetched over * (glob)
$ hg debugdatapack $CACHEDIR/master/packs/manifests/*.dataidx $ hg debugdatapack --config extensions.remotefilelog=$TESTDIR/../remotefilelog \
> $CACHEDIR/master/packs/manifests/*.dataidx
$TESTTMP/hgcache/master/packs/manifests/4ee15de76c068ec1c80e3e61f2c3c476a779078a: $TESTTMP/hgcache/master/packs/manifests/4ee15de76c068ec1c80e3e61f2c3c476a779078a:
dir: dir:
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
a18d21674e76 000000000000 43 (missing) a18d21674e76 000000000000 43 (missing)
subdir: subdir:
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
ddb35f099a64 000000000000 43 (missing) ddb35f099a64 000000000000 43 (missing)
(empty name): (empty name):
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
60a7f7acb6bb 000000000000 95 (missing) 60a7f7acb6bb 000000000000 95 (missing)
- Test prefetching only the new tree parts for a commit who's parent tree is not - Test prefetching only the new tree parts for a commit who's parent tree is not
- downloaded already. Note that subdir/z was not downloaded this time. - downloaded already. Note that subdir/z was not downloaded this time.
$ hg pull --config treemanifest.pullprefetchrevs='tip' $ hg pull --config treemanifest.pullprefetchrevs='tip'
pulling from ssh://user@dummy/master pulling from ssh://user@dummy/master
searching for changes searching for changes
no changes found no changes found
prefetching trees prefetching trees
2 trees fetched over * (glob) 2 trees fetched over * (glob)
$ hg debugdatapack $CACHEDIR/master/packs/manifests/99050e724a9236121684523ba3f4db270e62fb58.dataidx $ hg debugdatapack --config extensions.remotefilelog=$TESTDIR/../remotefilelog \
> $CACHEDIR/master/packs/manifests/99050e724a9236121684523ba3f4db270e62fb58.dataidx
$TESTTMP/hgcache/master/packs/manifests/99050e724a9236121684523ba3f4db270e62fb58: $TESTTMP/hgcache/master/packs/manifests/99050e724a9236121684523ba3f4db270e62fb58:
dir: dir:
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
bf22bc15398b 000000000000 43 (missing) bf22bc15398b 000000000000 43 (missing)
(empty name): (empty name):
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
aa52a49be522 000000000000 95 (missing) aa52a49be522 000000000000 95 (missing)
Test that prefetch refills just part of a tree when the cache is deleted Test that prefetch refills just part of a tree when the cache is deleted
$ echo >> dir/x $ echo >> dir/x
$ hg commit -m 'edit x locally' $ hg commit -m 'edit x locally'
created new head created new head
1 files fetched over 1 fetches - (1 misses, 0.00% hit ratio) over * (glob) (remotefilelog.true.shallowrepo.true !)
$ rm -rf $CACHEDIR/master/* $ rm -rf $CACHEDIR/master/*
$ hg cat subdir/z $ hg cat subdir/z
3 trees fetched over * (glob) 3 trees fetched over * (glob)
z z
1 files fetched over 1 fetches - (1 misses, 0.00% hit ratio) over * (glob) (remotefilelog.true.shallowrepo.true !)
Test prefetch non-parent commits with no base node (should fetch minimal Test prefetch non-parent commits with no base node (should fetch minimal
trees - in this case 3 trees for commit 2, and 2 for commit 4 despite it having trees - in this case 3 trees for commit 2, and 2 for commit 4 despite it having
3 directories) 3 directories)
$ rm -rf $CACHEDIR/master $ rm -rf $CACHEDIR/master
$ hg prefetchtrees -r '2 + 4' $ hg prefetch -r '2 + 4'
5 trees fetched over * (glob) 5 trees fetched over * (glob)
3 files fetched over 1 fetches - (3 misses, 0.00% hit ratio) over * (glob) (remotefilelog.true.shallowrepo.true !)
Test repack option Test repack option
$ rm -rf $CACHEDIR/master $ rm -rf $CACHEDIR/master
$ hg prefetchtrees -r '0' $ hg prefetch -r '0'
2 trees fetched over *s (glob) 2 trees fetched over * (glob)
$ hg prefetchtrees -r '2' 1 files fetched over 1 fetches - (1 misses, 0.00% hit ratio) over * (glob) (remotefilelog.true.shallowrepo.true !)
3 trees fetched over *s (glob) $ hg prefetch -r '2'
3 trees fetched over * (glob)
2 files fetched over 1 fetches - (2 misses, 0.00% hit ratio) over * (glob) (remotefilelog.true.shallowrepo.true !)
$ hg prefetchtrees -r '4' --repack #if remotefilelog.false
3 trees fetched over *s (glob) $ hg prefetch -r '4' --repack
abort: repack requires remotefilelog extension
[255]
#else
$ hg prefetch -r '4' --repack
3 trees fetched over * (glob)
(running background incremental repack) (running background incremental repack)
1 files fetched over 1 fetches - (1 misses, 0.00% hit ratio) over * (glob) (remotefilelog.true.shallowrepo.true !)
$ sleep 0.5 $ sleep 0.5
$ hg debugwaitonrepack $ hg debugwaitonrepack
$ ls_l $CACHEDIR/master/packs/manifests | grep datapack | wc -l $ ls_l $CACHEDIR/master/packs/manifests | grep datapack | wc -l
\s*1 (re) \s*1 (re)
#endif

treemanifest/__init__.py

localrepo, localrepo,
manifest, manifest,
mdiff, mdiff,
phases, phases,
policy, policy,
registrar, registrar,
repair, repair,
revlog, revlog,
scmutil,
sshserver, sshserver,
templatekw, templatekw,
util, util,
wireproto, wireproto,
) )
from mercurial.i18n import _ from mercurial.i18n import _
from mercurial.node import bin, hex, nullid from mercurial.node import bin, hex, nullid
from remotefilelog import (
cmdtable as remotefilelogcmdtable,
resolveprefetchopts,
)
from remotefilelog.contentstore import ( from remotefilelog.contentstore import (
manifestrevlogstore, manifestrevlogstore,
unioncontentstore, unioncontentstore,
) )
from remotefilelog.metadatastore import ( from remotefilelog.metadatastore import (
unionmetadatastore, unionmetadatastore,
) )
from remotefilelog.datapack import ( from remotefilelog.datapack import (
_registerbundle2parts() _registerbundle2parts()
extensions.wrapfunction(templatekw, 'showmanifest', showmanifest) extensions.wrapfunction(templatekw, 'showmanifest', showmanifest)
templatekw.keywords['manifest'] = templatekw.showmanifest templatekw.keywords['manifest'] = templatekw.showmanifest
# Change manifest template output # Change manifest template output
templatekw.defaulttempl['manifest'] = '{node}' templatekw.defaulttempl['manifest'] = '{node}'
def _wrapremotefilelog(loaded):
if loaded:
remotefilelogmod = extensions.find('remotefilelog')
extensions.wrapcommand(
remotefilelogmod.cmdtable, 'prefetch', _prefetchwrapper)
else:
# There is no prefetch command to wrap around. In this case, we use
# the command table entry for prefetch in the remotefilelog to
# define the prefetch command, wrap it, and then override it
# completely. This ensures that the options to the prefetch command
# are consistent.
cmdtable['prefetch'] = remotefilelogcmdtable['prefetch']
extensions.wrapcommand(cmdtable, 'prefetch', _overrideprefetch)
extensions.afterloaded('remotefilelog', _wrapremotefilelog)
def showmanifest(orig, **args): def showmanifest(orig, **args):
"""Same implementation as the upstream showmanifest, but without the 'rev' """Same implementation as the upstream showmanifest, but without the 'rev'
field.""" field."""
ctx, templ = args[r'ctx'], args[r'templ'] ctx, templ = args[r'ctx'], args[r'templ']
mnode = ctx.manifestnode() mnode = ctx.manifestnode()
if mnode is None: if mnode is None:
# just avoid crash, we might want to use the 'ff...' hash in future # just avoid crash, we might want to use the 'ff...' hash in future
return return
return wrapper(origfn, *args, **kwargs) return wrapper(origfn, *args, **kwargs)
currcls.__dict__[propname].func = wrap currcls.__dict__[propname].func = wrap
break break
if currcls is object: if currcls is object:
raise AttributeError(_("%s has no property '%s'") % raise AttributeError(_("%s has no property '%s'") %
(type(currcls), propname)) (type(currcls), propname))
@command('prefetchtrees', [ # Wrapper around the 'prefetch' command which also allows for prefetching the
('r', 'rev', '', _("revs to prefetch the trees for")), # trees along with the files.
('', 'repack', False, _('run repack after prefetch')), def _prefetchwrapper(orig, ui, repo, *pats, **opts):
('b', 'base', '', _("rev that is assumed to already be local")), # The wrapper will take care of the repacking.
] + commands.walkopts, _('--rev REVS PATTERN..')) repackrequested = opts.pop('repack')
durhamUnsubmitted
Not Done

I'd add comments to these two functions, since it's not obvious from the names how they differ.

durham: I'd add comments to these two functions, since it's not obvious from the names how they differ.
singhsrbAuthorUnsubmitted
Not Done

I was hoping the comment at line 188 would help in sorting that out. But no harm in adding more comments.

singhsrb: I was hoping the comment at line 188 would help in sorting that out. But no harm in adding more…
def prefetchtrees(ui, repo, *args, **opts):
revs = repo.revs(opts.get('rev')) _prefetchonlytrees(repo, opts)
_prefetchonlyfiles(orig, ui, repo, *pats, **opts)
if repackrequested:
durhamUnsubmitted
Not Done

Kinda awkward that this is done in three locations now (here, below, and in remotefilelog). Maybe in _prefetchwrapper we delete the repack option from opts and then have a if opts.get('repack'): backgroundrepack(..) call in there. At least then the pattern is a little clearer. Versus the code below which requires that we know repack is an option and that not calling orig means we don't execute that option.

Not much cleaner, but a good start for now.

durham: Kinda awkward that this is done in three locations now (here, below, and in remotefilelog).
singhsrbAuthorUnsubmitted
Not Done

I would prefer that too. This crossed my mind at some point and then I just ignored it. Thanks for noticing this!

singhsrb: I would prefer that too. This crossed my mind at some point and then I just ignored it. Thanks…
backgroundrepack(repo, incremental=True)
# Wrapper around the 'prefetch' command which overrides the command completely
# and only allows for prefetching trees. This is only required when the
# 'prefetch' command is not available because the remotefilelog extension is not
# loaded and we want to be able to at least prefetch trees. The wrapping just
# ensures that we get a consistent interface to the 'prefetch' command.
def _overrideprefetch(orig, ui, repo, *pats, **opts):
if opts.get('repack'):
raise error.Abort(_('repack requires remotefilelog extension'))
_prefetchonlytrees(repo, opts)
def _prefetchonlyfiles(orig, ui, repo, *pats, **opts):
if shallowrepo.requirement in repo.requirements:
orig(ui, repo, *pats, **opts)
def _prefetchonlytrees(repo, opts):
opts = resolveprefetchopts(repo.ui, opts)
revs = scmutil.revrange(repo, opts.get('rev'))
mfnodes = set() mfnodes = set()
for rev in revs: for rev in revs:
mfnodes.add(repo[rev].manifestnode()) mfnodes.add(repo[rev].manifestnode())
basemfnode = set() basemfnode = set()
base = opts.get('base') base = opts.get('base')
if base: if base is not None:
basemfnode.add(repo[base].manifestnode()) basemfnode.add(repo[base].manifestnode())
_prefetchtrees(repo, '', mfnodes, basemfnode, []) _prefetchtrees(repo, '', mfnodes, basemfnode, [])
# Run repack in background
if opts.get('repack'):
backgroundrepack(repo, incremental=True)
def _prefetchtrees(repo, rootdir, mfnodes, basemfnodes, directories): def _prefetchtrees(repo, rootdir, mfnodes, basemfnodes, directories):
# If possible, use remotefilelog's more expressive fallbackpath # If possible, use remotefilelog's more expressive fallbackpath
if util.safehasattr(repo, 'fallbackpath'): if util.safehasattr(repo, 'fallbackpath'):
fallbackpath = repo.fallbackpath fallbackpath = repo.fallbackpath
else: else:
fallbackpath = repo.ui.config('paths', 'default') fallbackpath = repo.ui.config('paths', 'default')
start = time.time() start = time.time()