Re: [PATCH v2 11/19] pack-objects: use bitmaps when packing objects

[Duy Nguyen <pclouds@gmail.com>]

On Wed, Oct 30, 2013 at 2:36 PM, Jeff King <peff@peff.net> wrote:
> On Sat, Oct 26, 2013 at 05:25:14PM +0700, Nguyen Thai Ngoc Duy wrote:
>
>> > For bitmaps to be used, the following must be true:
>> >
>> >   1. We must be packing to stdout (as a normal `pack-objects` from
>> >      `upload-pack` would do).
>> >
>> >   2. There must be a .bitmap index containing at least one of the
>> >      "have" objects that the client is asking for.
>> >
>> >   3. Bitmaps must be enabled (they are enabled by default, but can be
>> >      disabled by setting `pack.usebitmaps` to false, or by using
>> >      `--no-use-bitmap-index` on the command-line).
>> >
>> > If any of these is not true, we fall back to doing a normal walk of the
>> > object graph.
>>
>> I haven't read the bitmap creation code yet. But it probably does not
>> matter. If the client requests a shallow fetch, you probably want to
>> fall back to normal walk too.
>
> One other criterion I should have mentioned: we must be using the
> internal rev-list. That prevented us in v1.8.4.1 and earlier from using
> bitmaps for shallow fetches. But as of v1.8.4.2, we always use
> pack-objects' rev-walker. We may need to pass --no-use-bitmap-index for
> shallow fetches.

I don't think a new option is needed. The code just needs to check if
there are any commit grafts. If there are, fall back to the old way.
That covers both shallow fetches and some rare grafted repos. I think
refs/replace/* does not impact rev walking, so we should be fine if
it's used.

> As for repos that are themselves shallow, I do not know how doing a
> "repack -b" would fare. Probably not well.
-- 
Duy