Summary: This will limit what we cache in the first place
Test Plan:
No test yet, I will add a test in a diff later checking cache
eviction.
Reviewers: ttung, durham
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3346515
Summary:
This diff adds documentation for all the config options and their
default values.
Test Plan: No test change.
Reviewers: ttung, durham
Differential Revision: https://phabricator.intern.facebook.com/D3346491
Summary:
As discussed in the group, we want to shuffle by batches to keep
an approximate ordering and still avoid caching process fighting for the
same entries.
Test Plan: Add a new test
Reviewers: durham, ttung
Differential Revision: https://phabricator.intern.facebook.com/D3344144
Summary:
If we want to cache revision [300, 350, 320, 380], we would first order the
revisions to be [380, 350, 320, 300] and cache them in that order. Assuming that
only 3 revisions can fit in the cache, we write all revisions and prune the
one that overflew. Afterwards, we change the mtime of all the cached entries
to ensure that for each of the newly cached entries, more recent mtime == more
relevant entry.
Test Plan: Test output changes slightly, as expected
Reviewers: mitrandir, ttung, simonfar
Differential Revision: https://phabricator.intern.facebook.com/D3344125
Summary: This will be useful to implement the new cache filling strategy.
Test Plan: Refactoring does not change test output (y)
Reviewers: ttung, simonfar, mitrandir
Differential Revision: https://phabricator.intern.facebook.com/D3344111
Summary:
This will be useful later on and exercised by the new cache filling
strategy
Test Plan: Not exercised yet
Reviewers: mitrandir, ttung
Differential Revision: https://phabricator.intern.facebook.com/D3344107
Summary: Interpret revrange inside the worker process. The reason this is needed is that `triggercacheondirstatechange` happens in the wlock release callback. However, the lock is not released at this point. Interpreting the revrange requires wlock, so then we try to acquire it but we'll never succeed (because we need to finish the wlock release callback). If we do it in the scope of the worker process, it'll try to acquire it, find that the primary process owns it, and wait to acquire it.
Test Plan: run unit tests
Reviewers: durham, lcharignon
Reviewed By: lcharignon
Subscribers: quark, mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3339805
Tasks: 11385141
Signature: t1:3339805:1464209208:ec2424f84570d489aa5c0c629a15b69b6b126c60
Summary:
This diff changes the fastmanifestocache revset to also cache parents
of revisions in the revset.
Test Plan: Check that it works calling debugcachemanifest manually with
hg sl -r "fastmanifesttocache()"
Reviewers: durham, ttung
Differential Revision: https://phabricator.intern.facebook.com/D3344222
Summary:
This will be useful to improve the cache filling process to stop once the limit
hits the total size.
Test Plan: Test output does not change
Reviewers: mitrandir, ttung, simonfar
Differential Revision: https://phabricator.intern.facebook.com/D3344099
Summary: This could be useful to have the number of valid entries in the cache
Test Plan: Test output changes accordingly
Reviewers: ttung
Differential Revision: https://phabricator.intern.facebook.com/D3343488
Summary:
This time I tested the full workflow filling up the cache entierly respecting
the system limit. It went well with no crash.
Test Plan:
Tested filling up the cache using the systemawarecachelimit and then
pruning some of the cache to exercise all the codepaths in systemawarecachelimit.
Reviewers: ttung
Differential Revision: https://phabricator.intern.facebook.com/D3343474
Summary:
Before this patch with fastmanifest.cacheonchange on, we would cache
manifest on change to dirstate, bookmarks and remotenames. This patch makes
these operation in the background by default. Also it changes the cache strategy
used by default to be the new strategy that adapts based on the system resources.
Test Plan: Tests does not change
Reviewers: ttung, simonfar, quark
Differential Revision: https://phabricator.intern.facebook.com/D3341681
Summary: Wrong variable name lead to a crash
Test Plan: The crash does not happen anymore
Reviewers: ttung, simonfar, quark
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3341687
Summary:
I didn't fix the non determinism properly before, this patch fixes
the issue with a more defensive code.
Test Plan: Ran the test 32 times and the result didn't vary
Reviewers: ttung, simonfar, quark
Differential Revision: https://phabricator.intern.facebook.com/D3341032
Summary: We don't actually want to daemonize the process. We just want to spawn a worker process to do some work. Instead of exitting the initial process, we fork, do a bit of cleanup of the output streams, and then fork once again to prevent zombies.
Test Plan: run unit tests
Reviewers: lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3339788
Tasks: 11385109, 11385154
Signature: t1:3339788:1464107186:95bdaeb1f612ab058c4cc004af70c4d0bd9d2c58
Summary: If we don't, then a lazily-evaluated set is returned. That lazily-evaluated set will suppress the hidden bit when dirstate changes are involved.
Test Plan: on a dirstate change with a lot of hidden revs, observe that only a small set of revs are cached.
Reviewers: lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters, #mercurial
Differential Revision: https://phabricator.intern.facebook.com/D3338252
Signature: t1:3338252:1464107195:fd0e342fb4e4eb485057b7fda9679f7ff18f4e7b
Summary:
Before this patch, we were iterating over the cache entries in a non-
deterministic fashion. This patch makes the iteration stable by ordering the
entries by filename.
Test Plan: Changed the tests accordingly
Reviewers: ttung
Differential Revision: https://phabricator.intern.facebook.com/D3336880
Summary:
We order by mtime and then by filename and not just by mtime. Otherwise, we
wrongfully relied on os.listdir to return the same thing on different platforms.
Test Plan: Change the test
Reviewers: ttung
Differential Revision: https://phabricator.intern.facebook.com/D3336874
Summary:
When we start worker process to cache revisions, since we don't use
a lock, it is better to have the workers cache revisions in a random order to
avoid duplicated effort as much as possible.
Test Plan: No test for now, not sure how to test that.
Reviewers: simonfar, quark, ttung
Differential Revision: https://phabricator.intern.facebook.com/D3335918
Summary:
This patch reorganizes the import statements in fastmanifest.py and
fixes 3 issues flagged by pyflakes
Test Plan: Test pass
Reviewers: ttung
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3335411
Summary:
This adds a new strategy for the cache limit that depends on the free
space availabe in the system. For system with more than 100GB of free space
we allocate 5GB for the cache. With more than 20 GB of free space we allocate
2GB of free space and otherwise 10% of the free space at most.
Test Plan: Add a new test
Reviewers: ttung, durham, rmcelroy
Differential Revision: https://phabricator.intern.facebook.com/D3330894
Summary:
This adds the first, basic implementation of size limitation for
the fastmanifest cache
Test Plan: Add a few tests that cover edge cases
Reviewers: ttung, simonfar, durham
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3326149
Summary:
This patch makes it possible to cache fastmanifest asynchronously. I added
a test an also tested with fbsource:
hg debugcachemanifest --debug --config extensions.fastmanifest=/data/users/lcharignon/facebook-hg-rpms/fb-hgext/fastmanifest.py -R ~/fbsource --all
hg debugcachemanifest --debug --config extensions.fastmanifest=/data/users/lcharignon/facebook-hg-rpms/fb-hgext/fastmanifest.py -R ~/fbsource --list
This is a tricky code, and I think @quark would be best to review it.
Test Plan: Added a new test
Reviewers: quark, durham, ttung
Subscribers: quark
Differential Revision: https://phabricator.intern.facebook.com/D3323540
Summary: This is a refactoring of the _cachemanifest function
Test Plan: test output does not change
Reviewers: ttung, simonfar
Differential Revision: https://phabricator.intern.facebook.com/D3330501
Summary:
We use the manifestnode from the changelog and avoid having to reach the manifest
completely to check if it is already in cache.
This goes with a refactoring of the contains method. Before this patch, the
contains method was misleading because an entry could be contained in the cache
but not show up when you iterate over the cache. This happened because contains
was operating on nodes and iterating showed filesnames (so keys + prefix). To
make it clear that contains operates on nodes, we change its name to containsnode
We also rename key to hexnode to make it clearer that the keys are hexnodes.
Test Plan: Existing tests pass. We also add a new test to show that the fast
pass is hit.
Reviewers: durham, ttung
Differential Revision: https://phabricator.intern.facebook.com/D3325335
Summary: This will be useful for implementing garbage collection later.
Test Plan: Add a new test that exercises the new function
Reviewers: ttung
Differential Revision: https://phabricator.intern.facebook.com/D3322426
Summary:
This patch changes fastmanifest to add automatic caching of relevant
manifest when bookmarks change or if the parent of the working copy changes.
Test Plan: Added a test
Reviewers: ttung
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3318775
Summary:
Touching on access allows us to order the entries by access time and
implement a LRU caching strategy, see D3326149.
Test Plan: Tests pass
Reviewers: ttung
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3328205
Summary:
Before this patch, if a second process was to remove a cached entries during
a pruneall operation from another process, we could crash.
Test Plan: Tests pass
Reviewers: ttung, rmcelroy
Differential Revision: https://phabricator.intern.facebook.com/D3326100
Summary:
This will makes implementation of other things easier like garbage
collection.
Test Plan:
Existing test pass and the existing code exercise the new code path
so we should be good.
Reviewers: ttung
Differential Revision: https://phabricator.intern.facebook.com/D3322423
Summary:
This is typically what we do with other extensions, either the top or the
bottom of the file to increase legibility.
Test Plan: Tests pass
Reviewers: ttung
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3318766
Summary:
Adding a pruneall method to prune all cache manifest. This will be
useful to test the caching logic without having to run shell commands to remove
manifests.
Test Plan: Add a new test
Reviewers: ttung, durham
Differential Revision: https://phabricator.intern.facebook.com/D3314158
Summary:
We just materialize an empty fastmanifest and return it.
Also, not emptymanifest => True. Did I mention how much I hate implicit casts?
Test Plan: pass test-subrepo.t
Reviewers: durham, lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3320669
Tasks: 11320498
Signature: t1:3320669:1463668266:0e7fd7d7123d37c7e71215f0632314ea804f1d43
Summary: This is the behavior of lazymanifest (c implementation) and _lazymanifest (python implementation).
Test Plan: pass all but one of the subrepo tests
Reviewers: durham, lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3320470
Tasks: 11319460
Signature: t1:3320470:1463668278:01968bf3a95070a695fbac8ffec675c2ff616434
Summary: We want to always present a fastmanifest, so we always return true for `_incache()`. `_cachedmanifest()` is modified to extract the flat manifest when debugfastmanifest is set, and convert the extracted manifest.
Test Plan: down to 45 failures with this and the `filtercopy()` fixes. #10644559
Reviewers: lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3320099
Tasks: 11278537
Signature: t1:3320099:1463668296:3f0e6b37524d8a74f531bc418a5594e13994d2b2
Summary:
* Handle the scenario where ui is not set (like in test-fastmanifest.py).
* Add in the same logic for `fastmanifestcache` as `hybridmanifest`
Test Plan: run test-fastmanifest.py test-fastmanifest.t
Reviewers: lcharignon, durham
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3311487
Summary:
When fastmanifest.debugfastmanifest is set to True, we always return a fastmanifest. If the cache is empty, we will convert a flat manifest into a fastmanifest and use that for all operations.
Same goal as D3247484.
Test Plan: pass some unit tests in the mercurial suite. need to investigate the remaining issues.
Reviewers: #mercurial, lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3295434
Signature: t1:3295434:1463439017:0a6d20fa23fe409094e35a46eb195495455b8602
Summary: Unlike manifestdict, we don't accept a 0-argument constructor. Instead, we build up a cfastmanifest and then wrap a fastmanifestdict around it.
Test Plan: pass more unit tests
Reviewers: #mercurial, lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3295344
Signature: t1:3295344:1463438969:8e66c79aac0d5914549852b233d07b1ab3bf11ff
Summary: Since there's no flatmanifestcache, we can collapse the two classes.
Test Plan: `python ~/work/mercurial/facebook-hg-rpms/hg-crew/tests/run-tests.py test-fastmanifest.t test-fastmanifest.py`
Reviewers: #mercurial, lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3294646
Signature: t1:3294646:1463438882:6d9c3eaa7ac739f27672d4557f6031abbd1033a7
Summary:
* `_manifest()` should attempt to retrieve a fastmanifest if possible.
* `self.incache` represents a tristate indicating True if the fastmanifest is available, False if the fastmanifest is not, and None if we haven't tried to determine its availablility.
* clarified the debug messages a bit
* re-introduce the test removed in D3277498 (since it actually can work now).
Test Plan: pass unit tests!
Reviewers: lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3288967
Signature: t1:3288967:1463416544:416f28ecdc5e6a26545f3d4215fc8baa9222af7c
Summary: `fastmanifestdict`'s constructor takes a fastmanifest. there is no default constructor.
Test Plan: used in later diffs
Reviewers: lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters, #mercurial
Differential Revision: https://phabricator.intern.facebook.com/D3288939
Signature: t1:3288939:1463416036:a6475f5693d48175c81262b412ebdf0525b60fb2
Summary: `manifest.node` is a bin node value. To get the hex version, we need to call `revlog.hex(..)` on it.
Test Plan: used in later diff.
Reviewers: durham, lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters, #mercurial
Differential Revision: https://phabricator.intern.facebook.com/D3288910
Signature: t1:3288910:1463416018:2c6e09a6c80b2e0a04ddd712d7b7b506c334afb1
Summary: Since self.node is passed to `copy()`, we should pass the unadulterated node. Otherwise, we end up calling `revlog.hex(..)` on a hex hash.
Test Plan: pass tests/test-fastmanifest.py
Reviewers: lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3284150
Signature: t1:3284150:1463067635:938e45fcd706844f86ed5a84b6dd0462a80adcbe
Summary: since it's a singleton, we can have one instance of the code for retrieving the instance.
Test Plan: run tests/test-fastmanifest.t
Reviewers: lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3284113
Signature: t1:3284113:1463416610:207c7e9954cc0b830e77f2fecc0a65acfcfd1096
Summary:
Right now, neither `copy()` nor `matches(..)` behave as one might expect. If the hybridmanifest is wrapping a flat manifest, then calling `copy()` or `matches(..)` results in an exception as it explicitly calls for a flatmanifest.
These methods should call the method (`copy` or `matches` on whatever manifest the hybridmanifest is wrapping), and wrap the result in a hybridmanifest. To support this change, we modify `hybridmanifest()` to accept three possible data sources:
# A flat manifest
# A fast manifest
# A method to return a flat manifest
Test Plan: tests/test-fastmanifest.t passes
Reviewers: durham, lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters, #mercurial
Differential Revision: https://phabricator.intern.facebook.com/D3284044
Tasks: 10589051
Signature: t1:3284044:1463063458:c0636e0db1ee726e7db55cc62b7dae419049b584
Summary: Flat manifests are cached in memory by manifest.py. Unclear whether there's any reason for us to cache flat manifests on disk.
Test Plan: ran test-fastmanifest.py
Reviewers: durham, lcharignon
Reviewed By: lcharignon
Subscribers: trunkagent, mitrandir, mjpieters, #mercurial
Differential Revision: https://phabricator.intern.facebook.com/D3277498
Tasks: 10589051
Signature: t1:3277498:1463416673:74cab287b9045166dc0322a45f8a325f65da5d1f
