RE: [PATCH v4] Add git-grep threads param

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Nov 9, 2015 at 9:28 AM, Victor Leschuk
<vleschuk@xxxxxxxxxxxxxxxx> wrote:
>
> Maybe use the simplest version (and keep num_numbers == 0 also as flag for all other checks in code like if(num_flags) .... ):
>
> if (list.nr || cached )
>   num_threads = 0; // do not use threads
> else if (num_threads == 0)
>   num_threads = online_cpus() <= 1 ? 0 : GREP_NUM_THREADS_DEFAULT;

> I will say this AGAIN.

> The number of threads is *not* about the number of CPU's. Stop this
craziness. It's wrong.

Actually I have never said the nCPUs played main role in it. The patch is intended to provide user ability to change this threads number according to their needs and to touch as small amount of other code as possible.

> The number of threads is about parallelism. Yes, CPU's is a small part
> of it. But as mentioned earlier, the *big* wins are for slow
> filesystems, NFS in particular. On NFS, even if you have things
> cached, the cache revalidation is going to cause network traffic
> almost every time. Being able to have several of those outstanding is
> a big deal.

> So stop with the "online_cpus()" stuff. And don't base your benchmarks
> purely on the CPU-bound case. Because the CPU-bound case is the case
> that is already generally so good that few people will care all *that*
> deeply.

I have performed a cold-cached FS test (in previous thread to minimize the CPU part in the results) and it showed high correlation between speed and thread_num. Isn't it what you said? Even on systems with small number of cores we can gain profit of multithreading. That's I why I suggest this param to be customizable and not HARDCODED.

We need to create a clear text for the documentation that this number should not based on number of CPU-s only. Currently do not mention anything on it.

--
Victor
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]