<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Jan 18, 2016 at 12:51 AM, Adrian Buehlmann <span dir="ltr"><<a href="mailto:adrian@cadifra.com" target="_blank">adrian@cadifra.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="">On 2016-01-18 03:44, Matt Mackall wrote:<br>
> On Thu, 2016-01-14 at 13:45 -0800, Gregory Szorc wrote:<br>
>> # HG changeset patch<br>
>> # User Gregory Szorc <<a href="mailto:gregory.szorc@gmail.com">gregory.szorc@gmail.com</a>><br>
>> # Date 1452807841 28800<br>
>> #      Thu Jan 14 13:44:01 2016 -0800<br>
>> # Node ID 3cc9fde87e3284c2653a25254b22f03339dbd075<br>
>> # Parent  d73af0bac3890fd4b7b43d888ff61718a166cbdd<br>
>> streamclone: use backgroundfilecloser (issue4889)<br>
><br>
> I've gone ahead and queued these. I've left the 2048 default low to make sure we<br>
> maximize coverage during the freeze: please get people to test it!<br>
<br>
</span>I won't test it myself (I stopped playing guinea pig quite a while ago),<br>
but I think - as I already said - that a default of 2048 is way too low.<br>
<br></blockquote><div><br>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.<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
This patch tries to "fix" a problem which does not exist for such small<br>
numbers of files. The potential problems it may introduce are however<br>
now inflicted on many users - due to the small default. On a platform,<br>
which is already shaky enough (Windows).<br></blockquote><div><br></div><div>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). <br></div></div></div></div>