On Thu, Oct 13, 2016 at 12:30:52AM +0200, Vegard Nossum wrote:

> However, the commit found by 'git blame' above appears just fine to me,
> I haven't been able to spot a bug in it.
> A closer inspection reveals the problem to really be that this is an
> extremely hot path with more than -- holy cow -- 4,106,756,451
> iterations on the 'packed_git' list for a single 'git fetch' on my
> repository. I'm guessing the patch above just made the inner loop
> ever so slightly slower.
> My .git/objects/pack/ has ~2088 files (1042 idx files, 1042 pack files,
> and 4 tmp_pack_* files).

Yeah. I agree that the commit you found makes the check a little more
expensive, but I think the root of the problem is calling
prepare_packed_git_one many times. This _should_ happen once for each
pack at program startup, and possibly again if we need to re-scan the
pack directory to account for racing with a simultaneous repack.

The latter is generally triggered when we fail to look up an object we
expect to exist. So I'd suspect 45e8a74 (has_sha1_file: re-check pack
directory before giving up, 2013-08-30) is playing a part. We dealt with
that to some degree in 0eeb077 (index-pack: avoid excessive re-reading
of pack directory, 2015-06-09), but it would not surprise me if there is
another spot that needs similar treatment.

Does the patch below help?

diff --git a/builtin/fetch.c b/builtin/fetch.c
index d5329f9..c0f3c2c 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -243,7 +243,7 @@ static void find_non_local_tags(struct transport *transport,
                if (ends_with(ref->name, "^{}")) {
                        if (item && !has_object_file(&ref->old_oid) &&
                            !will_fetch(head, ref->old_oid.hash) &&
-                           !has_sha1_file(item->util) &&
+                           !has_sha1_file_with_flags(item->util, 
                            !will_fetch(head, item->util))
                                item->util = NULL;
                        item = NULL;
@@ -256,7 +256,8 @@ static void find_non_local_tags(struct transport *transport,
                 * to check if it is a lightweight tag that we want to
                 * fetch.
-               if (item && !has_sha1_file(item->util) &&
+               if (item &&
+                   !has_sha1_file_with_flags(item->util, HAS_SHA1_QUICK) &&
                    !will_fetch(head, item->util))
                        item->util = NULL;
@@ -276,7 +277,8 @@ static void find_non_local_tags(struct transport *transport,
         * We may have a final lightweight tag that needs to be
         * checked to see if it needs fetching.
-       if (item && !has_sha1_file(item->util) &&
+       if (item &&
+           !has_sha1_file_with_flags(item->util, HAS_SHA1_QUICK) &&
            !will_fetch(head, item->util))
                item->util = NULL;


Reply via email to