{"thread":{"id":"40545","subject":"[PATCH] Add fetch.recurseSubmoduleParallelism config option","startedAt":"2015-10-12T22:52:30Z","lastAt":"2015-10-16T17:26:21Z","messageCount":9,"participants":["Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"271544","messageId":"1444690350-6486-1-git-send-email-sbeller@google.com","threadId":"40545","inReplyTo":null,"subject":"[PATCH] Add fetch.recurseSubmoduleParallelism config option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-10-12T22:52:30Z","receivedAt":"2015-10-12T22:52:30Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This allows to configure fetching in parallel without having the annoying\ncommand line option.\n\nThis moved the responsibility to determine how many parallel processes\nto start from builtin/fetch to submodule.c as we need a way to communicate\n\"The user did not specify the number of parallel processes in the command\nline options\" in the builtin fetch. The submodule code takes care of\nthe precedence (CLI > config > default)\n\nSigned-off-by: Stefan Beller \n---\n Documentation/config.txt | 6 ++++++\n builtin/fetch.c | 2 +-\n submodule.c | 14 ++++++++++++++\n 3 files changed, 21 insertions(+), 1 deletion(-)\n \n I just monkey tested the code and it worked once! The problem with testing\n this parallelizing option is that the expected behavior doesn't change\n except for being faster. And I don't want to add timing tests to the test\n suite because they are unreliable.\n \n Any idea how to test this properly?\n \n This applies on top of sb/submodule-parallel-fetch\n \n Thanks,\n Stefan\n \n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 315f271..1172db0 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1140,6 +1140,12 @@ fetch.recurseSubmodules::\n \twhen its superproject retrieves a commit that updates the submodule's\n \treference.\n \n+fetch.recurseSubmoduleParallelism\n+\tThis is used to determine how many submodules can be fetched in\n+\tparallel. Specifying a positive integer allows up to that number\n+\tof submodules being fetched in parallel. Specifying 0 the number\n+\tof cpus will be taken as the maximum number.\n+\n fetch.fsckObjects::\n \tIf it is set to true, git-fetch-pack will check all fetched\n \tobjects. It will abort in the case of a malformed object or a\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex f28eac6..b1399dc 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -37,7 +37,7 @@ static int prune = -1; /* unspecified */\n static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity;\n static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n static int tags = TAGS_DEFAULT, unshallow, update_shallow;\n-static int max_children = 1;\n+static int max_children = -1;\n static const char *depth;\n static const char *upload_pack;\n static struct strbuf default_rla = STRBUF_INIT;\ndiff --git a/submodule.c b/submodule.c\nindex c21b265..c85d3ef 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -15,6 +15,7 @@\n #include \"thread-utils.h\"\n \n static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;\n+static int config_fetch_parallel_submodules = -1;\n static struct string_list changed_submodule_paths;\n static int initialized_fetch_ref_tips;\n static struct sha1_array ref_tips_before_fetch;\n@@ -179,6 +180,14 @@ int submodule_config(const char *var, const char *value, void *cb)\n \telse if (!strcmp(var, \"fetch.recursesubmodules\")) {\n \t\tconfig_fetch_recurse_submodules = parse_fetch_recurse_submodules_arg(var, value);\n \t\treturn 0;\n+\t} else if (!strcmp(var, \"fetch.recursesubmoduleparallelism\")) {\n+\t\tchar *end;\n+\t\tint ret;\n+\t\tconfig_fetch_parallel_submodules = strtol(value, &end, 0);\n+\t\tret = (*end == '\\0');\n+\t\tif (!ret)\n+\t\t\twarning(\"value for fetch.recurseSubmoduleParallelism not recognized\");\n+\t\treturn ret;\n \t}\n \treturn 0;\n }\n@@ -759,6 +768,11 @@ int fetch_populated_submodules(const struct argv_array *options,\n \targv_array_push(&spf.args, \"--recurse-submodules-default\");\n \t/* default value, \"--submodule-prefix\" and its value are added later */\n \n+\tif (max_parallel_jobs < 0)\n+\t\tmax_parallel_jobs = config_fetch_parallel_submodules;\n+\tif (max_parallel_jobs < 0)\n+\t\tmax_parallel_jobs = 1;\n+\n \tcalculate_changed_submodule_paths();\n \trun_processes_parallel(max_parallel_jobs,\n \t\t\t get_next_submodule,\n-- \n2.5.0.267.g8d6e698.dirty\n"},{"id":"271545","messageId":"xmqqeggzbrx5.fsf@gitster.mtv.corp.google.com","threadId":"40545","inReplyTo":"1444690350-6486-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH] Add fetch.recurseSubmoduleParallelism config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-12T23:14:46Z","receivedAt":"2015-10-12T23:14:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> This allows to configure fetching in parallel without having the annoying\n> command line option.\n\ns/annoying//;\n\nI think this is a sane thing to do, but the name of the variable may\nwant to be bikeshedded a bit.\n\n> This moved the responsibility to determine how many parallel processes\n> to start from builtin/fetch to submodule.c as we need a way to communicate\n> \"The user did not specify the number of parallel processes in the command\n> line options\" in the builtin fetch. The submodule code takes care of\n> the precedence (CLI > config > default)\n>\n> Signed-off-by: Stefan Beller \n> ---\n> Documentation/config.txt | 6 ++++++\n> builtin/fetch.c | 2 +-\n> submodule.c | 14 ++++++++++++++\n> 3 files changed, 21 insertions(+), 1 deletion(-)\n> \n> I just monkey tested the code and it worked once! The problem with testing\n> this parallelizing option is that the expected behavior doesn't change\n> except for being faster. And I don't want to add timing tests to the test\n> suite because they are unreliable.\n>\n> Any idea how to test this properly?\n\nI agree that a test in t/ would catch bugs in the functionality. If\nyour parallel implementation is somehow broken in the future and\nstops functioning correctly, fetching all submodules with a single\ntask and fetching them with N tasks will produce different results\n;-).\n\nBut it would not help you much in seeing if the parallelism is\nreally taking place. Adding t/perf/ tests to show how much benefit\nyou are getting may be of more value.\n\nThe parallel_process API could learn a new \"verbose\" feature that it\nby itself shows some messages like\n\n \"processing the 'frotz' job with N tasks\"\n \"M tasks finished (N still running)\" \n\nin the output stream from strategic places. For example, the first\nmessage will come at the end of pp_init(), and the second message\nwill be appended at the end of buffered output of a task that has\njust been finished. Once you have something like that, you could\ncheck for them in a test in t/.\n\nJust a thought.\n\n> \n> This applies on top of sb/submodule-parallel-fetch\n> \n> Thanks,\n> Stefan\n> \n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 315f271..1172db0 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -1140,6 +1140,12 @@ fetch.recurseSubmodules::\n> \twhen its superproject retrieves a commit that updates the submodule's\n> \treference.\n> \n> +fetch.recurseSubmoduleParallelism\n> +\tThis is used to determine how many submodules can be fetched in\n> +\tparallel. Specifying a positive integer allows up to that number\n> +\tof submodules being fetched in parallel. Specifying 0 the number\n> +\tof cpus will be taken as the maximum number.\n> +\n> fetch.fsckObjects::\n> \tIf it is set to true, git-fetch-pack will check all fetched\n> \tobjects. It will abort in the case of a malformed object or a\n> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> index f28eac6..b1399dc 100644\n> --- a/builtin/fetch.c\n> +++ b/builtin/fetch.c\n> @@ -37,7 +37,7 @@ static int prune = -1; /* unspecified */\n> static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity;\n> static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n> static int tags = TAGS_DEFAULT, unshallow, update_shallow;\n> -static int max_children = 1;\n> +static int max_children = -1;\n> static const char *depth;\n> static const char *upload_pack;\n> static struct strbuf default_rla = STRBUF_INIT;\n> diff --git a/submodule.c b/submodule.c\n> index c21b265..c85d3ef 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -15,6 +15,7 @@\n> #include \"thread-utils.h\"\n> \n> static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;\n> +static int config_fetch_parallel_submodules = -1;\n> static struct string_list changed_submodule_paths;\n> static int initialized_fetch_ref_tips;\n> static struct sha1_array ref_tips_before_fetch;\n> @@ -179,6 +180,14 @@ int submodule_config(const char *var, const char *value, void *cb)\n> \telse if (!strcmp(var, \"fetch.recursesubmodules\")) {\n> \t\tconfig_fetch_recurse_submodules = parse_fetch_recurse_submodules_arg(var, value);\n> \t\treturn 0;\n> +\t} else if (!strcmp(var, \"fetch.recursesubmoduleparallelism\")) {\n> +\t\tchar *end;\n> +\t\tint ret;\n> +\t\tconfig_fetch_parallel_submodules = strtol(value, &end, 0);\n> +\t\tret = (*end == '\\0');\n> +\t\tif (!ret)\n> +\t\t\twarning(\"value for fetch.recurseSubmoduleParallelism not recognized\");\n> +\t\treturn ret;\n> \t}\n> \treturn 0;\n> }\n> @@ -759,6 +768,11 @@ int fetch_populated_submodules(const struct argv_array *options,\n> \targv_array_push(&spf.args, \"--recurse-submodules-default\");\n> \t/* default value, \"--submodule-prefix\" and its value are added later */\n> \n> +\tif (max_parallel_jobs < 0)\n> +\t\tmax_parallel_jobs = config_fetch_parallel_submodules;\n> +\tif (max_parallel_jobs < 0)\n> +\t\tmax_parallel_jobs = 1;\n> +\n> \tcalculate_changed_submodule_paths();\n> \trun_processes_parallel(max_parallel_jobs,\n> \t\t\t get_next_submodule,\n"},{"id":"271546","messageId":"CAGZ79kZuZZivs8czV2P6uHWaU6ay1hG21k-_G9tgN5KbV6jW8w@mail.gmail.com","threadId":"40545","inReplyTo":"xmqqeggzbrx5.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Add fetch.recurseSubmoduleParallelism config option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-10-12T23:31:18Z","receivedAt":"2015-10-12T23:31:18Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Oct 12, 2015 at 4:14 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> This allows to configure fetching in parallel without having the annoying\n>> command line option.\n>\n> s/annoying//;\n>\n> I think this is a sane thing to do, but the name of the variable may\n> want to be bikeshedded a bit.\n\nThat's actually what I thought about most for this patch. I expected\nbikeshedding\nas well in the workflow (passing around -1 for each unset option).\n\nWe should not include (threads/processes) as that is implementation detail,\nand should not be exposed to the user (Looking at pack.threads)\n\nThere are some options using max_* for configuring parallel stuff\nsuch as http.maxRequests.\n\nThere is core.preloadIndex to enable parallel index preload, but\nthat is boolean and not giving fine control to the user. We want to give\nfine control to the user here I'd assume.\n\nMaybe also the more fundamental question needs to be answered,\nif we want to stay in the \"fetch.\" prefix. We could also make it a\nsubmodule specifc thing (submodule.jobs), but that would collide\nwith submodule.. maybe? (Originally I wanted to\npostpone this patch until I have parallelized git submodule update,\nso a \"fetch.\" prefix may not be good, as we want these 2 operations\nto use the same config option I'd guess)\n\n\n\n\n\n>\n>> This moved the responsibility to determine how many parallel processes\n>> to start from builtin/fetch to submodule.c as we need a way to communicate\n>> \"The user did not specify the number of parallel processes in the command\n>> line options\" in the builtin fetch. The submodule code takes care of\n>> the precedence (CLI > config > default)\n>>\n>> Signed-off-by: Stefan Beller \n>> ---\n>> Documentation/config.txt | 6 ++++++\n>> builtin/fetch.c | 2 +-\n>> submodule.c | 14 ++++++++++++++\n>> 3 files changed, 21 insertions(+), 1 deletion(-)\n>>\n>> I just monkey tested the code and it worked once! The problem with testing\n>> this parallelizing option is that the expected behavior doesn't change\n>> except for being faster. And I don't want to add timing tests to the test\n>> suite because they are unreliable.\n>>\n>> Any idea how to test this properly?\n>\n> I agree that a test in t/ would catch bugs in the functionality. If\n> your parallel implementation is somehow broken in the future and\n> stops functioning correctly, fetching all submodules with a single\n> task and fetching them with N tasks will produce different results\n> ;-).\n>\n> But it would not help you much in seeing if the parallelism is\n> really taking place. Adding t/perf/ tests to show how much benefit\n> you are getting may be of more value.\n>\n> The parallel_process API could learn a new \"verbose\" feature that it\n> by itself shows some messages like\n>\n> \"processing the 'frotz' job with N tasks\"\n> \"M tasks finished (N still running)\"\n\nI know what to fill in for M and N, 'frotz' is a bit unclear to me.\nWould you imagine that to be passed in as a hardcoded string?\ngit fetch --recurse-submodules would pass in \"Fetching submodules\",\nbut wuld pass in actual \"frotz\", or would you assume that to be\ncomputed from the task data somehow?\n\n>\n> in the output stream from strategic places. For example, the first\n> message will come at the end of pp_init(), and the second message\n> will be appended at the end of buffered output of a task that has\n> just been finished. Once you have something like that, you could\n> check for them in a test in t/.\n>\n> Just a thought.\n\nI like that thought. :)\n\n>\n>>\n>> This applies on top of sb/submodule-parallel-fetch\n>>\n>> Thanks,\n>> Stefan\n>>\n>>\n>> diff --git a/Documentation/config.txt b/Documentation/config.txt\n>> index 315f271..1172db0 100644\n>> --- a/Documentation/config.txt\n>> +++ b/Documentation/config.txt\n>> @@ -1140,6 +1140,12 @@ fetch.recurseSubmodules::\n>> when its superproject retrieves a commit that updates the submodule's\n>> reference.\n>>\n>> +fetch.recurseSubmoduleParallelism\n>> + This is used to determine how many submodules can be fetched in\n>> + parallel. Specifying a positive integer allows up to that number\n>> + of submodules being fetched in parallel. Specifying 0 the number\n>> + of cpus will be taken as the maximum number.\n>> +\n>> fetch.fsckObjects::\n>> If it is set to true, git-fetch-pack will check all fetched\n>> objects. It will abort in the case of a malformed object or a\n>> diff --git a/builtin/fetch.c b/builtin/fetch.c\n>> index f28eac6..b1399dc 100644\n>> --- a/builtin/fetch.c\n>> +++ b/builtin/fetch.c\n>> @@ -37,7 +37,7 @@ static int prune = -1; /* unspecified */\n>> static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity;\n>> static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n>> static int tags = TAGS_DEFAULT, unshallow, update_shallow;\n>> -static int max_children = 1;\n>> +static int max_children = -1;\n>> static const char *depth;\n>> static const char *upload_pack;\n>> static struct strbuf default_rla = STRBUF_INIT;\n>> diff --git a/submodule.c b/submodule.c\n>> index c21b265..c85d3ef 100644\n>> --- a/submodule.c\n>> +++ b/submodule.c\n>> @@ -15,6 +15,7 @@\n>> #include \"thread-utils.h\"\n>>\n>> static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;\n>> +static int config_fetch_parallel_submodules = -1;\n>> static struct string_list changed_submodule_paths;\n>> static int initialized_fetch_ref_tips;\n>> static struct sha1_array ref_tips_before_fetch;\n>> @@ -179,6 +180,14 @@ int submodule_config(const char *var, const char *value, void *cb)\n>> else if (!strcmp(var, \"fetch.recursesubmodules\")) {\n>> config_fetch_recurse_submodules = parse_fetch_recurse_submodules_arg(var, value);\n>> return 0;\n>> + } else if (!strcmp(var, \"fetch.recursesubmoduleparallelism\")) {\n>> + char *end;\n>> + int ret;\n>> + config_fetch_parallel_submodules = strtol(value, &end, 0);\n>> + ret = (*end == '\\0');\n>> + if (!ret)\n>> + warning(\"value for fetch.recurseSubmoduleParallelism not recognized\");\n>> + return ret;\n>> }\n>> return 0;\n>> }\n>> @@ -759,6 +768,11 @@ int fetch_populated_submodules(const struct argv_array *options,\n>> argv_array_push(&spf.args, \"--recurse-submodules-default\");\n>> /* default value, \"--submodule-prefix\" and its value are added later */\n>>\n>> + if (max_parallel_jobs < 0)\n>> + max_parallel_jobs = config_fetch_parallel_submodules;\n>> + if (max_parallel_jobs < 0)\n>> + max_parallel_jobs = 1;\n>> +\n>> calculate_changed_submodule_paths();\n>> run_processes_parallel(max_parallel_jobs,\n>> get_next_submodule,\n"},{"id":"271547","messageId":"xmqqa8rnbq9k.fsf@gitster.mtv.corp.google.com","threadId":"40545","inReplyTo":"CAGZ79kZuZZivs8czV2P6uHWaU6ay1hG21k-_G9tgN5KbV6jW8w@mail.gmail.com","subject":"Re: [PATCH] Add fetch.recurseSubmoduleParallelism config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-12T23:50:31Z","receivedAt":"2015-10-12T23:50:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> There is core.preloadIndex to enable parallel index preload, but\n> that is boolean and not giving fine control to the user. We want to give\n> fine control to the user here I'd assume.\n\nI'd approach this as \"fetching multiple submodules at a time\", if I\nwere deciding its name.\n\n> ... We could also make it a\n> submodule specifc thing (submodule.jobs), but that would collide\n> with submodule.. maybe?\n\nI do not think so. You can have\n\n\tarea.attr1\n\tarea.attr3\n area.\"userthing1\".attr1\n area.\"userthing2\".attr1\n\nand the parser can differenciate them just fine.\n\nSo if you want\n\n [submodule]\n fetchParallel = 16\n\tupdateParallel = 4\n\nI do not think that would interfere with any\n\n [submodule \"name\"]\n \tvar = val\n\nYou can choose to even allow an attribute that is fundamentally per\n\"userthing\" (e.g. the branch, the remote, the submodule) defined\nwith area.\"userthing\".attr, but make area.attr to be the fallback\nvalue for unspecified area.\"userthing9\".attr (I think http.*.*\nhierarchy takes that approach), but I do not think the parallelism\nof fetching is something that should be specified per submodule.\n\n>> The parallel_process API could learn a new \"verbose\" feature that it\n>> by itself shows some messages like\n>>\n>> \"processing the 'frotz' job with N tasks\"\n>> \"M tasks finished (N still running)\"\n>\n> I know what to fill in for M and N, 'frotz' is a bit unclear to me.\n\nThe caller would pass the label to pp_init(); in this codepath\nperhaps it will say 'submodule fetch' or something.\n"},{"id":"271558","messageId":"xmqqzizn9qaf.fsf@gitster.mtv.corp.google.com","threadId":"40545","inReplyTo":"CAGZ79kZuZZivs8czV2P6uHWaU6ay1hG21k-_G9tgN5KbV6jW8w@mail.gmail.com","subject":"Re: [PATCH] Add fetch.recurseSubmoduleParallelism config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-13T07:32:56Z","receivedAt":"2015-10-13T07:32:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n>> The parallel_process API could learn a new \"verbose\" feature that it\n>> by itself shows some messages like\n>>\n>> \"processing the 'frotz' job with N tasks\"\n>> \"M tasks finished (N still running)\"\n>\n> I know what to fill in for M and N, 'frotz' is a bit unclear to me.\n\nAt least I don't know what M and N should be, and I'm curious how\nyou'll define them. See below.\n\n>> in the output stream from strategic places. For example, the first\n>> message will come at the end of pp_init(), and the second message\n>> will be appended at the end of buffered output of a task that has\n>> just been finished. Once you have something like that, you could\n>> check for them in a test in t/.\n>>\n>> Just a thought.\n>\n> I like that thought. :)\n\n\nA few more random thoughts:\n\n * The only thing you could rely on if you were to use the above in\n your tests is the one from pp_init() that declares how many\n processes the machinery is going to use. M/N will be unstable,\n depending on the scheduling order (e.g. the foreground process\n may take a lot of time to finish, while many other processes\n finish first).\n\n * Every time the foreground process (i.e. the one whose output is\n tee-ed to the overall output from the machinery) finishes, you\n can emit \"M tasks finished (N still running)\", but I am not sure\n what M should be. It is debatable how to account for background\n processes that have already completed but whose output haven't\n been shown.\n\n One school of thought that is in line with the \"pretend as if the\n background tasks are started immediately after the foreground\n task finishes, and they run at infinite speed and produce output\n in no time\" idea, on which the \"queue output from the background\n processes and emit at once in order to avoid intermixing\" design\n was based on, would be not to include them in M (i.e. finished\n ones), because their output haven't been emitted and we are\n pretending that they haven't even been started. If you take this\n approach, you however may have to include them in N (i.e. still\n running), but that would likely bump N beyond the maximum number\n of simultaneous processes.\n\n The other school of thought would of course tell the truth and\n include the number of finished background processes in M, as they\n have finished already in the reality. This will not risk showing\n N that is beyond the maximum, but your first \"progress\" output\n might say \"3 tasks finished\", which will make it look odd in a\n different way.\n"},{"id":"271577","messageId":"CAGZ79kYZ+RyjvkgY8+aeRpnVtaCyqFxQqQ3wggjtef7y6MNLcA@mail.gmail.com","threadId":"40545","inReplyTo":"xmqqzizn9qaf.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Add fetch.recurseSubmoduleParallelism config option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-10-13T16:03:24Z","receivedAt":"2015-10-13T16:03:24Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Oct 13, 2015 at 12:32 AM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>>> The parallel_process API could learn a new \"verbose\" feature that it\n>>> by itself shows some messages like\n>>>\n>>> \"processing the 'frotz' job with N tasks\"\n>>> \"M tasks finished (N still running)\"\n>>\n>> I know what to fill in for M and N, 'frotz' is a bit unclear to me.\n>\n> At least I don't know what M and N should be, and I'm curious how\n> you'll define them. See below.\n\nI presumed the second school of thought. Another alternative there would\nbe to have 3 numbers:\n\n \"M tasks finished (N still running, K pending output)\"\n\n>\n>>> in the output stream from strategic places. For example, the first\n>>> message will come at the end of pp_init(), and the second message\n>>> will be appended at the end of buffered output of a task that has\n>>> just been finished. Once you have something like that, you could\n>>> check for them in a test in t/.\n>>>\n>>> Just a thought.\n>>\n>> I like that thought. :)\n>\n>\n> A few more random thoughts:\n>\n> * The only thing you could rely on if you were to use the above in\n> your tests is the one from pp_init() that declares how many\n> processes the machinery is going to use. M/N will be unstable,\n> depending on the scheduling order (e.g. the foreground process\n> may take a lot of time to finish, while many other processes\n> finish first).\n>\n> * Every time the foreground process (i.e. the one whose output is\n> tee-ed to the overall output from the machinery) finishes, you\n> can emit \"M tasks finished (N still running)\", but I am not sure\n> what M should be. It is debatable how to account for background\n> processes that have already completed but whose output haven't\n> been shown.\n\nAssuming we go with your second school of thought (N are the real\nrunning processes, M including the finished but still pending output tasks),\nwe may have confusing output, as the output order may confuse the\nreader:\n\n N=8 M=13 (output from live task)\n ...\n N=8 M=12 (output from buffered task)\n ...\n\nAnyone who has no knowledge of the internals, wonders why\nM goes *down* ?\n\n>\n> One school of thought that is in line with the \"pretend as if the\n> background tasks are started immediately after the foreground\n> task finishes, and they run at infinite speed and produce output\n> in no time\" idea, on which the \"queue output from the background\n> processes and emit at once in order to avoid intermixing\" design\n> was based on, would be not to include them in M (i.e. finished\n> ones), because their output haven't been emitted and we are\n> pretending that they haven't even been started. If you take this\n> approach, you however may have to include them in N (i.e. still\n> running), but that would likely bump N beyond the maximum number\n> of simultaneous processes.\n>\n> The other school of thought would of course tell the truth and\n> include the number of finished background processes in M, as they\n> have finished already in the reality. This will not risk showing\n> N that is beyond the maximum, but your first \"progress\" output\n> might say \"3 tasks finished\", which will make it look odd in a\n> different way.\n>\n"},{"id":"271594","messageId":"xmqqeggya34i.fsf@gitster.mtv.corp.google.com","threadId":"40545","inReplyTo":"CAGZ79kYZ+RyjvkgY8+aeRpnVtaCyqFxQqQ3wggjtef7y6MNLcA@mail.gmail.com","subject":"Re: [PATCH] Add fetch.recurseSubmoduleParallelism config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-13T21:07:57Z","receivedAt":"2015-10-13T21:07:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> Assuming we go with your second school of thought (N are the real\n> running processes, M including the finished but still pending output tasks),\n\nThat's neither of my two, I would think.\n\nAnyway, I think it is now clear that it not very easy to say \"I know\nwhat to fill in M and N\" there ;-).\n"},{"id":"271820","messageId":"CAGZ79kZDMawq6f3Jet0o-93-vSamy3skji63brdT3puzaLty5Q@mail.gmail.com","threadId":"40545","inReplyTo":"xmqqa8rnbq9k.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Add fetch.recurseSubmoduleParallelism config option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-10-16T17:04:27Z","receivedAt":"2015-10-16T17:04:27Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Oct 12, 2015 at 4:50 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> There is core.preloadIndex to enable parallel index preload, but\n>> that is boolean and not giving fine control to the user. We want to give\n>> fine control to the user here I'd assume.\n>\n> I'd approach this as \"fetching multiple submodules at a time\", if I\n> were deciding its name.\n>\n\nso maybe\n fetch.recurseSubmoduleJobs\n fetch.submoduleJobs\n fetch.jobs\n fetch.connectionsToUse\n\nEventually we want to also be parallel in git fetch --all, when using\nthe latter two\nwe could reuse these then too, no need to support different options for\nfetch --all and fetch --recurseSubmodules.\n\n\n> So if you want\n>\n> [submodule]\n> fetchParallel = 16\n> updateParallel = 4\n\nSo you would have different settings here for only slightly different things?\nSo the series I sent out yesterday evening, would make use of updateParallel\nfor parallel cloning then instead?\n"},{"id":"271823","messageId":"xmqqh9lqg1xe.fsf@gitster.mtv.corp.google.com","threadId":"40545","inReplyTo":"CAGZ79kZDMawq6f3Jet0o-93-vSamy3skji63brdT3puzaLty5Q@mail.gmail.com","subject":"Re: [PATCH] Add fetch.recurseSubmoduleParallelism config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-16T17:26:21Z","receivedAt":"2015-10-16T17:26:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> so maybe\n> fetch.recurseSubmoduleJobs\n> fetch.submoduleJobs\n> fetch.jobs\n> fetch.connectionsToUse\n\n\"git remote update\" is another example that may want to run multiple\nindependent 'git fetch' in parallel. I think \"When the operation I\nask involves fetching from multiple places, I want N instances of\nthem to be executed\", regardless of the kind of operation (\"remote\nupdate\" or \"submodule update\", etc.), would match the end-user's\npoint of view the best, if you want to give them \"set this single\nthing to apply to all of them\" fallback default.\n\nIf you want to give them a finer-grained control, you would need to\ndifferentiate what kind of fetch would use N tasks (as opposed to\nother kind of fetch that uses M tasks) and the name would need to\nhave \"submodule\" in it for that differentiation.\n\n>> So if you want\n>>\n>> [submodule]\n>> fetchParallel = 16\n>> updateParallel = 4\n>\n> So you would have different settings here for only slightly different things?\n\nI was just showing you that it is _possible_ if you want to give\nfiner control. For example, you can define:\n\n * 'submodule.parallel', if defined gives the values for the\n following more specific ones if they aren't given.\n\n * 'submodule.fetchParallel' specifies how many tasks are run in\n 'fetch --recurse-submodules'.\n\n * 'submodule.fetchParallel' specifies how many tasks are run in\n 'submodule update'.\n\nso that those who want finer controls can, and those who don't can\nset a single one to apply to all.\n\nIf you want to start with a globally single setting, that is\nperfectly fine.\n"}]}