Re: [PATCH 02/10] revision: learn '--no-kept-objects'

Taylor Blau [off-list ref] writes:

Some callers want to perform a reachability traversal that terminates
when an object is found in a kept pack. The closest existing option is
'--honor-pack-keep', but this isn't quite what we want. Instead of
halting the traversal midway through, a full traversal is always
performed, and the results are only trimmed afterwords.

True.  

Is there a reason to keep both kinds?  It is obvious that stopping
traversal once we hit a kept pack would be more time and space
efficient (I presume that the reason why .kept pack matters is
because we are repacking everything else) to enumerate the objects
that need to be repacked than traversing all the way and filtering
out objects that appear in .kept packs, but would there be some
correctness implications to replace the existing use of
"--honor-pack-keep" with "--no-kept-objects=on-disk"?  

What it means to be excluded by the former is quite clear: any
object that appears in a kept pack, whether another copy of it
appears elsewhere, is excluded from getting enumerated for
repacking.  It is quite unclear what it means to enumerate objects
with "--no-kept-objects".  It is clear from the implementation side
of the thing (stop traversal at objects that appear in any kept
pack), but it is totally unclear what such a meaning defined
operationally affects the resulting enumeration.  We know that the
enumerated objects do not appear in any of the kept pack, but it
does not mean all objects that are reachable/in-use that are not in
any kept packs are enumerated.

Is it explained anywhere how "in-core kept state" is bootstrapped,
modified and maintained?

The patch to C-part itself is a trivially correct implementation of
"stop at an object that can be found in a kept pack", and there is
no comment, but it is not clear to me what we want to achieve by
this.  Is the underlying assumption that no objects in .kept pack
would refer to outside world, either loose or packs that are not
kept?  How are we guaranteeing it?

On Thu, Jan 28, 2021 at 07:10:04PM -0800, Junio C Hamano wrote:

We know that the enumerated objects do not appear in any of the kept
pack, but it does not mean all objects that are reachable/in-use that
are not in any kept packs are enumerated.

You raise a very valid point. FWIW, I originally wrote these patches as
just "enumerate the objects in these small packs, make a new pack out of
those, and then (optionally) delete the small ones. I abandoned that
idea because it needs special handling for loose objects, and it has no
idea which objects are unreachable, etc.

But maybe it is time to go back to the drawing board there. Perhaps a
`--geometric` repack implies that we keep unreachable objects in effect,
and that a full repack (i.e., one that does reachability analysis) is
required to drop them.

Other ideas are welcome.

Thanks,
Taylor