fetch: skip on-demand checking when no submodules are configured

PATCH In review Jens Lehmann 2 participants 2 messages Jun 15, 2016

Share & tools

Jens Lehmann

author

2016-06-15 22:52 UTC #0 lore raw

PATCH 1/1 fetch: skip on-demand checking when no submodules are configured

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 <[email protected]>

Signed-off-by Jens Lehmann <[email protected]>

---

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(-)

builtin/fetch.c +9 -6

index 93c9938..e422ced 100644
@@ -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,

submodule.c +4 -0

index 7a76edf..ad86534 100644
@@ -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);

Signature · 1 line

1.7.7.rc0.189.gf9175

Junio C Hamano

2016-06-15 22:52 UTC #1 lore raw

Quoted 19 lines · Jens Lehmann

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 <[email protected]>
Signed-off-by Jens Lehmann <[email protected]>
---
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.

Quoted 4 lines

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.