Commit Graph

166 Commits

Author SHA1 Message Date
Durham Goode
52a399a6a9 fastmanifest: refactor add()'s fastdelta usage
Summary:
A future patch will make treemanifests also use the fastdelta fast path. To
support that, let's refactor the fast path to separate the fastmanifest specific
part.

Test Plan: Ran the tests

Reviewers: #mercurial, ikostia

Reviewed By: ikostia

Subscribers: rmcelroy, ikostia, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4249369

Signature: t1:4249369:1480509029:b6d08a4f9ef6581a4a2c1bed7f7db8ff084fc35d
2016-12-02 14:37:37 -08:00
Durham Goode
d4cfb31a2b fastmanifest: move fastdelta to a top level function
Summary:
A future patch will make treemanifests able to use fastdelta as well, so let's
move the function to be global so we can reuse it.

Test Plan: Ran the tests

Reviewers: #mercurial, ikostia

Reviewed By: ikostia

Subscribers: rmcelroy, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4249364

Signature: t1:4249364:1480507778:a00bb7b6301ca2f716c542f37f7150f2f6231640
2016-12-02 14:37:35 -08:00
Durham Goode
3de4f9ec15 treemanifest: always use flat manifest to get full text
Summary:
Treemanifests are naturally slower to produce full texts than just reading the
text directly from the manifest. So let's change our hybrid manifest
implementation to prefer flat manifests over tree manifests for text(). Fast
manifests are still faster to produce full texts, so we leave those as the
highest preference. This speeds up commit time when using treemanifest since
we don't need to construct the text from the tree for producing the delta that
goes into the revlog.

Test Plan:
Ran rebase with tree manifests enabled and verified it was
significantly faster. Also manually timed the fast vs flat text logic to ensure
fast was actually faster for text() than flat.

Reviewers: #mercurial, ikostia

Reviewed By: ikostia

Subscribers: mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4249354

Signature: t1:4249354:1480505430:f9e712a2db49c56680ce34cd3f68c85841393b39
2016-12-02 14:37:33 -08:00
Durham Goode
6492dbbf03 hybridmanifest: don't reuse node in match results
Summary:
Previously, _converttohybridmanifest would always create a new hybrid manifest
with the same node as the original. This meant that some code paths would
attempt to use the treemanifest from the node, instead of the already prepared
matches result. This meant the output could contain all the values from the
original tree, instead of just the matches output.

This is actually a regression from 98ba34a5194c09. Prior to that, matches did
not reuse the node.

Test Plan: Manually inspected the results in the debugger during a rebase.

Reviewers: #mercurial, ikostia

Reviewed By: ikostia

Subscribers: rmcelroy, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4247821

Signature: t1:4247821:1480499268:27f4a1b92ecf5d10009996b5b8f22bac02f3f38e
2016-12-02 14:37:28 -08:00
Durham Goode
a0dc16174d treemanifest: write deltas for trees
Summary:
Previously, when we wrote each tree entry into a pack file, it wasn't delta'd in
any way. This patch makes it store the delta against p1 in the pack file.

Testing in a large repo shows this reduces tree pack size by about 22x.

Test Plan:
Ran the tests. Did a pull in a large repo and saw the pack file was
22x smaller than before (and still usable).

Reviewers: #mercurial

Differential Revision: https://phabricator.intern.facebook.com/D4202088
2016-11-29 15:37:58 -08:00
Jun Wu
d203784d68 pyflakes: fix all pyflakes issues
Summary: As the title.

Test Plan: `arc unit`

Reviewers: #sourcecontrol, stash, rmcelroy

Reviewed By: stash, rmcelroy

Subscribers: rmcelroy, stash, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4232321

Signature: t1:4232321:1480067588:54e91ece8fa6b5ff13b3ebc9279217c76bf96a24
2016-11-25 00:23:21 +00:00
Durham Goode
4419e39db5 fastmanifest: add find to hybridmanifestctx
The main manifestctx implementation has a find function, so the hybrid one needs
it as well. I tried to add a test for this, but the only known case is in a
weird situation where there are tags, but no tag caches and some other criteria
I haven't figured out yet. So I gave up.
2016-11-18 08:56:35 -08:00
Adam Simpkins
4b57fdd6ae [fastmanifest] fix crash in hybridmanifest.__nonzero__()
Summary:
hybridmanifest.__nonzero__() was explicitly trying to call __nonzero__() on the
underlying manifest object.  This breaks when the underlying manifest is a
manifestdict.  manifestdict does not implement __nonzero__ (it instead has a
__len__ method that gets used instead when evaluating the manifestdict as a
boolean).

Because of this issue I was getting a crash in a local script when calling
repo.commitctx() with a memctx object.

Test Plan: Confirmed that calling repo.commitctx() with a memctx no longer crashes.

Reviewers: #sourcecontrol, durham, quark

Reviewed By: quark

Subscribers: net-systems-diffs@, yogeshwer, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4191858

Signature: t1:4191858:1479330743:7adb668c9529a8c8abaf9a8a1e64c8ee78b64d86
2016-11-16 13:16:31 -08:00
Durham Goode
b1109deca8 treemanifest: update to work with manifestlog
Summary:
Upstream has refactored the manifest class into several classes, so we need to
update treemanifest to work with the new structure. Notably, the factory add
function previously relied on the ability for the revlog class to create a new
manifestdict (via manifest.maniest.read()), since this isn't possible anymore,
we have to construct the hybridmanifest ourselves and provide an appropriate
loadflat function to get the flat manifest if necessary.

Test Plan: Ran the tests

Reviewers: #mercurial, rmcelroy

Reviewed By: rmcelroy

Subscribers: mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4180891

Signature: t1:4180891:1479285991:82bc546a1eb682d3cfd8b4724bda575410405d0f
2016-11-16 12:11:15 -08:00
Durham Goode
4ddfb704d3 fastmanifest: update to work with manifestlog
Summary:
Upstream Mercurial has refactored the manifest to get rid of the manifest class.
This patch updates fastmanifest to work with the new class structure. In
particular, it removes the hacky wrapping of 3 different manifest construction
functions, with a single wrapping of manifestlog.get(), which makes the code
simpler and more robust.

Test Plan: Ran the tests

Reviewers: #mercurial, rmcelroy

Reviewed By: rmcelroy

Subscribers: rmcelroy, stash, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4180874

Signature: t1:4180874:1479286125:bbd2a36aa86237d68036b9d7b0a580829219d869
2016-11-16 12:11:06 -08:00
Stanislau Hlebik
8b420cd314 fastmanifest: fallback to old manifest in readshallowfast
Summary:
fastmanifest makes bundling slower because fastmanifest `readshallowfast` always returns full manifest.
That's a big problem for infinitepush.
Let's copy-paste readshallowfast implementation from upstream. It uses readshallowdelta() if possible.

Test Plan:
1) Run all the tests for fb-hgext
2) Run infinitepush with this extension enabled. Make it is fast

Reviewers: durham, simonfar, rmcelroy, quark

Reviewed By: quark

Subscribers: mjpieters, #sourcecontrol

Differential Revision: https://phabricator.intern.facebook.com/D4088360

Tasks: 13907166

Signature: t1:4088360:1477580931:746e4054380403abbc52d1922583021b81f31bb6
2016-10-27 09:23:05 -07:00
Simon Farnsworth
7de29e4a87 fastmanifest: copy the readdelta() implementation and amend
Summary: Tests are failing because readdelta() in fastmanifest doesn't work after an upstream change. Copy and fix the upstream code to work in this case.

Test Plan: Run the test on my devserver

Reviewers: #mercurial, quark

Reviewed By: quark

Subscribers: mjpieters, #sourcecontrol

Differential Revision: https://phabricator.intern.facebook.com/D4074517

Tasks: 14070483

Signature: t1:4074517:1477412975:bdfb6e81e4ea3bccafa59a5cb0a6e71fcb1e7753
2016-10-25 09:30:26 -07:00
Durham Goode
e63b8ce924 treemanifest: write trees during local commits
Summary:
This makes local commits get written to trees (as well as flat manifest) when a
commit happens where the parent commit has a tree manifest already. During a
transaction where multiple trees are written (like when rebasing multiple
nodes), we reuse the same pack file for all the trees produced by tieing into
the transaction abort and close hooks.

Test Plan:
Ran the tests. Ran hg commit with the extension enabled. A future
patch will add an integration test for the treemanifest extension.

Reviewers: #mercurial, quark

Reviewed By: quark

Subscribers: mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4055851

Signature: t1:4055851:1477059659:91b1c2f93ef986e910cea752ebf2466cb20ac921
2016-10-21 11:02:26 -07:00
Durham Goode
8bc51e248b treemanifest: add in memory caching layer to hybridmanifest lookups
Summary:
In a future diff we will start allowing local trees to be written as part of
transactions. Part of this involves making those written trees available to
readers in this process before the transaction is committed. Since all manifest
reads go through hybridmanifest, we can insert a cache at this layer (just like
fastmanifest already has a cache at this layer).

The future patch which adds new manifests will populate this cache, and clear
this cache when the transaction completes.

Test Plan:
Ran the tests. Also tested the actual flow with the future patch that
causes commit to write to the cache

Reviewers: #mercurial, quark

Reviewed By: quark

Subscribers: mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4055838

Signature: t1:4055838:1477059568:0bf1d79cc1ca6f9bcefb881bab35c61c13641f4e
2016-10-21 11:02:24 -07:00
Durham Goode
122664649d fastmanifest: allow converting treemanifest into flat
Summary:
It's possible for treemanifests to be created that aren't backed by a flat
manifest (like if you did `foo = treemf.matches(...)`). In these cases, if we
ever need to diff it with a flat text, we need to be able to convert it to flat.

This patches adds the ability to convert it to a flat manifest via the text()
function.

Test Plan: Ran the tests

Reviewers: #mercurial, quark

Reviewed By: quark

Subscribers: quark, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4055656

Signature: t1:4055656:1477056921:7417c9ca06d61c5a283c6cf9693d83650e39f196
2016-10-21 11:02:13 -07:00
Durham Goode
148717dbde fastmanifest: fix some check code errors
Summary:
This fixes some (but not all) of the fastmanifest errors. The remaining ones are
from an import * and one that will require slightly more refactoring to fix.

Test Plan: Ran the tests

Reviewers: #mercurial, quark

Reviewed By: quark

Subscribers: mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4055633

Signature: t1:4055633:1477050763:09e758b1bd72b18799cdabf510eb0b2332077833
2016-10-21 11:02:11 -07:00
Durham Goode
f7249fd2e2 treemanifest: implement treemanifest.__nonzero__
This implements the __nonzero__ function, which is necessary for things like
`if mymanifest:`
2016-10-14 16:01:12 -07:00
Durham Goode
68f61c682a fastmanifest: fix cache race condition
Summary:
The fastmanifest code had a race condition where committing quickly in
a loop could cause a stack trace. It was caused by the code check m.incache()
and then using m._cachedmanifest(), and between those two lines a background
process could have cleared the entry fromm the cache. So now we just check if
_cachedmanifest() actually returns a value.

Test Plan:
With and without the fix:
```
cd ~/local && rm -rf temprepo ; hg init temprepo && cd temprepo && for j in `seq -f "%03g" 1 100`; do for i in `seq -f "%03g" 1 10`; do echo "line$j" >> file$i.txt; done; echo "Commit $j done"; hg ci -Am "Commit $j" ; done
```

It would stack trace reliably without the fix, and never with the fix.

Reviewers: ikostia, #mercurial, quark

Reviewed By: quark

Subscribers: mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4004376

Tasks: 13828535

Signature: t1:4004376:1476266037:0ff33a43fd35b5c7309fbfbf546799d75cc03ebc
2016-10-12 08:50:47 -07:00
Jun Wu
417bff83a4 fastmanifest: add the missing readfast method
Summary:
Upstream 4718718ed358 requires a `readfast` method for manifestctx.
`hybridmanifestctx` does not have such method so let's add it.

Test Plan: Added a test.

Reviewers: durham, ttung, #sourcecontrol, rmcelroy

Reviewed By: rmcelroy

Subscribers: mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3955478

Signature: t1:3955478:1475582074:f80c9eb97bcd42846625727ef1da4bce3256c88c
2016-10-01 01:34:50 +01:00
Jun Wu
1093ee0826 fastmanifest: implement readdelta for hybridmanifestctx
Summary:
This fixes a crash that `hybridmanifestctx` does not have `readdelta`
after upstream's b404425704fa. `readdelta` should be fast for vanilla
manifest so just forward the call there.

Test Plan: Modify a file that `.` touched, run `hg amend` and it won't crash.

Reviewers: ttung, #sourcecontrol, durham

Reviewed By: durham

Subscribers: mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3948989

Signature: t1:3948989:1475198696:d8ae194500fc093c03cc115f4613a9b8c8bfbff2
2016-09-30 02:09:43 +01:00
Durham Goode
c9e2cde0ad fastmanifest: fix hybridmanifestctx to match upstream
Summary:
Upstream no longer has manifestctx derive from manifestdict; instead it uses a
read function. Let's update hybridmanifestctx to do the same.

Test Plan: Ran the fastmanifest tests

Reviewers: #sourcecontrol, ttung

Reviewed By: ttung

Subscribers: mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3863656

Signature: t1:3863656:1473878502:6fb01f9f23fe64aafdc366a7ae98d510f3fb7927
2016-09-14 16:08:55 -07:00
Tony Tung
7b6879b1ef [fastmanifest] fix integration point for fastmanifest to match upstream changes
Summary: @durham's changes to manifest.py landed, so we need to update where we plug into fastmanifest.

Test Plan: `PYTHONPATH=~/work/mercurial/facebook-hg-rpms/fb-hgext/:~/work/mercurial/facebook-hg-rpms/lz4revlog/:~/work/mercurial/facebook-hg-rpms/remotenames/ /opt/local/bin/python2.7 ~/work/mercurial/facebook-hg-rpms/hg-crew/tests/run-tests.py -j32 test-fastmanifest*.{py,t}`

Reviewers: rmcelroy, durham

Reviewed By: durham

Subscribers: mitrandir, durham, mjpieters, lcharignon

Differential Revision: https://phabricator.intern.facebook.com/D3838608

Signature: t1:3838608:1473439470:1a689f60e72d351f1cf12d9e2cf1d2da9ab96bd9
2016-09-09 11:06:43 -07:00
Durham Goode
a2f69f2e93 fastmanifest: support tree manifests in hybrid manifest
Summary:
This adds support for the optional ctreemanifest type. If the module is
available and if `fastmanifest.usetree` is enabled (default: False), we'll use
tree manifests before falling back to flat manifests.

Test Plan: Ran the ctreemanifest perf suite and the fastmanifest test suite

Reviewers: #fastmanifest, ttung

Reviewed By: ttung

Subscribers: ttung, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3754309

Signature: t1:3754309:1471929802:3ccba8ce6160b7a6d20b599626efd5876bd969f6
2016-08-24 11:31:06 -07:00
Durham Goode
41e13076e1 fastmanifest: fix broken assert
Summary:
This assert is meant to check that at least one source was provided to the
hybridmanifest. The old version was broken though.

The test requires an update because it attempts to construct a hybridmanifest
with no source.

Test Plan: Ran the tests

Reviewers: #fastmanifest, ttung

Reviewed By: ttung

Subscribers: mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3760229

Signature: t1:3760229:1472062113:695085444915cb15a93db7f317580dcffc2f115d
2016-08-24 11:31:06 -07:00
Durham Goode
cede3a3be7 fastmanifest: add config option for disabling cache usage
Summary:
This adds the `fastmaniest.usecache` config option (default: True) which lets
users disable the use of the fastmanifest cache without disabling the entire
extension. This will be useful in a future patch where we'll use this same
hybrid manifest infrastructure to allow swapping in tree manifests (even on
repos that don't want fast manifest caches).

Also contains a minior fix to an assert that didn't appear to make sense (I
think it should be asserting that at least one data source is provided).

Test Plan: Ran the ctreemanifest perf suite and the fastmanifest test suite

Reviewers: #fastmanifest, ttung

Reviewed By: ttung

Subscribers: ttung, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3754301

Signature: t1:3754301:1471990417:0cde06f271d10c7f0c839e4f644310a4f525f3f6
2016-08-24 11:31:06 -07:00
Tony Tung
24341537bf [fastmanifest] prune the cache only when the revset is empty
Summary:
Because we now prune in parallel to priming the cache, we no longer need the prune at the end.  The only scenario where we still need to prune is where we never enter the priming loop, i.e., when the revset is empty.

Depends on D3545267, D3544997

Test Plan: since we don't prune in many circumstances any more, the test output is slighty affected.  otherwise, the tests pass.

Reviewers: #fastmanifest, durham

Reviewed By: durham

Subscribers: durham, mitrandir, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3547219

Signature: t1:3547219:1468373775:e4535e3a66fb362385a23b992749eb7c3fcf7ea1
2016-07-26 23:35:24 -07:00
Tony Tung
2c7c082bd9 [fastmanifest] use a standardized mechanism for fetching the vfs's base.
Summary: vfs.join(None) will return its base.  doesn't matter if it's nested or whatever.

Test Plan:
cloned fbjava, then stripped a revision (with evolve off).  then added a new revision, and ran `hg incoming`.  this resulted in the same stacktrace!

then with the fix run `hg incoming` again.  no crash this time.

Reviewers: #fastmanifest, simonfar

Reviewed By: simonfar

Subscribers: mitrandir, simonfar, mjpieters, lcharignon

Differential Revision: https://phabricator.intern.facebook.com/D3598348

Tasks: 12305684

Signature: t1:3598348:1469093045:440cf3d314589f46dd8083901b584d7cfd54a95a
2016-07-21 12:11:21 -07:00
Simon Farnsworth
eafe319549 Dive deeper into the opener to try and find the base.
Summary: Sometimes opener.vfs is an _fncachevfs, which has a .vfs member, not a .base member. When you get one of these, go deeper.

Test Plan:
Run testrunner.py flib/intern/sandcastle/vcs/ without this change, see it traceback.

Rerun testrunner.py flib/intern/sandcastle/vcs/ with this change, see it fail in the same way as the current released version.

Reviewers: durham, lcharignon, ttung, rmcelroy

Reviewed By: rmcelroy

Subscribers: rmcelroy, mjpieters, #sourcecontrol

Differential Revision: https://phabricator.intern.facebook.com/D3593097

Tasks: 12305684

Signature: t1:3593097:1469033487:aec4c4a5de6f92a64f4730f03e9780e02f086819
2016-07-20 10:13:23 -07:00
Tony Tung
c04d72741a [fastmanifest] tie a fastmanifestcache instance to each opener
Summary: Rather than have fastmanifestcache as a singleton, we attach it to each opener.

Test Plan: pass existing unit tests.

Reviewers: #fastmanifest, durham

Reviewed By: durham

Subscribers: mitrandir, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3553542

Tasks: 12169797

Signature: t1:3553542:1468373683:b0a7fcdf5dbf12046e5b4be9aa0bc5d46e55ce06
2016-07-13 00:02:48 -07:00
Tony Tung
40f173e380 [fastmanifest] implement prune using makeroomfor
Summary: `prune` is essentially `makeroomfor(0, set())`.

Test Plan: pass existing unit tests.  output is slightly different, but that's just because we no longer output that debugging line.

Reviewers: #fastmanifest, durham

Reviewed By: durham

Subscribers: mitrandir, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3547069

Signature: t1:3547069:1468352305:6185132648f871e26d3cfd449c059523b5eb6d6b
2016-07-12 17:09:15 -07:00
Tony Tung
73ff5ee3de [fastmanifest] don't write the cache entry just to blow it away
Summary:
It's kind of silly to write the FM cache entry, and then immediately blow away the entry if it doesn't fit in the limit.  Instead, just calculate if we have enough space, and raise CacheFullException if we don't.

Depends on D3538944

Test Plan: pass existing tests.

Reviewers: lcharignon, durham

Reviewed By: durham

Subscribers: mitrandir, mjpieters, quark

Differential Revision: https://phabricator.intern.facebook.com/D3545446

Signature: t1:3545446:1468283437:7e4167e3b5a13bf3d01fca45a965d7aaa8843166
2016-07-11 21:58:19 -07:00
Tony Tung
f58e14a80d [fastmanifest] improve setwithlimit
Summary:
There's two issues with `setwithlimit`:
1) It does not have a docblock, nor an API that fully covers all the possible conditions.
2) It returns None if the cache entry is already present.

This modifies `setwithlimit` to return True if the cache entry ultimately makes it (whether it previously existed or the write was successful), False if the write fails, and raises CacheFullException if the cache is full.

Test Plan: pass existing tests.  the already-in-cache case is a race condition that is not practical to reproduce.

Reviewers: lcharignon, durham

Reviewed By: durham

Subscribers: mitrandir, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3545407

Signature: t1:3545407:1468283390:64d6cdba7a17cca9bd4465c6e03b9143e4b4cc4b
2016-07-11 21:58:04 -07:00
Tony Tung
fa9ab92f9b [fastmanifest] make room for cache entries as we are populating
Summary:
Instead of pruning at the end, we need to prune as we are writing entries.

Depends on D3545060

Test Plan: pass test in D3545060

Reviewers: lcharignon, durham

Reviewed By: durham

Subscribers: mitrandir, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3545267

Signature: t1:3545267:1468279898:5332838efa52bb55733d0e6dcf613b27e71e9d3e
2016-07-11 17:34:01 -07:00
Tony Tung
2d41746595 [fastmanifest] refactor limit code
Summary:
Previously, depending on the code path, the limit specified would not actually take effect.  For instance, if we came in from debugmanifestcache, and attempted to populated the cache, we would use `systemawarecachelimit` when filling the cache, and the fixedsize limit specified by the user when pruning.

With this change, we unify the all the cache limit decisions to `fastmanifestcache`.  If the user actually overrides the limit, we set the limit in `fastmanifestcache` and let that make the decisions.

We also change the definitions of limit in `hg debugcachemanifest` to:
1) >0 => it's the limit.
2) =0 => use systemawarecachelimit
3) <0 => no limit!

Test Plan: pass existing unit tests. there's a small change in the test output, because we always evaluate the limit now, plus we remove the test for limit=0, since it means something different now.

Reviewers: lcharignon, durham

Reviewed By: durham

Subscribers: trunkagent, mitrandir, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3544997

Signature: t1:3544997:1468281604:8f78f00ebf2afd8f3f1fbefbd82316b97cc4b193
2016-07-11 17:33:37 -07:00
Tony Tung
5d9ea99776 [fastmanifest] convert the fastmanifest once
Summary:
We need a fastmanifest object in order to size it.  Once we know its size, we can make room in the cache.

This slightly affects one of the tests, as we request the manifest text earlier than we previously did.

Depends on D3537904

Test Plan: used in later diff.

Reviewers: lcharignon, durham

Reviewed By: durham

Subscribers: mitrandir, mjpieters, quark

Differential Revision: https://phabricator.intern.facebook.com/D3538991

Signature: t1:3538991:1468280648:41c65d91529babe0559eac7b75509481adf2765f
2016-07-11 17:33:23 -07:00
Tony Tung
4d30825f32 [fastmanifest] allow fastmanifests to be passed into the caches
Summary: This allows us to convert a manifest once, and try to repeatedly jam it into the cache (presumably while evicting things).

Test Plan: pass existing tests.

Reviewers: lcharignon, durham

Reviewed By: durham

Subscribers: mitrandir, mjpieters, quark

Differential Revision: https://phabricator.intern.facebook.com/D3537904

Signature: t1:3537904:1468268209:6f1746d3cee5071321d830bb91efd591a31a8616
2016-07-11 14:08:45 -07:00
Tony Tung
4e58c6f814 [fastmanifest] remove shuffle by batch
Summary: We don't need this since we loosely lock the cache worker.

Test Plan: pass existing tests.

Reviewers: lcharignon, durham

Reviewed By: durham

Subscribers: mitrandir, mjpieters, quark

Differential Revision: https://phabricator.intern.facebook.com/D3537355

Signature: t1:3537355:1468268248:ebf7de5bf5030b90039823e1111a073d7eacc447
2016-07-11 14:06:56 -07:00
Laurent Charignon
5b6ae49741 fastmanifest: fix ratio computation and operation naming
Summary:
Before this patch we were doing two mistakes in the ratio computation.
- We were not recording global cache hit or miss, it was always a hit because
a function is truthy is python
- We were not deduping cache miss for the same manifest multiple times
This patch fixes these two mistakes.
It also changes the logging of cache hit and miss to include the name of the
operation that triggered the cache hit or miss (diff or filesnotin).

Test Plan: Test output changed and commented

Reviewers: durham, ttung

Differential Revision: https://phabricator.intern.facebook.com/D3484658
2016-07-06 13:39:42 -07:00
Laurent Charignon
b6eae920e2 fastmanifest: fix logic error for debugmetrics and simplify test
Summary:
Before this patch we were using config instead of configbool for reading
the debugmetrics config causing "False" to be evaluated as a truthy value
for the config. This patch fixes the issue and sets the config to false for
some of the tests to reduce the noise of the output.

Test Plan: Tests pass

Reviewers: ttung

Differential Revision: https://phabricator.intern.facebook.com/D3524501
2016-07-06 13:42:32 -07:00
Tony Tung
bbd7b6fa47 [fastmanifest] move extension wrapping functions to extsetup
Summary:
There's no need for the `setup()` to be called in `reposetup()` any more.

Depends on D3510966

Test Plan:
1) pass existing unit tests.
2) kill chg server instances, then run `USE_CHG= hg diff -c . --debug` repeatedly.  (USE_CHG tells my hg wrapper to use chg)

Reviewers: quark, lcharignon

Reviewed By: lcharignon

Subscribers: mitrandir, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3511004

Signature: t1:3511004:1467410145:0761c80da77d5a193fbdb08527fcc4aae9f8065e
2016-07-05 16:32:22 -07:00
Tony Tung
13718aed04 [fastmanifest] add a uiproxy object that forwards requests to a real ui object
Summary: Whenever fastmanifest needs an ui object, we pass it a global proxy object.  Whenever someone calls `reposetup(..)`, we update the backing object.

Test Plan:
1) pass existing unit tests
2) enable chg, then run `hg diff -c . --debug`.  saw fastmanifest debug output in the console.

Reviewers: lcharignon, quark

Reviewed By: lcharignon

Subscribers: trunkagent, mitrandir, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3510966

Signature: t1:3510966:1467410140:88fe810d03b9a9aa6818f17eb605a70eee931364
2016-07-05 16:32:05 -07:00
Tony Tung
0df49ab211 [fastmanifest] unconditionally wrap trigger methods
Summary: Use the ui object inside the repo object to run the checks instead.

Test Plan:
```
[andromeda]:~/work/mercurial/facebook-hg-rpms/fb-hgext/tests:e81e328|remote/@> PYTHONPATH=~/work/mercurial/facebook-hg-rpms/remotenames/:~/work/mercurial/facebook-hg-rpms/mutable-history/hgext  python ~/work/mercurial/facebook-hg-rpms/hg-crew/tests/run-tests.py -j32 test-fastmanifest*.{py,t} test-sampling.t
........
# Ran 8 tests, 0 skipped, 0 warned, 0 failed.
```

Reviewers: lcharignon, quark

Reviewed By: quark

Subscribers: mitrandir, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3510964

Signature: t1:3510964:1467405336:32d74410f3c40293fddddfccb7e7725675534ad7
2016-07-05 16:31:02 -07:00
Laurent Charignon
0918180a4d fastmanifest: add logging for blackbox
Summary:
There are two parts to add logging to blackbox. Logging events
and changing the configuration of blackbox. This diff does the former.
We don't reuse all the metrics event as:
- blackbox does not support wildcard for events like fastmanifest-*
- they are not meant to be human readable, they are for performance logging

We instead log a few new events:
- What triggered a caching operation?
- What command we run to cache the manifest?
- What revisions are to be cached?
- What revisions are actuallt cached?
- Do we overflow the cache?

This will allow us to understand all the cases where caching didn't trigger.
Like in t11877434.

Test Plan:
Test output does not change
Verified manually by changing the blackbox configuration that events get logged:



  CHGDISABLE=1 hg book food --config blackbox.track='command, commandfinish, commandexception, exthook, fsmonitor, pythonhook, fastmanifest' --config extensions.fastmanifest=/home/lcharignon/facebook-hg-rpms/fb-hgext/fastmanifest --config extensions.blackbox=/home/lcharignon/facebook-hg-rpms/hg-crew/hgext/blackbox.py



Reviewers: ttung

Subscribers: mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3504158

Tasks: 11877434
2016-07-05 12:24:14 -07:00
Tony Tung
38faebe31d [fastmanifest] the key of of the hit ratio should be the aggregating key
Summary: If the key as always 'ratio', then all the ratios are aliased when we send off aggregate stats.  That means we end up sending the last entry, rather than each individual entry.

Test Plan:
along with D3504940

run FB_HG_DIAGS= hg diff -c .:

```
    "int": {
        "builddate": 1467289392,
        "cachehitratio": 100,
        "consumed": 489,
        "diffcachehitratio": -1,
        "elapsed": 280,
        "errorcode": 0,
        "filesnotincachehitratio": 0,
        "time": 1467318481
    },
```

Reviewers: lcharignon

Reviewed By: lcharignon

Subscribers: mitrandir, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3505464

Tasks: 12019647

Signature: t1:3505464:1467320144:327e0d306b9afa90ed9fc2d704a8ca02a79f3038
2016-06-30 13:59:40 -07:00
Jun Wu
b75516fcff fastmanifest: do not store ui in metricscollector
Summary:
This diff removes `self.ui = ui` in metricscollector to address stale ui
issues with chg. Also clean up unnecessary sample merges since there is
only one collector per process.

Test Plan:
Run with `fastmanifest.debugmetrics=1` and confirm see lines like:

  [FM-METRICS] Begin metrics
  [FM-METRICS] kind: cachehitratio, kwargs: [('ratio', 100.0)]
  [FM-METRICS] kind: diffcachehitratio, kwargs: [('ratio', -1)]
  [FM-METRICS] kind: filesnotincachehitratio, kwargs: [('ratio', 0.0)]
  [FM-METRICS] End metrics

Reviewers: #mercurial, ttung, lcharignon

Reviewed By: lcharignon

Subscribers: mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3504442

Signature: t1:3504442:1467311563:5904bd06b210fa8550b2e1f7e85c2133ad7a1a0e
2016-06-30 11:51:06 -07:00
Laurent Charignon
f47791bbbc fastmanifest: fix setup to work with chg
Summary:
chg runs extsetup and uisetup with dummy ui objects. Before this
patch we were relying on ui objects in extsetup and uisetup. After this
patch, we move the logic to reposetup. We do things a little differently
because reposetup can be called multiple times and extsetup and uisetup are
called only once.

This caused one problem for `hg clone` in the test as reposetup is not
called for clone operation. I adapted the test a little bit to cover this case.

Test Plan: Modified tests pass

Reviewers: mitrandir, quark, lcharignon

Reviewed By: lcharignon

Subscribers: mitrandir, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3499176

Signature: t1:3499176:1467244335:a70d718725c1d19ab6b6feb4558a8ffd1a020ab2
2016-06-29 17:13:41 -07:00
Tony Tung
746cf154bc [fastmanifest] allow the worker subprocess executable to be overridden
Summary: This will allow us to submit scuba profiling data.

Test Plan:
along with the corresponding wrapper change, run `FB_HG_DIAGS= hg boo abc` and observed the following rows in scuba:

```
scuba> select * from dev_command_timers where user='tonytung' and source = 'hg' and time > 1467178050
+--------+----------+------------+----------+---------+-----------+--------+----------------+--------------+------------+------+-----------+-------------+---------+---------+-------+---------+---------------+--------+---------------+--------------+--------------+------------+---------------+----------------+-------------------------------------------------------------------------------------------------------------------------------------------------+--------------+------------+---------------+-----------+--------------+---------+-------------+--------+--------+--------+-------------------+---------------+---------------+---------------+----------+-----+-----+--------+-------------+-----------+-----------------+---------------+------------+----------------+---------------+-------------+-------------+----------------+------------+----------+------------------+--------+
| abcabc | base_rev | builddate  | consumed | elapsed | errorcode | errors | noninteractive | target_count |    time    | when | abcabcdef | base_status | blahrgh | buildid |  chg  | chicken |    command    | defghi |   evolution   | evolutionext | fastmanifest | filesystem | fsmonitor_ext | fsmonitor_mode |                                                                   fullcommand                                                                   | generaldelta | hgwatchman | hgwatchmanext |   host    |    iftype    | inhibit | interactive | kernel | killed | linter | manifestdiskcache |    parent     | remoteFilelog | remotefilelog |   repo   | scm | sid | source | sqldirstate | sshclient | sshclientregion | sshclienttier | testing123 | tw_job_cluster | tw_job_handle | tw_job_name | tw_job_user | tw_oncall_team | tw_task_id |   user   |     version      | whence |
+--------+----------+------------+----------+---------+-----------+--------+----------------+--------------+------------+------+-----------+-------------+---------+---------+-------+---------+---------------+--------+---------------+--------------+--------------+------------+---------------+----------------+-------------------------------------------------------------------------------------------------------------------------------------------------+--------------+------------+---------------+-----------+--------------+---------+-------------+--------+--------+--------+-------------------+---------------+---------------+---------------+----------+-----+-----+--------+-------------+-----------+-----------------+---------------+------------+----------------+---------------+-------------+-------------+----------------+------------+----------+------------------+--------+
|   null |     null | 1467154509 |     2019 |    1668 |         0 |   null |           null |         null | 1467178055 | null | null      | null        | null    | null    | false | null    | bookmarks     | null   | createmarkers |              | null         | hfs        |               | on             | buck-out/gen/scm/wrappers//mac-hg/hg boo abc                                                                                                    | true         | null       | null          | andromeda | disconnected | null    | true        | 15.5.0 | null   | null   | null              | -bash         | null          | true          | fbsource | hg  |     | hg     | false       |           | null            | null          | null       |                | null          |             |             |                |            | tonytung | 3.8.3 309-aa1d56 | null   |
|   null |     null | 1467154509 |     3269 |    2964 |         0 |   null |           null |         null | 1467178058 | null | null      | null        | null    | null    | false | null    | cachemanifest | null   | createmarkers |              | null         | hfs        |               | on             | /Users/tonytung/work/fbsource-fbcode/fbcode/buck-out/gen/scm/wrappers/mac-hg/hg --repository /Users/tonytung/work/fbsource-fbcode cachemanifest | true         | null       | null          | andromeda | disconnected | null    | true        | 15.5.0 | null   | null   | null              | /sbin/launchd | null          | true          | fbsource | hg  |     | hg     | false       |           | null            | null          | null       |                | null          |             |             |                |            | tonytung |                  | null   |
+--------+----------+------------+----------+---------+-----------+--------+----------------+--------------+------------+------+-----------+-------------+---------+---------+-------+---------+---------------+--------+---------------+--------------+--------------+------------+---------------+----------------+-------------------------------------------------------------------------------------------------------------------------------------------------+--------------+------------+---------------+-----------+--------------+---------+-------------+--------+--------+--------+-------------------+---------------+---------------+---------------+----------+-----+-----+--------+-------------+-----------+-----------------+---------------+------------+----------------+---------------+-------------+-------------+----------------+------------+----------+------------------+--------+
2 row(s) in set (0.29 sec)
scuba>
```

Reviewers: lcharignon, mjpieters

Reviewed By: mjpieters

Subscribers: mitrandir, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3496584

Tasks: 11893315

Signature: t1:3496584:1467198422:ef3bcf2ddc13601645bdf00576f48cd31a838b06
2016-06-29 12:10:29 -07:00
Tony Tung
e749cff990 [fastmanifest] ui.log's msg[0] must be a format string
Summary:
Since metrics doesn't send a string, let's just stub out the call.

The alternative is to put a '' in every `recordsample()` call.

Test Plan: pass existing tests.

Reviewers: lcharignon

Reviewed By: lcharignon

Subscribers: mitrandir, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3479138

Tasks: 11900487

Signature: t1:3479138:1466787619:910917093cf0361ed181af5597ab702b936f8d1e
2016-06-24 16:17:56 -07:00
Tony Tung
d24d1c4c86 [fastmanifest] fix incorrect table label
Summary: It's actually most relevant to least relevant.  Add a docblock.

Test Plan: pass unit tests

Reviewers: lcharignon

Reviewed By: lcharignon

Subscribers: mitrandir, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3478616

Signature: t1:3478616:1466787566:b38eceddf91c3e88341f9173e07244ac33ab6345
2016-06-24 16:13:39 -07:00
Tony Tung
1060bda026 [fastmanifest] update comment to reflect the ugly truth
Test Plan: it's a comment

Reviewers: durham, mitrandir, rmcelroy, lcharignon

Reviewed By: lcharignon

Subscribers: mitrandir, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3478159

Signature: t1:3478159:1466787557:90af1e7c952e3b1ecb4b9b07fbc5613ca7940f3a
2016-06-24 16:13:21 -07:00