Re: [PATCH v5] pack-objects mru

On Thu, Aug 11, 2016 at 08:11:33AM -0700, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

So considering "--depth" as a space-saving measure for --aggressive does
not seem that effective. But it feels weird to quietly drop actions
people might have done with previous aggressive runs.

That argument cuts both ways, doesn't it?

If the user explicitly asks to use lower "--depth" from the command
line when the second repack runs, the intention is clear: the
existing pack may use delta chains that are too long and is
detrimental to the run-time performance, and the user wants to
correct it by repacking with shorter delta chain.

Should the act of letting "gc --auto" use lower "--depth", by not
configuring to always use deeper chain, be interpreted the same way?
I am not sure.  The old packing with large --depth is something the
user did long time ago, and the decision the user made not to use
large depth always is also something the user did long time ago.  I
do not think it is so cut-and-dried which one of the two conflicting
wishes we should honor when running the second repack, especially
when it is run unattended like "gc --auto" does.

Good points. Explicitly saying "repack --depth=..." carries a lot more
weight to me than "git gc --auto" randomly kicking in, as far as knowing
that what the user actually wants. My patch doesn't differentiate, of
course, but I think it could.

The other problem with my patch is the fact that we don't do a good job
of finding new, in-limit deltas for the ones we discard. If you want to
do that, you really need to "git repack -f" (at least with the current
code). At which point we do not reuse the on-disk deltas at all, and the
problem is moot (you could also interpret the fact that the user did
_not_ pass "-f" as "you want to reuse deltas, which means you want to
reuse even long chains", but as you've argued above, you can make a lot
of guesses about the user's intention from what they did or did not
say).

So if we were to go this route, I don't think my patch is quite
sufficient; we'd want something else on top to do a better job of
finding replacement deltas.

Regarding my "does not seem that effective" above, I think we should
drop the aggressive depth to 50, and I just posted a patch with
reasoning and numbers:

  http://public-inbox.org/git/20160811161309.ecmebaafcz6rkg6o@sigill.intra.peff.net/

That's maybe orthogonal, but it does remove the weird "gc --aggressive
followed by gc --auto produces a bad pack" issue, because unless you are
doing something clever, the depth will always be 50 (modulo people who
did an aggressive pack with an older version of git :-/ ).

-Peff