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

From: Stefan Beller <sbeller@google.com>
Date: Oct 28, 2015, 21:40 UTC
Message-ID: <CAGZ79kbm_aucoEADLFt3VjShP5Kgi0Wwyb6m1dRJtQWu9_ZtBA@mail.gmail.com>
In-Reply-To: <xmqqk2q8ni2i.fsf@gitster.mtv.corp.google.com>

On Tue, Oct 27, 2015 at 1:59 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 5 quoted lines

> 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.