From: Junio C Hamano <gitster@pobox•com>
To: Stefan Beller <sbeller@google•com>
Cc: jrnieder@gmail•com, git@vger•kernel.org, Jens.Lehmann@web•de
Subject: Re: [PATCHv9 3/6] fetching submodules: respect `submodule.fetchJobs` config option
Date: Tue, 09 Feb 2016 13:12:13 -0800 [thread overview]
Message-ID: <xmqqd1s537wy.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <1455051274-15256-4-git-send-email-sbeller@google.com> (Stefan Beller's message of "Tue, 9 Feb 2016 12:54:31 -0800")
Stefan Beller <sbeller@google•com> writes:
> This allows to configure fetching and updating in parallel
> without having the command line option.
>
> This moved the responsibility to determine how many parallel processes
> to start from builtin/fetch to submodule.c as we need a way to communicate
> "The user did not specify the number of parallel processes in the command
> line options" in the builtin fetch. The submodule code takes care of
> the precedence (CLI > config > default).
>
> Signed-off-by: Stefan Beller <sbeller@google•com>
> ---
This got a lot simpler to read, I think, even though this comes with
a better error checking ;-)
Thanks.
> Documentation/config.txt | 6 ++++++
> builtin/fetch.c | 2 +-
> submodule.c | 17 ++++++++++++++++-
> submodule.h | 1 +
> t/t5526-fetch-submodules.sh | 14 ++++++++++++++
> 5 files changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 2d06b11..3b02732 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2646,6 +2646,12 @@ submodule.<name>.ignore::
> "--ignore-submodules" option. The 'git submodule' commands are not
> affected by this setting.
>
> +submodule.fetchJobs::
> + Specifies how many submodules are fetched/cloned at the same time.
> + A positive integer allows up to that number of submodules fetched
> + in parallel. A value of 0 will give some reasonable default.
> + If unset, it defaults to 1.
> +
> tag.sort::
> This variable controls the sort ordering of tags when displayed by
> linkgit:git-tag[1]. Without the "--sort=<value>" option provided, the
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 586840d..5aa1c2d 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -37,7 +37,7 @@ static int prune = -1; /* unspecified */
> static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity;
> static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
> static int tags = TAGS_DEFAULT, unshallow, update_shallow;
> -static int max_children = 1;
> +static int max_children = -1;
> static const char *depth;
> static const char *upload_pack;
> static struct strbuf default_rla = STRBUF_INIT;
> diff --git a/submodule.c b/submodule.c
> index b83939c..fd763f5 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -15,6 +15,7 @@
> #include "thread-utils.h"
>
> static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
> +static int parallel_jobs = 1;
> static struct string_list changed_submodule_paths;
> static int initialized_fetch_ref_tips;
> static struct sha1_array ref_tips_before_fetch;
> @@ -169,7 +170,13 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
>
> int submodule_config(const char *var, const char *value, void *cb)
> {
> - if (starts_with(var, "submodule."))
> + if (!strcmp(var, "submodule.fetchjobs")) {
> + unsigned long val;
> + if (!git_parse_ulong(value, &val) || 0 > val || val >= INT_MAX)
> + die(_("Error parsing submodule.fetchJobs %s"), value);
> + parallel_jobs = val;
> + return 0;
> + } else if (starts_with(var, "submodule."))
> return parse_submodule_config_option(var, value);
> else if (!strcmp(var, "fetch.recursesubmodules")) {
> config_fetch_recurse_submodules = parse_fetch_recurse_submodules_arg(var, value);
> @@ -751,6 +758,9 @@ int fetch_populated_submodules(const struct argv_array *options,
> argv_array_push(&spf.args, "--recurse-submodules-default");
> /* default value, "--submodule-prefix" and its value are added later */
>
> + if (max_parallel_jobs < 0)
> + max_parallel_jobs = parallel_jobs;
> +
> calculate_changed_submodule_paths();
> run_processes_parallel(max_parallel_jobs,
> get_next_submodule,
> @@ -1097,3 +1107,8 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
> strbuf_release(&rel_path);
> free((void *)real_work_tree);
> }
> +
> +int parallel_submodules(void)
> +{
> + return parallel_jobs;
> +}
> diff --git a/submodule.h b/submodule.h
> index cbc0003..95babc5 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -41,5 +41,6 @@ int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam
> struct string_list *needs_pushing);
> int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);
> void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
> +int parallel_submodules(void);
>
> #endif
> diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
> index 1241146..954d0e4 100755
> --- a/t/t5526-fetch-submodules.sh
> +++ b/t/t5526-fetch-submodules.sh
> @@ -471,4 +471,18 @@ test_expect_success "don't fetch submodule when newly recorded commits are alrea
> test_i18ncmp expect.err actual.err
> '
>
> +test_expect_success 'fetching submodules respects parallel settings' '
> + git config fetch.recurseSubmodules true &&
> + (
> + cd downstream &&
> + GIT_TRACE=$(pwd)/trace.out git fetch --jobs 7 &&
> + grep "7 tasks" trace.out &&
> + git config submodule.fetchJobs 8 &&
> + GIT_TRACE=$(pwd)/trace.out git fetch &&
> + grep "8 tasks" trace.out &&
> + GIT_TRACE=$(pwd)/trace.out git fetch --jobs 9 &&
> + grep "9 tasks" trace.out
> + )
> +'
> +
> test_done
next prev parent reply other threads:[~2016-02-09 21:12 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-09 20:54 [PATCHv9 0/6] Expose submodule parallelism to the user Stefan Beller
2016-02-09 20:54 ` [PATCHv9 1/6] submodule-config: keep update strategy around Stefan Beller
2016-02-09 21:08 ` Junio C Hamano
2016-02-09 22:19 ` Stefan Beller
2016-02-09 22:29 ` Junio C Hamano
2016-02-09 21:49 ` Jonathan Nieder
2016-02-09 22:32 ` Junio C Hamano
2016-02-11 20:00 ` Junio C Hamano
2016-02-09 20:54 ` [PATCHv9 2/6] submodule-config: drop check against NULL Stefan Beller
2016-02-09 21:50 ` Jonathan Nieder
2016-02-09 20:54 ` [PATCHv9 3/6] fetching submodules: respect `submodule.fetchJobs` config option Stefan Beller
2016-02-09 21:12 ` Junio C Hamano [this message]
2016-02-09 22:34 ` Jonathan Nieder
2016-02-10 0:11 ` Stefan Beller
2016-02-10 1:12 ` Junio C Hamano
2016-02-10 2:04 ` Junio C Hamano
2016-02-09 20:54 ` [PATCHv9 4/6] git submodule update: have a dedicated helper for cloning Stefan Beller
2016-02-09 21:24 ` Junio C Hamano
2016-02-10 0:37 ` Jonathan Nieder
2016-02-10 2:26 ` Jacob Keller
2016-02-10 17:49 ` Stefan Beller
2016-02-11 7:46 ` Jacob Keller
2016-02-09 20:54 ` [PATCHv9 5/6] submodule update: expose parallelism to the user Stefan Beller
2016-02-09 20:54 ` [PATCHv9 6/6] clone: allow an explicit argument for parallel submodule clones Stefan Beller
2016-02-09 21:39 ` [PATCHv9 0/6] Expose submodule parallelism to the user Junio C Hamano
2016-02-09 21:46 ` Stefan Beller
2016-02-09 22:03 ` Junio C Hamano
2016-02-11 20:23 ` Junio C Hamano
2016-02-11 20:28 ` Junio C Hamano
2016-02-11 20:33 ` Stefan Beller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqd1s537wy.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=Jens.Lehmann@web$(echo .)de \
--cc=git@vger$(echo .)kernel.org \
--cc=jrnieder@gmail$(echo .)com \
--cc=sbeller@google$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox