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

From: Junio C Hamano <gitster@pobox.com>
Date: May 3, 2013, 16:15 UTC
Message-ID: <7vwqrgxcoc.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1367584514-19806-5-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

Show 6 quoted lines

> 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 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).

Show 16 quoted lines

> If all of the
> following are met, we can be sure that all objects reachable from the
> new refs exist, which is the intention of running "rev-list ...":
>
>  - all refs point to an object in the pack
>  - there are no dangling pointers in any object in the pack
>  - no objects in the pack point to objects outside the pack
>
> The second and third checks can be done with the help of index-pack as
> a slight variation of --strict check (which introduces a new condition
> for the shortcut: pack transfer must be used and the number of objects
> large enough to call index-pack). The first is checked in
> check_everything_connected after we get an "ok" from index-pack.
>
> "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?

> This shortcut is not applied to shallow clones, partly because shallow
> clones should have no more objects than a usual fetch and the cost of
> rev-list is acceptable, partly to avoid dealing with corner cases when
> grafting is involved.