changegroup: extract _revisiondeltanormal() to standalone function It wasn't accessing anything important on the cgpacker that warranted it being a method instead of a function. Differential Revision: https://phab.mercurial-scm.org/D4142

changegroup: inline _revchunk() into group() _revchunk() was pretty minimal. I think having all the code for generating data composing the changegroup in one function makes things easier to understand. As part of the refactor, we now call the _revisiondelta* functions explicitly. This paves the road to refactor their argument signatures. Differential Revision: https://phab.mercurial-scm.org/D4141

changegroup: pass mfdicts properly With the narrow code part of core, the hacky pass-argument-via- attribute-on-self can be accomplished with a regular function argument. Differential Revision: https://phab.mercurial-scm.org/D4140

changegroup: pass sorted revisions into group() (API) Currently, group() receives a list of nodes and calls _sortgroup() to sort them and turn them into revs. Since the sorting behavior varies depending on the type of data being transferred, I think it makes sense to perform the sorting before group() is invoked. This commit extracts _sortgroup() to a pair of standalone functions. It then moves the calling of these functions to the 3 call sites of group(). group() now receives an iterable of revs instead of nodes. Differential Revision: https://phab.mercurial-scm.org/D4139

changegroup: pull _fileheader out of cgpacker It doesn't need any state from the packer. Differential Revision: https://phab.mercurial-scm.org/D4138

changegroup: factor changelogdone into an argument The variable was basically tracking whether the current operation is being performed against the changelog or something else. So let's just pass such a flag to everything that needs to access it. I'm still not a huge fan of building changelog awareness into low-level functions like revision delta generation. But passing an argument is strictly better than state on the packer instance. Differential Revision: https://phab.mercurial-scm.org/D4137

changegroup: record changelogdone after fully consuming its data Setting this as a side-effect of calling _close() is wonky. There's only one group for changelog data. So we can wait until after all data has been emitted before recording it. Differential Revision: https://phab.mercurial-scm.org/D4136

changegroup: key off changelogdone We use self._changelogdone for similar checks. Let's make things consistent. Differential Revision: https://phab.mercurial-scm.org/D4135

perf: call _generatechangelog() instead of group() Now that we have a separate function for generating just the changelog bits, the perf command should call it so it gets more accurate behavior. This changes the results of this command on my hg repo significantly: ! wall 1.390502 comb 1.390000 user 1.370000 sys 0.020000 (best of 8) ! wall 1.768750 comb 1.760000 user 1.760000 sys 0.000000 (best of 6) Profiling seems to reveal that ~20% of execution time is spent in progress bar accounting and printing! If we run with progress.disable=true: ! wall 1.639134 comb 1.650000 user 1.630000 sys 0.020000 (best of 7) A nice speedup. But profiling still shows a good chunk of time being spent in progress bar accounting code. The reason is that the progress bar is conditionally enabled via an argument to cgpacker.group(). The previous code in perf.py calling into group() did not enable the progress bar but _generatechangelog() always does. I think it is important for the perf* commands to capture real-world use cases. And this code always runs with an active progress bar. So the regression is acceptable. That being said, terminal printing performance can vary substantially. I don't think perf* commands should test terminal printing unless explicitly desired. So I've disabled progress bar printing in this command. Differential Revision: https://phab.mercurial-scm.org/D4134

changegroup: factor changelog chunk generation into own function We have separate functions for generating manifests and filelogs. Let's split changelog into its own function so things are consistent. As part of this, we refactor the code slightly. Before, the changelog linknode callback was updating state on variables inherited via a closure. Since the closure is now separate from generate(), we need to a way pass state between generate() and _generatechangelog(). The return value of _generatechangelog() is a 2-tuple where the first item is a dict containing accumulated state. We then alias some of its members into the scope of generate() to reduce code churn. I will be converting other functions to a similar pattern in future commits. Differential Revision: https://phab.mercurial-scm.org/D4133