[PATCH 3 of 3] streamclone: use backgroundfilecloser (issue4889)

Gregory Szorc gregory.szorc at gmail.com
Mon Jan 18 12:17:30 CST 2016


On Mon, Jan 18, 2016 at 12:51 AM, Adrian Buehlmann <adrian at cadifra.com>
wrote:

> On 2016-01-18 03:44, Matt Mackall wrote:
> > On Thu, 2016-01-14 at 13:45 -0800, Gregory Szorc wrote:
> >> # HG changeset patch
> >> # User Gregory Szorc <gregory.szorc at gmail.com>
> >> # Date 1452807841 28800
> >> #      Thu Jan 14 13:44:01 2016 -0800
> >> # Node ID 3cc9fde87e3284c2653a25254b22f03339dbd075
> >> # Parent  d73af0bac3890fd4b7b43d888ff61718a166cbdd
> >> streamclone: use backgroundfilecloser (issue4889)
> >
> > I've gone ahead and queued these. I've left the 2048 default low to make
> sure we
> > maximize coverage during the freeze: please get people to test it!
>
> I won't test it myself (I stopped playing guinea pig quite a while ago),
> but I think - as I already said - that a default of 2048 is way too low.
>
>
My measurements on my i7-6700K (one of the fastest consumer grade CPUs
available currently) show that file closes take 1-3ms. I arrived at 2048
because I feel it is larger than most casual repositories but small enough
to show benefits. At 1ms, 2048 closes is >2s. I don't think the 3.4x
speedup in the commit message is reasonable because of startup and shutdown
overhead on a smaller file set size. But even a 2x speedup will reduce
operation time on 2049 files by >1s. And the impact only goes up from
there. That's how I arrived at 2048.


> This patch tries to "fix" a problem which does not exist for such small
> numbers of files. The potential problems it may introduce are however
> now inflicted on many users - due to the small default. On a platform,
> which is already shaky enough (Windows).
>

I understand your concern about Windows stability. Do you have any concrete
concerns about potential problems this patch may introduce? We take care to
limit the total number of open files. And the context managers ensure files
are closed before locks are released. Exceptions in threads will get raised
on the main thread. I'm not spotting an obvious weakness in this approach
(other than potentially slowdown due to thread startup/shutdown overhead
and more I/O contention - but I think it's OK to let the kernel sort these
things out).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20160118/b3259250/attachment.html>


More information about the Mercurial-devel mailing list