{"thread":{"id":"40028","subject":"[RFC PATCH 2/4] Add a workdispatcher to get work done in parallel","startedAt":"2015-08-06T17:35:20Z","lastAt":"2015-08-06T20:44:41Z","messageCount":11,"participants":["Stefan Beller","Eric Sunshine","Jeff King","Jens Lehmann"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"267573","messageId":"1438882524-21215-1-git-send-email-sbeller@google.com","threadId":"40028","inReplyTo":null,"subject":"[RFC/PATCH 0/4] parallel fetch for submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-06T17:35:20Z","receivedAt":"2015-08-06T17:35:20Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When I was looking at the branches of Jens for work done on submodules\nnot yet upstream I found a commit \"WIP threaded submodule fetching[1],\nand I was side tracked wanting to present a different approach to that.\n\nThe first patch is a bit unrelated as it relates to the rewrite of git-submodule.sh\nbut also has code in submodule.c and the following patches modify code just around\nthat, so I did not remove that patch from this series. It is the same I sent \nyesterday.\n\nThe next patch 2/4 presents a framework for parallel threaded work.\nIt allows to setup a worker pool of <n> threads and then have a queue\nof tasks which are worked on by the threads. The patch is a the one which\nI'd request most comments on as I think that can be reused in a variety of\nsituations (parallel checkout of files, parallel fetch of different remotes,\nor such).\n\nI consider the third patch farely boring as it adds argv_array_copy, so I\nwould not expect much discussion there.\n\nThe last patch 4/4 presents the new workdispatcher from 2/4 in use\nwith just one unsolved problem of how to handle the output of the\nparallel commands to stdout and stderr. It may be useful to put\nhandling of parallel outputs into the work dispatcher.\n\n[1] https://github.com/jlehmann/git-submod-enhancements/commit/47597753206d40e234a47392e258065c9489e2b3\n\nThis series applies on top of origin/sb/submodule-helper (d2c6c09ac819,\nsubmodule: implement `module_list` as a builtin helper) and can also be found\nat https://github.com/stefanbeller/git/tree/parallel-submodule-fetch\n\nStefan Beller (4):\n  submodule: implement `module_name` as a builtin helper\n  Add a workdispatcher to get work done in parallel\n  argv_array: add argv_array_clone to clone an existing argv array\n  submodule: add infrastructure to fetch submodules in parallel\n\n Makefile                    |   1 +\n argv-array.c                |  13 ++++\n argv-array.h                |   1 +\n builtin/fetch.c             |   3 +-\n builtin/submodule--helper.c |  23 ++++++\n git-submodule.sh            |  32 ++------\n submodule.c                 |  92 ++++++++++++++++------\n submodule.h                 |   3 +-\n workdispatcher.c            | 184 ++++++++++++++++++++++++++++++++++++++++++++\n workdispatcher.h            |  29 +++++++\n 10 files changed, 332 insertions(+), 49 deletions(-)\n create mode 100644 workdispatcher.c\n create mode 100644 workdispatcher.h\n\n-- \n2.5.0.239.g9728e1d.dirty\n"},{"id":"267572","messageId":"1438882524-21215-2-git-send-email-sbeller@google.com","threadId":"40028","inReplyTo":"1438882524-21215-1-git-send-email-sbeller@google.com","subject":"[PATCH 1/4] submodule: implement `module_name` as a builtin helper","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-06T17:35:21Z","receivedAt":"2015-08-06T17:35:21Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This implements the helper `module_name` in C instead of shell,\nyielding a nice performance boost.\n\nBefore this patch, I measured a time (best out of three):\n\n  $ time ./t7400-submodule-basic.sh  >/dev/null\n    real\t0m11.066s\n    user\t0m3.348s\n    sys\t0m8.534s\n\nWith this patch applied I measured (also best out of three)\n\n  $ time ./t7400-submodule-basic.sh  >/dev/null\n    real\t0m10.063s\n    user\t0m3.044s\n    sys\t0m7.487s\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/submodule--helper.c | 23 +++++++++++++++++++++++\n git-submodule.sh            | 32 +++++++-------------------------\n submodule.c                 | 18 +++++++++++++-----\n submodule.h                 |  1 +\n 4 files changed, 44 insertions(+), 30 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex cb18ddf..3713c4c 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -5,6 +5,8 @@\n #include \"pathspec.h\"\n #include \"dir.h\"\n #include \"utf8.h\"\n+#include \"submodule.h\"\n+#include \"string-list.h\"\n \n static char *ps_matched;\n static const struct cache_entry **ce_entries;\n@@ -98,6 +100,24 @@ static int module_list(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+static int module_name(int argc, const char **argv, const char *prefix)\n+{\n+\tconst char *name;\n+\n+\tif (argc != 1)\n+\t\tusage(\"git submodule--helper module_name <path>\\n\");\n+\n+\tgitmodules_config();\n+\tname = submodule_name_for_path(argv[0]);\n+\n+\tif (name)\n+\t\tprintf(\"%s\\n\", name);\n+\telse\n+\t\tdie(\"No submodule mapping found in .gitmodules for path '%s'\", argv[0]);\n+\n+\treturn 0;\n+}\n+\n int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n {\n \tif (argc < 2)\n@@ -106,6 +126,9 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n \tif (!strcmp(argv[1], \"module_list\"))\n \t\treturn module_list(argc - 1, argv + 1, prefix);\n \n+\tif (!strcmp(argv[1], \"module_name\"))\n+\t\treturn module_name(argc - 2, argv + 2, prefix);\n+\n usage:\n \tusage(\"git submodule--helper module_list\\n\");\n }\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex af9ecef..e6ff38d 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -178,24 +178,6 @@ get_submodule_config () {\n \tprintf '%s' \"${value:-$default}\"\n }\n \n-\n-#\n-# Map submodule path to submodule name\n-#\n-# $1 = path\n-#\n-module_name()\n-{\n-\t# Do we have \"submodule.<something>.path = $1\" defined in .gitmodules file?\n-\tsm_path=\"$1\"\n-\tre=$(printf '%s\\n' \"$1\" | sed -e 's/[].[^$\\\\*]/\\\\&/g')\n-\tname=$( git config -f .gitmodules --get-regexp '^submodule\\..*\\.path$' |\n-\t\tsed -n -e 's|^submodule\\.\\(.*\\)\\.path '\"$re\"'$|\\1|p' )\n-\ttest -z \"$name\" &&\n-\tdie \"$(eval_gettext \"No submodule mapping found in .gitmodules for path '\\$sm_path'\")\"\n-\tprintf '%s\\n' \"$name\"\n-}\n-\n #\n # Clone a submodule\n #\n@@ -498,7 +480,7 @@ cmd_foreach()\n \t\tthen\n \t\t\tdisplaypath=$(relative_path \"$sm_path\")\n \t\t\tsay \"$(eval_gettext \"Entering '\\$prefix\\$displaypath'\")\"\n-\t\t\tname=$(module_name \"$sm_path\")\n+\t\t\tname=$(git submodule--helper module_name \"$sm_path\")\n \t\t\t(\n \t\t\t\tprefix=\"$prefix$sm_path/\"\n \t\t\t\tclear_local_git_env\n@@ -554,7 +536,7 @@ cmd_init()\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n-\t\tname=$(module_name \"$sm_path\") || exit\n+\t\tname=$(git submodule--helper module_name \"$sm_path\") || exit\n \n \t\tdisplaypath=$(relative_path \"$sm_path\")\n \n@@ -636,7 +618,7 @@ cmd_deinit()\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n-\t\tname=$(module_name \"$sm_path\") || exit\n+\t\tname=$(git submodule--helper module_name \"$sm_path\") || exit\n \n \t\tdisplaypath=$(relative_path \"$sm_path\")\n \n@@ -758,7 +740,7 @@ cmd_update()\n \t\t\techo >&2 \"Skipping unmerged submodule $prefix$sm_path\"\n \t\t\tcontinue\n \t\tfi\n-\t\tname=$(module_name \"$sm_path\") || exit\n+\t\tname=$(git submodule--helper module_name \"$sm_path\") || exit\n \t\turl=$(git config submodule.\"$name\".url)\n \t\tbranch=$(get_submodule_config \"$name\" branch master)\n \t\tif ! test -z \"$update\"\n@@ -1022,7 +1004,7 @@ cmd_summary() {\n \t\t\t# Respect the ignore setting for --for-status.\n \t\t\tif test -n \"$for_status\"\n \t\t\tthen\n-\t\t\t\tname=$(module_name \"$sm_path\")\n+\t\t\t\tname=$(git submodule--helper module_name \"$sm_path\")\n \t\t\t\tignore_config=$(get_submodule_config \"$name\" ignore none)\n \t\t\t\ttest $status != A && test $ignore_config = all && continue\n \t\t\tfi\n@@ -1184,7 +1166,7 @@ cmd_status()\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n-\t\tname=$(module_name \"$sm_path\") || exit\n+\t\tname=$(git submodule--helper module_name \"$sm_path\") || exit\n \t\turl=$(git config submodule.\"$name\".url)\n \t\tdisplaypath=$(relative_path \"$prefix$sm_path\")\n \t\tif test \"$stage\" = U\n@@ -1261,7 +1243,7 @@ cmd_sync()\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n-\t\tname=$(module_name \"$sm_path\")\n+\t\tname=$(git submodule--helper module_name \"$sm_path\")\n \t\turl=$(git config -f .gitmodules --get submodule.\"$name\".url)\n \n \t\t# Possibly a url relative to parent\ndiff --git a/submodule.c b/submodule.c\nindex 15e90d1..872967f 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -686,6 +686,16 @@ static void calculate_changed_submodule_paths(void)\n \tinitialized_fetch_ref_tips = 0;\n }\n \n+const char* submodule_name_for_path(const char* path)\n+{\n+\tstruct string_list_item *item;\n+\titem = unsorted_string_list_lookup(&config_name_for_path, path);\n+\tif (item)\n+\t\treturn item->util;\n+\telse\n+\t\treturn NULL;\n+}\n+\n int fetch_populated_submodules(const struct argv_array *options,\n \t\t\t       const char *prefix, int command_line_option,\n \t\t\t       int quiet)\n@@ -693,7 +703,7 @@ int fetch_populated_submodules(const struct argv_array *options,\n \tint i, result = 0;\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tstruct argv_array argv = ARGV_ARRAY_INIT;\n-\tstruct string_list_item *name_for_path;\n+\tconst char *name_for_path;\n \tconst char *work_tree = get_git_work_tree();\n \tif (!work_tree)\n \t\tgoto out;\n@@ -723,10 +733,8 @@ int fetch_populated_submodules(const struct argv_array *options,\n \t\tif (!S_ISGITLINK(ce->ce_mode))\n \t\t\tcontinue;\n \n-\t\tname = ce->name;\n-\t\tname_for_path = unsorted_string_list_lookup(&config_name_for_path, ce->name);\n-\t\tif (name_for_path)\n-\t\t\tname = name_for_path->util;\n+\t\tname_for_path = submodule_name_for_path(ce->name);\n+\t\tname =  name_for_path ? name_for_path : ce->name;\n \n \t\tdefault_argv = \"yes\";\n \t\tif (command_line_option == RECURSE_SUBMODULES_DEFAULT) {\ndiff --git a/submodule.h b/submodule.h\nindex 7beec48..e3dd854 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -41,5 +41,6 @@ int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam\n \t\tstruct string_list *needs_pushing);\n int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);\n void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n+const char* submodule_name_for_path(const char* path);\n \n #endif\n-- \n2.5.0.239.g9728e1d.dirty\n"},{"id":"267570","messageId":"1438882524-21215-3-git-send-email-sbeller@google.com","threadId":"40028","inReplyTo":"1438882524-21215-1-git-send-email-sbeller@google.com","subject":"[RFC PATCH 2/4] Add a workdispatcher to get work done in parallel","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-06T17:35:22Z","receivedAt":"2015-08-06T17:35:22Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This adds infrastructure code to work a set of tasks from a thread pool.\n\nThe whole life cycle of such a thread pool would look like\n\n    struct workdispatcher *wd;\n    struct return_values *rv;\n    \n    wd = create_workdispatcher(&command_for_task, max_parallel_jobs);\n    for (...) {\n        prepare(pointer_to_task);\n        add_task(wd, pointer_to_task);\n    }\n    rv = wait_workdispatcher(wd);\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n Makefile         |   1 +\n workdispatcher.c | 184 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n workdispatcher.h |  29 +++++++++\n 3 files changed, 214 insertions(+)\n create mode 100644 workdispatcher.c\n create mode 100644 workdispatcher.h\n\ndiff --git a/Makefile b/Makefile\nindex 6fb7484..2d8803c 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -805,6 +805,7 @@ LIB_OBJS += version.o\n LIB_OBJS += versioncmp.o\n LIB_OBJS += walker.o\n LIB_OBJS += wildmatch.o\n+LIB_OBJS += workdispatcher.o\n LIB_OBJS += wrapper.o\n LIB_OBJS += write_or_die.o\n LIB_OBJS += ws.o\ndiff --git a/workdispatcher.c b/workdispatcher.c\nnew file mode 100644\nindex 0000000..adfedd9\n--- /dev/null\n+++ b/workdispatcher.c\n@@ -0,0 +1,184 @@\n+#include \"cache.h\"\n+#include \"workdispatcher.h\"\n+\n+#ifndef NO_PTHREADS\n+#include <pthread.h>\n+#include <semaphore.h>\n+#include <stdio.h>\n+#include <unistd.h>\n+\n+#include \"git-compat-util.h\"\n+struct job_list {\n+\tvoid *item;\n+\tstruct job_list *next;\n+};\n+#endif\n+\n+struct workdispatcher {\n+#ifndef NO_PTHREADS\n+\t/*\n+\t * To avoid deadlocks always aquire the semaphores with lowest priority\n+\t * first, priorites are in descending order as listed.\n+\t *\n+\t * The `mutex` is a general purpose lock for modifying data in the work\n+\t * dispatcher, such as adding a new task or adding a return value from\n+\t * an already run task.\n+\t *\n+\t * `workingcount` and `freecount` are opposing semaphores, the sum of\n+\t * their values should equal `max_threads` at any time while the `mutex`\n+\t * is available.\n+\t */\n+\tsem_t mutex;\n+\tsem_t workingcount;\n+\tsem_t freecount;\n+\n+\tpthread_t *threads;\n+\tunsigned max_threads;\n+\n+\tstruct job_list *first;\n+\tstruct job_list *last;\n+#endif\n+\tvoid *(*function)(void*);\n+\tstruct return_values *ret;\n+};\n+\n+#ifndef NO_PTHREADS\n+static unsigned number_cores(void)\n+{\n+\tint count = sysconf(_SC_NPROCESSORS_ONLN);\n+\tif (count < 1) {\n+\t\tfprintf(stderr, \"Number of CPUs online reported %d. \"\n+\t\t\t\"Using one core.\\n\", count);\n+\t\tcount = 1;\n+\t}\n+\treturn count;\n+}\n+\n+void *get_task(struct workdispatcher *wd)\n+{\n+\tvoid *ret;\n+\tstruct job_list *job;\n+\n+\tsem_wait(&wd->workingcount);\n+\tsem_wait(&wd->mutex);\n+\n+\tif (!wd->first)\n+\t\tdie(\"BUG: internal error with dequeuing jobs for threads\");\n+\tjob = wd->first;\n+\tret = job->item;\n+\twd->first = job->next;\n+\tif (!wd->first)\n+\t\twd->last = NULL;\n+\n+\tsem_post(&wd->freecount);\n+\tsem_post(&wd->mutex);\n+\n+\tfree(job);\n+\treturn ret;\n+}\n+\n+void* dispatcher(void *args)\n+{\n+\tstruct workdispatcher *wd = args;\n+\tvoid *job = get_task(wd);\n+\twhile (job) {\n+\t\tvoid *retvalue = wd->function(job);\n+\n+\t\tsem_wait(&wd->mutex);\n+\t\tstruct return_values *rv = wd->ret;\n+\t\tALLOC_GROW(rv->ret, rv->count + 1, rv->alloc);\n+\t\twd->ret->ret[rv->count++] = retvalue;\n+\t\tsem_post(&wd->mutex);\n+\n+\t\tjob = get_task(wd);\n+\t}\n+\n+\tpthread_exit(0);\n+}\n+#endif\n+\n+struct workdispatcher *create_workdispatcher(void *function(void*),\n+\t\t\t\t\t     unsigned max_threads)\n+{\n+\tstruct workdispatcher *wd = xmalloc(sizeof(*wd));\n+\n+#ifndef NO_PTHREADS\n+\tint i;\n+\tif (!max_threads)\n+\t\twd->max_threads = number_cores();\n+\telse\n+\t\twd->max_threads = max_threads;\n+\n+\tsem_init(&wd->mutex, 0, 1);\n+\tsem_init(&wd->workingcount, 0, 0);\n+\tsem_init(&wd->freecount, 0, wd->max_threads);\n+\twd->threads = xmalloc(wd->max_threads * sizeof(pthread_t));\n+\n+\tfor (i = 0; i < wd->max_threads; i++)\n+\t\tpthread_create(&wd->threads[i], 0, &dispatcher, wd);\n+\n+\twd->first = NULL;\n+\twd->last = NULL;\n+#endif\n+\twd->function = function;\n+\twd->ret = xmalloc(sizeof(*wd->ret));\n+\twd->ret->ret = NULL;\n+\twd->ret->count = 0;\n+\twd->ret->alloc = 0;\n+\n+\treturn wd;\n+}\n+\n+void add_task(struct workdispatcher *wd, void *job)\n+{\n+#ifndef NO_PTHREADS\n+\tstruct job_list *job_list;\n+\n+\tjob_list = xmalloc(sizeof(*job_list));\n+\tjob_list->item = job;\n+\tjob_list->next = NULL;\n+\n+\tsem_wait(&wd->freecount);\n+\tsem_wait(&wd->mutex);\n+\n+\tif (!wd->last) {\n+\t\twd->last = job_list;\n+\t\twd->first = wd->last;\n+\t} else {\n+\t\twd->last->next = job_list;\n+\t\twd->last = wd->last->next;\n+\t}\n+\n+\tsem_post(&wd->workingcount);\n+\tsem_post(&wd->mutex);\n+#else\n+\tALLOC_GROW(wd->ret->ret, wd->ret->count + 1, wd->ret->alloc);\n+\twd->ret->ret[wd->ret->count++] = wd->function(job);\n+#endif\n+}\n+\n+struct return_values *wait_workdispatcher(struct workdispatcher *wd)\n+{\n+\tstruct return_values *ret;\n+#ifndef NO_PTHREADS\n+\tint i;\n+\tfor (i = 0; i < wd->max_threads; i++)\n+\t\tadd_task(wd, NULL);\n+\n+\tfor (i = 0; i < wd->max_threads; i++)\n+\t\tpthread_join(wd->threads[i], 0);\n+\n+\tsem_destroy(&wd->mutex);\n+\tsem_destroy(&wd->workingcount);\n+\tsem_destroy(&wd->freecount);\n+\n+\tif (wd->first)\n+\t\tdie(\"BUG: internal error with queuing jobs for threads\");\n+\n+\tfree(wd->threads);\n+#endif\n+\tret = wd->ret;\n+\n+\tfree(wd);\n+\treturn ret;\n+}\ndiff --git a/workdispatcher.h b/workdispatcher.h\nnew file mode 100644\nindex 0000000..9f78124\n--- /dev/null\n+++ b/workdispatcher.h\n@@ -0,0 +1,29 @@\n+#ifndef WORKDISPATCHER\n+#define WORKDISPATCHER\n+\n+struct return_values {\n+\tvoid **ret;\n+\tint count, alloc;\n+};\n+\n+/*\n+ * Creates a struct workdispatcher, which holds a job list and assigns the\n+ * jobs to be processed to a number of threads `maxthreads`.\n+ * Within the threads the function `fct` is called with the pointer as\n+ * given in add_task.\n+ *\n+ * If `maxthreads` is zero the number of cores available will be used.\n+ *\n+ * Currently this only works in environments with pthreads, in other\n+ * environments, the task will be processed directly in `add_task`.\n+ */\n+struct workdispatcher *create_workdispatcher(void *fct(void*),\n+\t\t\t\t\t     unsigned maxthreads);\n+\n+/* Waits for all tasks to be done and frees the object. */\n+struct return_values *wait_workdispatcher(struct workdispatcher *wd);\n+\n+/* `task` must not be NULL, as that's used internally to signal shutdown. */\n+void add_task(struct workdispatcher *wd, void *task);\n+\n+#endif\n-- \n2.5.0.239.g9728e1d.dirty\n"},{"id":"267571","messageId":"1438882524-21215-4-git-send-email-sbeller@google.com","threadId":"40028","inReplyTo":"1438882524-21215-1-git-send-email-sbeller@google.com","subject":"[PATCH 3/4] argv_array: add argv_array_copy","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-06T17:35:23Z","receivedAt":"2015-08-06T17:35:23Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The copied argv array shall be an identical deep copy except for\nthe internal allocation value.\n\nCC: Jeff King <peff@peff.net>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n argv-array.c | 13 +++++++++++++\n argv-array.h |  1 +\n 2 files changed, 14 insertions(+)\n\ndiff --git a/argv-array.c b/argv-array.c\nindex 256741d..6d9c1dd 100644\n--- a/argv-array.c\n+++ b/argv-array.c\n@@ -68,3 +68,16 @@ void argv_array_clear(struct argv_array *array)\n \t}\n \targv_array_init(array);\n }\n+\n+void argv_array_copy(struct argv_array *src, struct argv_array *dst)\n+{\n+\tint i;\n+\n+\tdst->argv = xmalloc((src->argc + 1) * sizeof(*dst->argv));\n+\tdst->argc = src->argc;\n+\tdst->alloc = src->argc;\n+\tfor (i = 0; i < dst->argc ; i++)\n+\t\tdst->argv[i] = xstrdup(src->argv[i]);\n+\tdst->argv[dst->argc] = NULL;\n+}\n+\ndiff --git a/argv-array.h b/argv-array.h\nindex c65e6e8..247627da 100644\n--- a/argv-array.h\n+++ b/argv-array.h\n@@ -19,5 +19,6 @@ LAST_ARG_MUST_BE_NULL\n void argv_array_pushl(struct argv_array *, ...);\n void argv_array_pop(struct argv_array *);\n void argv_array_clear(struct argv_array *);\n+void argv_array_copy(struct argv_array *src, struct argv_array *dst);\n \n #endif /* ARGV_ARRAY_H */\n-- \n2.5.0.239.g9728e1d.dirty\n"},{"id":"267574","messageId":"1438882524-21215-5-git-send-email-sbeller@google.com","threadId":"40028","inReplyTo":"1438882524-21215-1-git-send-email-sbeller@google.com","subject":"[RFC PATCH 4/4] submodule: add infrastructure to fetch submodules in parallel","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-06T17:35:24Z","receivedAt":"2015-08-06T17:35:24Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This makes use of the new workdispatcher to fetch a number\nof submodules at the same time.\n\nStill todo: sort the output of the fetch commands. I am unsure\nif this should be hooked into the workdispatcher as the problem\nof sorted output will appear likely again, so a general solution\nwould not hurt.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/fetch.c |  3 ++-\n submodule.c     | 74 ++++++++++++++++++++++++++++++++++++++++++++-------------\n submodule.h     |  2 +-\n 3 files changed, 60 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 8d5b2db..9053e8b 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1207,7 +1207,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \t\tresult = fetch_populated_submodules(&options,\n \t\t\t\t\t\t    submodule_prefix,\n \t\t\t\t\t\t    recurse_submodules,\n-\t\t\t\t\t\t    verbosity < 0);\n+\t\t\t\t\t\t    verbosity < 0,\n+\t\t\t\t\t\t    1);\n \t\targv_array_clear(&options);\n \t}\n \ndiff --git a/submodule.c b/submodule.c\nindex 872967f..0b2842b 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -11,6 +11,7 @@\n #include \"sha1-array.h\"\n #include \"argv-array.h\"\n #include \"blob.h\"\n+#include \"workdispatcher.h\"\n \n static struct string_list config_name_for_path;\n static struct string_list config_fetch_recurse_submodules_for_name;\n@@ -696,13 +697,49 @@ const char* submodule_name_for_path(const char* path)\n \t\treturn NULL;\n }\n \n+struct submodule_parallel_fetch {\n+\tstruct child_process cp;\n+\tstruct argv_array argv;\n+\tstruct strbuf sb;\n+\tint quiet;\n+};\n+\n+void submodule_parallel_fetch_init(struct submodule_parallel_fetch *spf)\n+{\n+\tchild_process_init(&spf->cp);\n+\targv_array_init(&spf->argv);\n+\tstrbuf_init(&spf->sb, 0);\n+\tspf->quiet = 0;\n+}\n+\n+void *run_command_and_cleanup(void *arg)\n+{\n+\tstruct submodule_parallel_fetch *spf = arg;\n+\tvoid *ret = NULL;\n+\n+\tif (!spf->quiet)\n+\t\tputs(spf->sb.buf);\n+\n+\tspf->cp.argv = spf->argv.argv;\n+\n+\tif (run_command(&spf->cp))\n+\t\tret = (void *)1;\n+\n+\tstrbuf_release(&spf->cp);\n+\targv_array_clear(spf->argv);\n+\tfree(spf);\n+\treturn ret;\n+}\n+\n int fetch_populated_submodules(const struct argv_array *options,\n \t\t\t       const char *prefix, int command_line_option,\n-\t\t\t       int quiet)\n+\t\t\t       int quiet, int max_parallel_jobs)\n {\n \tint i, result = 0;\n-\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tstruct workdispatcher *wd;\n+\tstruct return_values *rv;\n \tstruct argv_array argv = ARGV_ARRAY_INIT;\n+\tstruct submodule_parallel_fetch *spf;\n \tconst char *name_for_path;\n \tconst char *work_tree = get_git_work_tree();\n \tif (!work_tree)\n@@ -717,12 +754,9 @@ int fetch_populated_submodules(const struct argv_array *options,\n \targv_array_push(&argv, \"--recurse-submodules-default\");\n \t/* default value, \"--submodule-prefix\" and its value are added later */\n \n-\tcp.env = local_repo_env;\n-\tcp.git_cmd = 1;\n-\tcp.no_stdin = 1;\n-\n \tcalculate_changed_submodule_paths();\n \n+\twd = create_workdispatcher(&run_command_and_cleanup, max_parallel_jobs);\n \tfor (i = 0; i < active_nr; i++) {\n \t\tstruct strbuf submodule_path = STRBUF_INIT;\n \t\tstruct strbuf submodule_git_dir = STRBUF_INIT;\n@@ -771,24 +805,30 @@ int fetch_populated_submodules(const struct argv_array *options,\n \t\tif (!git_dir)\n \t\t\tgit_dir = submodule_git_dir.buf;\n \t\tif (is_directory(git_dir)) {\n+\t\t\tspf = xmalloc(sizeof(*spf));\n+\t\t\tsubmodule_parallel_fetch_init(spf);\n+\t\t\tspf->cp.env = local_repo_env;\n+\t\t\tspf->cp.git_cmd = 1;\n+\t\t\tspf->cp.no_stdin = 1;\n+\t\t\tspf->cp.dir = strbuf_detach(&submodule_path, NULL);\n+\t\t\tspf->quiet = quiet;\n \t\t\tif (!quiet)\n-\t\t\t\tprintf(\"Fetching submodule %s%s\\n\", prefix, ce->name);\n-\t\t\tcp.dir = submodule_path.buf;\n-\t\t\targv_array_push(&argv, default_argv);\n-\t\t\targv_array_push(&argv, \"--submodule-prefix\");\n-\t\t\targv_array_push(&argv, submodule_prefix.buf);\n-\t\t\tcp.argv = argv.argv;\n-\t\t\tif (run_command(&cp))\n-\t\t\t\tresult = 1;\n-\t\t\targv_array_pop(&argv);\n-\t\t\targv_array_pop(&argv);\n-\t\t\targv_array_pop(&argv);\n+\t\t\t\tstrbuf_addf(&spf->sb, \"Fetching submodule %s%s\", prefix, ce->name);\n+\t\t\targv_array_copy(&argv, &spf->argv);\n+\t\t\targv_array_push(&spf->argv, default_argv);\n+\t\t\targv_array_push(&spf->argv, \"--submodule-prefix\");\n+\t\t\targv_array_push(&spf->argv, submodule_prefix.buf);\n+\t\t\tadd_task(wd, spf);\n \t\t}\n \t\tstrbuf_release(&submodule_path);\n \t\tstrbuf_release(&submodule_git_dir);\n \t\tstrbuf_release(&submodule_prefix);\n \t}\n \targv_array_clear(&argv);\n+\trv = wait_workdispatcher(wd);\n+\tfor (i = 0; i < rv->count; i++)\n+\t\tif (rv->ret[i])\n+\t\t\tresult = 1;\n out:\n \tstring_list_clear(&changed_submodule_paths, 1);\n \treturn result;\ndiff --git a/submodule.h b/submodule.h\nindex e3dd854..51195ea 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -31,7 +31,7 @@ void set_config_fetch_recurse_submodules(int value);\n void check_for_new_submodule_commits(unsigned char new_sha1[20]);\n int fetch_populated_submodules(const struct argv_array *options,\n \t\t\t       const char *prefix, int command_line_option,\n-\t\t\t       int quiet);\n+\t\t\t       int quiet, int max_parallel_jobs);\n unsigned is_submodule_modified(const char *path, int ignore_untracked);\n int submodule_uses_gitfile(const char *path);\n int ok_to_remove_submodule(const char *path);\n-- \n2.5.0.239.g9728e1d.dirty\n"},{"id":"267575","messageId":"CAPig+cQ3bAML7WMyS3Q1YQCjR2SfgB47AEkTmqbycAJcig0=tg@mail.gmail.com","threadId":"40028","inReplyTo":"1438882524-21215-4-git-send-email-sbeller@google.com","subject":"Re: [PATCH 3/4] argv_array: add argv_array_copy","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-08-06T18:18:26Z","receivedAt":"2015-08-06T18:18:26Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Aug 6, 2015 at 1:35 PM, Stefan Beller <sbeller@google.com> wrote:\n> The copied argv array shall be an identical deep copy except for\n> the internal allocation value.\n>\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n> diff --git a/argv-array.c b/argv-array.c\n> index 256741d..6d9c1dd 100644\n> --- a/argv-array.c\n> +++ b/argv-array.c\n> @@ -68,3 +68,16 @@ void argv_array_clear(struct argv_array *array)\n> +void argv_array_copy(struct argv_array *src, struct argv_array *dst)\n\n'src' should be 'const'.\n\nTypical Unix argument order has 'dst' first and 'src' second, i.e\nstrcpy(dst, src). Is it worth deviating from that precedent?\n\n> +{\n> +       int i;\n> +\n> +       dst->argv = xmalloc((src->argc + 1) * sizeof(*dst->argv));\n\nWhat happens if 'dst' already has content? Isn't that being leaked\nhere? At the very least, don't you want to argv_array_clear(dst)?\n\n> +       dst->argc = src->argc;\n> +       dst->alloc = src->argc;\n\nThis is wrong, of course. The number allocated is actually argc+1, not argc.\n\n> +       for (i = 0; i < dst->argc ; i++)\n\nWhile it's not wrong per se to use dst->argc as the terminating\ncondition, it is potentially misleading and confusing. Instead using\nsrc->argc as the terminating condition will better telegraph that the\ncopy process is indeed predicated upon 'src'.\n\n> +               dst->argv[i] = xstrdup(src->argv[i]);\n> +       dst->argv[dst->argc] = NULL;\n\nIt's not clear why you want to hand-code the low-level functionality\nagain (such as array allocation and string duplication), risking (and\nindeed making) errors in the process, when you could instead re-use\nexisting argv_array code. I would have expected to see\nargv_array_copy() implemented as:\n\n    argv_array_clear(dst);\n    for (i = 0; i < src->argc; i++)\n        argv_array_push(dst, src->argv[i]);\n\nwhich provides far fewer opportunities for errors to creep in.\n\nMoreover, this function might be too special-purpose. That is, why\ndoes it need to overwrite 'dst'? Can't you achieve the same\nfunctionality by merely appending to 'dst', and leave it up to the\ncaller to decide whether 'dst' should be cleared beforehand or not? If\nso, then you can drop the argv_array_clear(dst) from the above.\n\nHowever, that begs the question: Why do you need argv_array_copy() at\nall? Isn't the same functionality already provided by\nargv_array_pushv()? To wit, a caller which wants to copy from 'src' to\n'dst' can already do:\n\n    struct argv_array src = ...;\n    struct argv_array dst = ARGV_ARRAY_INIT;\n    argv_array_pushv(&dst, src->argv);\n\n> +}\n> +\n> diff --git a/argv-array.h b/argv-array.h\n> index c65e6e8..247627da 100644\n> --- a/argv-array.h\n> +++ b/argv-array.h\n> @@ -19,5 +19,6 @@ LAST_ARG_MUST_BE_NULL\n>  void argv_array_pushl(struct argv_array *, ...);\n>  void argv_array_pop(struct argv_array *);\n>  void argv_array_clear(struct argv_array *);\n> +void argv_array_copy(struct argv_array *src, struct argv_array *dst);\n>\n>  #endif /* ARGV_ARRAY_H */\n> --\n> 2.5.0.239.g9728e1d.dirty\n"},{"id":"267581","messageId":"20150806185221.GA1444@sigill.intra.peff.net","threadId":"40028","inReplyTo":"CAPig+cQ3bAML7WMyS3Q1YQCjR2SfgB47AEkTmqbycAJcig0=tg@mail.gmail.com","subject":"Re: [PATCH 3/4] argv_array: add argv_array_copy","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-08-06T18:52:22Z","receivedAt":"2015-08-06T18:52:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 06, 2015 at 02:18:26PM -0400, Eric Sunshine wrote:\n\n> However, that begs the question: Why do you need argv_array_copy() at\n> all? Isn't the same functionality already provided by\n> argv_array_pushv()? To wit, a caller which wants to copy from 'src' to\n> 'dst' can already do:\n> \n>     struct argv_array src = ...;\n>     struct argv_array dst = ARGV_ARRAY_INIT;\n>     argv_array_pushv(&dst, src->argv);\n\nThanks for reviewing this, Eric. Everything you said is exactly what I\nwas thinking, too, especially this last part.\n\n-Peff\n"},{"id":"267583","messageId":"55C3BA48.6050600@web.de","threadId":"40028","inReplyTo":"1438882524-21215-2-git-send-email-sbeller@google.com","subject":"Re: [PATCH 1/4] submodule: implement `module_name` as a builtin helper","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2015-08-06T19:49:28Z","receivedAt":"2015-08-06T19:49:28Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 06.08.2015 um 19:35 schrieb Stefan Beller:\n> This implements the helper `module_name` in C instead of shell,\n> yielding a nice performance boost.\n>\n> Before this patch, I measured a time (best out of three):\n>\n>    $ time ./t7400-submodule-basic.sh  >/dev/null\n>      real\t0m11.066s\n>      user\t0m3.348s\n>      sys\t0m8.534s\n>\n> With this patch applied I measured (also best out of three)\n>\n>    $ time ./t7400-submodule-basic.sh  >/dev/null\n>      real\t0m10.063s\n>      user\t0m3.044s\n>      sys\t0m7.487s\n>\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n\nPlease see my comments in the other thread concerning this patch.\n\nAnd wouldn't it make more sense to keep this patch together with\nthe \"submodule: implement `module_list` as a builtin helper\" in\nits own \"submodule-helper\" series and have the following three\npatches in a separate \"parallel fetch for submodules\" series?\n\n>   builtin/submodule--helper.c | 23 +++++++++++++++++++++++\n>   git-submodule.sh            | 32 +++++++-------------------------\n>   submodule.c                 | 18 +++++++++++++-----\n>   submodule.h                 |  1 +\n>   4 files changed, 44 insertions(+), 30 deletions(-)\n>\n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index cb18ddf..3713c4c 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -5,6 +5,8 @@\n>   #include \"pathspec.h\"\n>   #include \"dir.h\"\n>   #include \"utf8.h\"\n> +#include \"submodule.h\"\n> +#include \"string-list.h\"\n>\n>   static char *ps_matched;\n>   static const struct cache_entry **ce_entries;\n> @@ -98,6 +100,24 @@ static int module_list(int argc, const char **argv, const char *prefix)\n>   \treturn 0;\n>   }\n>\n> +static int module_name(int argc, const char **argv, const char *prefix)\n> +{\n> +\tconst char *name;\n> +\n> +\tif (argc != 1)\n> +\t\tusage(\"git submodule--helper module_name <path>\\n\");\n> +\n> +\tgitmodules_config();\n> +\tname = submodule_name_for_path(argv[0]);\n> +\n> +\tif (name)\n> +\t\tprintf(\"%s\\n\", name);\n> +\telse\n> +\t\tdie(\"No submodule mapping found in .gitmodules for path '%s'\", argv[0]);\n> +\n> +\treturn 0;\n> +}\n> +\n>   int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n>   {\n>   \tif (argc < 2)\n> @@ -106,6 +126,9 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n>   \tif (!strcmp(argv[1], \"module_list\"))\n>   \t\treturn module_list(argc - 1, argv + 1, prefix);\n>\n> +\tif (!strcmp(argv[1], \"module_name\"))\n> +\t\treturn module_name(argc - 2, argv + 2, prefix);\n> +\n>   usage:\n>   \tusage(\"git submodule--helper module_list\\n\");\n>   }\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index af9ecef..e6ff38d 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -178,24 +178,6 @@ get_submodule_config () {\n>   \tprintf '%s' \"${value:-$default}\"\n>   }\n>\n> -\n> -#\n> -# Map submodule path to submodule name\n> -#\n> -# $1 = path\n> -#\n> -module_name()\n> -{\n> -\t# Do we have \"submodule.<something>.path = $1\" defined in .gitmodules file?\n> -\tsm_path=\"$1\"\n> -\tre=$(printf '%s\\n' \"$1\" | sed -e 's/[].[^$\\\\*]/\\\\&/g')\n> -\tname=$( git config -f .gitmodules --get-regexp '^submodule\\..*\\.path$' |\n> -\t\tsed -n -e 's|^submodule\\.\\(.*\\)\\.path '\"$re\"'$|\\1|p' )\n> -\ttest -z \"$name\" &&\n> -\tdie \"$(eval_gettext \"No submodule mapping found in .gitmodules for path '\\$sm_path'\")\"\n> -\tprintf '%s\\n' \"$name\"\n> -}\n> -\n>   #\n>   # Clone a submodule\n>   #\n> @@ -498,7 +480,7 @@ cmd_foreach()\n>   \t\tthen\n>   \t\t\tdisplaypath=$(relative_path \"$sm_path\")\n>   \t\t\tsay \"$(eval_gettext \"Entering '\\$prefix\\$displaypath'\")\"\n> -\t\t\tname=$(module_name \"$sm_path\")\n> +\t\t\tname=$(git submodule--helper module_name \"$sm_path\")\n>   \t\t\t(\n>   \t\t\t\tprefix=\"$prefix$sm_path/\"\n>   \t\t\t\tclear_local_git_env\n> @@ -554,7 +536,7 @@ cmd_init()\n>   \twhile read mode sha1 stage sm_path\n>   \tdo\n>   \t\tdie_if_unmatched \"$mode\"\n> -\t\tname=$(module_name \"$sm_path\") || exit\n> +\t\tname=$(git submodule--helper module_name \"$sm_path\") || exit\n>\n>   \t\tdisplaypath=$(relative_path \"$sm_path\")\n>\n> @@ -636,7 +618,7 @@ cmd_deinit()\n>   \twhile read mode sha1 stage sm_path\n>   \tdo\n>   \t\tdie_if_unmatched \"$mode\"\n> -\t\tname=$(module_name \"$sm_path\") || exit\n> +\t\tname=$(git submodule--helper module_name \"$sm_path\") || exit\n>\n>   \t\tdisplaypath=$(relative_path \"$sm_path\")\n>\n> @@ -758,7 +740,7 @@ cmd_update()\n>   \t\t\techo >&2 \"Skipping unmerged submodule $prefix$sm_path\"\n>   \t\t\tcontinue\n>   \t\tfi\n> -\t\tname=$(module_name \"$sm_path\") || exit\n> +\t\tname=$(git submodule--helper module_name \"$sm_path\") || exit\n>   \t\turl=$(git config submodule.\"$name\".url)\n>   \t\tbranch=$(get_submodule_config \"$name\" branch master)\n>   \t\tif ! test -z \"$update\"\n> @@ -1022,7 +1004,7 @@ cmd_summary() {\n>   \t\t\t# Respect the ignore setting for --for-status.\n>   \t\t\tif test -n \"$for_status\"\n>   \t\t\tthen\n> -\t\t\t\tname=$(module_name \"$sm_path\")\n> +\t\t\t\tname=$(git submodule--helper module_name \"$sm_path\")\n>   \t\t\t\tignore_config=$(get_submodule_config \"$name\" ignore none)\n>   \t\t\t\ttest $status != A && test $ignore_config = all && continue\n>   \t\t\tfi\n> @@ -1184,7 +1166,7 @@ cmd_status()\n>   \twhile read mode sha1 stage sm_path\n>   \tdo\n>   \t\tdie_if_unmatched \"$mode\"\n> -\t\tname=$(module_name \"$sm_path\") || exit\n> +\t\tname=$(git submodule--helper module_name \"$sm_path\") || exit\n>   \t\turl=$(git config submodule.\"$name\".url)\n>   \t\tdisplaypath=$(relative_path \"$prefix$sm_path\")\n>   \t\tif test \"$stage\" = U\n> @@ -1261,7 +1243,7 @@ cmd_sync()\n>   \twhile read mode sha1 stage sm_path\n>   \tdo\n>   \t\tdie_if_unmatched \"$mode\"\n> -\t\tname=$(module_name \"$sm_path\")\n> +\t\tname=$(git submodule--helper module_name \"$sm_path\")\n>   \t\turl=$(git config -f .gitmodules --get submodule.\"$name\".url)\n>\n>   \t\t# Possibly a url relative to parent\n> diff --git a/submodule.c b/submodule.c\n> index 15e90d1..872967f 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -686,6 +686,16 @@ static void calculate_changed_submodule_paths(void)\n>   \tinitialized_fetch_ref_tips = 0;\n>   }\n>\n> +const char* submodule_name_for_path(const char* path)\n> +{\n> +\tstruct string_list_item *item;\n> +\titem = unsorted_string_list_lookup(&config_name_for_path, path);\n> +\tif (item)\n> +\t\treturn item->util;\n> +\telse\n> +\t\treturn NULL;\n> +}\n> +\n>   int fetch_populated_submodules(const struct argv_array *options,\n>   \t\t\t       const char *prefix, int command_line_option,\n>   \t\t\t       int quiet)\n> @@ -693,7 +703,7 @@ int fetch_populated_submodules(const struct argv_array *options,\n>   \tint i, result = 0;\n>   \tstruct child_process cp = CHILD_PROCESS_INIT;\n>   \tstruct argv_array argv = ARGV_ARRAY_INIT;\n> -\tstruct string_list_item *name_for_path;\n> +\tconst char *name_for_path;\n>   \tconst char *work_tree = get_git_work_tree();\n>   \tif (!work_tree)\n>   \t\tgoto out;\n> @@ -723,10 +733,8 @@ int fetch_populated_submodules(const struct argv_array *options,\n>   \t\tif (!S_ISGITLINK(ce->ce_mode))\n>   \t\t\tcontinue;\n>\n> -\t\tname = ce->name;\n> -\t\tname_for_path = unsorted_string_list_lookup(&config_name_for_path, ce->name);\n> -\t\tif (name_for_path)\n> -\t\t\tname = name_for_path->util;\n> +\t\tname_for_path = submodule_name_for_path(ce->name);\n> +\t\tname =  name_for_path ? name_for_path : ce->name;\n>\n>   \t\tdefault_argv = \"yes\";\n>   \t\tif (command_line_option == RECURSE_SUBMODULES_DEFAULT) {\n> diff --git a/submodule.h b/submodule.h\n> index 7beec48..e3dd854 100644\n> --- a/submodule.h\n> +++ b/submodule.h\n> @@ -41,5 +41,6 @@ int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam\n>   \t\tstruct string_list *needs_pushing);\n>   int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);\n>   void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n> +const char* submodule_name_for_path(const char* path);\n>\n>   #endif\n>\n"},{"id":"267585","messageId":"55C3BB6D.5090905@web.de","threadId":"40028","inReplyTo":"55C3BA48.6050600@web.de","subject":"Re: [PATCH 1/4] submodule: implement `module_name` as a builtin helper","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2015-08-06T19:54:21Z","receivedAt":"2015-08-06T19:54:21Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 06.08.2015 um 21:49 schrieb Jens Lehmann:\n> And wouldn't it make more sense to keep this patch together with\n> the \"submodule: implement `module_list` as a builtin helper\" in\n> its own \"submodule-helper\" series and have the following three\n> patches in a separate \"parallel fetch for submodules\" series?\n\nPlease scratch that, I just now read your comment in the cover\nletter on that ...\n"},{"id":"267586","messageId":"55C3BEDB.20809@web.de","threadId":"40028","inReplyTo":"1438882524-21215-1-git-send-email-sbeller@google.com","subject":"Re: [RFC/PATCH 0/4] parallel fetch for submodules","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2015-08-06T20:08:59Z","receivedAt":"2015-08-06T20:08:59Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 06.08.2015 um 19:35 schrieb Stefan Beller:\n> When I was looking at the branches of Jens for work done on submodules\n> not yet upstream I found a commit \"WIP threaded submodule fetching[1],\n> and I was side tracked wanting to present a different approach to that.\n\nCool. I didn't follow that route further than building a proof of\nconcept because I ran into a nasty DNS-timeout on my router at home\nand at work we host all repos on a not-so-beefy server making parallel\nfetch rather pointless. But I suspect this approach will bring down\nfetch times for some users.\n\nMaybe we could also re-use parallel fetch for multiple upstreams in\nthe superproject when doing a \"git fetch --all\" without too much\nextra work?\n"},{"id":"267588","messageId":"CAGZ79kZquSJmLQYQqp3DW3KU4tMaNaaSD0==0Sky6TdiD1F8kw@mail.gmail.com","threadId":"40028","inReplyTo":"55C3BEDB.20809@web.de","subject":"Re: [RFC/PATCH 0/4] parallel fetch for submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-06T20:44:41Z","receivedAt":"2015-08-06T20:44:41Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Aug 6, 2015 at 1:08 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:\n> Am 06.08.2015 um 19:35 schrieb Stefan Beller:\n>>\n>> When I was looking at the branches of Jens for work done on submodules\n>> not yet upstream I found a commit \"WIP threaded submodule fetching[1],\n>> and I was side tracked wanting to present a different approach to that.\n>\n>\n> Cool. I didn't follow that route further than building a proof of\n> concept because I ran into a nasty DNS-timeout on my router at home\n> and at work we host all repos on a not-so-beefy server making parallel\n> fetch rather pointless. But I suspect this approach will bring down\n> fetch times for some users.\n\nThe difference between your proof of concept and mine is to have the number\nof threads easier configurable. (Think of \"git fetch --recurse-submodules=yes\n -j4\", similar to \"make -j4\" splitting work up to 4 different programs\nat the same\ntime. And that thing would be possible to add in this series)\n\nIf you fetch lots of submodules, both the client and server load should come\nin an ping-pong on-off pattern as the client waits for the server to\nprepare stuff\nand get it sent to it and then the client needs time to resolve deltas\nand write to\ndisk. Depending on the duty cycle of each, a different number of\nparallel threads\nmake sense (I would expected that they shift their phases against each other\nby pure randomness, i.e. one thread is currently resolving deltas\nwhile the other\nthread is telling the server to get some work done, so both the client\nand server\nget utilized at the same time).\n\n>\n> Maybe we could also re-use parallel fetch for multiple upstreams in\n> the superproject when doing a \"git fetch --all\" without too much\n> extra work?\n\nThat's why I tried advertising RFC patch 2/4 as I believe it could be\neasily reused for such things like \"git fetch --all\", but maybe other\npeople see problems with it (over/under engineered, wrong things\nadded, but other critical things missing) so I'd like to hear opinions\non that. :)\n"}]}