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

Correct. I think adding the option (both to command line and to config file) is good, as long as the IO issues are documented. And default to just the fixed number of threads for now - and with the option, maybe people can then more easily try out different scenarios and maybe improve on the

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

> if (list.nr || cached) >num_threads = 1; > if (!num_threads) > num_threads = GREP_NUM_THREADS_DEFAULT; > and then later, instead of use_threads, do: > if (num_threads <= 1) { ... do single-threaded version ... > } else { ... do multi-threaded version

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

On Mon, Nov 09, 2015 at 09:55:34AM -0800, Linus Torvalds wrote: > > 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

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

On Mon, Nov 9, 2015 at 9:55 AM, Linus Torvalds wrote: > > 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

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

On Mon, Nov 09, 2015 at 08:34:27AM -0800, Victor Leschuk wrote: > > Why don't we leave it at 8, then? That's the conservative choice, and > > once we have --threads, people can easily experiment with different > > values and we can follow-up with a change to the default if need be. > > I'd

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

On Mon, Nov 9, 2015 at 10:32 AM, Victor Leschuk wrote: > On Mon, Nov 9, 2015 at 9:28 AM, Victor Leschuk >> num_threads = online_cpus() <= 1 ? 0 : GREP_NUM_THREADS_DEFAULT; > > Actually I have never said the nCPUs played main role in it. T The pseudo-code you sent

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

On Mon, Nov 9, 2015 at 9:28 AM, Victor Leschuk 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

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

On Mon, Nov 09, 2015 at 09:28:12AM -0800, Victor Leschuk 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)

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

On Mon, Nov 9, 2015 at 9:28 AM, Victor Leschuk 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

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

On Mon, Nov 09, 2015 at 03:36:16AM -0800, Victor Leschuk wrote: > Yeah do also think it would be more reasonable to use "0" for > "autodetect" default value. However chat this autodetect value should > be? > > For index index-pack and pack-objects we use ncpus() for this, however > according to

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

> Why don't we leave it at 8, then? That's the conservative choice, and > once we have --threads, people can easily experiment with different > values and we can follow-up with a change to the default if need be. I'd propose the following: if (list.nr || cached) {

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

. > In the meantime I'd argue for just getting rid of the online_cpu's > check, because > (a) I think it's actively misleading > (b) the threaded grep probably doesn't hurt much even on a single > CPU, and the _potential_ upside from IO could easily dwarf the cost. > (c) do developers

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

From: Jeff King [p...@peff.net] Sent: Tuesday, November 03, 2015 22:40 To: Junio C Hamano Cc: Victor Leschuk; git@vger.kernel.org; Victor Leschuk; torva...@linux-foundation.org; j...@keeping.me.uk Subject: Re: [PATCH v4] Add git-grep threads param On Tue

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

>> do we have any objections on this patch? > The question you should be asking is "do we have any support". Hello all, CCing participated reviewers. As Junio has correctly mentioned: "Do we have any support for including this functionality?" I think this kind of customization can be useful

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

Victor Leschuk writes: > Make number of git-grep worker threads a configuration parameter. > According to several tests on systems with different number of CPU cores > the hard-coded number of 8 threads is not optimal for all systems: > tuning this parameter can significantly

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

On Tue, Nov 03, 2015 at 09:22:09AM -0800, Junio C Hamano wrote: > > +grep.threads:: > > + Number of grep worker threads, use it to tune up performance on > > + multicore machines. Default value is 8. Set to 0 to disable threading. > > + > > I am not enthused by this "Set to 0 to disable".

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

Victor Leschuk writes: > do we have any objections on this patch? The question you should be asking is "do we have any support". It is not like the default for any series is to be included; it is quite the opposite. "Is this worth having in our tree?" is the

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

Hello all, do we have any objections on this patch? -- Best Regards, Victor From: Victor Leschuk [vlesc...@gmail.com] Sent: Tuesday, October 27, 2015 14:22 To: git@vger.kernel.org Cc: Victor Leschuk Subject: [PATCH v4] Add git-grep threads param Make

[PATCH v4] Add git-grep threads param

Make number of git-grep worker threads a configuration parameter. According to several tests on systems with different number of CPU cores the hard-coded number of 8 threads is not optimal for all systems: tuning this parameter can significantly speed up grep performance. Signed-off-by: Victor