On Mon, Jul 25, 2016 at 02:40:25PM -0400, Jeff King wrote:
> > @@ -1052,6 +1053,9 @@ static int add_object_entry_from_bitmap(const unsigned char *sha1,
> > {
> > uint32_t index_pos;
> >
> > + if (local && has_loose_object_nonlocal(sha1))
> > + return 0;
> > +
> > if (have_duplicate_entry(sha1, 0, &index_pos))
> > return 0;
>
> Hrm. Adding entries from the bitmap should ideally be very fast, but
> here we're introducing extra lookups in the object database. I guess it
> only kicks in when --local is given, though, which most bitmap-using
> paths would not do.
>
> But is this check enough? The non-bitmap code path calls
> want_object_in_pack, which checks not only loose objects, but also
> non-local packs, and .keep.
>
> Those don't kick in for your use case. I wonder if we should simply have
> something like:
>
> if (local || ignore_packed_keep)
> use_bitmap_index = 0;
>
> and just skip bitmaps for those cases. That's easy to reason about, and
> I don't think anybody would care (your use case does not, and the repack
> use case is already not going to use bitmaps).
BTW, I thought we had more optimizations in this area, but I realized
that I had never sent them to the list. I just did, and you may want to
take a peek at:
http://thread.gmane.org/gmane.comp.version-control.git/300218
I doubt it will speed up your case much (unless you really do have tons
of packs in your extraction). And I think it is still worth doing
disabling I showed above, even with the optimizations, just because it's
easier to reason about.
So I _think_ those optimizations are orthogonal to what we're discussing
here, but I wanted to point you at them just in case.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html