From: Ben Peart <peartben@gmail.com>
Date: Mon, 29 Oct 2018 17:07:25 GMT
Subject: Re: [PATCH v2 08/10] read-cache.c: remove #ifdef NO_PTHREADS
Message-ID: <4e3c20f1-580f-0a34-eb86-028bf8dd1b4b@gmail.com>
In-Reply-To: <20181029143033.GF17668@sigill.intra.peff.net>



On 10/29/2018 10:30 AM, Jeff King wrote:
> On Sat, Oct 27, 2018 at 07:30:06PM +0200, Nguyễn Thái Ngọc Duy wrote:
> 
>> -#ifndef NO_PTHREADS
>> -	nr_threads = git_config_get_index_threads();
>> +	if (HAVE_THREADS) {
>> +		nr_threads = git_config_get_index_threads();
>>   
>> -	/* TODO: does creating more threads than cores help? */
>> -	if (!nr_threads) {
>> -		nr_threads = istate->cache_nr / THREAD_COST;
>> -		cpus = online_cpus();
>> -		if (nr_threads > cpus)
>> -			nr_threads = cpus;
>> +		/* TODO: does creating more threads than cores help? */
>> +		if (!nr_threads) {
>> +			nr_threads = istate->cache_nr / THREAD_COST;
>> +			cpus = online_cpus();
>> +			if (nr_threads > cpus)
>> +				nr_threads = cpus;
>> +		}
>> +	} else {
>> +		nr_threads = 1;
>>   	}
> 
> I'd have thought we could just rely on online_cpus() returning 1 here to
> avoid having to ask "do we even have thread support?". But I guess that
> TODO comment implies that we might one day two 2 threads on a single
> CPU.
> 
> -Peff
> 

See my earlier response - the HAVE_THREADS tests are not needed.

We can remove the "TODO" comment - I tested it and it wasn't faster to 
have more threads than CPUs.