[PATCH 09 of 11 sparse] dirstate: move validation of files in sparse checkout from extension

Martin von Zweigbergk martinvonz at google.com
Tue Jul 11 01:46:23 EDT 2017


On Sat, Jul 8, 2017 at 4:29 PM, Gregory Szorc <gregory.szorc at gmail.com> wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc at gmail.com>
> # Date 1499554173 25200
> #      Sat Jul 08 15:49:33 2017 -0700
> # Node ID 151bee7bb0e2ea8e612bbb0e16fa45d6e446ec05
> # Parent  711945cedb514e81299059d69bfeb72da388fad0
> dirstate: move validation of files in sparse checkout from extension
>
> The sparse extension wraps various dirstate methods to validate that
> the path being modified in dirstate exists in the sparse checkout.
> This commit moves that code to dirstate itself.
>
> As part of the move, the error messages were tweaked slightly.
> And the code for the validation was reworked slightly. The most
> notable change is the removal of the "f is not None" check.
> This check has instead been inlined into copy(), which is the only
> place where it should be relevant.
>
> I'm not crazy about the references to sparse's UI adjustments
> leaking into core. But what can you do.

How about raising SparseError()? You can give it the file that was
outside if that's useful.

>
> I'm not a dirstate expert and am not sure if this is the ideal
> way to hook in path validation into dirstate now that the code is in
> core. But this does preserve the behavior of the extension. We can
> always refactor later.
>
> Unless the sparse feature is enabled via the sparse extension,
> the matcher will be an alwaysmatcher and the new code will
> essentially no-op. This change does add the overhead of calling
> a few functions, however. If the modified methods are called in
> tight loops (this can occur for working directory updates) this could
> have a performance impact. I haven't explicitly measured the perf
> impact because I view this code as temporary until it can be
> refactored. The end state should essentially be an attribute
> lookup, so I'm not too worried about long-term perf impact.

The worst case I could think of for this was debugrebuilddirstate,
because it writes the entire dirstate without reading the working
copy. That took 184ms before this patch and 195ms after (best of 10).
The increase remained (as expected) for the longer-running "hg
status": 591ms before and 599ms after. Also as expected, the relative
difference there is pretty small.

I'm fine with that slow-down. It seems like we should be able to
improve it later, but it's not a big deal to me even if we couldn't.


More information about the Mercurial-devel mailing list