Re: [PATCH v2 7/7] pack-objects: use mru list when iterating over packs

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> Here's a list of approaches I think we can use to fix this:
>
>   1. squelch the warning and ignore it. The downside here, besides not
>      warning the user about true in-pack cycles, is that we have no
>      opportunity to actually find a new delta (because we realize the
>      problem only after the delta-compression phase).
>
>      My test repository is a bad packing of all of the forks of
>      torvalds/linux, with 3600 packs. I'm happy to share it if anybody
>      wants to see it, but note that it is 11GB.
>
>      The space overhead of the resulting pack in this case is ~3.2%
>      (versus a pack generated by the original code, using the static
>      pack order).  Which is really not that bad, but I'm sure there are
>      more pathological cases (i.e., there were on the order of hundreds
>      or maybe thousands of cycles that needed broken, out of about 15
>      million total objects; but one could imagine the worst case as
>      nr_of_objects/2).
> ...
>
>     So I dunno. I really like the MRU approach if we can salvage it.

I think I share the same feeling.  As long as the chance is small
enough that the pack reordering creates a new cycle, the resulting
pack would not become too bloated by the last-ditch cycle breaking
code and finding a replacement delta instead of inflating it may not
be worth the trouble.

It worries me a lot to lose the warning unconditionally, though.
That's the (only) coal-mine canary that lets us notice a problem
when we actually start hitting that last-ditch cycle breaking too
often.