{"thread":{"id":"28347","subject":"[PATCH] fetch: skip on-demand checking when no submodules are configured","startedAt":"2011-09-09T18:22:03Z","lastAt":"2011-09-09T21:00:29Z","messageCount":2,"participants":["Jens Lehmann","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"175222","messageId":"4E6A594B.9030108@web.de","threadId":"28347","inReplyTo":null,"subject":"[PATCH] fetch: skip on-demand checking when no submodules are configured","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-09-09T18:22:03Z","receivedAt":"2011-09-09T18:22:03Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"It makes no sense to do the - possibly very expensive - call to \"rev-list\n --not --all\" in check_for_new_submodule_commits() when\nthere aren't any submodules configured.\n\nLeave check_for_new_submodule_commits() early when no name <-> path\nmappings for submodules are found in the configuration. To make that work\nreading the configuration had to be moved further up in cmd_fetch(), as\ndoing that after the actual fetch of the superproject was too late.\n\nReported-by: Martin Fick \nSigned-off-by: Jens Lehmann \n---\n\nThis achieves the first goal: Don't let people pay a performance penalty\nwhen they don't even use submodules. On Michael's test repo from [1] the\ntime for a full fetch went down from 142 seconds (current master) to one\nsecond which is - not surprisingly - the same as using current master\nwith the --no-recurse-submodules option.\n\nNow back to the drawing board to fix the performance regression for those\npeople who are using submodules ...\n\n[1] http://comments.gmane.org/gmane.comp.version-control.git/177103\n\n builtin/fetch.c | 15 +++++++++------\n submodule.c | 4 ++++\n 2 files changed, 13 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 93c9938..e422ced 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -941,6 +941,15 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix,\n \t\t\t builtin_fetch_options, builtin_fetch_usage, 0);\n\n+\tif (recurse_submodules != RECURSE_SUBMODULES_OFF) {\n+\t\tif (recurse_submodules_default) {\n+\t\t\tint arg = parse_fetch_recurse_submodules_arg(\"--recurse-submodules-default\", recurse_submodules_default);\n+\t\t\tset_config_fetch_recurse_submodules(arg);\n+\t\t}\n+\t\tgitmodules_config();\n+\t\tgit_config(submodule_config, NULL);\n+\t}\n+\n \tif (all) {\n \t\tif (argc == 1)\n \t\t\tdie(_(\"fetch --all does not take a repository argument\"));\n@@ -976,12 +985,6 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \tif (!result && (recurse_submodules != RECURSE_SUBMODULES_OFF)) {\n \t\tconst char *options[10];\n \t\tint num_options = 0;\n-\t\tif (recurse_submodules_default) {\n-\t\t\tint arg = parse_fetch_recurse_submodules_arg(\"--recurse-submodules-default\", recurse_submodules_default);\n-\t\t\tset_config_fetch_recurse_submodules(arg);\n-\t\t}\n-\t\tgitmodules_config();\n-\t\tgit_config(submodule_config, NULL);\n \t\tadd_options_to_argv(&num_options, options);\n \t\tresult = fetch_populated_submodules(num_options, options,\n \t\t\t\t\t\t submodule_prefix,\ndiff --git a/submodule.c b/submodule.c\nindex 7a76edf..ad86534 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -481,6 +481,10 @@ void check_for_new_submodule_commits(unsigned char new_sha1[20])\n \tconst char *argv[] = {NULL, NULL, \"--not\", \"--all\", NULL};\n \tint argc = ARRAY_SIZE(argv) - 1;\n\n+\t/* No need to check if there are no submodules configured */\n+\tif (!config_name_for_path.nr)\n+\t\treturn;\n+\n \tinit_revisions(&rev, NULL);\n \targv[1] = xstrdup(sha1_to_hex(new_sha1));\n \tsetup_revisions(argc, argv, &rev, NULL);\n-- \n1.7.7.rc0.189.gf9175\n"},{"id":"175246","messageId":"7vaaadtor6.fsf@alter.siamese.dyndns.org","threadId":"28347","inReplyTo":"4E6A594B.9030108@web.de","subject":"Re: [PATCH] fetch: skip on-demand checking when no submodules are configured","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-09T21:00:29Z","receivedAt":"2011-09-09T21:00:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann writes:\n\n> It makes no sense to do the - possibly very expensive - call to \"rev-list\n> --not --all\" in check_for_new_submodule_commits() when\n> there aren't any submodules configured.\n>\n> Leave check_for_new_submodule_commits() early when no name <-> path\n> mappings for submodules are found in the configuration. To make that work\n> reading the configuration had to be moved further up in cmd_fetch(), as\n> doing that after the actual fetch of the superproject was too late.\n>\n> Reported-by: Martin Fick \n> Signed-off-by: Jens Lehmann \n> ---\n>\n> This achieves the first goal: Don't let people pay a performance penalty\n> when they don't even use submodules. On Michael's test repo from [1] the\n> time for a full fetch went down from 142 seconds (current master) to one\n> second which is - not surprisingly - the same as using current master\n> with the --no-recurse-submodules option.\n>\n> Now back to the drawing board to fix the performance regression for those\n> people who are using submodules ...\n>\n> [1] http://comments.gmane.org/gmane.comp.version-control.git/177103\n\nThanks.\n"}]}