git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] fetch: skip on-demand checking when no submodules are configured
@ 2011-09-09 18:22 Jens Lehmann
  2011-09-09 21:00 ` Junio C Hamano
  0 siblings, 1 reply; 2+ messages in thread
From: Jens Lehmann @ 2011-09-09 18:22 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Git Mailing List, Martin Fick, Heiko Voigt, Michael Haggerty

It makes no sense to do the - possibly very expensive - call to "rev-list
<new-ref-sha1> --not --all" in check_for_new_submodule_commits() when
there aren't any submodules configured.

Leave check_for_new_submodule_commits() early when no name <-> path
mappings for submodules are found in the configuration. To make that work
reading the configuration had to be moved further up in cmd_fetch(), as
doing that after the actual fetch of the superproject was too late.

Reported-by: Martin Fick <mfick@codeaurora.org>
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

This achieves the first goal: Don't let people pay a performance penalty
when they don't even use submodules. On Michael's test repo from [1] the
time for a full fetch went down from 142 seconds (current master) to one
second which is - not surprisingly - the same as using current master
with the --no-recurse-submodules option.

Now back to the drawing board to fix the performance regression for those
people who are using submodules ...

[1] http://comments.gmane.org/gmane.comp.version-control.git/177103

 builtin/fetch.c |   15 +++++++++------
 submodule.c     |    4 ++++
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 93c9938..e422ced 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -941,6 +941,15 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix,
 			     builtin_fetch_options, builtin_fetch_usage, 0);

+	if (recurse_submodules != RECURSE_SUBMODULES_OFF) {
+		if (recurse_submodules_default) {
+			int arg = parse_fetch_recurse_submodules_arg("--recurse-submodules-default", recurse_submodules_default);
+			set_config_fetch_recurse_submodules(arg);
+		}
+		gitmodules_config();
+		git_config(submodule_config, NULL);
+	}
+
 	if (all) {
 		if (argc == 1)
 			die(_("fetch --all does not take a repository argument"));
@@ -976,12 +985,6 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	if (!result && (recurse_submodules != RECURSE_SUBMODULES_OFF)) {
 		const char *options[10];
 		int num_options = 0;
-		if (recurse_submodules_default) {
-			int arg = parse_fetch_recurse_submodules_arg("--recurse-submodules-default", recurse_submodules_default);
-			set_config_fetch_recurse_submodules(arg);
-		}
-		gitmodules_config();
-		git_config(submodule_config, NULL);
 		add_options_to_argv(&num_options, options);
 		result = fetch_populated_submodules(num_options, options,
 						    submodule_prefix,
diff --git a/submodule.c b/submodule.c
index 7a76edf..ad86534 100644
--- a/submodule.c
+++ b/submodule.c
@@ -481,6 +481,10 @@ void check_for_new_submodule_commits(unsigned char new_sha1[20])
 	const char *argv[] = {NULL, NULL, "--not", "--all", NULL};
 	int argc = ARRAY_SIZE(argv) - 1;

+	/* No need to check if there are no submodules configured */
+	if (!config_name_for_path.nr)
+		return;
+
 	init_revisions(&rev, NULL);
 	argv[1] = xstrdup(sha1_to_hex(new_sha1));
 	setup_revisions(argc, argv, &rev, NULL);
-- 
1.7.7.rc0.189.gf9175

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] fetch: skip on-demand checking when no submodules are configured
  2011-09-09 18:22 [PATCH] fetch: skip on-demand checking when no submodules are configured Jens Lehmann
@ 2011-09-09 21:00 ` Junio C Hamano
  0 siblings, 0 replies; 2+ messages in thread
From: Junio C Hamano @ 2011-09-09 21:00 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Git Mailing List, Martin Fick, Heiko Voigt, Michael Haggerty

Jens Lehmann <Jens.Lehmann@web.de> writes:

> It makes no sense to do the - possibly very expensive - call to "rev-list
> <new-ref-sha1> --not --all" in check_for_new_submodule_commits() when
> there aren't any submodules configured.
>
> Leave check_for_new_submodule_commits() early when no name <-> path
> mappings for submodules are found in the configuration. To make that work
> reading the configuration had to be moved further up in cmd_fetch(), as
> doing that after the actual fetch of the superproject was too late.
>
> Reported-by: Martin Fick <mfick@codeaurora.org>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> ---
>
> This achieves the first goal: Don't let people pay a performance penalty
> when they don't even use submodules. On Michael's test repo from [1] the
> time for a full fetch went down from 142 seconds (current master) to one
> second which is - not surprisingly - the same as using current master
> with the --no-recurse-submodules option.
>
> Now back to the drawing board to fix the performance regression for those
> people who are using submodules ...
>
> [1] http://comments.gmane.org/gmane.comp.version-control.git/177103

Thanks.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2011-09-09 21:00 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-09 18:22 [PATCH] fetch: skip on-demand checking when no submodules are configured Jens Lehmann
2011-09-09 21:00 ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).