Re: [PATCHv3 08/11] fetching submodules: respect `submodule.jobs` config option

[Stefan Beller <sbeller@google.com>]

On Fri, Nov 13, 2015 at 12:47 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> Am 12.11.2015 um 00:34 schrieb Stefan Beller:
>>
>> On Wed, Nov 11, 2015 at 11:55 AM, Jens Lehmann <Jens.Lehmann@web.de>
>> wrote:
>>>>
>>>>
>>>> TL;DR: checkout is serial, network-related stuff only will be using
>>>> submodule.jobs
>>>
>>>
>>>
>>> My point being: isn't "jobs" a bit too generic for a config option that
>>> is only relevant for network-related stuff? Maybe "submodule.fetchJobs"
>>> or similar would be better, as you are already thinking about adding
>>> other parallelisms with different constraints later?
>>
>>
>> Actually I don't think that far ahead.
>
>
> Maybe I've been bitten once too often by too generic names that became
> a problem later on ... ;-)
>
>> (I assume network to be the bottleneck for clone/fetch operations)
>> All I want is a saturated network all the time, and as the native git
>> protocol
>> doesn't provide that (tcp startup takes time until full band witdth is
>> reached,
>> local operations both on client and server) I added the parallel stuff
>> to 'smear' different submodule network traffics along the timeline,
>> such that we have a better approximation of an always fully saturated link
>> for the whole operation. So in the long term future, we maybe want to
>> reuse an http/ssh session for a different submodule, possibly interleaving
>> the different submodules on the wire to make it even faster. Though that
>> may not be helping much.
>>
>> So we're back at bike shedding about the name. submodule.fetchJobs
>> sounds like it only applies to fetching, do you think it's sufficient for
>> clone
>> as well?
>
>
> Hmm, to me fetching is a part of cloning, so I don't have a problem with
> that. And documenting it accordingly should make it clear to everyone.
>
>> Once upon a time, Junio used  'submodule.fetchParallel' or
>> 'submodule.paralle'
>> in a discussion[1] for the distinction of the local and networked things.
>> [1] Discussing "[PATCH] Add fetch.recurseSubmoduleParallelism config
>> option"
>>
>> How about submodules.parallelNetwork for the networking part and
>> submodules.parallelLocal for the local part? (I don't implement
>> parallelLocal in
>> the next few weeks I'd estimate).
>
>
> If 'submodules.parallelNetwork' will be used for submodule push too as
> soon as that learns parallel operation, I'm ok with that. But if we don't
> have good reason to believe the number of jobs for fetch can simply be
> reused for push, me thinks we should have one config option containing the
> term "fetch" now and another that contains "push" when we need it later,
> just to be on the safe side. Otherwise it might be hard to explain to
> users why 'submodules.parallelNetwork' is only used for fetch and clone
> and why they have to set 'submodules.parallelPush' for pushing ...
>
> So either 'submodule.fetchParallel' or 'submodule.fetchJobs' is fine for
> me, and 'submodules.parallelNetwork' is ok too as long as we have reason
> to believe this value can be used for push later too.

Ok, got it. So fetchJobs is fine with me.
Mind the difference in the first part, submodule[s] in singular/plural.
I thought submodule as a prefix for any individual submodule, but any
settings applying to all of the submodules, you'd take the plural submodules.*
settings.