[PATCH 1 of 3] scmutil: support background file closing

[Gregory Szorc]

On Thu, Feb 25, 2016 at 7:45 PM, Matt Harbison <mharbison72 at gmail.com>
wrote:

> On Thu, 14 Jan 2016 16:45:28 -0500, Gregory Szorc <gregory.szorc at gmail.com>
> wrote:
>
> # HG changeset patch
>> # User Gregory Szorc <gregory.szorc at gmail.com>
>> # Date 1452807299 28800
>> #      Thu Jan 14 13:34:59 2016 -0800
>> # Node ID 1e32fb544d387374d4caab11e9aeef5aadb9c54d
>> # Parent  e6e34c4e391668c5d8af8f98c004a27c77b1e2fa
>> scmutil: support background file closing
>>
>>
> [snip]
>
>
> +class backgroundfilecloser(object):
>> +    """Coordinates background closing of file handles on multiple
>> threads."""
>> +    def __init__(self, ui, expectedcount=-1):
>> +        self._running = False
>> +        self._entered = False
>> +        self._threads = []
>> +        self._threadexception = None
>> +
>> +        # Only Windows/NTFS has slow file closing. So only enable by
>> default
>> +        # on that platform. But allow to be enabled elsewhere for
>> testing.
>> +        defaultenabled = os.name == 'nt'
>> +        enabled = ui.configbool('worker', 'backgroundclose',
>> defaultenabled)
>> +
>> +        if not enabled:
>> +            return
>> +
>> +        # There is overhead to starting and stopping the background
>> threads.
>> +        # Don't do background processing unless the file count is large
>> enough
>> +        # to justify it.
>> +        minfilecount = ui.configint('worker',
>> 'backgroundcloseminfilecount',
>> +                                    2048)
>> +        # FUTURE dynamically start background threads after minfilecount
>> closes.
>> +        # (We don't currently have any callers that don't know their
>> file count)
>> +        if expectedcount > 0 and expectedcount < minfilecount:
>> +            return
>> +
>> +        # Windows defaults to a limit of 512 open files. A buffer of 128
>> +        # should give us enough headway.
>> +        maxqueue = ui.configint('worker', 'backgroundclosemaxqueue', 384)
>> +        threadcount = ui.configint('worker',
>> 'backgroundclosethreadcount', 4)
>> +
>> +        ui.debug('starting %d threads for background file closing\n' %
>> +                 threadcount)
>>
>
> This recently(?) broke a handful of Windows tests.  I figured adding the
> '(?)' marker was the right way to handle this, but it doesn't seem to
> work.  The (?) test in test-run-tests.t works though, and I don't see any
> obvious difference.  Any ideas?
>
> --- c:/Users/Matt/Projects/hg/tests/test-copy-move-merge.t
> +++ c:/Users/Matt/Projects/hg/tests/test-copy-move-merge.t.err
> @@ -35,6 +35,7 @@
>     preserving a for resolve of c
>    removing a
>    starting 4 threads for background file closing (?)
> +  starting 4 threads for background file closing
>     b: remote moved from a -> m (premerge)
>    picked tool ':merge' for b (binary False symlink False changedelete
> False)
>    merging a and b to b
>
> ERROR: test-copy-move-merge.t output changed
> !
>

The fallout is likely due to the recent change in merge.py to use
background file closing for working directory updates.

FWIW, as part of developing some recent patches I've noticed that test diff
output can be wonky. I think there may be a buffering issue between
stderr/stdout or a race condition somewhere?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20160227/84934ed1/attachment.html>