[PATCH] Disable useless check_for_new_submodule_commits() by default

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Imagine a project with 20k paths with a history 250k commits deep, without
any submodule.

Imagine another project with the same depth of history but with 1k paths,
among which there are 200 submodules.

Further, imagine fetching from one of the above repositories into a
repository that is very behind, and updating many remote tracking
branches.  Now, think about what check_for_new_submodule_commits() in
submodule.c that is called from update_local_ref() in builtin/fetch.c
would do.

For each updated remote tracking branch (or anything that is not a tag),
the problem function will run the equivalent of:

	git log --raw $new --not --all

which would mean 250k rounds of diff-tree to enumerate the submodules that
may have been updated, and it does it for each and every refs outside the
refs/tags hierarchy.

Presumably, this is so that it only has to actually fetch in the submodule
"on demand", but even if in a project _with_ submodules (i.e. the latter
example above), this is simply not acceptable. You could just enumerate
those 200 submodules at the tip that _might_ matter and that would be
million times cheaper. To add insult to injury, this "on demand" behaviour
is on by default, which hurts projects without any submodules (i.e. the
former example above) a lot.

In short, if "on demand" check is million times more expensive than
actually doing it, the check does not have any value.

In the longer term, people who want to have the on-demand behaviour need
to come up with a cheaper way to determine if it is necessary to recurse
into submodules by fixing check_for_new_submodule_commits(), but until
that happens, we should disable submodule recursion by default unless the
user explicitly asks for it from the command line or from the configuration.

Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---

 * This is meant for 1.7.6.X maintenance track to fix the huge regression
   caused by 88a2197 (fetch/pull: recurse into submodules when necessary,
   2011-03-06).  I would very much appreciate a simpler and trivially
   correct patch as a counter-proposal that makes sure that the expensive
   check_for_new_submodule_commits() and fetch_populated_submodules()
   functions are never called when the user does not have any command line
   option or configuration variable to tell otherwise. I suspect that this
   patch may be disabling more things than absolutely necessary.

   I also suspect that it might be just a matter of finding any and all
   comparison of the form:

	if (recurse != ON && recurse != OFF)

   that was present before the on-demand stuff and replace that with
        
	if (recurse == ON_DEMAND)

   but I didn't look very closely. At least this patch is much less error
   prone for the purpose of not triggering the expensive and pointless
   check ;-)

 builtin/fetch.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 93c9938..71232c1 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -29,7 +29,7 @@ enum {
 };
 
 static int all, append, dry_run, force, keep, multiple, prune, update_head_ok, verbosity;
-static int progress, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
+static int progress, recurse_submodules = RECURSE_SUBMODULES_OFF;
 static int tags = TAGS_DEFAULT;
 static const char *depth;
 static const char *upload_pack;

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]