> for_each_object_in_pack() is a fine way to make sure that you list
> everythning in a pack, but I suspect it is a horrible way to feed a
> list of objects to pack-objects, as it goes by the .idx order, which
> is by definition a way to enumerate objects in a randomly useless
> order.
That's true.
> Do we already have an access to the in-core reverse index for the
> pack at this point in the code?
As far as I can tell, no. These patches construct the list of promisor
objects in repack.c (which does not use the revindex at all), to be
processed by pack-objects in a different process (which does use the
revindex in reuse-delta mode, which is the default). I guess that we
could have access to the revindex if we were to libify pack-objects and
run it in the same process as repack.c or if we were to add a special
mode to pack-objects that reads for itself the list of all the promisor
objects.
> If so, we can enumerate the objects
> in the offset order without incurring a lot of cost (building the
> in-core revindex is the more expensive part). When writing a pack,
> we try to make sure that related objects are written out close to
> each other [*1*], so listing them in the offset order (with made-up
> pathname information to _force_ that objects that live close
> together in the original pack are grouped together by getting
> similar names) might give us a better than horrible deltification.
> I dunno.
Before I write the NEEDSWORK comment, just to clarify - do you mean
better than horrible object locality? I think deltification still occurs
because pack-objects sorts the input when doing the windowed
deltification, for example:
git rev-parse HEAD:fetch-pack.c HEAD HEAD^ HEAD^^ \
HEAD^^^ v2.17.0:fetch-pack.c | git pack-objects --window=2 abc
produces a packfile with a delta, as checked by `verify-pack -v`.
> Side note *1*: "related" has two axis, and one is relevant
> for better deltification, while the other is not useful.
> The one I have in mind here is that we write set of blobs
> that belong to the same "delta family" together.
As far as I can see, they do not need to be adjacent in the packfile to
have one be a delta against the other.
> I do not think such a "make it a bit better than horrible" is necessary
> in an initial version, but it deserves an in-code NEEDSWORK comment
> to remind us that we need to measure and experiment.
OK, I'll do this in the next reroll.
