On Thu, Jan 28, 2021 at 06:33:10PM -0800, Junio C Hamano wrote:
> Taylor Blau <me@xxxxxxxxxxxx> writes:
>
> > 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
