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

2015-10-27 Thread Victor Leschuk
Hello Linus, >> 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: > Did you also compare cold-cache filesystem performance? > One of the reasons for doing threaded grep is for CPU scaling. But another

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

2015-10-27 Thread John Keeping
On Mon, Oct 26, 2015 at 10:25:41PM -0700, Victor Leschuk wrote: > >> @@ -22,6 +22,7 @@ SYNOPSIS > >> [--color[=] | --no-color] > >> [--break] [--heading] [-p | --show-function] > >> [-A ] [-B ] [-C ] > >> +[--threads ] > > > Is this the best place for this

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

2015-10-27 Thread Victor Leschuk
Hello John, >> This thought also crossed my mind, however we already pass grep_opt to >> start_threads() function, so I think passing it to wait_all() is not >> that ugly, and kind of symmetric. And I do not like the idea of >> duplicating same information in different places. What do you think?

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

2015-10-27 Thread John Keeping
On Tue, Oct 27, 2015 at 06:54:16AM -0700, Victor Leschuk wrote: > Hello John, > > >> This thought also crossed my mind, however we already pass grep_opt to > >> start_threads() function, so I think passing it to wait_all() is not > >> that ugly, and kind of symmetric. And I do not like the idea

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

2015-10-26 Thread John Keeping
On Mon, Oct 26, 2015 at 03:32:13PM +0300, Victor Leschuk wrote: > 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

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

2015-10-26 Thread Victor Leschuk
Hello John, see comments inline. >> @@ -22,6 +22,7 @@ SYNOPSIS >> [--color[=] | --no-color] >> [--break] [--heading] [-p | --show-function] >> [-A ] [-B ] [-C ] >> +[--threads ] > Is this the best place for this option? I know the current list isn't > sorted