On Sun, Aug 14, 2022 at 10:32:12PM -0700, Junio C Hamano wrote:

quoted

We're in the middle of walking through the entries of a tree object via
process_tree_contents(). We see a blob (or it could even be another tree
entry) that we don't have, so we call is_promisor_object() to check it.
That function loops over all of the objects in the promisor packfile,
including the tree we're currently walking.

I forgot that the above "loops over" happens only once to populate
the oidset hashtable, and briefly wondered if we are being grossly
inefficient by scanning pack .idx file each time we encounter a
missing object.  "Upon first call, that function loops over
... walking, to prepare a hashtable to answer if any object id is
referred to by an object in promisor packs" would have helped ;-).

Right. When you have worked in an area, sometimes it is easy to forget
which things are common knowledge and which are not. :) I don't mind at
all if you want to amend the commit message as you apply.

quoted

It may also be a good direction for this function in general, as there
are other possible optimizations that rely on doing some analysis before
parsing:

  - we could detect blobs and avoid reading their contents; they can't
    link to other objects, but parse_object() doesn't know that we don't
    care about checking their hashes.

  - we could avoid allocating object structs entirely for most objects
    (since we really only need them in the oidset), which would save
    some memory.

  - promisor commits could use the commit-graph rather than loading the
    object from disk

This commit doesn't do any of those optimizations, but I think it argues
that this direction is reasonable, rather than relying on parse_object()
and trying to teach it to give us more information about whether it
parsed.

Yeah, all of the future bits sound sensible. 

I very intentionally didn't work on those things yet, because I wanted
to make sure we got a simple fix in as quickly as possible. That said, I
don't have immediate plans for them. They are perhaps not quite small
enough for #leftoverbits, but I think they might also be nice bite-sized
chunks for somebody wanting to get their feet wet in that part of the
code.

-Peff