Subject: Re: [PATCH v6 4/7] config: add new index.threads config setting

From: SZEDER Gábor <[email-masked]>
Date: Mon, 1 Oct 2018 17:06:13 +0200

On Mon, Oct 01, 2018 at 09:17:53AM -0400, Ben Peart wrote:
> 
> 
> On 9/28/2018 6:15 PM, Junio C Hamano wrote:
> >Ramsay Jones <[email-masked]> writes:
> >
> >>>                 if (!nr) {
> >>>                         ieot_blocks = istate->cache_nr / THREAD_COST;
> >>>-                       if (ieot_blocks < 1)
> >>>-                               ieot_blocks = 1;
> >>>                         cpus = online_cpus();
> >>>                         if (ieot_blocks > cpus - 1)
> >>>                                 ieot_blocks = cpus - 1;
> >>
> >>So, am I reading this correctly - you need cpus > 2 before an
> >>IEOT extension block is written out?
> >>
> >>OK.
> >
> >Why should we be even calling online_cpus() in this codepath to
> >write the index in a single thread to begin with?
> >
> >The number of cpus that readers would use to read this index file
> >has nothing to do with the number of cpus available to this
> >particular writer process.
> >
> 
> As I mentioned in my other reply, this is optimizing for the most common
> case where the index is read from the same machine that wrote it and the
> user is taking the default settings (ie index.threads=true).

I think this is a reasonable assumption to make, but it should be
mentioned in the relevant commit message.  Alas, as far as I can tell,
not a single commit message has been updated in v7.

> Aligning the number of blocks to the number of threads that will be
> processing them avoids situations where one thread may have up to double the
> work to do as the other threads (for example, if there were 3 blocks to be
> processed by 2 threads).