Re: [PATCH 2/2] fetch-pack: avoid repeatedly re-scanning pack directory

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jan 27, 2013, 23:20 UTC
Message-ID: <20130127232033.GA8206@elie.Belkin>
In-Reply-To: <7vvcaiv14s.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

> It is not about a rough estimate nor common commits, though.  The
> "everything local" check in question is interested in only one
> thing: are we _clearly_ up to date without fetching anything from
> them?

[...]

> Jonathan Nieder <jrnieder@gmail.com> writes:

Show 12 quoted lines

>>  * Why is 49bb805e ("Do not ask for objects known to be complete",
>>    2005-10-19) trying to do?  Are we hurting that in any way?
>
> An earlier fetch may have acquired all the necessary objects but may
> not have updated our refs for some reason (e.g. fast-forward check
> may have fired).  In such a case, we may already have a history that
> is good (i.e. not missing paths down to the common history) in our
> repository that is not connected to any of our refs, and we can
> update our refs (or write to FETCH_HEAD) without asking the remote
> end to do any common ancestor computation or object transfer.
>
> That was the primary thing the patch wanted to do.

Interesting.

After I fetch objects for branches a, b, and c which all have different commit times in a fetch that fails due to the fast-forward check, I have the following objects:

	a	commit date = 25 January 2013
	b	commit date = 26 January 2013
	c	commit date = 27 January 2013

When I try to fetch again (forcibly this time), git notices that the objects are available locally and sets "cutoff" to 27 January 2013 as a hint about the last time I fetched.

mark_recent_complete_commits() is called, and since these objects are not reachable from any local refs, none are visited in the walk, and 49bb805e does not affect the outcome of the fetch positively or negatively.

On the other hand, if I fetched a, b, and c to local branches and then built on top of them, 49bb805e ensures that a second fetch of the same refs will know right away not to request objects for c. So it brings some of the benefits of 2759cbc7 (git-fetch-pack: avoid unnecessary zero packing, 2005-10-18) when the receiving side has either (A) renamed refs or (B) built new history on top of them.

Correct?

It is only in case (B) that the cutoff matters. If we miscalculate the cutoff, that means either (i) cutoff == 0, and we would lose the benefit of the walk that finds complete commits, or (ii) cutoff is a little earlier, and the walk to find complete commits would take a little longer. Neither effects the result of the fetch, and neither is likely to be a significant enough performance difference to offset the benefit of Jeff's patch.

Thanks for explaining. Jonathan