Re: [PATCH] fetch: use "quick" has_sha1_file for tag following

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 17, 2016, 17:30 UTC
Message-ID: <xmqqeg3ex4qz.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20161014185953.k4b5xwihlgvxurjc@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 5 quoted lines

> Still an impressive speedup as a percentage, but negligible in absolute
> terms. But that's on a local filesystem on a Linux machine. I'd worry
> much more about a system with a slow readdir(), e.g., due to NFS.
> Somebody's real-world NFS case[1] was what prompted us to do 0eeb077
> (index-pack: avoid excessive re-reading of pack directory, 2015-06-09).

Yes.

> It looks like I _did_ look into optimizing this into a single stat()
> call in the thread at [1]. I completely forgot about that. I did find
> there that naively using stat_validity() on a directory is racy, though
> I wonder if we could do something clever with gettimeofday() instead.

It feels funny to hear an idea to compare fs timestamp with gettimeofday immedately after hearing the word NFS, though ;-).

Show 9 quoted lines

>> I agree that the fallout from the inaccuracy of "quick" approach is
>> probably acceptable and the next "fetch" will correct it anyway, so
>> let's do the "quick but inaccurate" for now and perhaps cook it in
>> 'next' for a bit longer than other topics?
>
> I doubt that cooking in 'next' for longer will turn up anything useful.
> The case we care about is the race between a repack and a fetch. We
> lived with the "quick" version of has_sha1_file() everywhere for 8
> years.

A very convincing argument.  I stand corrected.

Thanks.