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

From: Jeff King <peff@peff.net>
Date: Nov 9, 2015, 17:47 UTC
Message-ID: <20151109174738.GA29468@sigill.intra.peff.net>
In-Reply-To: <6AE1604EE3EC5F4296C096518C6B77EE5D0FDABA18@mail.accesssoftek.com>

On Mon, Nov 09, 2015 at 09:28:12AM -0800, Victor Leschuk wrote:

Show 6 quoted lines

> 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;

That's OK.

> else if (num_threads < 0)
>   die(...)

Do we really want to die here? I think "threads < 0" works the same as "threads==0" in other git programs. It's also a weird place to die. It would make:

  git grep --cached --threads=-1

silently work, while:

  git grep --threads=-1

would die.

If we do accept it, it may make sense to normalize it to 0 so that you can just check "!num_threads" elsewhere in the code.

-Peff