Re: [PATCH 5/9] submodule update: expose parallelism to the user

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 5/9] submodule update: expose parallelism to the user

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:07:04

Stefan Beller [off-list ref] writes:
quoted hunk
@@ -374,6 +374,10 @@ for linkgit:git-clone[1]'s `--reference` and `--shared` options carefully.
 	clone with a history truncated to the specified number of revisions.
 	See linkgit:git-clone[1]
 
+-j::
+--jobs::
This probably should be 

          -j <n>::
          --jobs <n>::

(see comments on [6/9]).  I know the option description in this file
is sloppy and does not say "--name <name>" etc., as it should (but
it does say "--reference <repository>"), and fixing them may not be
within the scope of this series, but we do not need to add more to
the existing problems.
+	This option is only valid for the update command.
+	Clone new submodules in parallel with as many jobs.
And when 0 starts to meaning something special, we would need to
describe that here (and/or submodule.jobs entry in config.txt).
As I already said, I do not think "0 means num_cpus" is a useful
default, and I would prefer if we reserved 0 to mean something more
useful we would figure out later.

Thanks.

Re: [PATCH 5/9] submodule update: expose parallelism to the user

From: Stefan Beller <hidden>
Date: 2016-06-15 23:07:05

On Tue, Oct 27, 2015 at 1:59 PM, Junio C Hamano [off-list ref] wrote:
And when 0 starts to meaning something special, we would need to
describe that here (and/or submodule.jobs entry in config.txt).
As I already said, I do not think "0 means num_cpus" is a useful
default, and I would prefer if we reserved 0 to mean something more
useful we would figure out later.
Ok I'll add that, too.

I am just debating with myself where the best place is.
In run-command.c in pp_init we have:

    if (n < 1)
        n = online_cpus();
    pp->max_processes = n;

we would need to change only that one place to insert an

    die("We haven't found the right default yet for 0");

However I think for most loads online_cpus makes sense as that
is ususally the bottleneck for local operations (if being excessive
memory may become an issue, but unlikely IMHO).
So instead I think it makes more sense to add it in the fetch/clone/update
to come up with a treatment for 0.

Maybe we want to make the explicit decision for the default value
for any user of the parallel processing, such that this code above
is misguided as it leads to bad defaults if reviewers are inattentive.

So having spelled out that, we may just want to bark in the pp_init
for having a number n < 1.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help