Re: [RFC/PATCH 0/1] cygwin: Remove the Win32 l/stat() functions

From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date: Jun 27, 2013, 23:09 UTC
Message-ID: <51CCC643.1050702@ramsay1.demon.co.uk>
In-Reply-To: <51CBD2FD.5070905@alum.mit.edu>

Michael Haggerty wrote:

> On 06/27/2013 12:35 AM, Jeff King wrote:

[ ... ]

>> I think Michael's assessment above is missing one thing.
> 
> Peff is absolutely right; for some unknown reason I was thinking of the
> consistency check as having been already fixed.

Well, the "cygwin: Remove the Win32 l/stat() functions" patch *does* fix the problem. :-D It's just a pity we can't use it on performance grounds. :(

Show 18 quoted lines

>> [...#ifdef out consistency check on cygwin when lock is held...]
> 
> Yes, this would work.
> 
> But, taking a step back, I think it is a bad idea to have an unreliable
> stat() masquerading as a real stat().  If we want to allow the use of an
> unreliable stat for certain purposes, let's have two stat() interfaces:
> 
> * the true stat() (in this case I guess cygwin's slow-but-correct
> implementation)
> 
> * some fast_but_maybe_unreliable_stat(), which would map to stat() on
> most platforms but might map to the Windows stat() on cygwin when so
> configured.
> 
> By default the true stat() would always be used.  It should have to be a
> conscious decision, taken only in specific, vetted scenarios, to use the
> unreliable stat.

You have just described my second patch! :D

Show 9 quoted lines

> 
> For example, I can't imagine that checking the freshness of the index or
> of the packed-refs file is ever going to be a bottleneck, so there is no
> reason at all to use an unreliable stat() here.
> 
> On the other hand, stat() seems definitely to be a bottleneck when
> testing for changes in a 100,000 file working tree, and here occasional
> mistakes might be considered acceptable.  So for this purpose the
> unreliable stat() might be used.

I have already written the first pass at this patch, but I'm having difficulty with naming (get_cached_stat_data, get_index_stat_data, get_stat_data, ... ;-)

ATB, Ramsay Jones