On Sun, Feb 25, 2018 at 08:30:48PM -0500, Jeff King wrote:
> > diff --git a/revision.c b/revision.c
> > index 5ce9b93..bc7def5 100644
> > --- a/revision.c
> > +++ b/revision.c
> > @@ -113,7 +113,8 @@ void mark_parents_uninteresting(struct commit *commit)
> > * it is popped next time around, we won't be trying
> > * to parse it and get an error.
> > */
> > - if (!has_object_file(&commit->object.oid))
> > + if (!commit->object.parsed &&
> > + !has_object_file(&commit->object.oid))
> > commit->object.parsed = 1;
>
> We don't actually need the object contents at all right here. This is
> just faking the "parsed" flag for later so that calls to parse_object()
> don't barf.
>
> This code comes originally form 454fbbcde3 (git-rev-list: allow missing
> objects when the parent is marked UNINTERESTING, 2005-07-10). But later,
> in aeeae1b771 (revision traversal: allow UNINTERESTING objects to be
> missing, 2009-01-27), we marked dealt with calling parse_object() on the
> parents more directly.
>
> So what I wonder is whether this code is simply redundant and can go
> away entirely. That would save the has_object_file() call in all cases.
There's a similar case for trees. In mark_tree_contents_uninteresting()
we do:
if (!has_object_file(&obj->oid))
return;
if (parse_tree(tree) < 0)
die("bad tree %s", oid_to_hex(&obj->oid));
which seems wasteful. Probably this could be:
if (parse_tree_gently(tree, 1) < 0)
return; /* missing uninteresting trees ok */
though technically the existing code allows _missing_ trees, but
not on corrupt ones.
I guess this is perhaps less interesting, because we only mark trees
directly fed from the pending array, not every tree of commits that we
traverse. Though if you had a really gigantic tree, it might be
measurable.
-Peff
