Fri, 07 Sep 2018 12:14:42 -0700 util: allow lrucachedict to track cost of entries
Gregory Szorc <gregory.szorc@gmail.com> [Fri, 07 Sep 2018 12:14:42 -0700] rev 39567
util: allow lrucachedict to track cost of entries Currently, lrucachedict allows tracking of arbitrary items with the only limit being the total number of items in the cache. Caches can be a lot more useful when they are bound by the size of the items in them rather than the number of elements in the cache. In preparation for teaching lrucachedict to enforce a max size of cached items, we teach lrucachedict to optionally associate a numeric cost value with each node. We purposefully let the caller define their own cost for nodes. This does introduce some overhead. Most of it comes from __setitem__, since that function now calls into insert(), thus introducing Python function call overhead. $ hg perflrucachedict --size 4 --gets 1000000 --sets 1000000 --mixed 1000000 ! gets ! wall 0.599552 comb 0.600000 user 0.600000 sys 0.000000 (best of 17) ! wall 0.614643 comb 0.610000 user 0.610000 sys 0.000000 (best of 17) ! inserts ! <not available> ! wall 0.655817 comb 0.650000 user 0.650000 sys 0.000000 (best of 16) ! sets ! wall 0.540448 comb 0.540000 user 0.540000 sys 0.000000 (best of 18) ! wall 0.805644 comb 0.810000 user 0.810000 sys 0.000000 (best of 13) ! mixed ! wall 0.651556 comb 0.660000 user 0.660000 sys 0.000000 (best of 15) ! wall 0.781357 comb 0.780000 user 0.780000 sys 0.000000 (best of 13) $ hg perflrucachedict --size 1000 --gets 1000000 --sets 1000000 --mixed 1000000 ! gets ! wall 0.621014 comb 0.620000 user 0.620000 sys 0.000000 (best of 16) ! wall 0.615146 comb 0.620000 user 0.620000 sys 0.000000 (best of 17) ! inserts ! <not available> ! wall 0.698115 comb 0.700000 user 0.700000 sys 0.000000 (best of 15) ! sets ! wall 0.560247 comb 0.560000 user 0.560000 sys 0.000000 (best of 18) ! wall 0.832495 comb 0.830000 user 0.830000 sys 0.000000 (best of 12) ! mixed ! wall 0.686172 comb 0.680000 user 0.680000 sys 0.000000 (best of 15) ! wall 0.841359 comb 0.840000 user 0.840000 sys 0.000000 (best of 12) We're still under 1us per insert, which seems like reasonable performance for a cache. If we comment out updating of self.totalcost during insert(), performance of insert() is identical to __setitem__ before. However, I don't want to make total cost evaluation lazy because it has significant performance implications for when we need to evaluate the total cost at mutation time (it requires a cache traversal, which could be expensive for large caches). Differential Revision: https://phab.mercurial-scm.org/D4502
Wed, 05 Sep 2018 23:15:20 -0700 util: add a popoldest() method to lrucachedict
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 05 Sep 2018 23:15:20 -0700] rev 39566
util: add a popoldest() method to lrucachedict This allows consumers to prune the oldest item from the cache. This could be useful for e.g. a consumer that wishes for the size of items tracked by the cache to remain under a high water mark. Differential Revision: https://phab.mercurial-scm.org/D4501
Thu, 06 Sep 2018 11:40:20 -0700 util: ability to change capacity when copying lrucachedict
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 06 Sep 2018 11:40:20 -0700] rev 39565
util: ability to change capacity when copying lrucachedict This will allow us to easily replace an lrucachedict with one with a higher or lower capacity as consumers deem necessary. IMO it is easier to just create a new cache instance than to muck with the capacity of an existing cache. Mutating an existing cache's capacity feels more prone to bugs. Differential Revision: https://phab.mercurial-scm.org/D4500
Thu, 06 Sep 2018 11:37:27 -0700 util: make capacity a public attribute on lrucachedict
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 06 Sep 2018 11:37:27 -0700] rev 39564
util: make capacity a public attribute on lrucachedict So others can query it. Useful for operations that may want to verify the cache has capacity for N items before it performs an operation that may cause cache eviction. Differential Revision: https://phab.mercurial-scm.org/D4499
Thu, 06 Sep 2018 11:33:40 -0700 util: properly copy lrucachedict instances
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 06 Sep 2018 11:33:40 -0700] rev 39563
util: properly copy lrucachedict instances Previously, copy() only worked if the cache was full. We teach copy() to only copy defined nodes. Differential Revision: https://phab.mercurial-scm.org/D4498
Thu, 06 Sep 2018 11:27:25 -0700 tests: rewrite test-lrucachedict.py to use unittest
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 06 Sep 2018 11:27:25 -0700] rev 39562
tests: rewrite test-lrucachedict.py to use unittest This makes the code so much easier to test and debug. Along the way, I discovered a bug in copy(), which I kind of added test coverage for. Differential Revision: https://phab.mercurial-scm.org/D4497
Wed, 29 Aug 2018 15:17:11 -0700 wireprotov2peer: stream decoded responses
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 29 Aug 2018 15:17:11 -0700] rev 39561
wireprotov2peer: stream decoded responses Previously, wire protocol version 2 would buffer all response data. Only once all data was received did we CBOR decode it and resolve the future associated with the command. This was obviously not desirable. In future commits that introduce large response payloads, this caused significant memory bloat and slowed down client operations due to waiting on the server. This commit refactors the response handling code so that response data can be streamed. Command response objects now contain a buffered CBOR decoder. As new data arrives, it is fed into the decoder. Decoded objects are made available to the generator as they are decoded. Because there is a separate thread processing incoming frames and feeding data into the response object, there is the potential for race conditions when mutating response objects. So a lock has been added to guard access to critical state variables. Because the generator emitting decoded objects needs to wait on those objects to become available, we've added an Event for the generator to wait on so it doesn't busy loop. This does mean there is the potential for deadlocks. And I'm pretty sure they can occur in some scenarios. We already have a handful of TODOs around this. But I've added some more. Fixing this will likely require moving the background thread receiving frames into clienthandler. We likely would have done this anyway when implementing the client bits for the SSH transport. Test output changes because the initial CBOR map holding the overall response state is now always handled internally by the response object. Differential Revision: https://phab.mercurial-scm.org/D4474
Wed, 29 Aug 2018 16:43:17 -0700 wireprotoframing: buffer emitted data to reduce frame count
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 29 Aug 2018 16:43:17 -0700] rev 39560
wireprotoframing: buffer emitted data to reduce frame count An upcoming commit introduces a wire protocol command that can emit hundreds of thousands of small objects. Without a buffering layer, we would emit a single, small frame for every object. Performance profiling revealed this to be a source of significant overhead for both client and server. This commit introduces a very crude buffering layer so that we emit fewer, bigger frames in such a scenario. This code will likely get rewritten in the future to be part of the streams API, as we'll need a similar strategy for compressing data. I don't want to think about it too much at the moment though. server before: user 32.500+0.000 sys 1.160+0.000 after: user 20.230+0.010 sys 0.180+0.000 client before: user 133.400+0.000 sys 93.120+0.000 after: user 68.370+0.000 sys 32.950+0.000 This appears to indicate we have significant overhead in the frame processing code on both client and server. It might be worth profiling that at some point... Differential Revision: https://phab.mercurial-scm.org/D4473
Wed, 05 Sep 2018 09:06:40 -0700 wireprotov2: implement commands as a generator of objects
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 05 Sep 2018 09:06:40 -0700] rev 39559
wireprotov2: implement commands as a generator of objects Previously, wire protocol version 2 inherited version 1's model of having separate types to represent the results of different wire protocol commands. As I implemented more powerful commands in future commits, I found I was using a common pattern of returning a special type to hold a generator. This meant the command function required a closure to do most of the work. That made logic flow more difficult to follow. I also noticed that many commands were effectively a sequence of objects to be CBOR encoded. I think it makes sense to define version 2 commands as generators. This way, commands can simply emit the data structures they wish to send to the client. This eliminates the need for a closure in command functions and removes encoding from the bodies of commands. As part of this commit, the handling of response objects has been moved into the serverreactor class. This puts the reactor in the driver's seat with regards to CBOR encoding and error handling. Having error handling in the function that emits frames is particularly important because exceptions in that function can lead to things getting in a bad state: I'm fairly certain that uncaught exceptions in the frame generator were causing deadlocks. I also introduced a dedicated error type for explicit error reporting in command handlers. This will be used in subsequent commits. There's still a bit of work to be done here, especially around formalizing the error handling "protocol." I've added yet another TODO to track this so we don't forget. Test output changed because we're using generators and no longer know we are at the end of the data until we hit the end of the generator. This means we can't emit the end-of-stream flag until we've exhausted the generator. Hence the introduction of 0-sized end-of-stream frames. Differential Revision: https://phab.mercurial-scm.org/D4472
Mon, 27 Aug 2018 13:30:44 -0700 internals: extract frame-based protocol docs to own document
Gregory Szorc <gregory.szorc@gmail.com> [Mon, 27 Aug 2018 13:30:44 -0700] rev 39558
internals: extract frame-based protocol docs to own document wireprotocol.txt is quite long and difficult to digest. The frame-based protocol is effectively a standalone concept (and could even be used outside of Mercurial). So this commit extracts its docs to a standalone file. The first few paragraphs were rewritten as part of the extraction. Sections headers were adjusted accordingly. Existing referalls in wireprotocol.txt were updated to refer to the new doc / concept, which I've started referring to as `hgrpc`. I'm on the fence as to whether to move the HTTP and SSH transport details to the new doc as well. For now, I'm leaving them in wireprotocol.txt. Differential Revision: https://phab.mercurial-scm.org/D4443
Wed, 12 Sep 2018 22:19:29 +0900 narrow: remove hack to write narrowspec to shared .hg directory
Yuya Nishihara <yuya@tcha.org> [Wed, 12 Sep 2018 22:19:29 +0900] rev 39557
narrow: remove hack to write narrowspec to shared .hg directory AFAIK, we no longer need it since the narrowspec file was move to the store directory in 576eef1ab43d, "narrow: move .hg/narrowspec to .hg/store/narrowspec."
Wed, 12 Sep 2018 22:15:43 +0900 narrowspec: remove parseserverpatterns() which isn't used anymore
Yuya Nishihara <yuya@tcha.org> [Wed, 12 Sep 2018 22:15:43 +0900] rev 39556
narrowspec: remove parseserverpatterns() which isn't used anymore Follows up 10a8472f6662, "narrow: drop support for remote expansion."
Tue, 11 Sep 2018 17:22:15 -0700 hg: write narrow patterns after repo creation
Gregory Szorc <gregory.szorc@gmail.com> [Tue, 11 Sep 2018 17:22:15 -0700] rev 39555
hg: write narrow patterns after repo creation Now that hg.clone() knows when a narrow clone is requested, it makes sense to have it update the narrow patterns for the repo soon after the repo is created, before any exchange occurs. Previously, the narrow extension was monkeypatching an exchange function to do this. The old code is redundant and has been removed. Differential Revision: https://phab.mercurial-scm.org/D4541
Tue, 11 Sep 2018 16:59:17 -0700 narrow: don't wrap exchange.pull() during clone
Gregory Szorc <gregory.szorc@gmail.com> [Tue, 11 Sep 2018 16:59:17 -0700] rev 39554
narrow: don't wrap exchange.pull() during clone The wrapped version was setting up the narrow repo requirement when a narrow clone was requested. Previous commits taught hg.clone() and repo creation to add the narrow requirement when a narrow clone was requested. So this requirement should already be set up for us and this code is no longer necessary. Differential Revision: https://phab.mercurial-scm.org/D4540
Tue, 11 Sep 2018 17:21:18 -0700 exchange: support defining narrow file patterns for pull
Gregory Szorc <gregory.szorc@gmail.com> [Tue, 11 Sep 2018 17:21:18 -0700] rev 39553
exchange: support defining narrow file patterns for pull This commit teaches exchange.pull() about the desire to perform a narrow file pull. We simply pass include and exclude patterns to the function. The values are validated and stored on the pulloperation instance. hg.clone() has been taught to pass these arguments to exchange.pull(). If the arguments are not passed to exchange.pull(), the active narrow patterns from the repository will automatically be used. We /could/ always use the narrow patterns from the repo. However, allowing explicit values to be passed in allows us to perform data fetching that doesn't necessarily align with the repo configuration. This provides more flexibility. Differential Revision: https://phab.mercurial-scm.org/D4539
Tue, 11 Sep 2018 17:20:14 -0700 commands: pass include and exclude options to hg.clone()
Gregory Szorc <gregory.szorc@gmail.com> [Tue, 11 Sep 2018 17:20:14 -0700] rev 39552
commands: pass include and exclude options to hg.clone() These arguments are defined by the narrow extension. Let's teach core to recognize them so we can delete some code from the narrow extension and start to exercise the in-core code for performing a narrow clone. We have no way of easily testing it, but this change should result in .hg/requires having the narrow requirement from the time the file is written rather than added as part of pull. We'll confirm this when we delete some monkeypatched functions from the narrow extension in later commits. Test output changed because hg.clone() is now receiving patterns and validation of those values is occurring sooner, before the exchange code runs and prints the message that was deleted. Differential Revision: https://phab.mercurial-scm.org/D4538
Tue, 11 Sep 2018 14:16:05 -0700 localrepo: add requirement when narrow files creation option present
Gregory Szorc <gregory.szorc@gmail.com> [Tue, 11 Sep 2018 14:16:05 -0700] rev 39551
localrepo: add requirement when narrow files creation option present The previous commit taught hg.clone() to define a creation option when file include or exclude patterns are passed. This commit teaches the new repo creation code to convert that creation option into a repository requirement. While not yet used by the narrow extension, the eventual side-effect of this change is that newly-created repositories will have the narrow requirement from their creation onset. Currently, the requirement is added to the repo at exchange.pull() time via a wrapped function in the narrow extension. Differential Revision: https://phab.mercurial-scm.org/D4537
Tue, 11 Sep 2018 17:15:35 -0700 hg: recognize include and exclude patterns when cloning
Gregory Szorc <gregory.szorc@gmail.com> [Tue, 11 Sep 2018 17:15:35 -0700] rev 39550
hg: recognize include and exclude patterns when cloning This commit teaches clone() to accept arguments defining file patterns to clone. This is the first step in teaching core code about the existence of a narrow clone. Right now, we only perform validation of the arguments and pass additional options into createopts to influence repository creation. Nothing of consequence happens with that creation option yet, however. For now, arbitrary restrictions exist, such as not allowing patterns for shared repos and disabling local copies when patterns are defined. We can potentially lift these restrictions in the future once partial clone/storage support is more flushed out. I figure it is best to reduce the surface area for bugs for the time being. It may seem weird to prefix these arguments with "store." However, clone is effectively pull + update and file patterns could apply to both the store and the working directory. The prefix is there to disambiguate in the future when this function may want to use different sets of patterns for the store and working directory. Differential Revision: https://phab.mercurial-scm.org/D4536
Tue, 11 Sep 2018 17:11:32 -0700 hg: allow extra arguments to be passed to repo creation (API)
Gregory Szorc <gregory.szorc@gmail.com> [Tue, 11 Sep 2018 17:11:32 -0700] rev 39549
hg: allow extra arguments to be passed to repo creation (API) Currently, repository creation is influenced by consulting the ui instance and turning config options into requirements. This means that in order to influence repository creation, you need to define and set a config option and that the option must translate to a requirement stored in the .hg/requires file. This commit introduces a new mechanism to influence repository creation. hg.repository() and hg.peer() have been taught to receive a new optional argument defining extra options to apply to repository creation. This value is passed along to the various instance() functions and can be used to influence repository creation. This will allow us to pass rich data directly to repository creation without having to go through the config layer. It also allows us to be more explicit about the features requested during repository creation and provides a natural point to detect unhandled options influencing repository creation. The new code detects when unknown creation options are present and aborts in that case. .. api:: options can now be passed to influence repository creation The various instance() functions to spawn new peers or repository instances now receive a ``createopts`` argument that can be a dict defining additional options to influence repository creation. localrepo.newreporequirements() also receives this argument. Differential Revision: https://phab.mercurial-scm.org/D4535
Tue, 11 Sep 2018 13:46:59 -0700 localrepo: move repo creation logic out of localrepository.__init__ (API)
Gregory Szorc <gregory.szorc@gmail.com> [Tue, 11 Sep 2018 13:46:59 -0700] rev 39548
localrepo: move repo creation logic out of localrepository.__init__ (API) It has long bothered me that local repository creation is handled as part of localrepository.__init__. Upcoming changes I want to make around how repositories are initialized and instantiated will make the continued existence of repository creation code in localrepository.__init__ even more awkward. localrepository instances are almost never constructed directly: instead, callers are supposed to go through hg.repository() to obtain a handle on a repository. And hg.repository() calls localrepo.instance() to return a new repo instance. This commit teaches localrepo.instance() to handle the create=True logic. Most of the code for repo construction has been moved to a standalone function. This allows extensions to monkeypatch the function to further customize freshly-created repositories. A few calls to localrepo.localrepository.__init__ that were passing create=True were converted to call localrepo.instance(). .. api:: local repo creation moved out of constructor ``localrepo.localrepository.__init__`` no longer accepts a ``create`` argument to create a new repository. New repository creation is now performed as part of ``localrepo.instance()`` and the bulk of the work is performed by ``localrepo.createrepository()``. Differential Revision: https://phab.mercurial-scm.org/D4534
Tue, 11 Sep 2018 13:52:17 -0400 subrepo: mask out passwords embedded in the messages displaying a URL stable
Matt Harbison <matt_harbison@yahoo.com> [Tue, 11 Sep 2018 13:52:17 -0400] rev 39547
subrepo: mask out passwords embedded in the messages displaying a URL I noticed the password in maintenance logs for the "no changes since last push" and "pushing to" messages when pushing with an explicit path. But the test case here with :pushurl was also affected. I didn't see that cloning or pulling subrepos on demand had this problem, but it seems safer to just mask that too. There's a bit of a disconnect here, because it looks like clone is slicing off the password (makes sense not to store it in the hgrc in cleartext). But not shearing it off of an explicit path causes the subrepo not to realize that it already pushed the latest stuff. This is the easiest fix, however.
Fri, 07 Sep 2018 15:57:55 -0700 localrepo: pass ui to newreporequirements() (API)
Gregory Szorc <gregory.szorc@gmail.com> [Fri, 07 Sep 2018 15:57:55 -0700] rev 39546
localrepo: pass ui to newreporequirements() (API) newreporequirements() is called as part of creating a new repository. It doesn't make much sense for it to receive a repo instance as part of determining what requirements for new repos should be. .. api:: localrepo.newreporequirements() receives a ui instead of a repo Differential Revision: https://phab.mercurial-scm.org/D4533
Tue, 11 Sep 2018 15:40:33 -0700 narrow: set opts['narrow'] instead of local variable
Gregory Szorc <gregory.szorc@gmail.com> [Tue, 11 Sep 2018 15:40:33 -0700] rev 39545
narrow: set opts['narrow'] instead of local variable This will allow the command function in core to infer the presence of the option without duplicating logic. Differential Revision: https://phab.mercurial-scm.org/D4532
Tue, 11 Sep 2018 15:53:12 -0700 narrow: drop support for remote expansion (BC)
Gregory Szorc <gregory.szorc@gmail.com> [Tue, 11 Sep 2018 15:53:12 -0700] rev 39544
narrow: drop support for remote expansion (BC) Previous patches to validate narrow patterns accidentically dropped support for the include: syntax that allows patterns to be expanded from a remote. This feature was never implemented in core and is only implemented on Google's custom server. Per @martinvonz's review comment in D4522, it is OK to drop this feature since it isn't used. The concept of this feature does seem useful. I anticipate it making a comeback some day in some shape or form. But for now, let's jettison the dead code. Differential Revision: https://phab.mercurial-scm.org/D4530
Fri, 07 Sep 2018 18:35:54 -0700 fastannotate: use repo.local()
Gregory Szorc <gregory.szorc@gmail.com> [Fri, 07 Sep 2018 18:35:54 -0700] rev 39543
fastannotate: use repo.local() This is the proper way to check whether we're dealing with a local repository, since extensions should be coding to an interface and not testing for exact types. Differential Revision: https://phab.mercurial-scm.org/D4542
Tue, 11 Sep 2018 16:04:55 -0700 tests: drop extra "file:" prefix from paths in narrow test
Martin von Zweigbergk <martinvonz@google.com> [Tue, 11 Sep 2018 16:04:55 -0700] rev 39542
tests: drop extra "file:" prefix from paths in narrow test It looks like these were added by mistake in f4d4bd8c8911 (narrow: add a --narrowspec flag to clone command, 2018-08-08). Differential Revision: https://phab.mercurial-scm.org/D4531
Tue, 11 Sep 2018 11:47:10 -0700 narrow: validate spec files are well-formed during clone (BC)
Gregory Szorc <gregory.szorc@gmail.com> [Tue, 11 Sep 2018 11:47:10 -0700] rev 39541
narrow: validate spec files are well-formed during clone (BC) Previously, specfiles would get read then normalized. We want specfiles to be normalized on read so there is no confusion about what the format of specfiles should be. This commit validates the parsed result of --specfile. If entries aren't prefixed, an error is raised. Previously, validation would occur at exchange time, hence why we dropped a line of test output related to server iteraction. Differential Revision: https://phab.mercurial-scm.org/D4526
Tue, 11 Sep 2018 10:59:21 -0700 narrow: validate patterns on incoming bundle2 part
Gregory Szorc <gregory.szorc@gmail.com> [Tue, 11 Sep 2018 10:59:21 -0700] rev 39540
narrow: validate patterns on incoming bundle2 part The remote data is untrusted and needs to be validated for pattern conformance. Differential Revision: https://phab.mercurial-scm.org/D4525
Tue, 11 Sep 2018 15:28:41 -0700 narrowspec: validate patterns when loading and saving spec file
Gregory Szorc <gregory.szorc@gmail.com> [Tue, 11 Sep 2018 15:28:41 -0700] rev 39539
narrowspec: validate patterns when loading and saving spec file Patterns should be normalized and validated before being passed into narrowspec.save(). Let's assert that by checking immediately before writing the narrow spec file. And let's assert that patterns loaded from the spec file also conform. Differential Revision: https://phab.mercurial-scm.org/D4524
Mon, 10 Sep 2018 22:34:19 +0900 ancestor: use heapreplace() in place of heappop/heappush()
Yuya Nishihara <yuya@tcha.org> [Mon, 10 Sep 2018 22:34:19 +0900] rev 39538
ancestor: use heapreplace() in place of heappop/heappush() This should be slightly faster. Overall perfancestors result:: cpython nginx mercurial ------------- ---------------- ---------------- ---------------- b6db2e80a9ce^ 0.103461 0.006303 0.035716 8eb2145ff0fb 0.192307 (x1.86) 0.012115 (x1.92) 0.052135 (x1.46) this patch 0.139986 (x1.35) 0.006389 (x1.01) 0.037176 (x1.04)
(0) -30000 -10000 -3000 -1000 -300 -100 -50 -30 +30 +50 +100 +300 +1000 +3000 +10000 tip