Re: [WIP v2 2/2] pack-objects: support --blob-max-bytes

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jun 2, 2017, 23:25 UTC
Message-ID: <20170602232508.GA21733@aiede.mtv.corp.google.com>
In-Reply-To: <20170602222640.u6vni5tdpjp3sayt@sigill.intra.peff.net>

Jeff King wrote:

> On Fri, Jun 02, 2017 at 12:38:45PM -0700, Jonathan Tan wrote:

Show 27 quoted lines

>> +--blob-max-bytes=<n>::
>> +	This option can only be used with --stdout. If specified, a blob
>> +	larger than this will not be packed unless a to-be-packed tree
>> +	has that blob with a filename beginning with ".git".  The size
>> +	can be suffixed with "k", "m", or "g", and may be "0".
>> ++
>> +If specified, after printing the packfile, pack-objects will print the
>> +count of excluded blobs (8 bytes) . Subsequently, for each excluded blob
>> +in hash order, pack-objects will print its hash (20 bytes) and its size
>> +(8 bytes). All numbers are in network byte order.
>> ++
>> +If pack-objects cannot efficiently determine, for an arbitrary blob,
>> +that no to-be-packed tree has that blob with a filename beginning with
>> +".git" (for example, if bitmaps are used to calculate the objects to be
>> +packed), it will pack all blobs regardless of size.
>
> Hmm. So this feature can't be used with bitmaps at all?  That seems like
> a big downside, as we still have to walk the whole graph to come up with
> the list of blobs (even if we're sending them). That's 30-40 seconds of
> CPU per clone on torvalds/linux. I imagine it's much worse on
> repositories big enough to need a full GVFS-style "don't send any blobs"
> approach.
>
> We have a name-hash cache extension in the bitmap file, but it doesn't
> carry enough information to deduce the .git-ness of a file. I don't
> think it would be too hard to add a "flags" extension, and give a single
> bit to "this is a .git file".

A nicer approach IMHO is to include an extra bitmap, like the existing object-type bitmaps (see the dump_bitmap calls in bitmap_writer_finish). This would would represent the set of all .git* blobs in the pack.

[...]

>                      If you're not just avoiding large blobs but trying
> to get a narrow clone, you don't want the .git files from the
> uninteresting parts of the tree.

This is something I've wondered about, too. Part of the story is that we haven't started omitting trees, so there is already O(number of trees) objects being sent and some additional small blobs for .git* specials doesn't make it much worse. Sending all .git* blobs keeps things simple since the server doesn't have to infer which .git* blobs are relevant to this client.

Longer term, we will likely want to allow clients to request omission of some trees, too. Omitting the corresponding .git* files becomes more straightforward at that point.

Does that make sense?

Jonathan