Re: Huge performance bottleneck reading packs

From: Jeff King <peff@peff.net>
Date: Oct 13, 2016, 15:26 UTC
Message-ID: <20161013152632.ynsabbv3yrthgidy@sigill.intra.peff.net>
In-Reply-To: <fb66dc02-fb75-5aad-74e5-01b969cf9f9a@oracle.com>

On Thu, Oct 13, 2016 at 09:20:07AM +0200, Vegard Nossum wrote:

Show 8 quoted lines

> > Does the patch below help?
> 
> Yes, ~2m10s -> ~1m25s when I test a git fetch this morning (the other
> variation in time may be due to different CPU usage by other programs,
> but I ran with/without the patch multiple times and the difference is
> consistent).
> [...]
> There are some 20k refs on the remote, closer to 25k locally.

OK, that makes some sense. For whatever reason, your remote has a bunch of tags that point to objects you do not already have. That could happen, I think, if the remote added a lot of tags since you cloned (because clone will grab all of the tags), but those tags do not point to history that you are otherwise fetching (since fetch by default will "auto-follow" such tags).

It might be interesting to see the results of:

  # all the objects we have
  git cat-file --batch-all-objects --batch-check='%(objectname)' >us

  # all the objects the remote is advertising
  git ls-remote origin | cut -f1 | sort -u >them

  # what they are advertising that we don't have
  comm -13 us them | wc -l

My guess is that the number is relatively high. And that would explain why nobody else has really complained much; such a pattern is probably uncommon.

> The fetch doesn't actually get anything from the remote as everything is
> already up to date (that makes the 2m40s times even more frustrating in
> a way :-)). Here's count-objects:

If the fetch is largely a noop, then that makes me wonder why we are spending even a minute in the "good" case. I wonder if there is some other spot that is wasting CPU on some inefficient data structure related to the number of refs you have. If you can do any profiling that points to a hot spot, that would be interesting to see (and also whether a gc improves things).

I see in find_non_local_tags() that we build up a sorted string_list via repeated calls to string_list_insert(), which will keep the thing sorted at each stage. That's not as efficient as just sorting at the end, but I think it's probably OK in practice because we actually feed it via for_each_ref(), whose output is sorted, and so we'd always just be appending to the end.

-Peff