Re: [PATCH v3 4/4] clone: open a shortcut for connectivity check

From: Duy Nguyen <pclouds@gmail.com>
Date: May 4, 2013, 01:10 UTC
Message-ID: <CACsJy8BSMkFGQcHxFD3wTn_PpC=qEFJ+vR_XygndzVnprye3Aw@mail.gmail.com>
In-Reply-To: <7vwqrgxcoc.fsf@alter.siamese.dyndns.org>

On Fri, May 3, 2013 at 11:15 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 13 quoted lines

> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> In order to make sure the cloned repository is good, we run "rev-list
>> --objects --not --all $new_refs" on the repository. This is expensive
>> on large repositories. This patch attempts to mitigate the impact in
>> this special case.
>>
>> In the "good" clone case, we only have one pack.
>
> If "On large repositories" is the focus, we need to take into
> account the fact that pack.packSizeLimit can split and store the
> incoming packstream to multiple packs, so "only have one pack" is
> misleading.

I only had a quick look. But I don't think index-pack respects packSizeLimit. pack-objects does but only when --stdout is not used, which is not the case for pack transfer.

Show 5 quoted lines

> I think you can still do the same trick even when we split the pack
> as index-pack will keep track of the objects it saw in the same
> incoming pack stream (but I am writing this from memory without
> looking at the original code you are touching, so please double
> check).

Yeah. As long we have only one incoming stream, we can still do the same verification.

Show 5 quoted lines

>> "index-pack + new checks" is still faster than the current "index-pack
>> + rev-list", which is the whole point of this patch. If any of the
>
> Does the same check apply if we end up on the unpack-objects
> codepath?

No. unpack-objects does not do this and check_everything_connected should invoke rev-list like before. -- Duy