Re: [PATCH v2] Add git-grep threads-num param

Please pay attention to your title. It no longer matches what the patch does.

It may also be beneficial to study recent titles and log messages from other developers (you can find them in "git log --no-merges" and "git shortlog --no-merges") and learn and imitate their format, style and tone. We want our log to tell a story in a consistent voice, no matter who the authors of individual commits are.

Victor Leschuk <vleschuk@gmail.com> writes:

Do you think anybody wants to see this line in the output from "git log" six months from now? I doubt it. The previous one will not be committed to my tree anyway, so the readers would not know (nor care) what other patch you are talking about.

This avoid --threads=0 to take the threading codepath and spawning no threads, which would have happend in the previous patch.

But it makes me wonder if the logic should be more like this:
    
 - Because the code is not prepared to go multi-thread when
   searching in the object data (not working tree), we always
   disable threading if 'list' is not empty or 'cached' is given;
   otherwise

 - If the user explicitly said that she wants N threads, we use that
   many threads; otherwise

 - If there is only one CPU, we do not do multi-thread; otherwise

 - We use the default number of threads.

IOW, I'd suggest making opt.num_threads an "int" (not "unsigned"), initialize it to -1 (unspecified), and then make this part more like this, perhaps?

	if (!opt.num_threads)
        	use_threads = 0; /* the user tells us not to use threads */
	else if (list.nr || cached)
        	use_threads = 0; /* cannot multi-thread object lookup */
	else if (opt.num_threads >= 1)
		use_threads = 1; /* the user explicitly wants this many */
	else if (online_cpus() <= 1)
        	use_threads = 0;
	else {
        	use_threads = 1;
                opt.num_threads = GREP_NUM_THREADS_DEFAULT;
	}

Something like this code structure makes it very clear what needs to be changed when we want to add some sort of auto-scaling (instead of assigning the DEFAULT constant, you'd see how many cores you have, how many files you will be grepping in, etc. and come up with a good number dynamically).

You forgot a required SP between a keyword for a syntactic construct and its open parenthesis.

Thanks.