Re: [PATCH 01/10] packfile: introduce 'find_kept_pack_entry()'

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

Taylor Blau [off-list ref] writes:

quoted

Future callers will want a function to fill a 'struct pack_entry' for a
given object id but _only_ from its position in any kept pack(s). They
could accomplish this by calling 'find_pack_entry()' and checking
whether the found pack is kept or not, but this is insufficient, since
there may be duplicate objects (and the mru cache makes it unpredictable
which variant we'll get).

I wonder if we eventually need a callback interface to walk _all_
pack entries for a given object, so that "I am only interested in
instances in kept packs" will be under total control of the callers.
As it stands, it is "just grab any one that is in a kept pack, any
one of them is fine", which is almost just of as narrow utility as
the original's "just grab the first one---any one of them is fine",
the latter of which is "insufficient" as the log message says.

We do that already in pack-objects, and that's the problem: it's really
slow. So if you have few kept packs, but a lot of other ones, you'd like
to pre-split the packs into two lists, and not bother walking the one
you know won't turn up interesting results.

I think the commit message here doesn't emphasize that reasoning enough.
It talks about using "find_pack_entry()", and that is definitely not
sufficient for our purposes. But the interesting part is replacing the
existing "walk all packs and see if any were kept" logic, which happens
in patch 6.

So the more compelling argument, I think, is something like:

  - you sometimes want to know if object X is any kept packs

  - you can't use find_pack_entry(), because it only gives you the first
    pack it finds

  - you can walk over all packs and look for the object in each.
    pack-objects does this. But it's slow, because you are looking in
    packs you don't care about.

  - so it's helpful for the lookup to know up front which packs are
    interesting to find objects in and which are not, to avoid looking
    in the uninteresting ones

-Peff