Re: [PATCH] pack-objects: Use reachability bitmap index when generating non-stdout pack too

[Jeff King <peff@peff.net>]

On Thu, Jul 07, 2016 at 10:09:17PM +0300, Kirill Smelkov wrote:

> Starting from 6b8fda2d (pack-objects: use bitmaps when packing objects)
> if a repository has bitmap index, pack-objects can nicely speedup
> "Counting objects" graph traversal phase. That however was done only for
> case when resultant pack is sent to stdout, not written into a file.
> 
> We can teach pack-objects to use bitmap index for initial object
> counting phase when generating resultant pack file too:

I'm not sure this is a good idea in general. When bitmaps are in use, we
cannot fill out the details in the object-packing list as thoroughly. In
particular:

  - we will not compute the same write order (which is based on
    traversal order), leading to packs that have less efficient cache
    characteristics

  - we don't learn about the filename of trees and blobs, which is going
    to make the delta step much less efficient. This might be mitigated
    by turning on the bitmap name-hash cache; I don't recall how much
    detail pack-objects needs on the name (i.e., the full name versus
    just the hash).

There may be other subtle things, too. The general idea of tying the
bitmap use to pack_to_stdout is that you _do_ want to use it for
serving fetches and pushes, but for a full on-disk repack via gc, it's
more important to generate a good pack.

Your use case:

> git-backup extracts many packs on repositories restoration. That was my
> initial motivation for the patch.

Seems to be somewhere in between. I'm not sure I understand how you're
invoking pack-objects here, but I wonder if you should be using
"pack-objects --stdout" yourself.

But even if it is the right thing for your use case to be using bitmaps
to generate an on-disk bitmap, I think we should be making sure it
_doesn't_ trigger when doing a normal repack.

-Peff