doc: fetch: document `--jobs=0` behavior
PATCH 1/1 doc: fetch: document `--jobs=0` behavior
From: "Daniel D. Beck" <[email protected]>
In c39952b92 (fetch: choose a sensible default with --jobs=0 again, 2023-02-20), the --jobs=0 behavior was (re)introduced, but it went undocumented. Since this is the same behavior as git -c fetch.parallel=0 fetch, which is documented, this change creates symmetry between the two documentation sections.
---
doc: fetch: document --jobs=0 behavior
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2047%2Fddbeck%2Fdoc-git-fetch-jobs-0-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2047/ddbeck/doc-git-fetch-jobs-0-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2047
Documentation/fetch-options.adoc | 2 ++ 1 file changed, 2 insertions(+)
Documentation/fetch-options.adoc +2 -0
@@ -234,6 +234,8 @@ endif::git-pull[] `--jobs=<n>`:: Parallelize all forms of fetching up to _<n>_ jobs at a time. + +A value of 0 will use some reasonable default. ++ If the `--multiple` option was specified, the different remotes will be fetched in parallel. If multiple submodules are fetched, they will be fetched in parallel. To control them independently, use the config settings
base-commit: 852829b3dd2fe4e7c7fc4d8badde644cf1b66c74
Signature · 1 line
gitgitgadget
Documentation/fetch-options.adoc endif::git-pull[] · line 237
Quoted 9 lines · Daniel D. Beck via GitGitGadget
Documentation/fetch-options.adoc +1
-0@@ -234,6 +234,8 @@ endif::git-pull[] `--jobs=<n>`:: Parallelize all forms of fetching up to _<n>_ jobs at a time. + +A value of 0 will use some reasonable default.
Can't we do better though than saying "some reasonable default"? As a user I would wonder what this is even supposed to mean. True, we don't do so either in the documentation of "fetch.parallel". But arguably, we should update both sites to reflect the status quo.
Going into the code we seem to fall back to online_cpus(). So should we document this accordingly?
Thanks!
Patrick
Quoted 16 lines · Patrick Steinhardt
Documentation/fetch-options.adoc endif::git-pull[] · line 237
Quoted 9 lines · Daniel D. Beck via GitGitGadget
Documentation/fetch-options.adoc +1
-0@@ -234,6 +234,8 @@ endif::git-pull[] `--jobs=<n>`:: Parallelize all forms of fetching up to _<n>_ jobs at a time. + +A value of 0 will use some reasonable default.Can't we do better though than saying "some reasonable default"? As a user I would wonder what this is even supposed to mean. True, we don't do so either in the documentation of "fetch.parallel". But arguably, we should update both sites to reflect the status quo.
Quoted 2 lines
Going into the code we seem to fall back to
online_cpus(). So should we document this accordingly?
I do not have time to dig this out myself from ancient discussion threads, but we probably had the same discussion when "git config --help" described the fetch.parallel with exactly the same phrasing and decided to leave the exact implementation detail out of the end-user facing documentation.
Thanks.
Quoted 18 lines · Junio C Hamano
Quoted 16 lines · Patrick Steinhardt
Documentation/fetch-options.adoc endif::git-pull[] · line 237
Quoted 9 lines · Daniel D. Beck via GitGitGadget
Documentation/fetch-options.adoc +1
-0@@ -234,6 +234,8 @@ endif::git-pull[] `--jobs=<n>`:: Parallelize all forms of fetching up to _<n>_ jobs at a time. + +A value of 0 will use some reasonable default.Can't we do better though than saying "some reasonable default"? As a user I would wonder what this is even supposed to mean. True, we don't do so either in the documentation of "fetch.parallel". But arguably, we should update both sites to reflect the status quo.
Quoted 8 lines
Quoted 2 lines
Going into the code we seem to fall back to
online_cpus(). So should we document this accordingly?I do not have time to dig this out myself from ancient discussion threads, but we probably had the same discussion when "git config --help" described the fetch.parallel with exactly the same phrasing and decided to leave the exact implementation detail out of the end-user facing documentation.
Doesn't look like it. The thread in question is [1], and neither the commit message nor the discussion around the patch mentioned why we don't document what the reasonable default is.
Dscho, do you remember by chance why you decided to not be more specific here?
Thanks!
Patrick
[1]: <[email protected]>
Quoted 31 lines · Patrick Steinhardt
Quoted 18 lines · Junio C Hamano
Quoted 16 lines · Patrick Steinhardt
Documentation/fetch-options.adoc
Quoted 9 lines · Daniel D. Beck via GitGitGadget
Documentation/fetch-options.adoc +0
-0@@ -234,6 +234,8 @@ endif::git-pull[]
--jobs=<n>::
Parallelize all forms of fetching up to _<n>_ jobs at a time.Documentation/fetch-options.adoc +2
-0+ +A value of 0 will use some reasonable default.Can't we do better though than saying "some reasonable default"? As a user I would wonder what this is even supposed to mean. True, we don't do so either in the documentation of "fetch.parallel". But arguably, we should update both sites to reflect the status quo.
Quoted 8 lines
Quoted 2 lines
Going into the code we seem to fall back to
online_cpus(). So should we document this accordingly?I do not have time to dig this out myself from ancient discussion threads, but we probably had the same discussion when "git config --help" described the fetch.parallel with exactly the same phrasing and decided to leave the exact implementation detail out of the end-user facing documentation.
Quoted 3 lines
Doesn't look like it. The thread in question is [1], and neither the commit message nor the discussion around the patch mentioned why we don't document what the reasonable default is.
(This is my first reply to this mailing list. Apologies in advance for any formatting mistakes.)
To set aside the history for a moment, I submitted this patch because, as a Git user, I was looking for someone to just tell me a reasonable number of jobs to use. I was pleased to find that Git already had a "don't make me think" value built in.
If there's a possibility to giving this behavior a name like --jobs=auto [1], then I'd recommend against specifically promising a strategy in the docs. It would preserve that "don't make me think" quality. Plus it would leave the door open to changing that strategy, if a better method came along.
That said, if the strategy is meant to be meaningful to users, then I'd suggest naming it something like --jobs=cpus at the same time as describing the workings of online_cpus().
In any case, thanks for the thoughtful consideration of my patch.
Daniel
[1]: <[email protected]>
Quoted 10 lines
Dscho, do you remember by chance why you decided to not be more specific here?
Thanks!
Patrick
[1]: <[email protected]>
Hi Patrick,
Quoted 28 lines · Patrick Steinhardt
Quoted 18 lines · Junio C Hamano
Quoted 16 lines · Patrick Steinhardt
Documentation/fetch-options.adoc endif::git-pull[] · line 237
Quoted 9 lines · Daniel D. Beck via GitGitGadget
Documentation/fetch-options.adoc +1
-0@@ -234,6 +234,8 @@ endif::git-pull[] `--jobs=<n>`:: Parallelize all forms of fetching up to _<n>_ jobs at a time. + +A value of 0 will use some reasonable default.Can't we do better though than saying "some reasonable default"? As a user I would wonder what this is even supposed to mean. True, we don't do so either in the documentation of "fetch.parallel". But arguably, we should update both sites to reflect the status quo.
Quoted 8 lines
Quoted 2 lines
Going into the code we seem to fall back to
online_cpus(). So should we document this accordingly?I do not have time to dig this out myself from ancient discussion threads, but we probably had the same discussion when "git config --help" described the fetch.parallel with exactly the same phrasing and decided to leave the exact implementation detail out of the end-user facing documentation.
Quoted 3 lines
Doesn't look like it. The thread in question is [1], and neither the commit message nor the discussion around the patch mentioned why we don't document what the reasonable default is.
Thank you for digging deeper. There was indeed no discussion about this in
https://lore.kernel.org/git/[email protected]/t/#u.
There was no discussion about that, either, in response to the What's Cooking email talking about the preceding pd/fetch-jobs branch:
https://lore.kernel.org/git/mhng-2c9b8fd0-22e7-4679-9d9b-f8128881fada@palmer-si-x1e/t/#mb58c71041bd41456ba0135437952ae15760e6724
Nor was there any discussion about the "reasonable default" in thr original pd/fetch-jobs contribution:
https://lore.kernel.org/git/mhng-0d288d1c-02fc-4280-bd8f-b7f611af3e8a@palmer-si-x1c4/t/#u
Quoted 2 lines
Dscho, do you remember by chance why you decided to not be more specific here?
Unfortunately not.
So I went on reconstructing the lay of the land back when d54dea77dba (fetch: let --jobs=<n> parallelize --multiple, too, 2019-10-05) landed. With that commit, the max_children variable (which --jobs=0 would set to 0) would be passed via fetch_multiple() [*1*] or fetch_populated_submodules() [*2*] to run_processes_parallel_tr2(), which would pass it through to run_processes_parallel() as the first parameter (called n) [*3*]. That function would pass that variable to pp_init() first thing [*4*], which would fall back to online_cpus() if it saw a value smaller than 1 [*5*].
So: The "reasonable default" is the number of CPUs, or more correctly, of CPU cores. It does seem, though, that that was considered common knowledge at the time, given e.g. v2.40.0's release notes saying [*6*]:
"git fetch --jobs=0" used to hit a BUG(), which has been corrected to use the available CPUs.
Ciao,
Johannes
Quoted 7 lines
Thanks!
Patrick
[1]: <[email protected]>
Footnote *1*:
https://gitlab.com/git-scm/git/-/blob/d54dea77dba081770fec7707110d8480ccaf9439/builtin/fetch.c#L1783
Footnote *2*:
https://gitlab.com/git-scm/git/-/blob/d54dea77dba081770fec7707110d8480ccaf9439/builtin/fetch.c#L1796
Footnote *3*:
https://gitlab.com/git-scm/git/-/blob/d54dea77dba081770fec7707110d8480ccaf9439/run-command.c#L1860
Footnote *4*:
https://gitlab.com/git-scm/git/-/blob/d54dea77dba081770fec7707110d8480ccaf9439/run-command.c#L1819
Footnote *5*:
https://gitlab.com/git-scm/git/-/blob/d54dea77dba081770fec7707110d8480ccaf9439/run-command.c#L1611-1612
Footnote *6*:
https://gitlab.com/git-scm/git/-/blob/v2.40.0/Documentation/RelNotes/2.40.0.txt#L57-58
Quoted 15 lines · Johannes Schindelin
Quoted 2 lines
Dscho, do you remember by chance why you decided to not be more specific here?
Unfortunately not.
So I went on reconstructing the lay of the land back when
d54dea77dba(fetch: let --jobs=<n> parallelize --multiple, too, 2019-10-05) landed. With that commit, themax_childrenvariable (which--jobs=0would set to 0) would be passed viafetch_multiple()[*1*] orfetch_populated_submodules()[*2*] torun_processes_parallel_tr2(), which would pass it through torun_processes_parallel()as the first parameter (calledn) [*3*]. That function would pass that variable topp_init()first thing [*4*], which would fall back toonline_cpus()if it saw a value smaller than 1 [*5*].
Quoted 6 lines
So: The "reasonable default" is the number of CPUs, or more correctly, of CPU cores. It does seem, though, that that was considered common knowledge at the time, given e.g. v2.40.0's release notes saying [*6*]:
"git fetch --jobs=0" used to hit a
BUG(), which has been corrected to use the available CPUs.
The belief that "available CPU cores is a reasonable default" turns out to be older than that.
Back in the days we didn't thread iterations of the same topic properly, so visiting the discussion thread and trying to find older iterations of the same topic was a nightmare, but I think I found where the phrasing came from:
https://lore.kernel.org/git/[email protected]/
This is a step in the second iteration of fetching submodules in parallel topic from 28 Oct 2015, where "some reasonable default" appears. I think it was done in response to a review comment on its earlier iteration which was:
https://lore.kernel.org/git/[email protected]/
Later the work resulted in a028a193 (fetching submodules: respect submodule.fetchJobs config option, 2016-02-29).
https://lore.kernel.org/git/[email protected]/
The variable fetch.parallel did not exist until d54dea77 (fetch: let --jobs=<n> parallelize --multiple, too, 2019-10-05) copied the famous "some reasonable default" phrasing to its documentation.
Quoted 7 lines · Daniel D. Beck via GitGitGadget
From: "Daniel D. Beck" <[email protected]>
In
c39952b92(fetch: choose a sensible default with --jobs=0 again, 2023-02-20), the--jobs=0behavior was (re)introduced, but it went undocumented. Since this is the same behavior asgit -c fetch.parallel=0 fetch, which is documented, this change creates symmetry between the two documentation sections.
Makes sense. In hindsight, we might have been better off if we also called this "--jobs=auto", but documenting the behaviour is a good first step.
Will queue. Thanks.
Quoted 27 lines
Signed-off-by Daniel D. Beck <[email protected]>---
doc: fetch: document --jobs=0 behaviorPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2047%2Fddbeck%2Fdoc-git-fetch-jobs-0-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2047/ddbeck/doc-git-fetch-jobs-0-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2047Documentation/fetch-options.adoc | 2 ++ 1 file changed, 2 insertions(+)Documentation/fetch-options.adoc +2
-0@@ -234,6 +234,8 @@ endif::git-pull[] `--jobs=<n>`:: Parallelize all forms of fetching up to _<n>_ jobs at a time. + +A value of 0 will use some reasonable default. ++ If the `--multiple` option was specified, the different remotes will be fetched in parallel. If multiple submodules are fetched, they will be fetched in parallel. To control them independently, use the config settingsbase-commit:
852829b3dd2fe4e7c7fc4d8badde644cf1b66c74