Re: [PATCH] list-objects: don't queue root trees unless revs->tree_objects is set

From: Jeff King <peff@peff.net>
Date: Sep 12, 2019, 22:34 UTC
Message-ID: <20190912223400.GA2580@sigill.intra.peff.net>
In-Reply-To: <xmqqd0g5h2xm.fsf@gitster-ct.c.googlers.com>

On Thu, Sep 12, 2019 at 09:52:53AM -0700, Junio C Hamano wrote:

Show 17 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> >> I was surprised we ever called repo_get_commit_tree() at all, since
> >> we're literally just traversing commits here. It looks like
> >> list-objects.c is very happy to queue pending trees for each commit,
> >> even if we're just going to throw them away when we get to
> >> process_tree()! I wonder if could be checking revs->tree_objects here
> >> and saving ourselves some work.
> >
> > Indeed, this seems to help quite a bit in the commit-graph case. I think
> > it's worth doing (and is independent of the other patch).
> 
> Yeah, I agree this is very much worth doing and is orthogonal to the
> other one.
> 
> Thanks for spotting it.  I wonder if it was broken like this forever
> since the beginning X-<.

Not quite since the beginning; it comes from 8d2dfc49b1 (process_{tree,blob}: show objects without buffering, 2009-04-10). I suspect nobody noticed because the cost for the normal case is really just shuffling some pointers around. It's only because it actively works against the commit-graph optimizations that it's so expensive there.

I was surprised (and pleased) by how much such a simple thing helped.

-Peff